fix(auth): prompt for recovery on startup by ndycode · Pull Request #106 · ndycode/codex-multi-auth

ndycode · 2026-03-16T08:51:10Z

Summary

Reopens the preserved startup-recovery prompt slice from archived PRs #77 and #92
Stacked PR on fresh/02-backup-restore-manager

What Changed

Prompts for actionable recovery before OAuth when interactive login starts empty, and suppresses the prompt after same-session deliberate fresh or reset actions.
Keeps this branch focused to the startup-recovery prompt and related recovery/storage tests.

Validation

Docs and Governance Checklist

README updated (if user-visible behavior changed)
docs/getting-started.md updated (if onboarding flow changed)
docs/features.md updated (if capability surface changed)
relevant docs/reference/* pages updated (if commands/settings/paths changed)
docs/upgrade.md updated (if migration behavior changed)
SECURITY.md and CONTRIBUTING.md reviewed for alignment

Risk and Rollback

Risk level: medium
Rollback plan: revert 12074e2 and 390f801

Additional Notes

This is a sibling branch to fresh/04-sync-center; later branches carry an equivalent cleanup commit but are not direct descendants of 390f801.
Preserved branch validation from the fresh-restack run passed lint, typecheck, build, and full tests.

note: greptile review for oc-chatgpt-multi-auth. cite files like `lib/foo.ts:123`. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

this pr adds a startup-recovery prompt to codex auth login: when the session starts with zero saved accounts and at least one actionable named backup exists, the user is offered a restore before oauth. the prompt is correctly suppressed in non-interactive/fallback paths and after same-session fresh or reset actions. the logic in resolveStartupRecoveryAction and the loginFlow continuation state machine (suppressRecoveryPrompt, pendingRecoveryState, allowEmptyStorageMenu) is well-structured and has thorough vitest coverage.

key concerns:

path containment check removed (lib/storage.ts): assertNamedBackupRestorePath — which used realpathSync.native to resolve canonical paths and verify backup files don't escape the backup root — has been deleted. the new isSymbolicLink() filter at listing time does not protect against windows ntfs directory junctions (which return false for isSymbolicLink()). resolveNamedBackupRestorePath now returns unvalidated paths for user-supplied backup names.
restoreNamedBackup drops eligibility guard: the old restoreAssessedNamedBackup verified eligibleForRestore before importing. the new restoreNamedBackup calls importAccounts directly; the ui path in runBackupRestoreManager does re-assess before calling it, but the exported function itself offers no protection for other callers.
retry jitter regressed: retryTransientFilesystemOperation now uses a fixed 0–9 ms jitter regardless of attempt (down from proportional 0–baseDelay ms), reducing spacing between retries and lowering effectiveness against windows av-scanner lock bursts.
unused warnSpy variables in three test cases will fail lint and silently swallow console.warn output in those tests.

Confidence Score: 2/5

not safe to merge — windows filesystem safety regressions (containment check, retry jitter) and missing eligibility guard on the exported restoreNamedBackup need resolution first
the startup-recovery prompt logic itself is sound and well-tested, but the accompanying storage.ts changes introduce three compounding windows-safety regressions: removal of the ntfs-junction-proof containment guard, reduced retry jitter quality, and loss of the exported eligibility check on restoreNamedBackup. the test removal of assertNamedBackupRestorePath coverage leaves no regression safety net for the path-traversal surface on windows.
lib/storage.ts requires the most attention — specifically the removed assertNamedBackupRestorePath containment logic, the jitter regression in retryTransientFilesystemOperation, and the missing eligibility check in restoreNamedBackup. test/storage.test.ts should be checked for any deduplication-related test coverage that was dropped.

Important Files Changed

Filename	Overview
lib/codex-manager.ts	major restructuring of runAuthLogin to add startup recovery prompt; introduces resolveStartupRecoveryAction and BackupRestoreManagerResult type; restore-backup command moved earlier in dispatch chain with setStoragePath(null); suppressRecoveryPrompt correctly set on deliberate fresh/reset; but candidateCache removed from loadBackupRestoreManagerAssessments, doubling assessment I/O concurrency from 4→8 on windows
lib/storage.ts	removes assertNamedBackupRestorePath/BackupContainmentError (path containment check), reduces retry attempts from 7→5 with worse jitter, expands EBUSY retry to all platforms, adds getActionableNamedBackupRestores/scanNamedBackups, drops restoreAssessedNamedBackup and its eligibility guard; multiple windows filesystem safety regressions
test/codex-manager-cli.test.ts	extensive new vitest coverage for startup recovery prompt (12+ new test cases covering scan failure, dismiss, re-prompt on failure, suppression after fresh/reset); three new tests declare warnSpy but never reference it, which will fail lint and silently suppress console.warn output; several pre-existing integration tests removed (doctor, report, settings hub)
test/recovery.test.ts	new test file covering resolveStartupRecoveryAction with multiple scenario branches; good coverage of assessments.length > 0, recoveryScanFailed, totalBackups > 0, and open-empty-storage-menu paths
test/storage-recovery-paths.test.ts	new test file covering getActionableNamedBackupRestores with fast-path vs slow-path, symlink filtering, failed assessment handling; no coverage for NTFS junction traversal scenario on windows
test/storage.test.ts	removes tests for assertNamedBackupRestorePath/BackupContainmentError and deduplication-scoped skipped-count scenarios; loss of coverage for path containment enforcement and duplicate-row skipped counting

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[runAuthLogin start] --> B{existingStorage\nhas accounts?}
    B -- yes --> C[Enter main login menu loop]
    B -- no --> D{canOpenEmptyStorageMenu?}
    D -- yes --> C
    D -- no --> E{canPromptForRecovery?\nnot suppressed, not attempted,\ncount=0, interactive}
    E -- no --> K[OAuth flow]
    E -- yes --> F[getActionableNamedBackupRestores]
    F -- throws --> G[warn + continue to OAuth]
    F --> H{resolveStartupRecoveryAction}
    H -- open-empty-storage-menu --> I[set allowEmptyStorageMenu=true\ncontinue loginFlow]
    I --> B
    H -- continue-with-oauth --> K
    H -- show-recovery-prompt --> J[confirm: Restore now?]
    J -- no --> K
    J -- yes --> L[runBackupRestoreManager\nwith allAssessments override]
    L -- restored --> M{existingCount > 0?}
    M -- yes --> C
    M -- no --> K
    L -- dismissed --> N[set suppressRecoveryPrompt=true\ncontinue loginFlow]
    N --> E
    L -- failed --> O[set pendingRecoveryState\nreset recoveryPromptAttempted\ncontinue loginFlow]
    O --> E
    C --> P{menu result}
    P -- fresh/reset --> Q[set suppressRecoveryPrompt=true\ncontinue loginFlow]
    Q --> E
    P -- restore-backup --> R[runBackupRestoreManager\nfresh scan]
    R --> C
    P -- cancel/add/oauth --> K
    K --> S[return exit code]

Comments Outside Diff (4)

lib/codex-manager.ts, line 4596-4602 (link)

autoSyncActiveAccountToCodex dropped — codex CLI auth state not updated after standalone restore

the old runBackupRestoreManager called autoSyncActiveAccountToCodex() after every successful write. the new restoreNamedBackup path skips this entirely. for the standalone codex auth restore-backup command the codex CLI active-account file is never rewritten, so the CLI stays pointing at whichever account was active before the restore (or at a stale entry).

when used inside runAuthLogin this is less severe because the user will eventually complete an OAuth step and syncSelectionToCodex fires. but for the direct command path the sync is the only opportunity and it's now gone.

the removed call:

// old — ran after every successful restore
const synced = await autoSyncActiveAccountToCodex();
if (!synced) console.warn("Backup restored, but Codex CLI auth state could not be synced.");

suggest calling autoSyncActiveAccountToCodex() (with a non-fatal catch) after restoreNamedBackup succeeds, at minimum for the standalone command path.

Prompt To Fix With AI

This is a comment left during a code review.
Path: lib/codex-manager.ts
Line: 4596-4602

Comment:
**`autoSyncActiveAccountToCodex` dropped — codex CLI auth state not updated after standalone restore**

the old `runBackupRestoreManager` called `autoSyncActiveAccountToCodex()` after every successful write. the new `restoreNamedBackup` path skips this entirely. for the standalone `codex auth restore-backup` command the codex CLI active-account file is never rewritten, so the CLI stays pointing at whichever account was active before the restore (or at a stale entry). 

when used inside `runAuthLogin` this is less severe because the user will eventually complete an OAuth step and `syncSelectionToCodex` fires. but for the direct command path the sync is the only opportunity and it's now gone.

the removed call:
```ts
// old — ran after every successful restore
const synced = await autoSyncActiveAccountToCodex();
if (!synced) console.warn("Backup restored, but Codex CLI auth state could not be synced.");
```

suggest calling `autoSyncActiveAccountToCodex()` (with a non-fatal catch) after `restoreNamedBackup` succeeds, at minimum for the standalone command path.

How can I resolve this? If you propose a fix, please make it concise.

lib/storage.ts, line 1082-1104 (link)

Retry jitter is now constant 0–9 ms regardless of attempt

the old code scaled jitter proportionally to the base delay:

const baseDelayMs = TRANSIENT_FILESYSTEM_BASE_DELAY_MS * 2 ** attempt; // 10, 20, 40, 80 ms
const jitterMs = Math.floor(Math.random() * baseDelayMs); // up to 79 ms at attempt 3
await setTimeout(resolve, baseDelayMs + jitterMs);

the new code always jitters 0–9 ms regardless of attempt:

setTimeout(resolve, 10 * 2 ** attempt + Math.floor(Math.random() * 10));
// attempt 0: 10-19 ms, attempt 1: 20-29 ms, ..., attempt 3: 80-89 ms

on windows, antivirus scanners hold locks for bursts that often exceed 100 ms. concentrated retry windows (e.g., all four retries landing within 90 ms) reduce the chance of clearing the lock between attempts. the old proportional jitter was more effective for distributing retry pressure. this regression is compounded by the already-reduced 5-attempt budget (down from 7). no vitest coverage for the jitter distribution.

Prompt To Fix With AI

This is a comment left during a code review.
Path: lib/storage.ts
Line: 1082-1104

Comment:
**Retry jitter is now constant 0–9 ms regardless of attempt**

the old code scaled jitter proportionally to the base delay:
```ts
const baseDelayMs = TRANSIENT_FILESYSTEM_BASE_DELAY_MS * 2 ** attempt; // 10, 20, 40, 80 ms
const jitterMs = Math.floor(Math.random() * baseDelayMs); // up to 79 ms at attempt 3
await setTimeout(resolve, baseDelayMs + jitterMs);
```

the new code always jitters 0–9 ms regardless of attempt:
```ts
setTimeout(resolve, 10 * 2 ** attempt + Math.floor(Math.random() * 10));
// attempt 0: 10-19 ms, attempt 1: 20-29 ms, ..., attempt 3: 80-89 ms
```

on windows, antivirus scanners hold locks for bursts that often exceed 100 ms. concentrated retry windows (e.g., all four retries landing within 90 ms) reduce the chance of clearing the lock between attempts. the old proportional jitter was more effective for distributing retry pressure. this regression is compounded by the already-reduced 5-attempt budget (down from 7). no vitest coverage for the jitter distribution.

How can I resolve this? If you propose a fix, please make it concise.

lib/storage.ts, line 1316-1318 (link)

restoreNamedBackup no longer enforces eligibleForRestore or the account limit

the old restoreAssessedNamedBackup validated eligibility before writing:

if (!assessment.eligibleForRestore) {
    throw new Error(assessment.error ?? "Backup is not eligible for restore.");
}

runBackupRestoreManager in codex-manager.ts does call assessNamedBackupRestore and check latestAssessment.eligibleForRestore before invoking restoreNamedBackup, so the current UI path is guarded. however, restoreNamedBackup is an exported function — any caller that reaches it directly (e.g., future automation, programmatic use, or a test that mocks around the UI layer) will bypass both the wouldExceedLimit guard and the eligibleForRestore check, landing straight in importAccounts.

the importAccounts function does enforce the limit via a throw, so catastrophic writes are blocked, but an ineligible restore due to an unparseable backup would still proceed and likely corrupt the import silently. suggest reinstating a minimal eligibility gate:

export async function restoreNamedBackup(name: string): Promise<ImportAccountsResult> {
    const assessment = await assessNamedBackupRestore(name);
    if (!assessment.eligibleForRestore) {
        throw new Error(assessment.error ?? "Backup is not eligible for restore.");
    }
    const backupPath = await resolveNamedBackupRestorePath(name);
    return importAccounts(backupPath);
}

Prompt To Fix With AI

This is a comment left during a code review.
Path: lib/storage.ts
Line: 1316-1318

Comment:
**`restoreNamedBackup` no longer enforces `eligibleForRestore` or the account limit**

the old `restoreAssessedNamedBackup` validated eligibility before writing:
```ts
if (!assessment.eligibleForRestore) {
    throw new Error(assessment.error ?? "Backup is not eligible for restore.");
}
```

`runBackupRestoreManager` in `codex-manager.ts` does call `assessNamedBackupRestore` and check `latestAssessment.eligibleForRestore` before invoking `restoreNamedBackup`, so the current UI path is guarded. however, `restoreNamedBackup` is an exported function — any caller that reaches it directly (e.g., future automation, programmatic use, or a test that mocks around the UI layer) will bypass both the `wouldExceedLimit` guard and the `eligibleForRestore` check, landing straight in `importAccounts`.

the `importAccounts` function does enforce the limit via a throw, so catastrophic writes are blocked, but an ineligible restore due to an unparseable backup would still proceed and likely corrupt the import silently. suggest reinstating a minimal eligibility gate:
```ts
export async function restoreNamedBackup(name: string): Promise<ImportAccountsResult> {
    const assessment = await assessNamedBackupRestore(name);
    if (!assessment.eligibleForRestore) {
        throw new Error(assessment.error ?? "Backup is not eligible for restore.");
    }
    const backupPath = await resolveNamedBackupRestorePath(name);
    return importAccounts(backupPath);
}
```

How can I resolve this? If you propose a fix, please make it concise.

lib/storage.ts, line 1579-1594 (link)

Path containment check removed — NTFS junction traversal on windows no longer blocked

the old code called assertNamedBackupRestorePath (with realpathSync.native) for every resolved path before use:

if (existingPath) {
    return assertNamedBackupRestorePath(existingPath, backupRoot);
}
// ...
return assertNamedBackupRestorePath(builtPath, backupRoot);

the new code drops this entirely:

if (existingPath) {
    return existingPath;   // no containment check
}
// ...
return buildNamedBackupPath(name);   // no containment check

scanNamedBackups and listNamedBackupsWithoutLoading do filter entry.isSymbolicLink(), which handles symlinks at listing time. however, on windows, ntfs directory junctions (mklink /j) are not symlinks — entry.isSymbolicLink() returns false for them while realpathSync.native resolves through them. a backups/ directory itself configured as a junction to a directory outside the data root would not be caught by the new filter.

more concretely: resolveNamedBackupRestorePath is also reached via assessNamedBackupRestore → resolveNamedBackupRestorePath, which is called directly from restoreNamedBackup. if the backup root is a junction, the canonical path will silently escape the expected directory boundary. no vitest coverage for this scenario.

Prompt To Fix With AI

This is a comment left during a code review.
Path: lib/storage.ts
Line: 1579-1594

Comment:
**Path containment check removed — NTFS junction traversal on windows no longer blocked**

the old code called `assertNamedBackupRestorePath` (with `realpathSync.native`) for every resolved path before use:
```ts
if (existingPath) {
    return assertNamedBackupRestorePath(existingPath, backupRoot);
}
// ...
return assertNamedBackupRestorePath(builtPath, backupRoot);
```

the new code drops this entirely:
```ts
if (existingPath) {
    return existingPath;   // no containment check
}
// ...
return buildNamedBackupPath(name);   // no containment check
```

`scanNamedBackups` and `listNamedBackupsWithoutLoading` do filter `entry.isSymbolicLink()`, which handles symlinks at listing time. however, on windows, ntfs directory junctions (`mklink /j`) are not symlinks — `entry.isSymbolicLink()` returns `false` for them while `realpathSync.native` resolves through them. a `backups/` directory itself configured as a junction to a directory outside the data root would not be caught by the new filter.

more concretely: `resolveNamedBackupRestorePath` is also reached via `assessNamedBackupRestore` → `resolveNamedBackupRestorePath`, which is called directly from `restoreNamedBackup`. if the backup root is a junction, the canonical path will silently escape the expected directory boundary. no vitest coverage for this scenario.

How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: lib/storage.ts
Line: 1082-1104

Comment:
**Retry jitter is now constant 0–9 ms regardless of attempt**

the old code scaled jitter proportionally to the base delay:
```ts
const baseDelayMs = TRANSIENT_FILESYSTEM_BASE_DELAY_MS * 2 ** attempt; // 10, 20, 40, 80 ms
const jitterMs = Math.floor(Math.random() * baseDelayMs); // up to 79 ms at attempt 3
await setTimeout(resolve, baseDelayMs + jitterMs);
```

the new code always jitters 0–9 ms regardless of attempt:
```ts
setTimeout(resolve, 10 * 2 ** attempt + Math.floor(Math.random() * 10));
// attempt 0: 10-19 ms, attempt 1: 20-29 ms, ..., attempt 3: 80-89 ms
```

on windows, antivirus scanners hold locks for bursts that often exceed 100 ms. concentrated retry windows (e.g., all four retries landing within 90 ms) reduce the chance of clearing the lock between attempts. the old proportional jitter was more effective for distributing retry pressure. this regression is compounded by the already-reduced 5-attempt budget (down from 7). no vitest coverage for the jitter distribution.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: lib/storage.ts
Line: 1316-1318

Comment:
**`restoreNamedBackup` no longer enforces `eligibleForRestore` or the account limit**

the old `restoreAssessedNamedBackup` validated eligibility before writing:
```ts
if (!assessment.eligibleForRestore) {
    throw new Error(assessment.error ?? "Backup is not eligible for restore.");
}
```

`runBackupRestoreManager` in `codex-manager.ts` does call `assessNamedBackupRestore` and check `latestAssessment.eligibleForRestore` before invoking `restoreNamedBackup`, so the current UI path is guarded. however, `restoreNamedBackup` is an exported function — any caller that reaches it directly (e.g., future automation, programmatic use, or a test that mocks around the UI layer) will bypass both the `wouldExceedLimit` guard and the `eligibleForRestore` check, landing straight in `importAccounts`.

the `importAccounts` function does enforce the limit via a throw, so catastrophic writes are blocked, but an ineligible restore due to an unparseable backup would still proceed and likely corrupt the import silently. suggest reinstating a minimal eligibility gate:
```ts
export async function restoreNamedBackup(name: string): Promise<ImportAccountsResult> {
    const assessment = await assessNamedBackupRestore(name);
    if (!assessment.eligibleForRestore) {
        throw new Error(assessment.error ?? "Backup is not eligible for restore.");
    }
    const backupPath = await resolveNamedBackupRestorePath(name);
    return importAccounts(backupPath);
}
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: test/codex-manager-cli.test.ts
Line: 1942-1943

Comment:
**Unused `warnSpy` variables will fail lint and silently suppress warnings**

three tests declare `warnSpy` but never reference or restore it:
- line 1942 ("prints usage when no subcommand is given")
- line 1950 ("prints implemented 40-feature matrix")
- line 1958 ("prints auth help when subcommand is --help")

this will trip `no-unused-vars` in eslint/typescript. it also silences `console.warn` for the duration of these tests without asserting on the output — meaning spurious startup-recovery warn messages from the new prompt path would be swallowed and hidden.

either use `_warnSpy` to signal intentional suppression, or remove the spy entirely if no warning assertions are needed:
```suggestion
		const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
		const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
```

same change needed on lines 1950 and 1958.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: lib/storage.ts
Line: 1579-1594

Comment:
**Path containment check removed — NTFS junction traversal on windows no longer blocked**

the old code called `assertNamedBackupRestorePath` (with `realpathSync.native`) for every resolved path before use:
```ts
if (existingPath) {
    return assertNamedBackupRestorePath(existingPath, backupRoot);
}
// ...
return assertNamedBackupRestorePath(builtPath, backupRoot);
```

the new code drops this entirely:
```ts
if (existingPath) {
    return existingPath;   // no containment check
}
// ...
return buildNamedBackupPath(name);   // no containment check
```

`scanNamedBackups` and `listNamedBackupsWithoutLoading` do filter `entry.isSymbolicLink()`, which handles symlinks at listing time. however, on windows, ntfs directory junctions (`mklink /j`) are not symlinks — `entry.isSymbolicLink()` returns `false` for them while `realpathSync.native` resolves through them. a `backups/` directory itself configured as a junction to a directory outside the data root would not be caught by the new filter.

more concretely: `resolveNamedBackupRestorePath` is also reached via `assessNamedBackupRestore` → `resolveNamedBackupRestorePath`, which is called directly from `restoreNamedBackup`. if the backup root is a junction, the canonical path will silently escape the expected directory boundary. no vitest coverage for this scenario.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: d92f36e}

Context used:

Rule used - What: Every code change must explain how it defend... (source)

chatgpt-codex-connector · 2026-03-16T08:51:18Z

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

coderabbitai · 2026-03-16T08:52:41Z

📝 Walkthrough

Walkthrough

added startup recovery prompt during interactive login when zero saved accounts exist and recoverable named backups are present. refactored backup restore manager and storage recovery apis, simplified import/restore return shapes, removed legacy ui copy, and expanded recovery tests.

Changes

Cohort / File(s)	Summary
Documentation `docs/getting-started.md`, `docs/troubleshooting.md`, `docs/upgrade.md`, `docs/reference/public-api.md`	added docs describing startup recovery prompt behavior and suppression rules; removed legacy importAccounts compatibility note.
cli interaction `lib/cli.ts`	added exported `isInteractiveLoginMenuAvailable()` and switched `promptLoginMode` to use it.
core recovery & manager flow `lib/codex-manager.ts`	added startup-recovery coordination state and `resolveStartupRecoveryAction()`; introduced `runBackupRestoreManager()` with explicit result types and assessments loading; integrated recovery flow into login loop and added `restore-backup` command.
storage layer & recovery apis `lib/storage.ts`	new public types and functions: `ActionableNamedBackupRecoveries`, `getActionableNamedBackupRestores()`, `StorageError`, `formatStorageErrorHint()`; refactored named-backup scanning/listing/assessment APIs; changed `importAccounts()` / `restoreNamedBackup()` return shape to `{ imported, total, skipped }`.
ui copy & messages `lib/ui/copy.ts`	removed `restoreBackupConfirm`, `restoreBackupSuccess`, `restoreBackupRefreshSuccess` from `UI_COPY.mainMenu`.
tests & validation `test/recovery.test.ts`, `test/storage.test.ts`, `test/storage-recovery-paths.test.ts`	expanded recovery tests, added removal retry helper and mtime adjustment in recovery path test; removed several public test-only storage exports and adjusted tests accordingly.

Sequence Diagram

sequenceDiagram
    participant user as User
    participant cli as CLI
    participant manager as CodexManager
    participant storage as Storage
    participant recovery as RecoveryManager
    participant fs as Filesystem

    user->>cli: run codex auth login
    cli->>manager: initiate login flow

    alt isInteractiveLoginMenuAvailable()
        manager->>storage: scanNamedBackups()
        storage->>fs: list backups in backups/
        fs-->>storage: backup entries
        storage-->>manager: backup scan result

        alt zero saved accounts AND backups exist
            manager->>recovery: resolveStartupRecoveryAction(recoveryState, scanFailed)
            recovery-->>manager: "show-recovery-prompt"
            manager->>storage: getActionableNamedBackupRestores()
            storage-->>manager: ActionableNamedBackupRecoveries
            manager->>user: display recovery prompt

            alt user selects backup
                user->>manager: select backup
                manager->>recovery: runBackupRestoreManager(displaySettings, assessments)
                recovery->>storage: restoreNamedBackup(name)
                storage->>fs: read & import backup
                fs-->>storage: imported accounts
                storage-->>recovery: { imported, total, skipped }
                recovery-->>manager: "restored"
                manager-->>user: restoration success
            else user skips
                user->>manager: skip recovery
                manager->>manager: proceed to oauth
            end
        else
            manager->>user: proceed to oauth
        end
    else non-interactive
        manager->>user: suppress recovery prompt, continue fallback
    end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Suggested labels

bug

key concerns for review

missing regression coverage: tests removed public test-only hooks in test/storage.test.ts:1. verify that removal of assertNamedBackupRestorePath, resolveNamedBackupRestorePath, and related exports did not leave gaps in validation for named-backup path containment and transient error handling. check callers and replace with new apis where needed (lib/storage.ts).
concurrency risk in recovery flow: lib/codex-manager.ts:60-90 introduces mutable state (suppressRecoveryPrompt, recoveryPromptAttempted, allowEmptyStorageMenu, pendingRecoveryState). audit for race conditions when login re-enters or simultaneous recovery scans/runBackupRestoreManager are triggered. ensure proper locking/serialisation.
windows edge case - filesystem mtime: test/storage-recovery-paths.test.ts:1 now uses fs.utimes to bias backup freshness. validate behavior on windows (mtime precision and timezone) and ensure getActionableNamedBackupRestores() uses a stable tiebreaker across platforms.
return type surface change: lib/storage.ts changed importAccounts() / restoreNamedBackup() to return { imported, total, skipped } (removed changed). audit all callers in lib/codex-manager.ts and lib/cli.ts for references to changed or legacy shapes. (lib/codex-manager.ts has many integrations).
public api additions - error handling: new StorageError and formatStorageErrorHint() in lib/storage.ts. review redaction logic for paths and ensure no sensitive info is leaked to UI or logs.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	⚠️ Warning	PR description is comprehensive with summary, changes, validation checklist (all checked), docs updates, risk level, and rollback plan; however, greptile flagged critical safety regressions and lint issues that are not addressed in the description.	Address greptile's four critical issues before merge: (1) restore proportional jitter in retryTransientFilesystemOperation (lib/storage.ts:1082), (2) reinstate eligibility check in restoreNamedBackup (lib/storage.ts:1316), (3) remove or prefix unused warnSpy vars with _ in test/codex-manager-cli.test.ts:1942/1950/1958, and (4) restore path containment check via realpathSync.native to block NTFS junctions (lib/storage.ts:1579).

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	title follows conventional commits format with type `fix`, scope `auth`, and lowercase summary within 72 chars.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings (stacked PR)
📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fresh/03-startup-recovery-prompt

✨ Simplify code

Create PR with simplified code
Commit simplified code in branch fresh/03-startup-recovery-prompt

📝 Coding Plan

Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Tip

CodeRabbit can generate a title for your PR based on the changes with custom instructions.

Set the reviews.auto_title_instructions setting to generate a title for your PR based on the changes in the PR with custom instructions.

Copilot

Pull request overview

Reintroduces and wires up a startup recovery prompt to offer named-backup restore options before OAuth when interactive login starts with an empty account store.

Changes:

Adds getActionableNamedBackupRestores() + startup decision logic (resolveStartupRecoveryAction) to drive the recovery prompt flow.
Refactors named-backup listing/assessment/restore plumbing and related tests.
Updates onboarding/upgrade/troubleshooting docs to describe the new startup recovery prompt behavior.

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
lib/storage.ts	Adds actionable-backup scanning API; refactors backup listing/assessment/restore + import/export behavior.
lib/codex-manager.ts	Implements startup recovery prompt flow and restore-backup command refactor.
lib/cli.ts	Centralizes “interactive menu available” detection for login/menu flows.
test/storage.test.ts	Updates storage/backup tests to match new restore/import behaviors and isolation patterns.
test/storage-recovery-paths.test.ts	Adjusts backup metadata ordering expectations by manipulating checkpoint mtimes.
test/recovery.test.ts	Adds coverage for new storage-backed recovery scanning + refactors existing recovery tests.
lib/ui/copy.ts	Removes restore-backup UI copy helpers in favor of inline messaging.
docs/getting-started.md	Documents the startup recovery prompt in the login flow.
docs/upgrade.md	Adds an upgrade note explaining the new startup recovery prompt behavior.
docs/troubleshooting.md	Notes when/why startup recovery prompting occurs.
docs/reference/public-api.md	Removes note about `importAccounts().changed` return shape.

Comments suppressed due to low confidence (1)

lib/storage.ts:1

The import result math can produce invalid values: imported can be negative if existingAccounts contains duplicates (since it’s not deduped), and skipped can become negative or exceed total as a result. Align the counting with the same deduplication strategy used to build deduplicatedAccounts (dedupe the existing snapshot for baseline), and clamp skipped to >= 0 after using consistent units (deduped incoming vs. raw incoming).

import { AsyncLocalStorage } from "node:async_hooks";

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

lib/storage.ts

 	const mergedAccounts = deduplicateAccounts([
-		...currentDeduplicatedAccounts,
-		...incomingDeduplicatedAccounts,
+		...currentAccounts,
+		...candidate.normalized.accounts,
 	]);
 	const wouldExceedLimit = mergedAccounts.length > ACCOUNT_LIMITS.MAX_ACCOUNTS;
 	const imported = wouldExceedLimit
 		? null
-		: mergedAccounts.length - currentDeduplicatedAccounts.length;
+		: mergedAccounts.length - currentAccounts.length;
 	const skipped = wouldExceedLimit
 		? null
-		: Math.max(0, incomingDeduplicatedAccounts.length - (imported ?? 0));
-	const changed = !haveEquivalentAccountRows(
-		mergedAccounts,
-		currentDeduplicatedAccounts,
-	);
+		: Math.max(0, candidate.normalized.accounts.length - (imported ?? 0));


lib/storage.ts

 	const transactionState = transactionSnapshotContext.getStore();
-	const currentStoragePath = normalizeStoragePathForComparison(getStoragePath());
+	const currentStoragePath = getStoragePath();
 	const storage = transactionState?.active
-		? normalizeStoragePathForComparison(transactionState.storagePath) ===
-			currentStoragePath
+		? transactionState.storagePath === currentStoragePath
 			? transactionState.snapshot


lib/storage.ts

 export async function importAccounts(
 	filePath: string,
-): Promise<ImportAccountsResult> {
+): Promise<{ imported: number; total: number; skipped: number }> {


lib/storage.ts

-		}
-		throw error;
-	}
+	const content = await fs.readFile(resolvedPath, "utf-8");


lib/codex-manager.ts

-	if (timestamp === null || timestamp === undefined || timestamp === 0)
-		return null;
-	if (!Number.isFinite(timestamp)) return null;
+	if (timestamp === null || timestamp === undefined) return null;


test/recovery.test.ts

+async function removeWithRetry(targetPath: string): Promise<void> {
+	for (let attempt = 0; attempt < 5; attempt += 1) {
+		try {
+			await fs.rm(targetPath, { recursive: true, force: true });
+			return;
+		} catch (error) {
+			const code = (error as NodeJS.ErrnoException).code;
+			if (
+				(code !== "EBUSY" && code !== "EPERM" && code !== "ENOTEMPTY") ||
+				attempt === 4
+			) {
+				throw error;
+			}
+			await new Promise((resolve) => setTimeout(resolve, 25 * 2 ** attempt));
+		}
+	}
+}


lib/storage.ts

lib/codex-manager.ts

lib/storage.ts

lib/codex-manager.ts

coderabbitai

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

lib/storage.ts (1)
1942-1985: ⚠️ Potential issue | 🟡 Minor

compute imported against the deduped current state here.

lib/storage.ts:1942-1985 still subtracts currentAccounts.length, while the write path in lib/storage.ts:3008-3042 was already fixed to measure against deduped existing storage. if a caller passes duplicate-laden currentStorage, getActionableNamedBackupRestores() can drop a genuinely useful restore because the assessment.imported > 0 filter in lib/storage.ts:1849-1857 sees 0 or a negative count. there is no matching assessment regression beside test/storage.test.ts:587-630.
suggested fix
 function assessNamedBackupRestoreCandidate(
 	backup: NamedBackupMetadata,
 	candidate: LoadedBackupCandidate,
 	currentStorage: AccountStorageV3 | null,
 ): BackupRestoreAssessment {
 	const currentAccounts = currentStorage?.accounts ?? [];
+	const effectiveCurrentAccounts = deduplicateAccounts([...currentAccounts]);

 	if (!candidate.normalized || !backup.accountCount || backup.accountCount <= 0) {
 		return {
 			backup,
@@
 	}

 	const mergedAccounts = deduplicateAccounts([
-		...currentAccounts,
+		...effectiveCurrentAccounts,
 		...candidate.normalized.accounts,
 	]);
 	const wouldExceedLimit = mergedAccounts.length > ACCOUNT_LIMITS.MAX_ACCOUNTS;
 	const imported = wouldExceedLimit
 		? null
-		: mergedAccounts.length - currentAccounts.length;
+		: mergedAccounts.length - effectiveCurrentAccounts.length;
as per coding guidelines, lib/**: focus on auth rotation, windows filesystem IO, and concurrency. verify every change cites affected tests (vitest) and that new queues handle EBUSY/429 scenarios. check for logging that leaks tokens or emails.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/storage.ts` around lines 1942 - 1985, In
assessNamedBackupRestoreCandidate, imported and skipped are currently computed
against currentAccounts.length (which may include duplicates); change the
calculations to use the deduplicated current state (mergedAccounts minus
deduplicated current count) — i.e., compute dedupedCurrentCount =
deduplicateAccounts(currentAccounts).length (or use mergedAccounts.length and
dedupedCurrentCount) and set imported = wouldExceedLimit ? null :
mergedAccounts.length - dedupedCurrentCount, and recalc skipped =
wouldExceedLimit ? null : Math.max(0, candidate.normalized.accounts.length -
(imported ?? 0)); keep wouldExceedLimit and error logic intact and ensure
eligibleForRestore uses !wouldExceedLimit.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/cli.ts`:
- Around line 266-268: The check calling isInteractiveLoginMenuAvailable()
before returning promptLoginModeFallback(existingAccounts) is redundant because
the same non-interactive early return gate is already handled earlier; remove
the conditional block that wraps promptLoginModeFallback(existingAccounts) (the
isInteractiveLoginMenuAvailable() check and its return) so
promptLoginModeFallback is only reached via the existing early-return logic,
leaving other code paths and function names (isInteractiveLoginMenuAvailable,
promptLoginModeFallback) intact.

In `@lib/codex-manager.ts`:
- Around line 4391-4411: Wrap each assessNamedBackupRestore(backup.name, {
currentStorage }) call with a transient-retry wrapper before invoking
Promise.allSettled: on error check error.code (handle at least
"EBUSY","EPERM","EACCES","EAGAIN" and HTTP 429-like transient indications) and
retry with small exponential backoff (e.g., 3 total attempts with increasing
delays) logging each retry attempt but avoid printing sensitive data
(tokens/emails); only if all attempts fail, let the rejection propagate so the
existing settledAssessments handling logs the final skip message using
collapseWhitespace and backup.name as before; update or add vitest cases
exercising transient EBUSY/EPERM and 429 behavior to assert retries occur and
that final logs do not leak secrets.
- Around line 4062-4142: Add vitest regression tests exercising the multi-branch
startup recovery flow in codex-manager: simulate
getActionableNamedBackupRestores throwing a filesystem error (use
getRedactedFilesystemErrorLabel expectations) to assert the scan failure path
logs and continues OAuth; simulate it returning empty assessments to assert
resolveStartupRecoveryAction triggers the "open-empty-storage-menu" branch
(allowEmptyStorageMenu behavior); simulate recoverable assessments where
runBackupRestoreManager returns a non-"restored" value to assert
pendingRecoveryState is set and recoveryPromptAttempted is reset; and simulate
confirm/restore interactions where confirm throws to exercise the prompt failure
branch ensuring recoveryPromptAttempted is reset only when promptWasShown is
false; stub/mocks to manipulate applyUiThemeFromDashboardSettings,
loadDashboardDisplaySettings, getNamedBackupsDirectoryPath, and confirm to
isolate behavior and assert state transitions on recoveryPromptAttempted and
pendingRecoveryState.

In `@lib/storage.ts`:
- Around line 1988-1993: The change removed the exported boolean "changed" from
result objects, causing a silent API break; update the functions that return the
import result (e.g., restoreNamedBackup and the functions around the other diff
region that currently return { imported, total, skipped }) to include a boolean
changed property derived as (imported > 0) before returning, and ensure
importAccounts (and any other exported helpers that produce the result type)
also returns the same shape so callers still see result.changed.
- Around line 1720-1770: The current catch in listNamedBackupsWithoutLoading
collapses any readdir failure into an empty listing; change the error handling
so that only ENOENT returns {backups: [], totalBackups: 0} and any other error
is rethrown (i.e., in the catch for the top-level
retryTransientFilesystemOperation call inspect (error as
NodeJS.ErrnoException).code and if !== "ENOENT" throw the error), leaving the
per-entry ENOENT swallowing in place; update callers/tests that rely on the
distinction (the recovery tests referenced around the existing test ranges) to
expect a thrown error for locked/EBUSY directory cases instead of an empty
result.
- Around line 2987-2994: Remove the pre-check that uses existsSync and instead
let fs.readFile (inside retryTransientFilesystemOperation) surface errors; catch
errors from the read of resolvedPath and translate an ENOENT into the friendly
throw new Error(`Import file not found: ${resolvedPath}`) while rethrowing other
errors unchanged. Update the block that calls
retryTransientFilesystemOperation(() => fs.readFile(resolvedPath, "utf-8")) to
wrap that call in a try/catch, check err.code === 'ENOENT' and throw the
user-friendly message; keep retryTransientFilesystemOperation, resolvedPath, and
fs.readFile references so the change is localized. Also adjust/extend
test/storage.test.ts:1356-1410 to add a delete-between-check-and-read scenario
(simulate removal during the operation) to ensure the ENOENT branch is covered
and no raw fs errors leak.

---

Outside diff comments:
In `@lib/storage.ts`:
- Around line 1942-1985: In assessNamedBackupRestoreCandidate, imported and
skipped are currently computed against currentAccounts.length (which may include
duplicates); change the calculations to use the deduplicated current state
(mergedAccounts minus deduplicated current count) — i.e., compute
dedupedCurrentCount = deduplicateAccounts(currentAccounts).length (or use
mergedAccounts.length and dedupedCurrentCount) and set imported =
wouldExceedLimit ? null : mergedAccounts.length - dedupedCurrentCount, and
recalc skipped = wouldExceedLimit ? null : Math.max(0,
candidate.normalized.accounts.length - (imported ?? 0)); keep wouldExceedLimit
and error logic intact and ensure eligibleForRestore uses !wouldExceedLimit.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 36ff8d22-40b5-4db9-b368-e1d6b7e7891b

📥 Commits

Reviewing files that changed from the base of the PR and between 8212831 and a7d87cd.

📒 Files selected for processing (12)

docs/getting-started.md
docs/reference/public-api.md
docs/troubleshooting.md
docs/upgrade.md
lib/cli.ts
lib/codex-manager.ts
lib/storage.ts
lib/ui/copy.ts
test/codex-manager-cli.test.ts
test/recovery.test.ts
test/storage-recovery-paths.test.ts
test/storage.test.ts

💤 Files with no reviewable changes (2)

docs/reference/public-api.md
lib/ui/copy.ts

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: Greptile Review

🧰 Additional context used

📓 Path-based instructions (3)

docs/**

⚙️ CodeRabbit configuration file

keep README, SECURITY, and docs consistent with actual CLI flags and workflows. whenever behavior changes, require updated upgrade notes and mention new npm scripts.

Files:

docs/getting-started.md
docs/troubleshooting.md
docs/upgrade.md

lib/**

⚙️ CodeRabbit configuration file

focus on auth rotation, windows filesystem IO, and concurrency. verify every change cites affected tests (vitest) and that new queues handle EBUSY/429 scenarios. check for logging that leaks tokens or emails.

Files:

lib/cli.ts
lib/storage.ts
lib/codex-manager.ts

test/**

⚙️ CodeRabbit configuration file

tests must stay deterministic and use vitest. demand regression cases that reproduce concurrency bugs, token refresh races, and windows filesystem behavior. reject changes that mock real secrets or skip assertions.

Files:

test/storage.test.ts
test/storage-recovery-paths.test.ts
test/recovery.test.ts

lib/cli.ts

coderabbitai · 2026-03-16T11:31:35Z

lib/codex-manager.ts

+		const canPromptForRecovery =
+			!suppressRecoveryPrompt &&
+			!recoveryPromptAttempted &&
+			existingCount === 0 &&
+			isInteractiveLoginMenuAvailable();
+		if (canPromptForRecovery) {
+			recoveryPromptAttempted = true;
+			let recoveryState: Awaited<
+				ReturnType<typeof getActionableNamedBackupRestores>
+			> | null = pendingRecoveryState;
+			pendingRecoveryState = null;
+			if (recoveryState === null) {
+				let recoveryScanFailed = false;
+				let scannedRecoveryState: Awaited<
+					ReturnType<typeof getActionableNamedBackupRestores>
+				>;
+				try {
+					scannedRecoveryState = await getActionableNamedBackupRestores({
+						currentStorage: refreshedStorage,
+					});
+				} catch (error) {
+					recoveryScanFailed = true;
+					const errorLabel = getRedactedFilesystemErrorLabel(error);
+					console.warn(
+						`Startup recovery scan failed (${errorLabel}). Continuing with OAuth.`,
+					);
+					scannedRecoveryState = {
+						assessments: [],
+						allAssessments: [],
+						totalBackups: 0,
+					};
+				}
+				recoveryState = scannedRecoveryState;
+				if (
+					resolveStartupRecoveryAction(scannedRecoveryState, recoveryScanFailed) ===
+					"open-empty-storage-menu"
+				) {
+					allowEmptyStorageMenu = true;
+					continue loginFlow;
+				}
+			}
+			if (recoveryState.assessments.length > 0) {
+				let promptWasShown = false;
+				try {
+					const displaySettings = await loadDashboardDisplaySettings();
+					applyUiThemeFromDashboardSettings(displaySettings);
+					const backupDir = getNamedBackupsDirectoryPath();
+					const backupLabel =
+						recoveryState.assessments.length === 1
+							? recoveryState.assessments
+									.map((assessment) => assessment.backup.name)
+									.join("")
+							: `${recoveryState.assessments.length} backups`;
+					promptWasShown = true;
+					const restoreNow = await confirm(
+						`Found ${recoveryState.assessments.length} recoverable backup${
+							recoveryState.assessments.length === 1 ? "" : "s"
+						} out of ${recoveryState.totalBackups} total (${backupLabel}) in ${backupDir}. Restore now?`,
+					);
+					if (restoreNow) {
+						const restoreResult = await runBackupRestoreManager(
+							displaySettings,
+							recoveryState.allAssessments,
+						);
+						if (restoreResult !== "restored") {
+							pendingRecoveryState = recoveryState;
+							recoveryPromptAttempted = false;
+						}
+						continue;
+					}
+				} catch (error) {
+					if (!promptWasShown) {
+						recoveryPromptAttempted = false;
+					}
+					const errorLabel = getRedactedFilesystemErrorLabel(error);
+					console.warn(
+						`Startup recovery prompt failed (${errorLabel}). Continuing with OAuth.`,
+					);
+				}
+			}
+		}


⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash set -euo pipefail echo "== candidate recovery/auth tests ==" fd -t f '.*(recovery|auth|login).*test\.(ts|tsx)$' test echo echo "== references to new startup-recovery symbols in tests ==" rg -n -C2 'resolveStartupRecoveryAction|show-recovery-prompt|open-empty-storage-menu|continue-with-oauth|suppressRecoveryPrompt|recoveryPromptAttempted|allowEmptyStorageMenu|pendingRecoveryState' test echo echo "== references to fresh/reset suppression in tests ==" rg -n -C2 'fresh|reset|startup recovery|recovery prompt' test

Repository: ndycode/codex-multi-auth

Length of output: 50381

🏁 Script executed:

wc -l test/recovery.test.ts

Repository: ndycode/codex-multi-auth

Length of output: 94

🏁 Script executed:

sed -n '783,900p' test/recovery.test.ts

Repository: ndycode/codex-multi-auth

Length of output: 3861

🏁 Script executed:

rg -n 'suppressRecoveryPrompt|recoveryPromptAttempted|pendingRecoveryState|allowEmptyStorageMenu' test/recovery.test.ts

Repository: ndycode/codex-multi-auth

Length of output: 50

🏁 Script executed:

rg -n -A5 'restore.*manager|runBackupRestoreManager' test/recovery.test.ts

Repository: ndycode/codex-multi-auth

Length of output: 50

🏁 Script executed:

sed -n '4062,4142p' lib/codex-manager.ts

Repository: ndycode/codex-multi-auth

Length of output: 2719

🏁 Script executed:

rg -n 'canPromptForRecovery|suppressRecoveryPrompt|recoveryPromptAttempted|pendingRecoveryState' lib/codex-manager.ts | head -20

Repository: ndycode/codex-multi-auth

Length of output: 709

🏁 Script executed:

fd -t f '.*(integration|e2e|startup).*test\.(ts|tsx)$' test

Repository: ndycode/codex-multi-auth

Length of output: 139

🏁 Script executed:

rg -n 'loginFlow\|runBackupRestoreManager' test/ --type ts -A3 | head -30

Repository: ndycode/codex-multi-auth

Length of output: 50

🏁 Script executed:

rg -n 'loginFlow|startup.*recovery|recovery.*prompt' test/oauth-server.integration.test.ts test/rotation-integration.test.ts

Repository: ndycode/codex-multi-auth

Length of output: 50

🏁 Script executed:

rg -n 'EBUSY|429|rate.*limit' lib/codex-manager.ts | head -20

Repository: ndycode/codex-multi-auth

Length of output: 711

🏁 Script executed:

rg -n 'getRedactedFilesystemErrorLabel' lib/codex-manager.ts -B2 -A5 | head -40

Repository: ndycode/codex-multi-auth

Length of output: 1321

🏁 Script executed:

rg -n -C3 'console.warn.*recovery' lib/codex-manager.ts

Repository: ndycode/codex-multi-auth

Length of output: 50

add vitest regression coverage for startup recovery state transitions.

lib/codex-manager.ts:4062-4142 adds a multi-branch state machine (suppressRecoveryPrompt, recoveryPromptAttempted, pendingRecoveryState, allowEmptyStorageMenu) with only unit test coverage of the resolveStartupRecoveryAction() helper. missing regression tests for:

scan failure recovery path (continues oauth, logs redacted error)

empty scan path (re-enters empty storage menu)

same-session suppression reset when restore manager returns non-"restored" value (lib/codex-manager.ts:4127-4128)

prompt failure path with promptWasShown guard resetting recoveryPromptAttempted (lib/codex-manager.ts:4134)

also add explicit coverage for windows filesystem io errors (ebusy scenarios) in the recovery scan path (lib/codex-manager.ts:4082-4087), since the current implementation catches but does not retry on transient errors.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@lib/codex-manager.ts` around lines 4062 - 4142, Add vitest regression tests exercising the multi-branch startup recovery flow in codex-manager: simulate getActionableNamedBackupRestores throwing a filesystem error (use getRedactedFilesystemErrorLabel expectations) to assert the scan failure path logs and continues OAuth; simulate it returning empty assessments to assert resolveStartupRecoveryAction triggers the "open-empty-storage-menu" branch (allowEmptyStorageMenu behavior); simulate recoverable assessments where runBackupRestoreManager returns a non-"restored" value to assert pendingRecoveryState is set and recoveryPromptAttempted is reset; and simulate confirm/restore interactions where confirm throws to exercise the prompt failure branch ensuring recoveryPromptAttempted is reset only when promptWasShown is false; stub/mocks to manipulate applyUiThemeFromDashboardSettings, loadDashboardDisplaySettings, getNamedBackupsDirectoryPath, and confirm to isolate behavior and assert state transitions on recoveryPromptAttempted and pendingRecoveryState.

coderabbitai · 2026-03-16T11:31:35Z

lib/codex-manager.ts

 		const settledAssessments = await Promise.allSettled(
 			chunk.map((backup) =>
-				assessNamedBackupRestore(backup.name, {
-					currentStorage,
-					candidateCache,
-				}),
+				assessNamedBackupRestore(backup.name, { currentStorage }),
 			),
 		);
 		for (const [resultIndex, result] of settledAssessments.entries()) {
 			if (result.status === "fulfilled") {
 				assessments.push(result.value);
 				continue;
 			}
-			if (isNamedBackupContainmentError(result.reason)) {
-				throw result.reason;
-			}
 			const backupName = chunk[resultIndex]?.name ?? "unknown";
 			const reason =
 				result.reason instanceof Error
 					? result.reason.message
 					: String(result.reason);
-			const normalizedReason =
-				collapseWhitespace(reason) || "unknown error";
-			assessmentFailures.push(`${backupName}: ${normalizedReason}`);
 			console.warn(
-				`Skipped backup assessment for "${backupName}": ${normalizedReason}`,
+				`Skipped backup assessment for "${backupName}": ${
+					collapseWhitespace(reason) || "unknown error"
+				}`,
 			);
 		}


⚠️ Potential issue | 🟠 Major

add transient fs retry before skipping backup assessments.

in lib/codex-manager.ts:4391-4411, assessment failures are skipped immediately. on windows, transient ebusy/eperm can hide recoverable backups and suppress the startup recovery prompt for this run.

proposed hardening

+async function assessNamedBackupWithRetry( + backupName: string, + currentStorage: Awaited<ReturnType<typeof loadAccounts>>, + maxAttempts = 3, +): Promise<BackupRestoreAssessment> { + let lastError: unknown; + for (let attempt = 1; attempt <= maxAttempts; attempt += 1) { + try { + return await assessNamedBackupRestore(backupName, { currentStorage }); + } catch (error) { + lastError = error; + const code = (error as { code?: string })?.code; + const retryable = code === "EBUSY" || code === "EPERM" || code === "EMFILE"; + if (!retryable || attempt === maxAttempts) throw error; + } + } + throw lastError; +} ... - const settledAssessments = await Promise.allSettled( - chunk.map((backup) => - assessNamedBackupRestore(backup.name, { currentStorage }), - ), - ); + const settledAssessments = await Promise.allSettled( + chunk.map((backup) => + assessNamedBackupWithRetry(backup.name, currentStorage), + ), + );

As per coding guidelines, lib/**: focus on auth rotation, windows filesystem IO, and concurrency. verify every change cites affected tests (vitest) and that new queues handle EBUSY/429 scenarios. check for logging that leaks tokens or emails.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@lib/codex-manager.ts` around lines 4391 - 4411, Wrap each assessNamedBackupRestore(backup.name, { currentStorage }) call with a transient-retry wrapper before invoking Promise.allSettled: on error check error.code (handle at least "EBUSY","EPERM","EACCES","EAGAIN" and HTTP 429-like transient indications) and retry with small exponential backoff (e.g., 3 total attempts with increasing delays) logging each retry attempt but avoid printing sensitive data (tokens/emails); only if all attempts fail, let the rejection propagate so the existing settledAssessments handling logs the final skip message using collapseWhitespace and backup.name as before; update or add vitest cases exercising transient EBUSY/EPERM and 429 behavior to assert retries occur and that final logs do not leak secrets.

lib/storage.ts

test/recovery.test.ts

lib/storage.ts

…recovery-prompt

coderabbitai

Actionable comments posted: 5

♻️ Duplicate comments (2)

lib/codex-manager.ts (2)
3840-3851: ⚠️ Potential issue | 🟠 Major

add vitest coverage for the new startup recovery state machine before merge.

line [4064] through line [4144] introduces multi-flag control flow that can regress silently without branch tests. i do not see evidence in this patch context that all new transitions are asserted (scan failure, open-empty-menu routing, prompt failure before/after prompt shown, non-restored manager result resetting state). refs: lib/codex-manager.ts:3840, lib/codex-manager.ts:4064, test/recovery.test.ts:783.
#!/bin/bash
set -euo pipefail

echo "== startup recovery symbols in tests =="
rg -n -C3 --type=ts 'resolveStartupRecoveryAction|open-empty-storage-menu|recoveryPromptAttempted|pendingRecoveryState|suppressRecoveryPrompt|Startup recovery scan failed|Startup recovery prompt failed' test

echo
echo "== backup manager result branches in tests =="
rg -n -C3 --type=ts 'runBackupRestoreManager|restored|dismissed|failed' test/recovery.test.ts test/codex-manager-cli.test.ts
as per coding guidelines, "lib/**: focus on auth rotation, windows filesystem io, and concurrency. verify every change cites affected tests (vitest) and that new queues handle EBUSY/429 scenarios."

Also applies to: 4064-4144
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/codex-manager.ts` around lines 3840 - 3851, Add vitest unit tests to
cover every branch of the new startup recovery state machine: exercise
resolveStartupRecoveryAction with recoveryState.assessments > 0,
recoveryScanFailed true, totalBackups > 0, and the default
open-empty-storage-menu case; add tests that simulate prompt flow toggles
(pendingRecoveryState, recoveryPromptAttempted, suppressRecoveryPrompt) both
before and after the prompt is shown and when the prompt fails; add tests that
stub/mock runBackupRestoreManager to return restored, dismissed, and failed
outcomes and assert the manager result correctly resets/updates state (e.g.,
pendingRecoveryState cleared, suppressRecoveryPrompt set/unset); put these tests
in test/recovery.test.ts (or adjacent vitest files), using the same symbols
referenced in the diff (resolveStartupRecoveryAction, pendingRecoveryState,
recoveryPromptAttempted, suppressRecoveryPrompt, runBackupRestoreManager,
restored, dismissed, failed) and ensure you assert transitions for the
scan-failure and open-empty-menu routing paths.
4387-4413: ⚠️ Potential issue | 🟠 Major

cover transient filesystem behavior for the backup assessment queue with vitest.

line [4393] batches restore assessments concurrently. this path should have explicit tests for transient EBUSY/EPERM and terminal failure behavior so queue semantics stay stable across windows io edge cases. refs: lib/codex-manager.ts:4387, lib/codex-manager.ts:4393, test/recovery.test.ts:1.
#!/bin/bash
set -euo pipefail

echo "== queue + fs transient handling in source =="
rg -n -C3 --type=ts 'NAMED_BACKUP_LIST_CONCURRENCY|Promise.allSettled|assessNamedBackupRestore|EBUSY|EPERM|EAGAIN' lib/codex-manager.ts lib/storage.ts

echo
echo "== corresponding vitest coverage =="
rg -n -C3 --type=ts 'EBUSY|EPERM|EAGAIN|retryTransientFilesystemOperation|assessNamedBackupRestore|loadBackupRestoreManagerAssessments' test
as per coding guidelines, "lib/**: focus on auth rotation, windows filesystem io, and concurrency. verify every change cites affected tests (vitest) and that new queues handle EBUSY/429 scenarios."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/codex-manager.ts` around lines 4387 - 4413, Add vitest coverage that
simulates transient Windows FS errors and terminal failures for the concurrent
batch path that uses NAMED_BACKUP_LIST_CONCURRENCY and Promise.allSettled in
loadBackupRestoreManagerAssessments/assessNamedBackupRestore: write tests in
test/recovery.test.ts that stub/mock the underlying filesystem calls (or stub
assessNamedBackupRestore) to first throw transient errors (EBUSY/EPERM/EAGAIN)
and then succeed, asserting the batch retry semantics allow the assessment to
eventually be pushed into assessments, and a separate test where the stub throws
a terminal error ensuring the code logs/skips the item (matching the
console.warn branch); use vitest vi.fn/mocks and, if code uses
retryTransientFilesystemOperation, assert it is invoked, otherwise assert
behavior by controlling timing and promises so the Promise.allSettled loop
handles transient vs terminal cases correctly.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/storage.ts`:
- Around line 3030-3039: The code currently calls deduplicateAccounts(merged)
twice in the import path (once inside the limit check and again when assigning
deduplicatedAccounts); compute dedupe once and reuse it: call const deduplicated
= deduplicateAccounts(merged) before the ACCOUNT_LIMITS.MAX_ACCOUNTS check, use
deduplicated.length in the guard and then assign deduplicatedAccounts =
deduplicated (or rename consistently) so you avoid redundant work on large
imports and keep references to merged, deduplicateAccounts, and
ACCOUNT_LIMITS.MAX_ACCOUNTS intact.
- Around line 1779-1806: Add Windows-specific tests that mock process.platform =
"win32" and assert that retryTransientFilesystemOperation actually retries on
EPERM and EBUSY: for each code, create a fake operation that throws a
NodeJS.ErrnoException with .code set to "EPERM" or "EBUSY" for the first N
attempts then resolves, call retryTransientFilesystemOperation and assert it
eventually returns and that the fake was called multiple times; restore
process.platform after each test. Use the same pattern to keep the Linux tests
(process.platform !== "win32") asserting EPERM/EBUSY rethrow immediately, and
reference the functions isRetryableFilesystemErrorCode and
retryTransientFilesystemOperation when locating the code under test.

In `@test/recovery.test.ts`:
- Around line 380-453: The test "keeps actionable backups when fast-path scan
hits EBUSY" relies on platform-specific EBUSY handling; wrap the part that calls
getActionableNamedBackupRestores with an explicit process.platform override
(save original, use Object.defineProperty to set process.platform to 'win32' for
the Windows behavior the test expects, then restore original afterward) so the
expectations remain deterministic; apply the same pattern to the other related
tests mentioned (the ones calling getActionableNamedBackupRestores and asserting
EBUSY behavior) to ensure consistent platform-specific behavior.
- Around line 783-815: Add regression tests for resolveStartupRecoveryAction to
cover the missing branches: call resolveStartupRecoveryAction with an input
object where assessments contains at least one actionable assessment (e.g.,
assessments: [{ actionable: true }]) and totalBackups > 0 and scanFailed = false
and assert it returns the recovery prompt route (the code path currently implied
by "open-empty-storage-menu"/recovery prompt); then add a test where assessments
contains an actionable item but scanning is suppressed (pass the
suppression/scanFailed flag that triggers same-session suppression) and assert
it returns "continue-with-oauth". Keep the existing tests for zero backups/no
scan error as-is. Reference resolveStartupRecoveryAction and the input shape
(assessments, totalBackups, scanFailed) when adding the two new cases.

In `@test/storage.test.ts`:
- Around line 1605-1617: The two tests are ambiguous about platform: ensure the
non-Windows test for EBUSY retry uses a mocked process.platform returning
"linux" and the Windows-retry test uses process.platform returning "win32" so
behavior is deterministic; in the non-Windows case (test around
listNamedBackups) keep vi.spyOn(process, "platform",
"get").mockReturnValue("linux") and assert readdir was called once, and in the
separate Windows-retry test (lines ~1638-1678) mock process.platform to "win32"
and set readdir to initially reject with an EBUSY error then succeed on retry to
assert retry behavior; also restore/clear spies/mocks after each test to avoid
cross-test leakage.

---

Duplicate comments:
In `@lib/codex-manager.ts`:
- Around line 3840-3851: Add vitest unit tests to cover every branch of the new
startup recovery state machine: exercise resolveStartupRecoveryAction with
recoveryState.assessments > 0, recoveryScanFailed true, totalBackups > 0, and
the default open-empty-storage-menu case; add tests that simulate prompt flow
toggles (pendingRecoveryState, recoveryPromptAttempted, suppressRecoveryPrompt)
both before and after the prompt is shown and when the prompt fails; add tests
that stub/mock runBackupRestoreManager to return restored, dismissed, and failed
outcomes and assert the manager result correctly resets/updates state (e.g.,
pendingRecoveryState cleared, suppressRecoveryPrompt set/unset); put these tests
in test/recovery.test.ts (or adjacent vitest files), using the same symbols
referenced in the diff (resolveStartupRecoveryAction, pendingRecoveryState,
recoveryPromptAttempted, suppressRecoveryPrompt, runBackupRestoreManager,
restored, dismissed, failed) and ensure you assert transitions for the
scan-failure and open-empty-menu routing paths.
- Around line 4387-4413: Add vitest coverage that simulates transient Windows FS
errors and terminal failures for the concurrent batch path that uses
NAMED_BACKUP_LIST_CONCURRENCY and Promise.allSettled in
loadBackupRestoreManagerAssessments/assessNamedBackupRestore: write tests in
test/recovery.test.ts that stub/mock the underlying filesystem calls (or stub
assessNamedBackupRestore) to first throw transient errors (EBUSY/EPERM/EAGAIN)
and then succeed, asserting the batch retry semantics allow the assessment to
eventually be pushed into assessments, and a separate test where the stub throws
a terminal error ensuring the code logs/skips the item (matching the
console.warn branch); use vitest vi.fn/mocks and, if code uses
retryTransientFilesystemOperation, assert it is invoked, otherwise assert
behavior by controlling timing and promises so the Promise.allSettled loop
handles transient vs terminal cases correctly.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: fad95d8e-47a4-43f3-a7eb-b8866b1fb048

📥 Commits

Reviewing files that changed from the base of the PR and between a7d87cd and 800d49f.

📒 Files selected for processing (7)

lib/cli.ts
lib/codex-manager.ts
lib/storage.ts
lib/ui/copy.ts
test/codex-manager-cli.test.ts
test/recovery.test.ts
test/storage.test.ts

💤 Files with no reviewable changes (1)

lib/ui/copy.ts

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: Greptile Review

🧰 Additional context used

📓 Path-based instructions (2)

lib/**

⚙️ CodeRabbit configuration file

focus on auth rotation, windows filesystem IO, and concurrency. verify every change cites affected tests (vitest) and that new queues handle EBUSY/429 scenarios. check for logging that leaks tokens or emails.

Files:

lib/cli.ts
lib/codex-manager.ts
lib/storage.ts

test/**

⚙️ CodeRabbit configuration file

tests must stay deterministic and use vitest. demand regression cases that reproduce concurrency bugs, token refresh races, and windows filesystem behavior. reject changes that mock real secrets or skip assertions.

Files:

test/recovery.test.ts
test/storage.test.ts

🔇 Additional comments (3)

lib/cli.ts (1)

29-31: good consolidation of interactive gating.

this keeps tty/non-interactive checks centralized and makes menu fallback behavior predictable. refs: lib/cli.ts:29, lib/cli.ts:258.

Also applies to: 258-260

lib/codex-manager.ts (1)

4510-4529: good fix: restore path now syncs codex active account.

post-restore sync closes the direct-restore drift between multi-auth state and codex cli state. refs: lib/codex-manager.ts:4510, lib/codex-manager.ts:4515.

lib/storage.ts (1)

2995-3005: nice hardening on import file read/enoent handling.

this correctly translates read-time ENOENT into a stable user-facing error and avoids pre-check race issues. refs: lib/storage.ts:2995, lib/storage.ts:3001.

coderabbitai · 2026-03-16T16:14:09Z

lib/storage.ts

 function isRetryableFilesystemErrorCode(
 	code: string | undefined,
 ): code is "EPERM" | "EBUSY" | "EAGAIN" {
 	if (code === "EAGAIN") {
 		return true;
 	}
-	if (process.platform !== "win32") {
-		return false;
-	}
-	return code === "EPERM" || code === "EBUSY";
+	return (code === "EPERM" || code === "EBUSY") && process.platform === "win32";
 }

 async function retryTransientFilesystemOperation<T>(
 	operation: () => Promise<T>,
 ): Promise<T> {
-	let attempt = 0;
-	while (true) {
+	for (let attempt = 0; attempt < 5; attempt += 1) {
 		try {
 			return await operation();
 		} catch (error) {
 			const code = (error as NodeJS.ErrnoException).code;
-			if (
-				!isRetryableFilesystemErrorCode(code) ||
-				attempt >= TRANSIENT_FILESYSTEM_MAX_ATTEMPTS - 1
-			) {
+			if (!isRetryableFilesystemErrorCode(code) || attempt === 4) {
 				throw error;
 			}
-			const baseDelayMs = TRANSIENT_FILESYSTEM_BASE_DELAY_MS * 2 ** attempt;
-			const jitterMs = Math.floor(Math.random() * baseDelayMs);
 			await new Promise((resolve) =>
-				setTimeout(resolve, baseDelayMs + jitterMs),
+				setTimeout(resolve, 10 * 2 ** attempt + Math.floor(Math.random() * 10)),
 			);
 		}
-		attempt += 1;
 	}
+
+	throw new Error("Retry loop exhausted unexpectedly");
 }


⚠️ Potential issue | 🟠 Major

❓ Verification inconclusive

Script executed:

#!/bin/bash set -euo pipefail echo "== retry policy implementation ==" rg -n -C3 --type=ts 'isRetryableFilesystemErrorCode|retryTransientFilesystemOperation|EPERM|EBUSY|EAGAIN|process.platform' lib/storage.ts echo echo "== retry policy tests ==" rg -n -C3 --type=ts 'retryTransientFilesystemOperation|EPERM|EBUSY|EAGAIN|win32|process.platform' test

Repository: ndycode/codex-multi-auth

Repository: ndycode/codex-multi-auth
Exit code: 0

stdout:

== retry policy implementation == 239-export function formatStorageErrorHint(error: unknown, path: string): string { 240- const err = error as NodeJS.ErrnoException; 241- const code = err?.code || "UNKNOWN"; 242: const isWindows = process.platform === "win32"; 243- 244- switch (code) { 245- case "EACCES": 246: case "EPERM": 247- return isWindows 248- ? `Permission denied writing to ${path}. Check antivirus exclusions for this folder. Ensure you have write permissions.` 249- : `Permission denied writing to ${path}. Check folder permissions. Try: chmod 755 ~/.codex`; 250: case "EBUSY": 251- return `File is locked at ${path}. The file may be open in another program. Close any editors or processes accessing it.`; 252- case "ENOSPC": 253- return `Disk is full. Free up space and try again. Path: ${path}`; -- 287- if (code === "ENOENT") { 288- return; 289- } 290: if ((code === "EPERM" || code === "EBUSY" || code === "EAGAIN") && attempt < 4) { 291- await new Promise((resolve) => setTimeout(resolve, 10 * 2 ** attempt)); 292- continue; 293- } -- 466- return; 467- } 468- const canRetry = 469: (code === "EPERM" || code === "EBUSY") && 470- attempt + 1 < BACKUP_COPY_MAX_ATTEMPTS; 471- if (canRetry) { 472- await new Promise((resolve) => -- 490- } catch (error) { 491- const code = (error as NodeJS.ErrnoException).code; 492- const canRetry = 493: (code === "EPERM" || code === "EBUSY" || code === "EAGAIN") && 494- attempt + 1 < BACKUP_COPY_MAX_ATTEMPTS; 495- if (!canRetry) { 496- throw error; -- 1651-async function scanNamedBackups(): Promise<NamedBackupScanResult> { 1652- const backupRoot = getNamedBackupRoot(getStoragePath()); 1653- try { 1654: const entries = await retryTransientFilesystemOperation(() => 1655- fs.readdir(backupRoot, { withFileTypes: true }), 1656- ); 1657- const backupEntries = entries -- 1720-async function listNamedBackupsWithoutLoading(): Promise<NamedBackupMetadataListingResult> { 1721- const backupRoot = getNamedBackupRoot(getStoragePath()); 1722- try { 1723: const entries = await retryTransientFilesystemOperation(() => 1724- fs.readdir(backupRoot, { withFileTypes: true }), 1725- ); 1726- const backups: NamedBackupMetadata[] = []; -- 1776- return scanResult.backups.map((entry) => entry.backup); 1777-} 1778- 1779:function isRetryableFilesystemErrorCode( 1780- code: string | undefined, 1781:): code is "EPERM" | "EBUSY" | "EAGAIN" { 1782: if (code === "EAGAIN") { 1783- return true; 1784- } 1785: return (code === "EPERM" || code === "EBUSY") && process.platform === "win32"; 1786-} 1787- 1788:async function retryTransientFilesystemOperation<T>( 1789- operation: () => Promise<T>, 1790-): Promise<T> { 1791- for (let attempt = 0; attempt < 5; attempt += 1) { -- 1793- return await operation(); 1794- } catch (error) { 1795- const code = (error as NodeJS.ErrnoException).code; 1796: if (!isRetryableFilesystemErrorCode(code) || attempt === 4) { 1797- throw error; 1798- } 1799- await new Promise((resolve) => -- 2022- 2023-async function loadBackupCandidate(path: string): Promise<LoadedBackupCandidate> { 2024- try { 2025: return await retryTransientFilesystemOperation(() => 2026- loadAccountsFromPath(path), 2027- ); 2028- } catch (error) { -- 2036-} 2037- 2038-function equalsNamedBackupEntry(left: string, right: string): boolean { 2039: return process.platform === "win32" 2040- ? left.toLowerCase() === right.toLowerCase() 2041- : left === right; 2042-} -- 2062- const requestedBaseName = stripNamedBackupJsonExtension(requestedWithExtension); 2063- 2064- try { 2065: const entries = await retryTransientFilesystemOperation(() => 2066- fs.readdir(backupRoot, { withFileTypes: true }), 2067- ); 2068- for (const entry of entries) { -- 2348- ctimeMs?: number; 2349- } | null = null; 2350- try { 2351: stats = await retryTransientFilesystemOperation(() => fs.stat(path)); 2352- } catch (error) { 2353- const code = (error as NodeJS.ErrnoException).code; 2354- if (code !== "ENOENT") { -- 2943- const transactionState = transactionSnapshotContext.getStore(); 2944- const currentStoragePath = getStoragePath(); 2945- const storage = transactionState?.active 2946: ? (process.platform === "win32" 2947- ? transactionState.storagePath?.toLowerCase() === 2948- currentStoragePath?.toLowerCase() 2949- : transactionState.storagePath === currentStoragePath) -- 2994- 2995- let content: string; 2996- try { 2997: content = await retryTransientFilesystemOperation(() => 2998- fs.readFile(resolvedPath, "utf-8"), 2999- ); 3000- } catch (error) { == retry policy tests == test/test-model-matrix-script.test.ts-188- it("filters non-path where output on Windows", async () => { test/test-model-matrix-script.test.ts-189- const platformSpy = vi test/test-model-matrix-script.test.ts-190- .spyOn(process, "platform", "get") test/test-model-matrix-script.test.ts:191: .mockReturnValue("win32"); test/test-model-matrix-script.test.ts-192- try { test/test-model-matrix-script.test.ts-193- spawnSync.mockReturnValue({ test/test-model-matrix-script.test.ts-194- stdout: -- test/test-model-matrix-script.test.ts-210- it("returns fallback command when where has no executable candidates", async () => { test/test-model-matrix-script.test.ts-211- const platformSpy = vi test/test-model-matrix-script.test.ts-212- .spyOn(process, "platform", "get") test/test-model-matrix-script.test.ts:213: .mockReturnValue("win32"); test/test-model-matrix-script.test.ts-214- try { test/test-model-matrix-script.test.ts-215- spawnSync.mockReturnValue({ test/test-model-matrix-script.test.ts-216- stdout: "", -- test/test-model-matrix-script.test.ts-231- it("serializes stopCodexServers calls and kills only tracked Windows PIDs", async () => { test/test-model-matrix-script.test.ts-232- const platformSpy = vi test/test-model-matrix-script.test.ts-233- .spyOn(process, "platform", "get") test/test-model-matrix-script.test.ts:234: .mockReturnValue("win32"); test/test-model-matrix-script.test.ts-235- try { test/test-model-matrix-script.test.ts-236- spawnSync.mockReturnValue({ status: 0, stdout: "", stderr: "" }); test/test-model-matrix-script.test.ts-237- -- test/unified-settings.test.ts-123- const renameSpy = vi.spyOn(fs, "rename"); test/unified-settings.test.ts-124- renameSpy.mockImplementationOnce(async () => { test/unified-settings.test.ts-125- const error = new Error("busy") as NodeJS.ErrnoException; test/unified-settings.test.ts:126: error.code = "EBUSY"; test/unified-settings.test.ts-127- throw error; test/unified-settings.test.ts-128- }); test/unified-settings.test.ts-129- try { -- test/unified-settings.test.ts-167- ).toBe(""); test/unified-settings.test.ts-168- }); test/unified-settings.test.ts-169- test/unified-settings.test.ts:170: it("retries async rename on windows-style EPERM lock", async () => { test/unified-settings.test.ts-171- const { saveUnifiedPluginConfig, loadUnifiedPluginConfigSync } = test/unified-settings.test.ts-172- await import("../lib/unified-settings.js"); test/unified-settings.test.ts-173- const renameSpy = vi.spyOn(fs, "rename"); test/unified-settings.test.ts-174- renameSpy.mockImplementationOnce(async () => { test/unified-settings.test.ts-175- const error = new Error("perm") as NodeJS.ErrnoException; test/unified-settings.test.ts:176: error.code = "EPERM"; test/unified-settings.test.ts-177- throw error; test/unified-settings.test.ts-178- }); test/unified-settings.test.ts-179- try { -- test/unified-settings.test.ts-188- } test/unified-settings.test.ts-189- }); test/unified-settings.test.ts-190- test/unified-settings.test.ts:191: it("cleans async temp file when rename repeatedly fails with EPERM", async () => { test/unified-settings.test.ts-192- const { saveUnifiedPluginConfig, getUnifiedSettingsPath } = await import( test/unified-settings.test.ts-193- "../lib/unified-settings.js" test/unified-settings.test.ts-194- ); test/unified-settings.test.ts-195- const renameSpy = vi.spyOn(fs, "rename"); test/unified-settings.test.ts-196- renameSpy.mockImplementation(async () => { test/unified-settings.test.ts-197- const error = new Error("perm locked") as NodeJS.ErrnoException; test/unified-settings.test.ts:198: error.code = "EPERM"; test/unified-settings.test.ts-199- throw error; test/unified-settings.test.ts-200- }); test/unified-settings.test.ts-201- try { -- test/unified-settings.test.ts-395- const readSpy = vi.spyOn(fs, "readFile"); test/unified-settings.test.ts-396- readSpy.mockImplementationOnce(async () => { test/unified-settings.test.ts-397- const error = new Error("file locked") as NodeJS.ErrnoException; test/unified-settings.test.ts:398: error.code = "EBUSY"; test/unified-settings.test.ts-399- throw error; test/unified-settings.test.ts-400- }); test/unified-settings.test.ts-401- -- test/storage-flagged.test.ts-12- setStoragePathDirect, test/storage-flagged.test.ts-13-} from "../lib/storage.js"; test/storage-flagged.test.ts-14- test/storage-flagged.test.ts:15:const RETRYABLE_REMOVE_CODES = new Set(["EBUSY", "EPERM", "ENOTEMPTY"]); test/storage-flagged.test.ts-16- test/storage-flagged.test.ts-17-async function removeWithRetry( test/storage-flagged.test.ts-18- targetPath: string, -- test/storage-flagged.test.ts-232- expect(flagged.accounts).toHaveLength(0); test/storage-flagged.test.ts-233- }); test/storage-flagged.test.ts-234- test/storage-flagged.test.ts:235: it.each(["EPERM", "EBUSY"] as const)( test/storage-flagged.test.ts-236- "retries transient %s when clearing flagged storage", test/storage-flagged.test.ts-237- async (code) => { test/storage-flagged.test.ts-238- await saveFlaggedAccounts({ -- test/storage-flagged.test.ts-276- }, test/storage-flagged.test.ts-277- ); test/storage-flagged.test.ts-278- test/storage-flagged.test.ts:279: it.each(["EPERM", "EBUSY"] as const)( test/storage-flagged.test.ts-280- "returns false when clearing flagged storage exhausts retryable %s failures", test/storage-flagged.test.ts-281- async (code) => { test/storage-flagged.test.ts-282- await saveFlaggedAccounts({ -- test/storage-flagged.test.ts-354- const [targetPath] = args; test/storage-flagged.test.ts-355- if (targetPath === flaggedPath) { test/storage-flagged.test.ts-356- const error = new Error( test/storage-flagged.test.ts:357: "EPERM flagged read", test/storage-flagged.test.ts-358- ) as NodeJS.ErrnoException; test/storage-flagged.test.ts:359: error.code = "EPERM"; test/storage-flagged.test.ts-360- throw error; test/storage-flagged.test.ts-361- } test/storage-flagged.test.ts-362- return originalReadFile(...args); -- test/storage.test.ts-711- flaggedRenameAttempts += 1; test/storage.test.ts-712- const error = Object.assign( test/storage.test.ts-713- new Error("flagged storage busy"), test/storage.test.ts:714: { code: "EBUSY" }, test/storage.test.ts-715- ); test/storage.test.ts-716- throw error; test/storage.test.ts-717- } -- test/storage.test.ts-822- flaggedRenameAttempts += 1; test/storage.test.ts-823- const error = Object.assign( test/storage.test.ts-824- new Error("flagged storage busy"), test/storage.test.ts:825: { code: "EBUSY" }, test/storage.test.ts-826- ); test/storage.test.ts-827- throw error; test/storage.test.ts-828- } -- test/storage.test.ts-831- if (accountRenameAttempts > 1) { test/storage.test.ts-832- const error = Object.assign( test/storage.test.ts-833- new Error("rollback account storage busy"), test/storage.test.ts:834: { code: "EBUSY" }, test/storage.test.ts-835- ); test/storage.test.ts-836- throw error; test/storage.test.ts-837- } -- test/storage.test.ts-939- if (flaggedRenameAttempts <= 2) { test/storage.test.ts-940- const error = Object.assign( test/storage.test.ts-941- new Error("flagged storage busy"), test/storage.test.ts:942: { code: "EBUSY" }, test/storage.test.ts-943- ); test/storage.test.ts-944- throw error; test/storage.test.ts-945- } -- test/storage.test.ts-1063- if (String(path) === exportPath && busyFailures === 0) { test/storage.test.ts-1064- busyFailures += 1; test/storage.test.ts-1065- const error = new Error("import file busy") as NodeJS.ErrnoException; test/storage.test.ts:1066: error.code = "EAGAIN"; test/storage.test.ts-1067- throw error; test/storage.test.ts-1068- } test/storage.test.ts-1069- return originalReadFile(...(args as Parameters<typeof fs.readFile>)); -- test/storage.test.ts-1574- it("rethrows unreadable backup directory errors while listing backups", async () => { test/storage.test.ts-1575- const readdirSpy = vi.spyOn(fs, "readdir"); test/storage.test.ts-1576- const error = new Error("backup directory locked") as NodeJS.ErrnoException; test/storage.test.ts:1577: error.code = "EPERM"; test/storage.test.ts-1578- readdirSpy.mockRejectedValue(error); test/storage.test.ts-1579- test/storage.test.ts-1580- try { test/storage.test.ts:1581: await expect(listNamedBackups()).rejects.toMatchObject({ code: "EPERM" }); test/storage.test.ts-1582- } finally { test/storage.test.ts-1583- readdirSpy.mockRestore(); test/storage.test.ts-1584- } -- test/storage.test.ts-1590- .mockReturnValue("linux"); test/storage.test.ts-1591- const readdirSpy = vi.spyOn(fs, "readdir"); test/storage.test.ts-1592- const error = new Error("backup directory locked") as NodeJS.ErrnoException; test/storage.test.ts:1593: error.code = "EPERM"; test/storage.test.ts-1594- readdirSpy.mockRejectedValue(error); test/storage.test.ts-1595- test/storage.test.ts-1596- try { test/storage.test.ts:1597: await expect(listNamedBackups()).rejects.toMatchObject({ code: "EPERM" }); test/storage.test.ts-1598- expect(readdirSpy).toHaveBeenCalledTimes(1); test/storage.test.ts-1599- } finally { test/storage.test.ts-1600- readdirSpy.mockRestore(); -- test/storage.test.ts-1602- } test/storage.test.ts-1603- }); test/storage.test.ts-1604- test/storage.test.ts:1605: it("rethrows EBUSY backup directory errors after one attempt on non-Windows platforms", async () => { test/storage.test.ts-1606- const platformSpy = vi test/storage.test.ts-1607- .spyOn(process, "platform", "get") test/storage.test.ts-1608- .mockReturnValue("linux"); test/storage.test.ts-1609- const readdirSpy = vi.spyOn(fs, "readdir"); test/storage.test.ts-1610- const error = new Error("backup directory busy") as NodeJS.ErrnoException; test/storage.test.ts:1611: error.code = "EBUSY"; test/storage.test.ts-1612- readdirSpy.mockRejectedValue(error); test/storage.test.ts-1613- test/storage.test.ts-1614- try { test/storage.test.ts:1615: await expect(listNamedBackups()).rejects.toMatchObject({ code: "EBUSY" }); test/storage.test.ts-1616- expect(readdirSpy).toHaveBeenCalledTimes(1); test/storage.test.ts-1617- } finally { test/storage.test.ts-1618- readdirSpy.mockRestore(); -- test/storage.test.ts-1623- it("rethrows unreadable backup directory errors while restoring backups", async () => { test/storage.test.ts-1624- const readdirSpy = vi.spyOn(fs, "readdir"); test/storage.test.ts-1625- const error = new Error("backup directory locked") as NodeJS.ErrnoException; test/storage.test.ts:1626: error.code = "EPERM"; test/storage.test.ts-1627- readdirSpy.mockRejectedValue(error); test/storage.test.ts-1628- test/storage.test.ts-1629- try { test/storage.test.ts-1630- await expect(restoreNamedBackup("Manual Backup")).rejects.toMatchObject({ test/storage.test.ts:1631: code: "EPERM", test/storage.test.ts-1632- }); test/storage.test.ts-1633- } finally { test/storage.test.ts-1634- readdirSpy.mockRestore(); -- test/storage.test.ts-1661- const error = new Error( test/storage.test.ts-1662- "backup directory busy", test/storage.test.ts-1663- ) as NodeJS.ErrnoException; test/storage.test.ts:1664: error.code = "EBUSY"; test/storage.test.ts-1665- throw error; test/storage.test.ts-1666- } test/storage.test.ts-1667- return originalReaddir(...(args as Parameters<typeof fs.readdir>)); -- test/storage.test.ts-1707- const error = new Error( test/storage.test.ts-1708- "backup directory busy", test/storage.test.ts-1709- ) as NodeJS.ErrnoException; test/storage.test.ts:1710: error.code = "EAGAIN"; test/storage.test.ts-1711- throw error; test/storage.test.ts-1712- } test/storage.test.ts-1713- return originalReaddir(...(args as Parameters<typeof fs.readdir>)); -- test/storage.test.ts-1780- if (String(path) === backup.path && busyFailures === 0) { test/storage.test.ts-1781- busyFailures += 1; test/storage.test.ts-1782- const error = new Error("backup file busy") as NodeJS.ErrnoException; test/storage.test.ts:1783: error.code = "EBUSY"; test/storage.test.ts-1784- throw error; test/storage.test.ts-1785- } test/storage.test.ts-1786- return originalReadFile(...(args as Parameters<typeof fs.readFile>)); -- test/storage.test.ts-1820- if (String(path) === backup.path && busyFailures === 0) { test/storage.test.ts-1821- busyFailures += 1; test/storage.test.ts-1822- const error = new Error("backup stat busy") as NodeJS.ErrnoException; test/storage.test.ts:1823: error.code = "EAGAIN"; test/storage.test.ts-1824- throw error; test/storage.test.ts-1825- } test/storage.test.ts-1826- return originalStat(...(args as Parameters<typeof fs.stat>)); -- test/storage.test.ts-2057- const testPath = "/home/user/.codex/accounts.json"; test/storage.test.ts-2058- test/storage.test.ts-2059- it("should return permission hint for EACCES on Windows", () => { test/storage.test.ts:2060: const originalPlatform = process.platform; test/storage.test.ts:2061: Object.defineProperty(process, "platform", { value: "win32" }); test/storage.test.ts-2062- test/storage.test.ts-2063- const err = { code: "EACCES" } as NodeJS.ErrnoException; test/storage.test.ts-2064- const hint = formatStorageErrorHint(err, testPath); -- test/storage.test.ts-2070- }); test/storage.test.ts-2071- test/storage.test.ts-2072- it("should return chmod hint for EACCES on Unix", () => { test/storage.test.ts:2073: const originalPlatform = process.platform; test/storage.test.ts-2074- Object.defineProperty(process, "platform", { value: "darwin" }); test/storage.test.ts-2075- test/storage.test.ts-2076- const err = { code: "EACCES" } as NodeJS.ErrnoException; -- test/storage.test.ts-2082- Object.defineProperty(process, "platform", { value: originalPlatform }); test/storage.test.ts-2083- }); test/storage.test.ts-2084- test/storage.test.ts:2085: it("should return permission hint for EPERM", () => { test/storage.test.ts:2086: const err = { code: "EPERM" } as NodeJS.ErrnoException; test/storage.test.ts-2087- const hint = formatStorageErrorHint(err, testPath); test/storage.test.ts-2088- test/storage.test.ts-2089- expect(hint).toContain("Permission denied"); test/storage.test.ts-2090- expect(hint).toContain(testPath); test/storage.test.ts-2091- }); test/storage.test.ts-2092- test/storage.test.ts:2093: it("should return file locked hint for EBUSY", () => { test/storage.test.ts:2094: const err = { code: "EBUSY" } as NodeJS.ErrnoException; test/storage.test.ts-2095- const hint = formatStorageErrorHint(err, testPath); test/storage.test.ts-2096- test/storage.test.ts-2097- expect(hint).toContain("locked"); -- test/storage.test.ts-2725- await expect(clearAccounts()).resolves.not.toThrow(); test/storage.test.ts-2726- }); test/storage.test.ts-2727- test/storage.test.ts:2728: it.each(["EPERM", "EBUSY", "EAGAIN"] as const)( test/storage.test.ts-2729- "retries transient %s when clearing saved account artifacts", test/storage.test.ts-2730- async (code) => { test/storage.test.ts-2731- await fs.writeFile(testStoragePath, "{}"); -- test/storage.test.ts-2780- await fs.rm(testWorkDir, { recursive: true, force: true }); test/storage.test.ts-2781- }); test/storage.test.ts-2782- test/storage.test.ts:2783: it.each(["EPERM", "EBUSY"] as const)( test/storage.test.ts-2784- "retries transient %s when clearing flagged account storage", test/storage.test.ts-2785- async (code) => { test/storage.test.ts-2786- const flaggedPath = getFlaggedAccountsPath(); -- test/storage.test.ts-2815- }, test/storage.test.ts-2816- ); test/storage.test.ts-2817- test/storage.test.ts:2818: it.each(["EPERM", "EBUSY"] as const)( test/storage.test.ts-2819- "returns false when clearing flagged account storage exhausts retryable %s failures", test/storage.test.ts-2820- async (code) => { test/storage.test.ts-2821- const flaggedPath = getFlaggedAccountsPath(); -- test/storage.test.ts-3010- await expect(clearAccounts()).resolves.not.toThrow(); test/storage.test.ts-3011- }); test/storage.test.ts-3012- test/storage.test.ts:3013: it.each(["EPERM", "EBUSY"] as const)( test/storage.test.ts-3014- "returns false when clearing saved accounts exhausts retryable %s failures", test/storage.test.ts-3015- async (code) => { test/storage.test.ts-3016- await fs.writeFile(testStoragePath, "{}"); -- test/storage.test.ts-3691- }); test/storage.test.ts-3692- }); test/storage.test.ts-3693- test/storage.test.ts:3694: describe("saveAccounts EPERM/EBUSY retry logic", () => { test/storage.test.ts-3695- const testWorkDir = join( test/storage.test.ts-3696- tmpdir(), test/storage.test.ts-3697- "codex-retry-" + Math.random().toString(36).slice(2), -- test/storage.test.ts-3711- await fs.rm(testWorkDir, { recursive: true, force: true }); test/storage.test.ts-3712- }); test/storage.test.ts-3713- test/storage.test.ts:3714: it("retries on EPERM and succeeds on second attempt", async () => { test/storage.test.ts-3715- const now = Date.now(); test/storage.test.ts-3716- const storage = { test/storage.test.ts-3717- version: 3 as const, -- test/storage.test.ts-3726- .mockImplementation(async (oldPath, newPath) => { test/storage.test.ts-3727- attemptCount++; test/storage.test.ts-3728- if (attemptCount === 1) { test/storage.test.ts:3729: const err = new Error("EPERM error") as NodeJS.ErrnoException; test/storage.test.ts:3730: err.code = "EPERM"; test/storage.test.ts-3731- throw err; test/storage.test.ts-3732- } test/storage.test.ts-3733- return originalRename(oldPath as string, newPath as string); -- test/storage.test.ts-3740- renameSpy.mockRestore(); test/storage.test.ts-3741- }); test/storage.test.ts-3742- test/storage.test.ts:3743: it("retries on EBUSY and succeeds on third attempt", async () => { test/storage.test.ts-3744- const now = Date.now(); test/storage.test.ts-3745- const storage = { test/storage.test.ts-3746- version: 3 as const, -- test/storage.test.ts-3755- .mockImplementation(async (oldPath, newPath) => { test/storage.test.ts-3756- attemptCount++; test/storage.test.ts-3757- if (attemptCount <= 2) { test/storage.test.ts:3758: const err = new Error("EBUSY error") as NodeJS.ErrnoException; test/storage.test.ts:3759: err.code = "EBUSY"; test/storage.test.ts-3760- throw err; test/storage.test.ts-3761- } test/storage.test.ts-3762- return originalRename(oldPath as string, newPath as string); -- test/storage.test.ts-3769- renameSpy.mockRestore(); test/storage.test.ts-3770- }); test/storage.test.ts-3771- test/storage.test.ts:3772: it("retries on EAGAIN and cleans up the WAL after rename succeeds", async () => { test/storage.test.ts-3773- const now = Date.now(); test/storage.test.ts-3774- const storage = { test/storage.test.ts-3775- version: 3 as const, -- test/storage.test.ts-3785- .mockImplementation(async (oldPath, newPath) => { test/storage.test.ts-3786- attemptCount++; test/storage.test.ts-3787- if (attemptCount === 1) { test/storage.test.ts:3788: const err = new Error("EAGAIN error") as NodeJS.ErrnoException; test/storage.test.ts:3789: err.code = "EAGAIN"; test/storage.test.ts-3790- throw err; test/storage.test.ts-3791- } test/storage.test.ts-3792- return originalRename(oldPath as string, newPath as string); -- test/storage.test.ts-3800- renameSpy.mockRestore(); test/storage.test.ts-3801- }); test/storage.test.ts-3802- test/storage.test.ts:3803: it("throws after 5 failed EPERM retries", async () => { test/storage.test.ts-3804- const now = Date.now(); test/storage.test.ts-3805- const storage = { test/storage.test.ts-3806- version: 3 as const, -- test/storage.test.ts-3811- let attemptCount = 0; test/storage.test.ts-3812- const renameSpy = vi.spyOn(fs, "rename").mockImplementation(async () => { test/storage.test.ts-3813- attemptCount++; test/storage.test.ts:3814: const err = new Error("EPERM error") as NodeJS.ErrnoException; test/storage.test.ts:3815: err.code = "EPERM"; test/storage.test.ts-3816- throw err; test/storage.test.ts-3817- }); test/storage.test.ts-3818- -- test/storage.test.ts-3824- renameSpy.mockRestore(); test/storage.test.ts-3825- }); test/storage.test.ts-3826- test/storage.test.ts:3827: it("throws immediately on non-EPERM/EBUSY errors", async () => { test/storage.test.ts-3828- const now = Date.now(); test/storage.test.ts-3829- const storage = { test/storage.test.ts-3830- version: 3 as const, -- test/storage.test.ts-3870- statSpy.mockRestore(); test/storage.test.ts-3871- }); test/storage.test.ts-3872- test/storage.test.ts:3873: it("retries backup copyFile on transient EBUSY and succeeds", async () => { test/storage.test.ts-3874- const now = Date.now(); test/storage.test.ts-3875- const storage = { test/storage.test.ts-3876- version: 3 as const, -- test/storage.test.ts-3888- .mockImplementation(async (src, dest) => { test/storage.test.ts-3889- copyAttempts += 1; test/storage.test.ts-3890- if (copyAttempts === 1) { test/storage.test.ts:3891: const err = new Error("EBUSY copy") as NodeJS.ErrnoException; test/storage.test.ts:3892: err.code = "EBUSY"; test/storage.test.ts-3893- throw err; test/storage.test.ts-3894- } test/storage.test.ts-3895- return originalCopy(src as string, dest as string); -- test/storage.test.ts-3908- } test/storage.test.ts-3909- }); test/storage.test.ts-3910- test/storage.test.ts:3911: it("retries backup copyFile on transient EPERM and succeeds", async () => { test/storage.test.ts-3912- const now = Date.now(); test/storage.test.ts-3913- const storage = { test/storage.test.ts-3914- version: 3 as const, -- test/storage.test.ts-3926- .mockImplementation(async (src, dest) => { test/storage.test.ts-3927- copyAttempts += 1; test/storage.test.ts-3928- if (copyAttempts === 1) { test/storage.test.ts:3929: const err = new Error("EPERM copy") as NodeJS.ErrnoException; test/storage.test.ts:3930: err.code = "EPERM"; test/storage.test.ts-3931- throw err; test/storage.test.ts-3932- } test/storage.test.ts-3933- return originalCopy(src as string, dest as string); -- test/storage.test.ts-3946- } test/storage.test.ts-3947- }); test/storage.test.ts-3948- test/storage.test.ts:3949: it("retries staged backup rename on transient EBUSY and succeeds", async () => { test/storage.test.ts-3950- const now = Date.now(); test/storage.test.ts-3951- const storagePath = getStoragePath(); test/storage.test.ts-3952- const storage = { -- test/storage.test.ts-3968- stagedRenameAttempts += 1; test/storage.test.ts-3969- if (stagedRenameAttempts === 1) { test/storage.test.ts-3970- const err = new Error( test/storage.test.ts:3971: "EBUSY staged rename", test/storage.test.ts-3972- ) as NodeJS.ErrnoException; test/storage.test.ts:3973: err.code = "EBUSY"; test/storage.test.ts-3974- throw err; test/storage.test.ts-3975- } test/storage.test.ts-3976- } -- test/storage.test.ts-4299- await expect(clearQuotaCache()).resolves.toBe(true); test/storage.test.ts-4300- }); test/storage.test.ts-4301- test/storage.test.ts:4302: it.each(["EPERM", "EBUSY"] as const)( test/storage.test.ts-4303- "retries transient %s when clearing the quota cache", test/storage.test.ts-4304- async (code) => { test/storage.test.ts-4305- const quotaPath = getQuotaCachePath(); -- test/storage.test.ts-4328- }, test/storage.test.ts-4329- ); test/storage.test.ts-4330- test/storage.test.ts:4331: it.each(["EPERM", "EBUSY"] as const)( test/storage.test.ts-4332- "returns false when quota-cache clear exhausts retryable %s failures", test/storage.test.ts-4333- async (code) => { test/storage.test.ts-4334- const quotaPath = getQuotaCachePath(); -- test/settings-hub-utils.test.ts-235- attempts += 1; test/settings-hub-utils.test.ts-236- if (attempts < 3) { test/settings-hub-utils.test.ts-237- const error = new Error("busy") as NodeJS.ErrnoException; test/settings-hub-utils.test.ts:238: error.code = attempts === 1 ? "EBUSY" : "EPERM"; test/settings-hub-utils.test.ts-239- throw error; test/settings-hub-utils.test.ts-240- } test/settings-hub-utils.test.ts-241- return "ok"; -- test/settings-hub-utils.test.ts-244- expect(attempts).toBe(3); test/settings-hub-utils.test.ts-245- }); test/settings-hub-utils.test.ts-246- test/settings-hub-utils.test.ts:247: it("retries queued writes for EAGAIN filesystem errors", async () => { test/settings-hub-utils.test.ts-248- const api = await loadSettingsHubTestApi(); test/settings-hub-utils.test.ts-249- let attempts = 0; test/settings-hub-utils.test.ts-250- const result = await api.withQueuedRetry( -- test/settings-hub-utils.test.ts-253- attempts += 1; test/settings-hub-utils.test.ts-254- if (attempts < 3) { test/settings-hub-utils.test.ts-255- const error = new Error("busy") as NodeJS.ErrnoException; test/settings-hub-utils.test.ts:256: error.code = "EAGAIN"; test/settings-hub-utils.test.ts-257- throw error; test/settings-hub-utils.test.ts-258- } test/settings-hub-utils.test.ts-259- return "ok"; -- test/settings-hub-utils.test.ts-662- })); test/settings-hub-utils.test.ts-663- const nodeFs = await import("node:fs"); test/settings-hub-utils.test.ts-664- const busyError = new Error("busy") as NodeJS.ErrnoException; test/settings-hub-utils.test.ts:665: busyError.code = "EBUSY"; test/settings-hub-utils.test.ts-666- const readSpy = vi test/settings-hub-utils.test.ts-667- .spyOn(nodeFs.promises, "readFile") test/settings-hub-utils.test.ts-668- .mockRejectedValueOnce(busyError) -- test/rotation-integration.test.ts-15-let testStoragePath: string; test/rotation-integration.test.ts-16- test/rotation-integration.test.ts-17-async function removeWithRetry(path: string, options: { recursive?: boolean; force?: boolean }): Promise<void> { test/rotation-integration.test.ts:18: const retryableCodes = new Set(["ENOTEMPTY", "EPERM", "EBUSY"]); test/rotation-integration.test.ts-19- const maxAttempts = 6; test/rotation-integration.test.ts-20- test/rotation-integration.test.ts-21- for (let attempt = 1; attempt <= maxAttempts; attempt += 1) { -- test/release-main-prs-regression.test.ts-192- const originalUnlink = fs.unlink.bind(fs); test/release-main-prs-regression.test.ts-193- const unlinkSpy = vi.spyOn(fs, "unlink").mockImplementation(async (targetPath) => { test/release-main-prs-regression.test.ts-194- if (targetPath === flaggedPath) { test/release-main-prs-regression.test.ts:195: const error = new Error("EPERM primary delete") as NodeJS.ErrnoException; test/release-main-prs-regression.test.ts:196: error.code = "EPERM"; test/release-main-prs-regression.test.ts-197- throw error; test/release-main-prs-regression.test.ts-198- } test/release-main-prs-regression.test.ts-199- return originalUnlink(targetPath); -- test/repo-hygiene.test.ts-7-import { execFileSync, spawn, spawnSync } from "node:child_process"; test/repo-hygiene.test.ts-8- test/repo-hygiene.test.ts-9-async function removeWithRetry(targetPath: string, options: { recursive?: boolean; force?: boolean }): Promise<void> { test/repo-hygiene.test.ts:10: const retryableCodes = new Set(["ENOTEMPTY", "EPERM", "EBUSY"]); test/repo-hygiene.test.ts-11- const maxAttempts = 6; test/repo-hygiene.test.ts-12- test/repo-hygiene.test.ts-13- for (let attempt = 1; attempt <= maxAttempts; attempt += 1) { -- test/refresh-queue.test.ts-887- vi.mocked(authModule.refreshAccessToken).mockResolvedValue(mockResult); test/refresh-queue.test.ts-888- test/refresh-queue.test.ts-889- const leaseCoordinator = { test/refresh-queue.test.ts:890: acquire: vi.fn().mockRejectedValue(new Error("EBUSY lease dir")), test/refresh-queue.test.ts-891- } as unknown as RefreshLeaseCoordinator; test/refresh-queue.test.ts-892- test/refresh-queue.test.ts-893- const queue = new RefreshQueue(30_000, leaseCoordinator); -- test/runtime-paths.test.ts-117- }); test/runtime-paths.test.ts-118- test/runtime-paths.test.ts-119- it("deduplicates Windows-style fallback paths case-insensitively", async () => { test/runtime-paths.test.ts:120: const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); test/runtime-paths.test.ts-121- try { test/runtime-paths.test.ts-122- homedir.mockReturnValue("C:\\Users\\Neil"); test/runtime-paths.test.ts-123- process.env.CODEX_HOME = "C:\\USERS\\NEIL\\.codex"; -- test/runtime-paths.test.ts-136- }); test/runtime-paths.test.ts-137- test/runtime-paths.test.ts-138- it("treats default Windows CODEX_HOME with a trailing separator as the default root", async () => { test/runtime-paths.test.ts:139: const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); test/runtime-paths.test.ts-140- try { test/runtime-paths.test.ts-141- homedir.mockReturnValue("C:\\Users\\Neil"); test/runtime-paths.test.ts-142- process.env.CODEX_HOME = "C:\\Users\\Neil\\.codex\\"; -- test/runtime-paths.test.ts-169- }); test/runtime-paths.test.ts-170- test/runtime-paths.test.ts-171- it("prefers USERPROFILE over os.homedir on Windows when CODEX_HOME is unset", async () => { test/runtime-paths.test.ts:172: const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); test/runtime-paths.test.ts-173- try { test/runtime-paths.test.ts-174- homedir.mockReturnValue("C:\\Windows\\System32\\config\\systemprofile"); test/runtime-paths.test.ts-175- process.env.USERPROFILE = "C:\\Users\\Alice"; -- test/runtime-paths.test.ts-182- }); test/runtime-paths.test.ts-183- test/runtime-paths.test.ts-184- it("falls back to HOME when USERPROFILE is missing on Windows", async () => { test/runtime-paths.test.ts:185: const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); test/runtime-paths.test.ts-186- try { test/runtime-paths.test.ts-187- homedir.mockReturnValue("C:\\Windows\\System32\\config\\systemprofile"); test/runtime-paths.test.ts-188- process.env.HOME = "D:\\Users\\Bob"; -- test/runtime-paths.test.ts-194- }); test/runtime-paths.test.ts-195- test/runtime-paths.test.ts-196- it("falls back to HOMEDRIVE and HOMEPATH when USERPROFILE and HOME are missing on Windows", async () => { test/runtime-paths.test.ts:197: const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); test/runtime-paths.test.ts-198- try { test/runtime-paths.test.ts-199- homedir.mockReturnValue("C:\\Windows\\System32\\config\\systemprofile"); test/runtime-paths.test.ts-200- process.env.HOMEDRIVE = "E:"; -- test/runtime-paths.test.ts-207- }); test/runtime-paths.test.ts-208- test/runtime-paths.test.ts-209- it("normalizes HOMEPATH without a leading slash on Windows", async () => { test/runtime-paths.test.ts:210: const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); test/runtime-paths.test.ts-211- try { test/runtime-paths.test.ts-212- homedir.mockReturnValue("C:\\Windows\\System32\\config\\systemprofile"); test/runtime-paths.test.ts-213- process.env.HOMEDRIVE = "E:"; -- test/refresh-lease.test.ts-121- if (String(path).endsWith(".lock") && busyCount < 2) { test/refresh-lease.test.ts-122- busyCount += 1; test/refresh-lease.test.ts-123- const error = new Error("busy") as NodeJS.ErrnoException; test/refresh-lease.test.ts:124: error.code = "EBUSY"; test/refresh-lease.test.ts-125- throw error; test/refresh-lease.test.ts-126- } test/refresh-lease.test.ts-127- return originalUnlink(path); -- test/refresh-lease.test.ts-173- unlink: vi.fn(async (path: Parameters<typeof fsPromises.unlink>[0]) => { test/refresh-lease.test.ts-174- if (String(path).endsWith(".lock")) { test/refresh-lease.test.ts-175- const error = new Error("busy") as NodeJS.ErrnoException; test/refresh-lease.test.ts:176: error.code = "EBUSY"; test/refresh-lease.test.ts-177- throw error; test/refresh-lease.test.ts-178- } test/refresh-lease.test.ts-179- return originalUnlink(path); -- test/quota-cache.test.ts-105- } test/quota-cache.test.ts-106- }); test/quota-cache.test.ts-107- test/quota-cache.test.ts:108: it.each(["EBUSY", "EPERM"] as const)( test/quota-cache.test.ts-109- "retries transient %s while clearing cache", test/quota-cache.test.ts-110- async (code) => { test/quota-cache.test.ts-111- const { clearQuotaCache, getQuotaCachePath, saveQuotaCache } = -- test/quota-cache.test.ts-139- }, test/quota-cache.test.ts-140- ); test/quota-cache.test.ts-141- test/quota-cache.test.ts:142: it.each(["EBUSY", "EPERM"] as const)( test/quota-cache.test.ts-143- "returns false when clearQuotaCache exhausts %s retries", test/quota-cache.test.ts-144- async (code) => { test/quota-cache.test.ts-145- vi.resetModules(); -- test/quota-cache.test.ts-183- }, test/quota-cache.test.ts-184- ); test/quota-cache.test.ts-185- test/quota-cache.test.ts:186: it("retries transient EBUSY while loading cache", async () => { test/quota-cache.test.ts-187- const { loadQuotaCache, getQuotaCachePath } = test/quota-cache.test.ts-188- await import("../lib/quota-cache.js"); test/quota-cache.test.ts-189- await fs.writeFile( -- test/quota-cache.test.ts-212- attempts += 1; test/quota-cache.test.ts-213- if (attempts === 1) { test/quota-cache.test.ts-214- const error = new Error("busy") as NodeJS.ErrnoException; test/quota-cache.test.ts:215: error.code = "EBUSY"; test/quota-cache.test.ts-216- throw error; test/quota-cache.test.ts-217- } test/quota-cache.test.ts-218- } -- test/quota-cache.test.ts-228- } test/quota-cache.test.ts-229- }); test/quota-cache.test.ts-230- test/quota-cache.test.ts:231: it.each(["EBUSY", "EPERM"] as const)( test/quota-cache.test.ts-232- "retries atomic rename on transient %s errors", test/quota-cache.test.ts-233- async (code) => { test/quota-cache.test.ts-234- const { saveQuotaCache, loadQuotaCache } = -- test/quota-cache.test.ts-276- const unlinkSpy = vi.spyOn(fs, "unlink"); test/quota-cache.test.ts-277- renameSpy.mockImplementation(async () => { test/quota-cache.test.ts-278- const error = new Error("locked") as NodeJS.ErrnoException; test/quota-cache.test.ts:279: error.code = "EPERM"; test/quota-cache.test.ts-280- throw error; test/quota-cache.test.ts-281- }); test/quota-cache.test.ts-282- -- test/recovery-constants.test.ts-3-import { homedir } from "node:os"; test/recovery-constants.test.ts-4- test/recovery-constants.test.ts-5-describe("recovery/constants.ts", () => { test/recovery-constants.test.ts:6: const originalPlatform = process.platform; test/recovery-constants.test.ts-7- const originalAppData = process.env.APPDATA; test/recovery-constants.test.ts-8- const originalXdgDataHome = process.env.XDG_DATA_HOME; test/recovery-constants.test.ts-9- -- test/recovery-constants.test.ts-42- test/recovery-constants.test.ts-43- describe("getXdgData on Windows", () => { test/recovery-constants.test.ts-44- it("should use APPDATA when set", async () => { test/recovery-constants.test.ts:45: Object.defineProperty(process, "platform", { value: "win32" }); test/recovery-constants.test.ts-46- process.env.APPDATA = "C:\\Users\\Test\\AppData\\Roaming"; test/recovery-constants.test.ts-47- test/recovery-constants.test.ts-48- const { CODEX_STORAGE } = await import("../lib/recovery/constants.js"); -- test/recovery-constants.test.ts-51- }); test/recovery-constants.test.ts-52- test/recovery-constants.test.ts-53- it("should fallback to AppData/Roaming when APPDATA is not set", async () => { test/recovery-constants.test.ts:54: Object.defineProperty(process, "platform", { value: "win32" }); test/recovery-constants.test.ts-55- delete process.env.APPDATA; test/recovery-constants.test.ts-56- test/recovery-constants.test.ts-57- const { CODEX_STORAGE } = await import("../lib/recovery/constants.js"); -- test/recovery.test.ts-54- } catch (error) { test/recovery.test.ts-55- const code = (error as NodeJS.ErrnoException).code; test/recovery.test.ts-56- if ( test/recovery.test.ts:57: (code !== "EBUSY" && code !== "EPERM" && code !== "ENOTEMPTY") || test/recovery.test.ts-58- attempt === 4 test/recovery.test.ts-59- ) { test/recovery.test.ts-60- throw error; -- test/recovery.test.ts-377- expect(result.assessments[0]?.imported).toBe(1); test/recovery.test.ts-378- }); test/recovery.test.ts-379- test/recovery.test.ts:380: it("keeps actionable backups when fast-path scan hits EBUSY", async () => { test/recovery.test.ts-381- const storage = await import("../lib/storage.js"); test/recovery.test.ts-382- const emptyStorage = { test/recovery.test.ts-383- version: 3, -- test/recovery.test.ts-427- const [path] = args; test/recovery.test.ts-428- if (path === lockedBackup?.path) { test/recovery.test.ts-429- const error = new Error("resource busy") as NodeJS.ErrnoException; test/recovery.test.ts:430: error.code = "EBUSY"; test/recovery.test.ts-431- throw error; test/recovery.test.ts-432- } test/recovery.test.ts-433- return originalReadFile(...args); -- test/recovery.test.ts-452- } test/recovery.test.ts-453- }); test/recovery.test.ts-454- test/recovery.test.ts:455: it("keeps actionable backups when fast-path metadata stat hits EBUSY", async () => { test/recovery.test.ts-456- const storage = await import("../lib/storage.js"); test/recovery.test.ts-457- const emptyStorage = { test/recovery.test.ts-458- version: 3, -- test/recovery.test.ts-504- normalizedPath.endsWith("/locked-backup.json") test/recovery.test.ts-505- ) { test/recovery.test.ts-506- const error = new Error("resource busy") as NodeJS.ErrnoException; test/recovery.test.ts:507: error.code = "EBUSY"; test/recovery.test.ts-508- throw error; test/recovery.test.ts-509- } test/recovery.test.ts-510- return originalStat(...args); -- test/recovery.test.ts-599- } test/recovery.test.ts-600- }); test/recovery.test.ts-601- test/recovery.test.ts:602: it("keeps injected-assessor backups when metadata stat hits EBUSY", async () => { test/recovery.test.ts-603- const storage = await import("../lib/storage.js"); test/recovery.test.ts-604- const emptyStorage = { test/recovery.test.ts-605- version: 3, -- test/recovery.test.ts-655- normalizedPath.endsWith("/locked-backup.json") test/recovery.test.ts-656- ) { test/recovery.test.ts-657- const error = new Error("resource busy") as NodeJS.ErrnoException; test/recovery.test.ts:658: error.code = "EBUSY"; test/recovery.test.ts-659- throw error; test/recovery.test.ts-660- } test/recovery.test.ts-661- return originalStat(...args); -- test/recovery.test.ts-694- } test/recovery.test.ts-695- }); test/recovery.test.ts-696- test/recovery.test.ts:697: it("keeps actionable backups when default assessment hits EBUSY", async () => { test/recovery.test.ts-698- const storage = await import("../lib/storage.js"); test/recovery.test.ts-699- const emptyStorage = { test/recovery.test.ts-700- version: 3, -- test/recovery.test.ts-743- const [path] = args; test/recovery.test.ts-744- if (path === lockedBackup?.path) { test/recovery.test.ts-745- const error = new Error("resource busy") as NodeJS.ErrnoException; test/recovery.test.ts:746: error.code = "EBUSY"; test/recovery.test.ts-747- throw error; test/recovery.test.ts-748- } test/recovery.test.ts-749- return originalReadFile(...args); -- test/paths.test.ts-117- process.env.HOME = "C:\\Users\\test"; test/paths.test.ts-118- process.env.USERPROFILE = "C:\\Users\\test"; test/paths.test.ts-119- process.env.CODEX_HOME = "C:\\Users\\test\\.codex"; test/paths.test.ts:120: const primary = path.win32.join("C:\\Users\\test\\.codex", "multi-auth"); test/paths.test.ts:121: const fallback = path.win32.join("C:\\Users\\test", "DevTools", "config", "codex", "multi-auth"); test/paths.test.ts:122: const normalizePath = (input: string) => path.win32.normalize(input.replace(/\//g, "\\")); test/paths.test.ts-123- test/paths.test.ts-124- mockedExistsSync.mockImplementation((candidate) => { test/paths.test.ts-125- if (typeof candidate !== "string") return false; test/paths.test.ts-126- const normalizedCandidate = normalizePath(candidate); test/paths.test.ts:127: if (normalizedCandidate === normalizePath(path.win32.join(primary, "settings.json"))) return true; test/paths.test.ts:128: if (normalizedCandidate === normalizePath(path.win32.join(primary, "openai-codex-accounts.json"))) return false; test/paths.test.ts:129: if (normalizedCandidate === normalizePath(path.win32.join(primary, "codex-accounts.json"))) return false; test/paths.test.ts:130: if (normalizedCandidate === normalizePath(path.win32.join(primary, "config.json"))) return false; test/paths.test.ts:131: if (normalizedCandidate === normalizePath(path.win32.join(primary, "dashboard-settings.json"))) return false; test/paths.test.ts:132: if (normalizedCandidate === normalizePath(path.win32.join(primary, "projects"))) return false; test/paths.test.ts:133: return normalizedCandidate === normalizePath(path.win32.join(fallback, "openai-codex-accounts.json")); test/paths.test.ts-134- }); test/paths.test.ts-135- test/paths.test.ts-136- try { -- test/paths.test.ts-207- const sharedRepoRoot = path.resolve("repo"); test/paths.test.ts-208- const sharedGitDir = path.join(sharedRepoRoot, ".git"); test/paths.test.ts-209- const normalize = (value: string) => test/paths.test.ts:210: process.platform === "win32" ? value.toLowerCase() : value; test/paths.test.ts-211- test/paths.test.ts-212- mockedExistsSync.mockImplementation((candidate) => { test/paths.test.ts-213- if (typeof candidate !== "string") return false; -- test/paths.test.ts-253- const sharedRepoRoot = path.resolve("repo"); test/paths.test.ts-254- const sharedGitDir = path.join(sharedRepoRoot, ".git"); test/paths.test.ts-255- const normalize = (value: string) => test/paths.test.ts:256: process.platform === "win32" ? value.toLowerCase() : value; test/paths.test.ts-257- test/paths.test.ts-258- mockedExistsSync.mockImplementation((candidate) => { test/paths.test.ts-259- if (typeof candidate !== "string") return false; -- test/paths.test.ts-291- }); test/paths.test.ts-292- test/paths.test.ts-293- it("supports Windows-style backslash gitdir pointers", () => { test/paths.test.ts:294: const projectRoot = path.win32.join("C:\\repo", "worktrees", "pr-8"); test/paths.test.ts:295: const gitEntry = path.win32.join(projectRoot, ".git"); test/paths.test.ts:296: const worktreeGitDir = path.win32.join("C:\\repo", ".git", "worktrees", "pr-8"); test/paths.test.ts:297: const commondirFile = path.win32.join(worktreeGitDir, "commondir"); test/paths.test.ts:298: const gitdirBackRefFile = path.win32.join(worktreeGitDir, "gitdir"); test/paths.test.ts-299- const sharedRepoRoot = "C:\\repo"; test/paths.test.ts:300: const sharedGitDir = path.win32.join(sharedRepoRoot, ".git"); test/paths.test.ts:301: const normalize = (value: string) => path.win32.normalize(value).toLowerCase(); test/paths.test.ts-302- test/paths.test.ts-303- mockedExistsSync.mockImplementation((candidate) => { test/paths.test.ts-304- if (typeof candidate !== "string") return false; -- test/paths.test.ts-325- return "..\\..\\\n"; test/paths.test.ts-326- } test/paths.test.ts-327- if (normalizedCandidate === normalize(gitdirBackRefFile)) { test/paths.test.ts:328: return `${path.win32.join(projectRoot, ".git")}\n`; test/paths.test.ts-329- } test/paths.test.ts-330- throw new Error(`Unexpected read path: ${String(candidate)}`); test/paths.test.ts-331- }); -- test/paths.test.ts-337- test/paths.test.ts-338- it("supports windows UNC gitdir pointers", () => { test/paths.test.ts-339- const sharedRepoRoot = "\\\\server\\share\\repo"; test/paths.test.ts:340: const projectRoot = path.win32.join(sharedRepoRoot, "worktrees", "pr-unc"); test/paths.test.ts:341: const gitEntry = path.win32.join(projectRoot, ".git"); test/paths.test.ts:342: const worktreeGitDir = path.win32.join(sharedRepoRoot, ".git", "worktrees", "pr-unc"); test/paths.test.ts:343: const commondirFile = path.win32.join(worktreeGitDir, "commondir"); test/paths.test.ts:344: const gitdirBackRefFile = path.win32.join(worktreeGitDir, "gitdir"); test/paths.test.ts:345: const sharedGitDir = path.win32.join(sharedRepoRoot, ".git"); test/paths.test.ts:346: const normalize = (value: string) => path.win32.normalize(value).toLowerCase(); test/paths.test.ts-347- test/paths.test.ts-348- mockedExistsSync.mockImplementation((candidate) => { test/paths.test.ts-349- if (typeof candidate !== "string") return false; -- test/paths.test.ts-371- return "..\\..\\\n"; test/paths.test.ts-372- } test/paths.test.ts-373- if (normalizedCandidate === normalize(gitdirBackRefFile)) { test/paths.test.ts:374: return `${path.win32.join(projectRoot, ".git")}\n`; test/paths.test ... [stdout truncated: output exceeded 50000 characters (92678 total). Please modify your command to reduce output size, for example by: - Using grep/rg to filter results - Using head/tail to limit lines - Using --max-count flag with grep/rg - Processing output with awk/sed to extract only needed information]

add windows-platform test coverage for EPERM/EBUSY retries in retryTransientFilesystemOperation.

platform-specific tests exist for Linux behavior at test/storage.test.ts:1590–1618 (EPERM/EBUSY rethrow without retry), but windows-specific coverage is missing. lib/storage.ts:1785 narrows EPERM/EBUSY retries to win32 only—this is a meaningful constraint shift from earlier retry logic at lib/storage.ts:290, lib/storage.ts:469, and lib/storage.ts:493 which retried all three codes universally. add explicit tests that mock process.platform = "win32" and verify EPERM/EBUSY are actually retried by the new function, alongside the existing Linux rethrow assertions.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@lib/storage.ts` around lines 1779 - 1806, Add Windows-specific tests that mock process.platform = "win32" and assert that retryTransientFilesystemOperation actually retries on EPERM and EBUSY: for each code, create a fake operation that throws a NodeJS.ErrnoException with .code set to "EPERM" or "EBUSY" for the first N attempts then resolves, call retryTransientFilesystemOperation and assert it eventually returns and that the fake was called multiple times; restore process.platform after each test. Use the same pattern to keep the Linux tests (process.platform !== "win32") asserting EPERM/EBUSY rethrow immediately, and reference the functions isRetryableFilesystemErrorCode and retryTransientFilesystemOperation when locating the code under test.

lib/storage.ts

coderabbitai · 2026-03-16T16:14:10Z

test/recovery.test.ts

+	it("keeps actionable backups when fast-path scan hits EBUSY", async () => {
+		const storage = await import("../lib/storage.js");
+		const emptyStorage = {
+			version: 3,
+			activeIndex: 0,
+			activeIndexByFamily: { codex: 0 },
+			accounts: [],
+		};
+		await storage.saveAccounts({
+			version: 3,
+			activeIndex: 0,
+			activeIndexByFamily: { codex: 0 },
+			accounts: [
+				{
+					email: "locked@example.com",
+					refreshToken: "locked-token",
+					addedAt: 1,
+					lastUsed: 1,
+				},
+			],
+		});
+		await storage.createNamedBackup("locked-backup");
+		await storage.saveAccounts({
+			version: 3,
+			activeIndex: 0,
+			activeIndexByFamily: { codex: 0 },
+			accounts: [
+				{
+					email: "valid@example.com",
+					refreshToken: "valid-token",
+					addedAt: 2,
+					lastUsed: 2,
+				},
+			],
+		});
+		await storage.createNamedBackup("valid-backup");
+		await storage.saveAccounts(emptyStorage);
+
+		const backups = await storage.listNamedBackups();
+		const lockedBackup = backups.find((backup) => backup.name === "locked-backup");
+		const validBackup = backups.find((backup) => backup.name === "valid-backup");
+		expect(lockedBackup).toBeDefined();
+		expect(validBackup).toBeDefined();
+
+		const originalReadFile = fs.readFile.bind(fs);
+		const readFileSpy = vi.spyOn(fs, "readFile").mockImplementation(
+			(async (...args: Parameters<typeof fs.readFile>) => {
+				const [path] = args;
+				if (path === lockedBackup?.path) {
+					const error = new Error("resource busy") as NodeJS.ErrnoException;
+					error.code = "EBUSY";
+					throw error;
+				}
+				return originalReadFile(...args);
+			}) as typeof fs.readFile,
+		);
+
+		try {
+			const result = await storage.getActionableNamedBackupRestores({
+				currentStorage: emptyStorage,
+			});
+
+			expect(result.totalBackups).toBe(2);
+			expect(result.assessments.map((item) => item.backup.name)).toEqual([
+				"valid-backup",
+			]);
+			expect(result.assessments[0]?.imported).toBe(1);
+			expect(readFileSpy.mock.calls.map(([path]) => path)).toEqual(
+				expect.arrayContaining([lockedBackup?.path, validBackup?.path]),
+			);
+		} finally {
+			readFileSpy.mockRestore();
+		}
+	});


⚠️ Potential issue | 🟠 Major

gate ebusy fast-path recovery tests to explicit platform behavior

these tests assert EBUSY handling outcomes but do not pin process.platform, so expectations can drift by runtime environment. make the platform explicit for each intended behavior to keep test/recovery.test.ts:380-453, test/recovery.test.ts:455-531, and test/recovery.test.ts:697-779 deterministic.

As per coding guidelines, test/**: tests must stay deterministic and use vitest, and demand regression cases that reproduce windows filesystem behavior.

Also applies to: 455-531, 697-779

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@test/recovery.test.ts` around lines 380 - 453, The test "keeps actionable backups when fast-path scan hits EBUSY" relies on platform-specific EBUSY handling; wrap the part that calls getActionableNamedBackupRestores with an explicit process.platform override (save original, use Object.defineProperty to set process.platform to 'win32' for the Windows behavior the test expects, then restore original afterward) so the expectations remain deterministic; apply the same pattern to the other related tests mentioned (the ones calling getActionableNamedBackupRestores and asserting EBUSY behavior) to ensure consistent platform-specific behavior.

coderabbitai · 2026-03-16T16:14:10Z

test/recovery.test.ts

+describe("resolveStartupRecoveryAction", () => {
+	it("falls through to OAuth when backups exist but none are actionable", async () => {
+		const { resolveStartupRecoveryAction } = await import(
+			"../lib/codex-manager.js"
+		);
+
+		expect(
+			resolveStartupRecoveryAction(
+				{ assessments: [], totalBackups: 2 },
+				false,
+			),
+		).toBe("continue-with-oauth");
+		expect(
+			resolveStartupRecoveryAction(
+				{ assessments: [], totalBackups: 2 },
+				false,
+			),
+		).not.toBe("open-empty-storage-menu");
+	});
+
+	it("falls through to OAuth when the startup recovery scan itself failed", async () => {
+		const { resolveStartupRecoveryAction } = await import(
+			"../lib/codex-manager.js"
+		);
+
+		expect(
+			resolveStartupRecoveryAction(
+				{ assessments: [], totalBackups: 0 },
+				true,
+			),
+		).toBe("continue-with-oauth");
+	});
 });


⚠️ Potential issue | 🟠 Major

add missing startup recovery routing regressions

test/recovery.test.ts:783-815 only covers oauth fallthrough. it does not cover the positive actionable-backup route or same-session suppression states, which are the highest-risk branches for startup behavior regressions.

please add cases for:

actionable backups + no suppression => recovery prompt path

actionable backups + suppression => continue oauth

zero backups + no scan error => continue oauth (already covered)

As per coding guidelines, test/**: demand regression cases that reproduce critical flow regressions.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@test/recovery.test.ts` around lines 783 - 815, Add regression tests for resolveStartupRecoveryAction to cover the missing branches: call resolveStartupRecoveryAction with an input object where assessments contains at least one actionable assessment (e.g., assessments: [{ actionable: true }]) and totalBackups > 0 and scanFailed = false and assert it returns the recovery prompt route (the code path currently implied by "open-empty-storage-menu"/recovery prompt); then add a test where assessments contains an actionable item but scanning is suppressed (pass the suppression/scanFailed flag that triggers same-session suppression) and assert it returns "continue-with-oauth". Keep the existing tests for zero backups/no scan error as-is. Reference resolveStartupRecoveryAction and the input shape (assessments, totalBackups, scanFailed) when adding the two new cases.

coderabbitai · 2026-03-16T16:14:10Z

test/storage.test.ts

+		it("rethrows EBUSY backup directory errors after one attempt on non-Windows platforms", async () => {
 			const platformSpy = vi
 				.spyOn(process, "platform", "get")
-				.mockReturnValue("win32");
+				.mockReturnValue("linux");
+			const readdirSpy = vi.spyOn(fs, "readdir");
+			const error = new Error("backup directory busy") as NodeJS.ErrnoException;
+			error.code = "EBUSY";
+			readdirSpy.mockRejectedValue(error);
+
+			try {
+				await expect(listNamedBackups()).rejects.toMatchObject({ code: "EBUSY" });
+				expect(readdirSpy).toHaveBeenCalledTimes(1);
+			} finally {


⚠️ Potential issue | 🟠 Major

fix platform ambiguity in ebusy retry coverage

test/storage.test.ts:1605-1617 explicitly asserts no EBUSY retries on non-windows, but test/storage.test.ts:1638-1678 expects EBUSY retry success without pinning the platform. this is contradictory and can mask regressions.

proposed test fix

it("retries transient backup directory errors while listing backups", async () => { + const platformSpy = vi + .spyOn(process, "platform", "get") + .mockReturnValue("win32"); await saveAccounts({ version: 3, activeIndex: 0, @@ try { const backups = await listNamedBackups(); @@ } finally { readdirSpy.mockRestore(); + platformSpy.mockRestore(); } });

As per coding guidelines, test/**: tests must stay deterministic and use vitest, and demand regression cases that reproduce windows filesystem behavior.

Also applies to: 1638-1678

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@test/storage.test.ts` around lines 1605 - 1617, The two tests are ambiguous about platform: ensure the non-Windows test for EBUSY retry uses a mocked process.platform returning "linux" and the Windows-retry test uses process.platform returning "win32" so behavior is deterministic; in the non-Windows case (test around listNamedBackups) keep vi.spyOn(process, "platform", "get").mockReturnValue("linux") and assert readdir was called once, and in the separate Windows-retry test (lines ~1638-1678) mock process.platform to "win32" and set readdir to initially reject with an EBUSY error then succeed on retry to assert retry behavior; also restore/clear spies/mocks after each test to avoid cross-test leakage.

lib/codex-manager.ts

ndycode added 5 commits March 15, 2026 23:19

fix(auth): define reset and delete safety flows

af7f137

feat(auth): add backup restore manager

f48ee06

feat(auth): prompt for recovery on startup

12074e2

test(storage): isolate empty export path

8212831

fix(auth): clean startup recovery prompt branch

390f801

Copilot AI review requested due to automatic review settings March 16, 2026 08:51

Copilot AI reviewed Mar 16, 2026

View reviewed changes

greptile-apps bot reviewed Mar 16, 2026

View reviewed changes

lib/storage.ts Outdated Show resolved Hide resolved

lib/storage.ts Outdated Show resolved Hide resolved

lib/codex-manager.ts Show resolved Hide resolved

lib/codex-manager.ts Show resolved Hide resolved

fix(storage): restore safe import and restore-backup handling

a7d87cd

greptile-apps bot reviewed Mar 16, 2026

View reviewed changes

lib/storage.ts Show resolved Hide resolved

lib/storage.ts Show resolved Hide resolved

lib/storage.ts Show resolved Hide resolved

lib/codex-manager.ts Show resolved Hide resolved

coderabbitai bot requested changes Mar 16, 2026

View reviewed changes

ndycode added 2 commits March 16, 2026 22:34

Block non-interactive backup restore mode

1eeec0b

Fix startup recovery backup accounting

474fbc4

greptile-apps bot reviewed Mar 16, 2026

View reviewed changes

test/recovery.test.ts Outdated Show resolved Hide resolved

lib/storage.ts Show resolved Hide resolved

ndycode added 3 commits March 16, 2026 23:30

fix(auth): harden restore backup fallback and file reads

326d5f1

fix(auth): sync direct restore and narrow retries

de6e256

Merge branch 'fresh/02-backup-restore-manager' into fresh/03-startup-…

800d49f

…recovery-prompt

coderabbitai bot requested changes Mar 16, 2026

View reviewed changes

greptile-apps bot reviewed Mar 16, 2026

View reviewed changes

lib/codex-manager.ts Show resolved Hide resolved

lib/codex-manager.ts Show resolved Hide resolved

fix(startup): restore backup recovery safeguards

d92f36e

ndycode changed the base branch from fresh/02-backup-restore-manager to dev March 18, 2026 06:47

Conversation

ndycode commented Mar 16, 2026 • edited by greptile-apps bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What Changed

Validation

Docs and Governance Checklist

Risk and Rollback

Additional Notes

note: greptile review for oc-chatgpt-multi-auth. cite files like lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

Confidence Score: 2/5

Important Files Changed

Flowchart

Comments Outside Diff (4)

Uh oh!

chatgpt-codex-connector bot commented Mar 16, 2026

Uh oh!

coderabbitai bot commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram

Estimated code review effort

key concerns for review

❌ Failed checks (2 warnings)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

ndycode commented Mar 16, 2026 •

edited by greptile-apps bot

Loading

note: greptile review for oc-chatgpt-multi-auth. cite files like `lib/foo.ts:123`. confirm regression tests + windows concurrency/token redaction coverage.

coderabbitai bot commented Mar 16, 2026 •

edited

Loading