Skip to content

Commit 3c9aab4

Browse files
committed
test(auth): cover remaining onboarding restore gaps
1 parent 198b3d6 commit 3c9aab4

6 files changed

Lines changed: 165 additions & 5 deletions

File tree

docs/getting-started.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,15 +44,17 @@ codex auth login
4444
Expected flow:
4545

4646
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.
47+
See upgrade note: [onboarding restore behavior](upgrade.md#onboarding-restore-note).
4748

4849
1. If no accounts are saved yet, the terminal opens directly to the sign-in menu.
4950
2. Choose one of the sign-in options:
5051
- `Open Browser (Easy)` for the normal OAuth flow
5152
- `Manual / Incognito` when you need to paste the callback yourself
52-
- `Recover saved accounts` when the current pool is empty and a 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`
53+
- `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`
5354
3. If you choose `Recover saved accounts`, the next menu lets you either:
5455
- load the newest valid backup automatically
5556
- pick a specific backup from a newest-first list
57+
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.
5658
4. If you use browser or manual sign-in, complete the official OAuth flow and return to the terminal.
5759
5. If you load a backup, the selected backup is restored, its active account is synced back into Codex CLI auth, and the login flow continues with that restored pool.
5860
6. Confirm the restored or newly signed-in account appears in the saved account list.

docs/upgrade.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,16 @@ For maintainer/debug flows, see advanced/internal controls in [development/CONFI
6464

6565
---
6666

67+
## Onboarding Restore Note
68+
69+
`codex auth login` now opens directly into the sign-in menu when the active pool is empty, instead of opening the account dashboard first.
70+
71+
- `Recover saved accounts` appears only when at least one valid named backup exists.
72+
- No new CLI flags or npm scripts were added for this flow.
73+
- The backup root remains `~/.codex/multi-auth/backups` by default, or `%CODEX_MULTI_AUTH_DIR%\\backups` when `CODEX_MULTI_AUTH_DIR` is set.
74+
75+
---
76+
6777
## Legacy Compatibility
6878

6979
Legacy files may still be discovered during migration-only compatibility checks.

lib/codex-manager.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1141,7 +1141,7 @@ async function promptManualCallback(state: string): Promise<string | null> {
11411141
type OAuthSignInMode = "browser" | "manual" | "restore-backup" | "cancel";
11421142
type BackupRestoreMode = "latest" | "manual" | "back";
11431143

1144-
function formatBackupSavedAt(mtimeMs: number): string {
1144+
export function formatBackupSavedAt(mtimeMs: number): string {
11451145
return new Date(mtimeMs).toLocaleString(undefined, {
11461146
month: "short",
11471147
day: "numeric",

lib/storage.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,11 @@ async function collectNamedBackups(storagePath: string): Promise<NamedBackupSumm
166166
}
167167
}
168168

169-
candidates.sort((left, right) => right.mtimeMs - left.mtimeMs);
169+
candidates.sort((left, right) => {
170+
const mtimeDelta = right.mtimeMs - left.mtimeMs;
171+
if (mtimeDelta !== 0) return mtimeDelta;
172+
return left.fileName.localeCompare(right.fileName);
173+
});
170174
return candidates;
171175
}
172176

@@ -898,8 +902,9 @@ export async function restoreAccountsFromBackup(
898902
}
899903
const relativePath = relative(resolvedBackupRoot, resolvedBackupPath);
900904
const isInsideBackupRoot =
901-
relativePath === "" ||
902-
(!relativePath.startsWith("..") && !isAbsolute(relativePath));
905+
relativePath.length > 0 &&
906+
!relativePath.startsWith("..") &&
907+
!isAbsolute(relativePath);
903908
if (!isInsideBackupRoot) {
904909
throw new Error(`Backup path must stay inside ${resolvedBackupRoot}: ${path}`);
905910
}

test/codex-manager-cli.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,26 @@ describe("codex manager cli commands", () => {
539539
vi.restoreAllMocks();
540540
});
541541

542+
it("formats backup saved-at timestamps with the runtime locale options", async () => {
543+
const localeSpy = vi
544+
.spyOn(Date.prototype, "toLocaleString")
545+
.mockReturnValue("Localized Saved Time");
546+
const { formatBackupSavedAt } = await import("../lib/codex-manager.js");
547+
548+
try {
549+
expect(formatBackupSavedAt(1_710_000_000_000)).toBe("Localized Saved Time");
550+
expect(localeSpy).toHaveBeenCalledWith(undefined, {
551+
month: "short",
552+
day: "numeric",
553+
year: "numeric",
554+
hour: "numeric",
555+
minute: "2-digit",
556+
});
557+
} finally {
558+
localeSpy.mockRestore();
559+
}
560+
});
561+
542562
it("runs forecast in json mode", async () => {
543563
const now = Date.now();
544564
loadAccountsMock.mockResolvedValueOnce({
@@ -3281,6 +3301,7 @@ describe("codex manager cli commands", () => {
32813301
.mockResolvedValueOnce("restore-backup")
32823302
.mockResolvedValueOnce("latest");
32833303
promptLoginModeMock.mockResolvedValueOnce({ mode: "cancel" });
3304+
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
32843305
const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js");
32853306

32863307
const exitCode = await runCodexMultiAuthCli(["auth", "login"]);
@@ -3297,6 +3318,9 @@ describe("codex manager cli commands", () => {
32973318
email: "warn@example.com",
32983319
}),
32993320
);
3321+
expect(logSpy).toHaveBeenCalledWith(
3322+
expect.stringContaining("Codex auth sync did not complete"),
3323+
);
33003324
expect(promptLoginModeMock).toHaveBeenCalledTimes(1);
33013325
expect(selectMock).toHaveBeenCalledTimes(2);
33023326
});
@@ -3330,6 +3354,9 @@ describe("codex manager cli commands", () => {
33303354
"C:/mock/backups/locked.json",
33313355
{ persist: false },
33323356
);
3357+
expect(saveAccountsMock).not.toHaveBeenCalled();
3358+
expect(promptLoginModeMock).not.toHaveBeenCalled();
3359+
expect(selectMock).toHaveBeenCalledTimes(3);
33333360
expect(errorSpy).toHaveBeenCalledWith("Backup restore failed: File is busy");
33343361
});
33353362

@@ -3609,6 +3636,7 @@ describe("codex manager cli commands", () => {
36093636
const exitCode = await runCodexMultiAuthCli(["auth", "login"]);
36103637

36113638
expect(exitCode).toBe(0);
3639+
expect(getNamedBackupsMock).toHaveBeenCalledTimes(1);
36123640
expect(restoreAccountsFromBackupMock).toHaveBeenCalledWith(
36133641
"/mock/backups/manual-choice.json",
36143642
{ persist: false },

test/storage-last-backup.test.ts

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,60 @@ describe("storage last backup restore", () => {
115115
]);
116116
});
117117

118+
it("breaks backup mtime ties with a deterministic file-name order", async () => {
119+
const alphaBackupPath = buildNamedBackupPath("backup-alpha");
120+
const betaBackupPath = buildNamedBackupPath("backup-beta");
121+
await fs.mkdir(dirname(alphaBackupPath), { recursive: true });
122+
for (const backupPath of [betaBackupPath, alphaBackupPath]) {
123+
await fs.writeFile(
124+
backupPath,
125+
JSON.stringify({
126+
version: 3,
127+
activeIndex: 0,
128+
activeIndexByFamily: { codex: 0 },
129+
accounts: [{ refreshToken: `${backupPath}-refresh`, addedAt: 1, lastUsed: 1 }],
130+
}),
131+
"utf-8",
132+
);
133+
await fs.utimes(
134+
backupPath,
135+
new Date("2026-03-04T00:00:00.000Z"),
136+
new Date("2026-03-04T00:00:00.000Z"),
137+
);
138+
}
139+
140+
const backups = await getNamedBackups();
141+
142+
expect(backups.map((backup) => backup.fileName)).toEqual([
143+
"backup-alpha.json",
144+
"backup-beta.json",
145+
]);
146+
});
147+
148+
it("ignores non-json files and subdirectories in the backup root", async () => {
149+
const backupPath = buildNamedBackupPath("real-backup");
150+
const backupRoot = dirname(backupPath);
151+
await fs.mkdir(backupRoot, { recursive: true });
152+
await fs.writeFile(
153+
backupPath,
154+
JSON.stringify({
155+
version: 3,
156+
activeIndex: 0,
157+
activeIndexByFamily: { codex: 0 },
158+
accounts: [{ refreshToken: "real-refresh", addedAt: 1, lastUsed: 1 }],
159+
}),
160+
"utf-8",
161+
);
162+
await fs.writeFile(join(backupRoot, "sidecar.tmp"), "lock", "utf-8");
163+
await fs.writeFile(join(backupRoot, "notes.bak"), "ignored", "utf-8");
164+
await fs.mkdir(join(backupRoot, "subdir"), { recursive: true });
165+
166+
const backups = await getNamedBackups();
167+
168+
expect(backups).toHaveLength(1);
169+
expect(backups[0]?.fileName).toBe("real-backup.json");
170+
});
171+
118172
it("skips a backup that disappears before stat and falls back to the next valid backup", async () => {
119173
const flakyBackupPath = buildNamedBackupPath("backup-flaky");
120174
const stableBackupPath = buildNamedBackupPath("backup-stable");
@@ -198,8 +252,14 @@ describe("storage last backup restore", () => {
198252

199253
expect(restored.accounts).toHaveLength(2);
200254
expect(restored.activeIndex).toBe(1);
255+
expect(restored.activeIndexByFamily).toEqual(
256+
expect.objectContaining({ codex: 1 }),
257+
);
201258
expect(loaded?.accounts).toHaveLength(2);
202259
expect(loaded?.activeIndex).toBe(1);
260+
expect(loaded?.activeIndexByFamily).toEqual(
261+
expect.objectContaining({ codex: 1 }),
262+
);
203263
expect(loaded?.accounts[1]?.refreshToken).toBe("refresh-2");
204264
});
205265

@@ -231,9 +291,15 @@ describe("storage last backup restore", () => {
231291

232292
expect(restored.accounts).toHaveLength(2);
233293
expect(restored.activeIndex).toBe(1);
294+
expect(restored.activeIndexByFamily).toEqual(
295+
expect.objectContaining({ codex: 1 }),
296+
);
234297
expect(loaded?.accounts).toHaveLength(1);
235298
expect(loaded?.accounts[0]?.refreshToken).toBe("current-refresh");
236299
expect(loaded?.activeIndex).toBe(0);
300+
expect(loaded?.activeIndexByFamily).toEqual(
301+
expect.objectContaining({ codex: 0 }),
302+
);
237303
});
238304

239305
it("throws when the named backup has no accounts", async () => {
@@ -314,6 +380,55 @@ describe("storage last backup restore", () => {
314380
);
315381
});
316382

383+
it("rejects the backup root directory itself as a restore target", async () => {
384+
const backupPath = buildNamedBackupPath("backup-root-guard");
385+
const backupRoot = dirname(backupPath);
386+
await fs.mkdir(backupRoot, { recursive: true });
387+
388+
await expect(restoreAccountsFromBackup(backupRoot)).rejects.toThrow(
389+
"Backup path must stay inside",
390+
);
391+
});
392+
393+
it("rejects a path inside the backup tree when realpath escapes outside the root", async () => {
394+
const linkedBackupPath = buildNamedBackupPath("backup-link");
395+
const backupRoot = dirname(linkedBackupPath);
396+
const escapedBackupPath = join(testRoot, "backup-outside-root.json");
397+
await fs.mkdir(backupRoot, { recursive: true });
398+
await fs.writeFile(
399+
escapedBackupPath,
400+
JSON.stringify({
401+
version: 3,
402+
activeIndex: 0,
403+
activeIndexByFamily: { codex: 0 },
404+
accounts: [{ refreshToken: "outside-refresh", addedAt: 1, lastUsed: 1 }],
405+
}),
406+
"utf-8",
407+
);
408+
409+
const originalRealpath = fs.realpath.bind(fs);
410+
const realpathSpy = vi.spyOn(fs, "realpath").mockImplementation(async (path, options) => {
411+
if (String(path) === linkedBackupPath) {
412+
return originalRealpath(
413+
escapedBackupPath as Parameters<typeof originalRealpath>[0],
414+
options as Parameters<typeof originalRealpath>[1],
415+
);
416+
}
417+
return originalRealpath(
418+
path as Parameters<typeof originalRealpath>[0],
419+
options as Parameters<typeof originalRealpath>[1],
420+
);
421+
});
422+
423+
try {
424+
await expect(restoreAccountsFromBackup(linkedBackupPath)).rejects.toThrow(
425+
"Backup path must stay inside",
426+
);
427+
} finally {
428+
realpathSpy.mockRestore();
429+
}
430+
});
431+
317432
it("throws a clear error when the backup root has never been created", async () => {
318433
const escapedBackupPath = join(testRoot, "backup-outside-root.json");
319434
await fs.writeFile(

0 commit comments

Comments
 (0)