From ea758d519f7a4d2db22b9322845cac039f2978d9 Mon Sep 17 00:00:00 2001 From: kagura-agent Date: Wed, 24 Jun 2026 20:18:35 +0800 Subject: [PATCH] fix(backup-all): catch per-sandbox errors to avoid aborting the batch When loadAgent() throws for a sandbox whose agent manifest is missing (e.g. an orphan from a previous higher-version install), the exception now gets caught inside the backup loop. The affected sandbox is counted as 'skipped' with a warning, and the remaining sandboxes continue to be backed up normally. This allows the pre-upgrade backup step to succeed and the installer to proceed. Closes #5734 Signed-off-by: kagura-agent --- src/lib/actions/maintenance.test.ts | 123 ++++++++++++++++++++++++++++ src/lib/actions/maintenance.ts | 10 ++- 2 files changed, 132 insertions(+), 1 deletion(-) create mode 100644 src/lib/actions/maintenance.test.ts diff --git a/src/lib/actions/maintenance.test.ts b/src/lib/actions/maintenance.test.ts new file mode 100644 index 00000000000..c4739e6959a --- /dev/null +++ b/src/lib/actions/maintenance.test.ts @@ -0,0 +1,123 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + listSandboxes: vi.fn(), + backupSandboxState: vi.fn(), + detectOpenShellStateRpcPreflightIssue: vi.fn().mockReturnValue(null), + detectOpenShellStateRpcResultIssue: vi.fn().mockReturnValue(null), + printOpenShellStateRpcIssue: vi.fn(), + captureSandboxListWithGatewayRecovery: vi.fn(), + printSandboxListFailureWithRecoveryContext: vi.fn(), + parseReadySandboxNames: vi.fn(), + dockerListImagesFormat: vi.fn().mockReturnValue(""), + dockerRmi: vi.fn(), + prompt: vi.fn(), +})); + +vi.mock("../state/registry", () => ({ + listSandboxes: mocks.listSandboxes, +})); +vi.mock("../state/sandbox", () => ({ + backupSandboxState: mocks.backupSandboxState, + BackupResult: {}, +})); +vi.mock("../adapters/openshell/gateway-drift", () => ({ + detectOpenShellStateRpcPreflightIssue: mocks.detectOpenShellStateRpcPreflightIssue, + detectOpenShellStateRpcResultIssue: mocks.detectOpenShellStateRpcResultIssue, + printOpenShellStateRpcIssue: mocks.printOpenShellStateRpcIssue, +})); +vi.mock("../openshell-sandbox-list", () => ({ + captureSandboxListWithGatewayRecovery: mocks.captureSandboxListWithGatewayRecovery, + printSandboxListFailureWithRecoveryContext: mocks.printSandboxListFailureWithRecoveryContext, +})); +vi.mock("../runtime-recovery", () => ({ + parseReadySandboxNames: mocks.parseReadySandboxNames, +})); +vi.mock("../adapters/docker", () => ({ + dockerListImagesFormat: mocks.dockerListImagesFormat, + dockerRmi: mocks.dockerRmi, +})); +vi.mock("../cli/branding", () => ({ + CLI_NAME: "nemoclaw", +})); +vi.mock("../credentials/store", () => ({ + prompt: mocks.prompt, +})); +vi.mock("../domain/lifecycle/options", () => ({ + normalizeGarbageCollectImagesOptions: (o: unknown) => o || {}, +})); +vi.mock("../domain/maintenance/images", () => ({ + findOrphanedSandboxImages: vi.fn().mockReturnValue([]), + parseSandboxImageRows: vi.fn().mockReturnValue([]), +})); + +import { backupAll } from "./maintenance"; + +describe("backupAll", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.captureSandboxListWithGatewayRecovery.mockResolvedValue({ + result: { status: 0, output: "sb-good\nsb-bad\n" }, + }); + mocks.parseReadySandboxNames.mockReturnValue(new Set(["sb-good", "sb-bad"])); + }); + + it("continues backup loop when backupSandboxState throws for one sandbox", async () => { + mocks.listSandboxes.mockReturnValue({ + sandboxes: [{ name: "sb-bad" }, { name: "sb-good" }], + defaultSandbox: null, + }); + + // First sandbox throws (simulating missing agent manifest) + mocks.backupSandboxState.mockImplementationOnce(() => { + throw new Error("Agent 'unknown-agent' not found: /path/to/manifest.yaml"); + }); + + // Second sandbox succeeds + mocks.backupSandboxState.mockImplementationOnce(() => ({ + success: true, + backedUpDirs: ["dir1"], + failedDirs: [], + backedUpFiles: [], + failedFiles: [], + manifest: { backupPath: "/backups/sb-good/timestamp" }, + })); + + // Should not throw — the loop should catch and continue + await backupAll(); + + // Both sandboxes should have been attempted + expect(mocks.backupSandboxState).toHaveBeenCalledTimes(2); + expect(mocks.backupSandboxState).toHaveBeenCalledWith("sb-bad"); + expect(mocks.backupSandboxState).toHaveBeenCalledWith("sb-good"); + }); + + it("counts thrown sandboxes as skipped, not failed", async () => { + mocks.listSandboxes.mockReturnValue({ + sandboxes: [{ name: "sb-bad" }], + defaultSandbox: null, + }); + mocks.parseReadySandboxNames.mockReturnValue(new Set(["sb-bad"])); + mocks.captureSandboxListWithGatewayRecovery.mockResolvedValue({ + result: { status: 0, output: "sb-bad\n" }, + }); + + mocks.backupSandboxState.mockImplementation(() => { + throw new Error("Agent 'orphan' not found"); + }); + + const consoleSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + await backupAll(); + + // Should log "Skipped" warning, not "backup failed" + const output = consoleSpy.mock.calls.map((c) => c[0]).join("\n"); + expect(output).toContain("Skipped"); + expect(output).toContain("orphan"); + expect(output).toContain("0 failed"); + expect(output).toContain("1 skipped"); + consoleSpy.mockRestore(); + }); +}); diff --git a/src/lib/actions/maintenance.ts b/src/lib/actions/maintenance.ts index c9aa0ad2871..bacabc53488 100644 --- a/src/lib/actions/maintenance.ts +++ b/src/lib/actions/maintenance.ts @@ -73,7 +73,15 @@ export async function backupAll(): Promise { continue; } console.log(` Backing up '${sb.name}'...`); - const result = sandboxState.backupSandboxState(sb.name); + let result: sandboxState.BackupResult; + try { + result = sandboxState.backupSandboxState(sb.name); + } catch (err: unknown) { + const msg = err instanceof Error ? err.message : String(err); + console.log(` ${YW}⚠${R} Skipped '${sb.name}': ${msg}`); + skipped++; + continue; + } if (result.success) { console.log( ` ${G}✓${R} ${sb.name}: ${result.backedUpDirs.length} dirs, ${result.backedUpFiles.length} files → ${result.manifest?.backupPath || "unknown"}`,