Skip to content

Commit ecfce8b

Browse files
committed
fix(auth): avoid false empty backup messaging
1 parent 192e4e3 commit ecfce8b

2 files changed

Lines changed: 104 additions & 6 deletions

File tree

lib/codex-manager.ts

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4348,8 +4348,15 @@ function getRedactedFilesystemErrorLabel(error: unknown): string {
43484348
return "UNKNOWN";
43494349
}
43504350

4351+
type BackupRestoreManagerAssessmentLoadResult = {
4352+
assessments: BackupRestoreAssessment[];
4353+
backupCount: number;
4354+
readFailed: boolean;
4355+
assessmentFailures: number;
4356+
};
4357+
43514358
async function loadBackupRestoreManagerAssessments(): Promise<
4352-
BackupRestoreAssessment[]
4359+
BackupRestoreManagerAssessmentLoadResult
43534360
> {
43544361
let backups: Awaited<ReturnType<typeof listNamedBackups>>;
43554362
try {
@@ -4361,14 +4368,25 @@ async function loadBackupRestoreManagerAssessments(): Promise<
43614368
collapseWhitespace(message) || "unknown error"
43624369
}`,
43634370
);
4364-
return [];
4371+
return {
4372+
assessments: [],
4373+
backupCount: 0,
4374+
readFailed: true,
4375+
assessmentFailures: 0,
4376+
};
43654377
}
43664378
if (backups.length === 0) {
4367-
return [];
4379+
return {
4380+
assessments: [],
4381+
backupCount: 0,
4382+
readFailed: false,
4383+
assessmentFailures: 0,
4384+
};
43684385
}
43694386

43704387
const currentStorage = await loadAccounts();
43714388
const assessments: BackupRestoreAssessment[] = [];
4389+
let assessmentFailures = 0;
43724390
for (
43734391
let index = 0;
43744392
index < backups.length;
@@ -4395,20 +4413,43 @@ async function loadBackupRestoreManagerAssessments(): Promise<
43954413
collapseWhitespace(reason) || "unknown error"
43964414
}`,
43974415
);
4416+
assessmentFailures += 1;
43984417
}
43994418
}
44004419

4401-
return assessments;
4420+
return {
4421+
assessments,
4422+
backupCount: backups.length,
4423+
readFailed: false,
4424+
assessmentFailures,
4425+
};
44024426
}
44034427

44044428
async function runBackupRestoreManager(
44054429
displaySettings: DashboardDisplaySettings,
44064430
assessmentsOverride?: BackupRestoreAssessment[],
44074431
): Promise<BackupRestoreManagerResult> {
44084432
const backupDir = getNamedBackupsDirectoryPath();
4409-
const assessments =
4410-
assessmentsOverride ?? (await loadBackupRestoreManagerAssessments());
4433+
const assessmentLoad =
4434+
assessmentsOverride === undefined
4435+
? await loadBackupRestoreManagerAssessments()
4436+
: {
4437+
assessments: assessmentsOverride,
4438+
backupCount: assessmentsOverride.length,
4439+
readFailed: false,
4440+
assessmentFailures: 0,
4441+
};
4442+
const { assessments } = assessmentLoad;
44114443
if (assessments.length === 0) {
4444+
if (assessmentLoad.readFailed) {
4445+
return "dismissed";
4446+
}
4447+
if (assessmentLoad.assessmentFailures > 0) {
4448+
console.warn(
4449+
"Could not inspect any named backups. Resolve the backup read errors and try again.",
4450+
);
4451+
return "dismissed";
4452+
}
44124453
console.log(`No named backups found. Place backup files in ${backupDir}.`);
44134454
return "dismissed";
44144455
}

test/codex-manager-cli.test.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3497,6 +3497,7 @@ describe("codex manager cli commands", () => {
34973497
.mockResolvedValueOnce({ mode: "restore-backup" })
34983498
.mockResolvedValueOnce({ mode: "cancel" });
34993499
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
3500+
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
35003501

35013502
try {
35023503
const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js");
@@ -3512,11 +3513,67 @@ describe("codex manager cli commands", () => {
35123513
"Could not read backup directory: EPERM: operation not permitted",
35133514
),
35143515
);
3516+
expect(logSpy).not.toHaveBeenCalledWith(
3517+
expect.stringContaining("No named backups found."),
3518+
);
35153519
} finally {
3520+
logSpy.mockRestore();
35163521
errorSpy.mockRestore();
35173522
}
35183523
});
35193524

3525+
it("does not claim backups are missing when every assessment fails", async () => {
3526+
setInteractiveTTY(true);
3527+
const now = Date.now();
3528+
loadAccountsMock.mockResolvedValue(null);
3529+
listNamedBackupsMock.mockResolvedValue([
3530+
{
3531+
name: "busy-backup",
3532+
path: "/mock/backups/busy-backup.json",
3533+
createdAt: null,
3534+
updatedAt: now,
3535+
sizeBytes: 128,
3536+
version: 3,
3537+
accountCount: 1,
3538+
schemaErrors: [],
3539+
valid: true,
3540+
loadError: undefined,
3541+
},
3542+
]);
3543+
assessNamedBackupRestoreMock.mockRejectedValueOnce(
3544+
makeErrnoError("backup directory busy", "EBUSY"),
3545+
);
3546+
promptLoginModeMock
3547+
.mockResolvedValueOnce({ mode: "restore-backup" })
3548+
.mockResolvedValueOnce({ mode: "cancel" });
3549+
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
3550+
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
3551+
3552+
try {
3553+
const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js");
3554+
const exitCode = await runCodexMultiAuthCli(["auth", "login"]);
3555+
3556+
expect(exitCode).toBe(0);
3557+
expect(promptLoginModeMock).toHaveBeenCalledTimes(2);
3558+
expect(selectMock).not.toHaveBeenCalled();
3559+
expect(restoreNamedBackupMock).not.toHaveBeenCalled();
3560+
expect(warnSpy).toHaveBeenCalledWith(
3561+
expect.stringContaining(
3562+
'Skipped backup assessment for "busy-backup": backup directory busy',
3563+
),
3564+
);
3565+
expect(warnSpy).toHaveBeenCalledWith(
3566+
"Could not inspect any named backups. Resolve the backup read errors and try again.",
3567+
);
3568+
expect(logSpy).not.toHaveBeenCalledWith(
3569+
expect.stringContaining("No named backups found."),
3570+
);
3571+
} finally {
3572+
logSpy.mockRestore();
3573+
warnSpy.mockRestore();
3574+
}
3575+
});
3576+
35203577
it("keeps healthy backups selectable when one assessment fails", async () => {
35213578
setInteractiveTTY(true);
35223579
loadAccountsMock.mockResolvedValue(null);

0 commit comments

Comments
 (0)