fix(auth): harden onboarding restore followups

Author: ndycode

docs/getting-started.md

@@ -43,15 +43,15 @@ codex auth login
 
 Expected flow:
 
-Backup restore appears under the `Recover saved accounts` heading in the onboarding menu. This flow does not add any new CLI flags or npm scripts.
+Backup restore appears as `Restore Saved Backup` under the `Recover saved accounts` heading in the onboarding menu. This flow does not add any new CLI flags or npm scripts.
 See upgrade note: [onboarding restore behavior](upgrade.md#onboarding-restore-note).
 
 1. If no accounts are saved yet, the terminal opens directly to the sign-in menu.
 2. Choose one of the sign-in options:
    - `Open Browser (Easy)` for the normal OAuth flow
    - `Manual / Incognito` when you need to paste the callback yourself
-   - `Recover saved accounts` when the current pool is empty and at least one valid named backup exists under `~/.codex/multi-auth/backups` by default, or under `%CODEX_MULTI_AUTH_DIR%\backups` if you override the storage root with `CODEX_MULTI_AUTH_DIR`
-3. If you choose `Recover saved accounts`, the next menu lets you either:
+   - `Restore Saved Backup` under the `Recover saved accounts` heading when the current pool is empty and at least one valid named backup exists under `~/.codex/multi-auth/backups` by default, or under `%CODEX_MULTI_AUTH_DIR%\backups` if you override the storage root with `CODEX_MULTI_AUTH_DIR`
+3. If you choose `Restore Saved Backup`, the next menu lets you either:
    - load the newest valid backup automatically
    - pick a specific backup from a newest-first list
    Empty, unreadable, or non-JSON backup sidecar files are skipped, so the menu entry appears only when at least one backup parses successfully and contains at least one account.

lib/codex-manager.ts

@@ -1155,6 +1155,7 @@ export function formatBackupSavedAt(mtimeMs: number): string {
 
 async function promptOAuthSignInMode(
 	backupOption: NamedBackupSummary | null,
+	backupDiscoveryWarning: string | null = null,
 ): Promise<OAuthSignInMode> {
 	if (!input.isTTY || !output.isTTY) {
 		return "browser";
@@ -1187,7 +1188,9 @@ async function promptOAuthSignInMode(
 
 	const selected = await select<OAuthSignInMode>(items, {
 		message: UI_COPY.oauth.chooseModeTitle,
-		subtitle: UI_COPY.oauth.chooseModeSubtitle,
+		subtitle: backupDiscoveryWarning
+			? `${UI_COPY.oauth.chooseModeSubtitle} ${backupDiscoveryWarning}`
+			: UI_COPY.oauth.chooseModeSubtitle,
 		help: backupOption
 			? UI_COPY.oauth.chooseModeHelpWithBackup
 			: UI_COPY.oauth.chooseModeHelp,
@@ -4357,11 +4360,14 @@ async function runAuthLogin(): Promise<number> {
 		const refreshedStorage = await loadAccounts();
 		let existingCount = refreshedStorage?.accounts.length ?? 0;
 		let forceNewLogin = existingCount > 0;
+		let onboardingBackupDiscoveryWarning: string | null = null;
 		const loadNamedBackupsForOnboarding = async (): Promise<NamedBackupSummary[]> => {
 			if (existingCount > 0) {
+				onboardingBackupDiscoveryWarning = null;
 				return [];
 			}
 			try {
+				onboardingBackupDiscoveryWarning = null;
 				return await getNamedBackups();
 			} catch (error) {
 				const code = (error as NodeJS.ErrnoException).code;
@@ -4370,17 +4376,24 @@ async function runAuthLogin(): Promise<number> {
 					error: error instanceof Error ? error.message : String(error),
 				});
 				if (code && code !== "ENOENT") {
+					onboardingBackupDiscoveryWarning =
+						"Named backup discovery failed. Continuing with browser or manual sign-in only.";
 					console.warn(
-						"Named backup discovery failed. Continuing with browser or manual sign-in only.",
+						onboardingBackupDiscoveryWarning,
 					);
+				} else {
+					onboardingBackupDiscoveryWarning = null;
 				}
 				return [];
 			}
 		};
 		let namedBackups = await loadNamedBackupsForOnboarding();
 		while (true) {
 			const latestNamedBackup = namedBackups[0] ?? null;
-			const signInMode = await promptOAuthSignInMode(latestNamedBackup);
+			const signInMode = await promptOAuthSignInMode(
+				latestNamedBackup,
+				onboardingBackupDiscoveryWarning,
+			);
 			if (signInMode === "cancel") {
 				if (existingCount > 0) {
 					console.log(stylePromptText(UI_COPY.oauth.cancelledBackToMenu, "muted"));

test/codex-manager-cli.test.ts

@@ -3100,6 +3100,12 @@ describe("codex manager cli commands", () => {
 				expect.objectContaining({
 					activeIndex: 1,
 					activeIndexByFamily: { codex: 1 },
+					accounts: expect.arrayContaining([
+						expect.objectContaining({
+							accountId: "acc_restored",
+							lastSwitchReason: "restore",
+						}),
+					]),
 				}),
 			);
 			expect(setCodexCliActiveSelectionMock).toHaveBeenCalledWith(
@@ -3344,6 +3350,12 @@ describe("codex manager cli commands", () => {
 		expect(signInItems.some((item) => item.value === "restore-backup")).toBe(
 			false,
 		);
+		const signInOptions = selectMock.mock.calls[0]?.[1] as {
+			subtitle?: string;
+		};
+		expect(signInOptions.subtitle).toContain(
+			"Named backup discovery failed. Continuing with browser or manual sign-in only.",
+		);
 		expect(loggerDebugMock).toHaveBeenCalledWith(
 			"getNamedBackups failed, skipping restore option",
 			{ code: "EPERM", error: "backups directory is locked" },
@@ -3560,6 +3572,61 @@ describe("codex manager cli commands", () => {
 		);
 	});
 
+	it("returns to the existing-account menu when restore fails after storage is already written", async () => {
+		const now = Date.now();
+		setInteractiveTTY(true);
+		const restoredStorage = {
+			version: 3 as const,
+			activeIndex: 0,
+			activeIndexByFamily: { codex: 0 },
+			accounts: [
+				{
+					email: "persisted-after-error@example.com",
+					accountId: "acc_persisted_after_error",
+					refreshToken: "refresh-persisted-after-error",
+					accessToken: "access-persisted-after-error",
+					expiresAt: now + 3_600_000,
+					addedAt: now - 1_000,
+					lastUsed: now - 1_000,
+					enabled: true,
+				},
+			],
+		};
+		let storageState: typeof restoredStorage | null = null;
+		loadAccountsMock.mockImplementation(async () =>
+			storageState == null ? null : structuredClone(storageState),
+		);
+		saveAccountsMock.mockImplementationOnce(async (nextStorage) => {
+			storageState = structuredClone(nextStorage);
+			throw new Error("save selected account failed");
+		});
+		getNamedBackupsMock.mockResolvedValue([
+			{
+				path: "/mock/backups/persisted-after-error.json",
+				fileName: "persisted-after-error.json",
+				accountCount: 1,
+				mtimeMs: now,
+			},
+		]);
+		restoreAccountsFromBackupMock.mockResolvedValue(structuredClone(restoredStorage));
+		selectMock
+			.mockResolvedValueOnce("restore-backup")
+			.mockResolvedValueOnce("latest");
+		promptLoginModeMock.mockResolvedValueOnce({ mode: "cancel" });
+		const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
+		const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js");
+
+		const exitCode = await runCodexMultiAuthCli(["auth", "login"]);
+
+		expect(exitCode).toBe(0);
+		expect(saveAccountsMock).toHaveBeenCalledTimes(1);
+		expect(promptLoginModeMock).toHaveBeenCalledTimes(1);
+		expect(selectMock).toHaveBeenCalledTimes(2);
+		expect(errorSpy).toHaveBeenCalledWith(
+			"Backup restore failed: save selected account failed",
+		);
+	});
+
 	it("does not restore the latest backup when confirmation is declined", async () => {
 		const now = Date.now();
 		setInteractiveTTY(true);

test/storage-last-backup.test.ts

@@ -266,7 +266,7 @@ describe("storage last backup restore", () => {
 		const originalStat = fs.stat.bind(fs);
 		const statSpy = vi.spyOn(fs, "stat").mockImplementation(async (path, options) => {
 			if (String(path) === flakyBackupPath) {
-				await fs.rm(flakyBackupPath, { force: true });
+				await removeWithRetry(flakyBackupPath, { force: true });
 				const error = new Error(
 					`ENOENT: no such file or directory, stat '${flakyBackupPath}'`,
 				) as NodeJS.ErrnoException;
@@ -405,7 +405,7 @@ describe("storage last backup restore", () => {
 		const originalRealpath = fs.realpath.bind(fs);
 		const realpathSpy = vi.spyOn(fs, "realpath").mockImplementation(async (path, options) => {
 			if (String(path) === backupPath) {
-				await fs.rm(backupPath, { force: true });
+				await removeWithRetry(backupPath, { force: true });
 				const error = new Error(
 					`ENOENT: no such file or directory, realpath '${backupPath}'`,
 				) as NodeJS.ErrnoException;
@@ -445,7 +445,7 @@ describe("storage last backup restore", () => {
 		const originalReadFile = fs.readFile.bind(fs);
 		const readFileSpy = vi.spyOn(fs, "readFile").mockImplementation(async (path, options) => {
 			if (String(path) === backupPath || String(path) === resolvedBackupPath) {
-				await fs.rm(backupPath, { force: true });
+				await removeWithRetry(backupPath, { force: true });
 				const error = new Error(
 					`ENOENT: no such file or directory, open '${backupPath}'`,
 				) as NodeJS.ErrnoException;
@@ -468,6 +468,9 @@ describe("storage last backup restore", () => {
 	});
 
 	it("accepts UNC-resolved backups inside the managed root and rejects cross-share escapes", async () => {
+		if (process.platform !== "win32") {
+			return;
+		}
 		const insideBackupPath = buildNamedBackupPath("backup-unc-inside");
 		const outsideBackupPath = buildNamedBackupPath("backup-unc-outside");
 		const backupRoot = dirname(insideBackupPath);