From a8ad1eec43555839fe3c7fb2a96d55b5defc4cdc Mon Sep 17 00:00:00 2001 From: Hokonoken <41166525+Hokonoken@users.noreply.github.com> Date: Fri, 10 Jul 2026 11:00:24 +0200 Subject: [PATCH 1/4] fix(backup): name the per-dir failure cause in backup failure messages backup-all and snapshot create collapsed every per-dir backup failure into an opaque list: 'backup failed (identity, devices, credentials)'. A permission problem (tar cannot read content the backup user does not own) and a directory that never materialized from a clean extraction are different operator problems, but the message could not tell them apart. backupSandboxState now records a per-dir cause alongside failedDirs: - 'permission denied' tar reported Permission denied for the dir - 'tar read error' tar reported other read errors for the dir - 'absent after extraction' tar succeeded but the dir never materialized and the backup-all / snapshot create failure messages render it: 'backup failed (identity (permission denied), ...)'. Dirs without an attributable cause render unchanged, and failedDirs keeps its shape for existing consumers. Verified live against a snapshot-restored clone with root-owned state dirs (the #6455 construction): the failure line names the cause, and restoring ownership returns backup-all to exit 0. Fixes #6455 Signed-off-by: Hokonoken <41166525+Hokonoken@users.noreply.github.com> --- src/lib/actions/maintenance.test.ts | 77 ++++++++++++++++++++++++++++- src/lib/actions/maintenance.ts | 7 ++- src/lib/actions/sandbox/snapshot.ts | 6 ++- src/lib/state/sandbox.ts | 50 +++++++++++++++++-- test/snapshot.test.ts | 5 ++ 5 files changed, 136 insertions(+), 9 deletions(-) diff --git a/src/lib/actions/maintenance.test.ts b/src/lib/actions/maintenance.test.ts index f0f057c99c8..102a5a9bf69 100644 --- a/src/lib/actions/maintenance.test.ts +++ b/src/lib/actions/maintenance.test.ts @@ -16,9 +16,13 @@ const mocks = vi.hoisted(() => ({ vi.mock("../state/registry", () => ({ listSandboxes: mocks.listSandboxes, })); -vi.mock("../state/sandbox", () => ({ +vi.mock("../state/sandbox", async (importOriginal) => ({ backupSandboxState: mocks.backupSandboxState, BackupResult: {}, + // Real formatter so the backup-failed message tests exercise the actual + // per-dir cause rendering (#6455). + formatFailedBackupItems: (await importOriginal()) + .formatFailedBackupItems, })); vi.mock("../openshell-sandbox-list", () => ({ captureSandboxListWithGatewayPreflightOrExit: mocks.captureSandboxListWithGatewayPreflightOrExit, @@ -384,6 +388,77 @@ describe("backupAll", () => { expect(exitSpy).toHaveBeenCalledWith(1); }); + it("names the per-dir failure cause in the backup-failed message (#6455)", async () => { + mocks.listSandboxes.mockReturnValue({ + sandboxes: [{ name: "clone-test" }], + defaultSandbox: null, + }); + mocks.parseReadySandboxNames.mockReturnValue(new Set(["clone-test"])); + mocks.captureSandboxListWithGatewayPreflightOrExit.mockResolvedValue({ + status: 0, + output: "clone-test\n", + }); + mocks.backupSandboxState.mockReturnValue({ + success: false, + backedUpDirs: [], + failedDirs: ["identity", "devices", "credentials"], + failedDirReasons: { + identity: "permission denied", + devices: "permission denied", + credentials: "absent after extraction", + }, + backedUpFiles: [], + failedFiles: [], + }); + vi.spyOn(console, "log").mockImplementation(() => undefined); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); + const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { + throw new Error(`exit:${code}`); + }) as never); + + await expect(backupAll()).rejects.toThrow("exit:1"); + + const errorOutput = errorSpy.mock.calls.map((c) => c[0]).join("\n"); + expect(errorOutput).toContain( + "backup failed (identity (permission denied), devices (permission denied), credentials (absent after extraction))", + ); + + errorSpy.mockRestore(); + exitSpy.mockRestore(); + }); + + it("renders the backup-failed message unchanged when no failure causes are recorded", async () => { + mocks.listSandboxes.mockReturnValue({ + sandboxes: [{ name: "sb-bad" }], + defaultSandbox: null, + }); + mocks.parseReadySandboxNames.mockReturnValue(new Set(["sb-bad"])); + mocks.captureSandboxListWithGatewayPreflightOrExit.mockResolvedValue({ + status: 0, + output: "sb-bad\n", + }); + mocks.backupSandboxState.mockReturnValue({ + success: false, + backedUpDirs: [], + failedDirs: ["memories"], + backedUpFiles: [], + failedFiles: ["settings.json"], + }); + vi.spyOn(console, "log").mockImplementation(() => undefined); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); + const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { + throw new Error(`exit:${code}`); + }) as never); + + await expect(backupAll()).rejects.toThrow("exit:1"); + + const errorOutput = errorSpy.mock.calls.map((c) => c[0]).join("\n"); + expect(errorOutput).toContain("backup failed (memories, settings.json)"); + + errorSpy.mockRestore(); + exitSpy.mockRestore(); + }); + it.each([ ["standalone backup", "", true], ["installer-strict backup", "1", false], diff --git a/src/lib/actions/maintenance.ts b/src/lib/actions/maintenance.ts index f50aef7a205..6d47723786d 100644 --- a/src/lib/actions/maintenance.ts +++ b/src/lib/actions/maintenance.ts @@ -113,8 +113,11 @@ export async function backupAll(): Promise { } unreachableRunning++; } - const failedItems = [...result.failedDirs, ...result.failedFiles]; - console.error(` ${RD}✗${R} ${sb.name}: backup failed (${failedItems.join(", ")})`); + const failedItems = sandboxState.formatFailedBackupItems( + [...result.failedDirs, ...result.failedFiles], + result.failedDirReasons, + ); + console.error(` ${RD}✗${R} ${sb.name}: backup failed (${failedItems})`); failed++; } } diff --git a/src/lib/actions/sandbox/snapshot.ts b/src/lib/actions/sandbox/snapshot.ts index b070e3d24ba..76c8cea4e49 100644 --- a/src/lib/actions/sandbox/snapshot.ts +++ b/src/lib/actions/sandbox/snapshot.ts @@ -545,7 +545,11 @@ function runSnapshotCreate( } else { console.error(" Snapshot failed."); if (result.failedDirs.length > 0) { - console.error(` Failed directories: ${result.failedDirs.join(", ")}`); + const failedDirs = sandboxState.formatFailedBackupItems( + result.failedDirs, + result.failedDirReasons, + ); + console.error(` Failed directories: ${failedDirs}`); } if (result.failedFiles.length > 0) { console.error(` Failed files: ${result.failedFiles.join(", ")}`); diff --git a/src/lib/state/sandbox.ts b/src/lib/state/sandbox.ts index 53f732011cb..d4bb7f9fcfa 100644 --- a/src/lib/state/sandbox.ts +++ b/src/lib/state/sandbox.ts @@ -136,6 +136,12 @@ export interface BackupResult { manifest?: RebuildManifest; backedUpDirs: string[]; failedDirs: string[]; + // Per-dir failure cause for entries in failedDirs, keyed by dir name. + // Distinguishes "permission denied" (tar could not read the content) from + // "absent after extraction" (tar succeeded but the dir never materialized) + // so operators can tell an ownership problem from a missing dir (#6455). + // Dirs failed for other reasons may be absent from this map. + failedDirReasons?: Record; // Set when the failure is a precondition (e.g. duplicate --name) rather // than a mid-backup error. CLI surfaces this to the user verbatim. error?: string; @@ -730,8 +736,15 @@ function stateFileRemotePath(dir: string, filePath: string): string { return `${dir.replace(/\/+$/, "")}/${filePath}`; } -function failedDirsFromTarStderr(stderr: string, existingDirs: string[]): Set { - const failed = new Set(); +/** Failure cause: tar reported "Permission denied" while reading the dir. */ +export const BACKUP_FAILURE_PERMISSION_DENIED = "permission denied"; +/** Failure cause: tar reported other read errors for the dir. */ +export const BACKUP_FAILURE_TAR_READ_ERROR = "tar read error"; +/** Failure cause: tar succeeded but the dir never materialized on the host. */ +export const BACKUP_FAILURE_ABSENT_AFTER_EXTRACTION = "absent after extraction"; + +function failedDirsFromTarStderr(stderr: string, existingDirs: string[]): Map { + const failed = new Map(); const dirs = [...existingDirs].sort((a, b) => b.length - a.length); for (const rawLine of stderr.split(/\r?\n/)) { const line = rawLine.trim(); @@ -743,7 +756,14 @@ function failedDirsFromTarStderr(stderr: string, existingDirs: string[]): Set | undefined, +): string { + return failedItems + .map((item) => (reasons?.[item] ? `${item} (${reasons[item]})` : item)) + .join(", "); +} + const SQLITE_BACKUP_PY = [ "import sqlite3, sys", "src, dst = sys.argv[1], sys.argv[2]", @@ -1128,6 +1162,7 @@ export function backupSandboxState(sandboxName: string, options: BackupOptions = const backedUpDirs: string[] = []; const failedDirs: string[] = []; + const failedDirReasons: Record = {}; const backedUpFiles: string[] = []; const failedFiles: string[] = []; let unreachable = false; @@ -1341,6 +1376,7 @@ export function backupSandboxState(sandboxName: string, options: BackupOptions = } else { _log(`Dir ${d} missing from clean tar extraction — marking failed`); failedDirs.push(d); + failedDirReasons[d] = BACKUP_FAILURE_ABSENT_AFTER_EXTRACTION; } } } else { @@ -1355,12 +1391,15 @@ export function backupSandboxState(sandboxName: string, options: BackupOptions = failedDirs.push(...existingDirs); } else { for (const d of existingDirs) { - if (tarFailedDirs.has(d)) { - _log(`Dir ${d} had tar read errors — marking failed`); + const tarFailureReason = tarFailedDirs.get(d); + if (tarFailureReason !== undefined) { + _log(`Dir ${d} had tar read errors (${tarFailureReason}) — marking failed`); failedDirs.push(d); + failedDirReasons[d] = tarFailureReason; } else if (!extractedDirs.has(d)) { _log(`Dir ${d} missing from partial tar extraction — marking failed`); failedDirs.push(d); + failedDirReasons[d] = BACKUP_FAILURE_ABSENT_AFTER_EXTRACTION; } else { backedUpDirs.push(d); } @@ -1425,6 +1464,7 @@ export function backupSandboxState(sandboxName: string, options: BackupOptions = manifest, backedUpDirs, failedDirs, + ...(Object.keys(failedDirReasons).length > 0 ? { failedDirReasons } : {}), backedUpFiles, failedFiles, }; diff --git a/test/snapshot.test.ts b/test/snapshot.test.ts index cacddad1404..3ee9ecd7e0d 100644 --- a/test/snapshot.test.ts +++ b/test/snapshot.test.ts @@ -591,6 +591,7 @@ process.exit(0); const backup = sandboxState.backupSandboxState("alpha"); expect(backup.success).toBe(false); expect(backup.failedDirs).toEqual(["agents"]); + expect(backup.failedDirReasons).toEqual({ agents: "permission denied" }); expect(backup.backedUpDirs).toEqual(["workspace", "extensions"]); expect(backup.manifest?.backedUpDirs).toEqual(["workspace", "extensions"]); expect(fs.existsSync(path.join(backup.manifest!.backupPath, "agents"))).toBe(true); @@ -960,6 +961,10 @@ process.exit(0); expect(backup.success).toBe(false); expect(backup.backedUpDirs).toEqual(["extensions"]); expect(backup.failedDirs).toEqual(["agents", "workspace"]); + expect(backup.failedDirReasons).toEqual({ + agents: "permission denied", + workspace: "absent after extraction", + }); expect(backup.manifest?.backedUpDirs).toEqual(["extensions"]); expect(fs.existsSync(path.join(backup.manifest!.backupPath, "workspace"))).toBe(false); } finally { From 21661524062d28cde5ce655dad0a3df146a572a1 Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Fri, 10 Jul 2026 18:58:21 -0700 Subject: [PATCH 2/4] test(snapshot): cover backup failure formatter mock Signed-off-by: Prekshi Vyas --- src/lib/actions/sandbox/snapshot.test.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/lib/actions/sandbox/snapshot.test.ts b/src/lib/actions/sandbox/snapshot.test.ts index 05077210389..51bc7147985 100644 --- a/src/lib/actions/sandbox/snapshot.test.ts +++ b/src/lib/actions/sandbox/snapshot.test.ts @@ -228,9 +228,11 @@ vi.mock("../../state/registry", () => ({ updateSandbox: updateSandboxMock, })); -vi.mock("../../state/sandbox", () => ({ +vi.mock("../../state/sandbox", async (importOriginal) => ({ backupSandboxState: backupSandboxStateMock, findBackup: findBackupMock, + formatFailedBackupItems: (await importOriginal()) + .formatFailedBackupItems, getLatestBackup: getLatestBackupMock, listBackups: listBackupsMock, restoreSandboxState: restoreSandboxStateMock, @@ -1474,6 +1476,7 @@ describe("runSandboxSnapshot", () => { backupSandboxStateMock.mockReturnValue({ success: false, failedDirs: ["workspace", "skills"], + failedDirReasons: { workspace: "permission denied" }, failedFiles: ["openclaw.json"], }); const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); @@ -1486,7 +1489,7 @@ describe("runSandboxSnapshot", () => { const errors = consoleError.mock.calls.flat().join("\n"); expect(errors).toContain("Snapshot failed."); - expect(errors).toContain("Failed directories: workspace, skills"); + expect(errors).toContain("Failed directories: workspace (permission denied), skills"); expect(errors).toContain("Failed files: openclaw.json"); }); }); From 4b4f22f75eb3a68e32e41937050162599dd1f0db Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Sat, 11 Jul 2026 01:24:56 -0700 Subject: [PATCH 3/4] test(backup): cover generic tar read failures Signed-off-by: Carlos Villela --- test/snapshot.test.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/test/snapshot.test.ts b/test/snapshot.test.ts index 3ee9ecd7e0d..f92598d442e 100644 --- a/test/snapshot.test.ts +++ b/test/snapshot.test.ts @@ -522,7 +522,7 @@ process.exit(0); } }); - it("excludes tar-failed directories from the restorable manifest", () => { + it("classifies tar-failed directories and excludes them from the restorable manifest", () => { const fixture = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-openclaw-partial-tar-")); const oldPath = process.env.PATH; const oldOpenshell = process.env.NEMOCLAW_OPENSHELL_BIN; @@ -570,6 +570,7 @@ if (cmd.includes("tar -cf -")) { }); if (r.stdout) fs.writeSync(1, r.stdout); process.stderr.write("tar: agents/main/sessions/sessions.json: Cannot open: Permission denied\\n"); + process.stderr.write("tar: workspace/marker.txt: Cannot read: Input/output error\\n"); process.stderr.write("tar: Exiting with failure status due to previous errors\\n"); process.exit(2); } @@ -590,15 +591,18 @@ process.exit(0); const backup = sandboxState.backupSandboxState("alpha"); expect(backup.success).toBe(false); - expect(backup.failedDirs).toEqual(["agents"]); - expect(backup.failedDirReasons).toEqual({ agents: "permission denied" }); - expect(backup.backedUpDirs).toEqual(["workspace", "extensions"]); - expect(backup.manifest?.backedUpDirs).toEqual(["workspace", "extensions"]); + expect(backup.failedDirs).toEqual(["agents", "workspace"]); + expect(backup.failedDirReasons).toEqual({ + agents: "permission denied", + workspace: "tar read error", + }); + expect(backup.backedUpDirs).toEqual(["extensions"]); + expect(backup.manifest?.backedUpDirs).toEqual(["extensions"]); expect(fs.existsSync(path.join(backup.manifest!.backupPath, "agents"))).toBe(true); const restore = sandboxState.restoreSandboxState("alpha", backup.manifest!.backupPath); expect(restore.success).toBe(true); - expect(restore.restoredDirs).toEqual(["workspace", "extensions"]); + expect(restore.restoredDirs).toEqual(["extensions"]); const loggedCommands = fs .readFileSync(sshLog, "utf-8") @@ -606,7 +610,7 @@ process.exit(0); .split("\n") .map((line) => JSON.parse(line).cmd as string); const cleanupCommand = loggedCommands.find((cmd) => cmd.includes("rm -rf")); - expect(cleanupCommand).toContain("/sandbox/.openclaw/workspace"); + expect(cleanupCommand).not.toContain("/sandbox/.openclaw/workspace"); expect(cleanupCommand).not.toContain("rm -rf -- /sandbox/.openclaw/extensions"); expect(cleanupCommand).toContain("/sandbox/.openclaw/extensions"); expect(cleanupCommand).toContain("! -name 'nemoclaw'"); From 27ad9f248c1d48af6efd9abc5cd9be9203923305 Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Sat, 11 Jul 2026 02:28:12 -0700 Subject: [PATCH 4/4] refactor(backup): extract failure diagnostics Signed-off-by: Carlos Villela --- src/lib/actions/maintenance.test.ts | 127 +++++++---------------- src/lib/actions/maintenance.ts | 3 +- src/lib/actions/sandbox/snapshot.test.ts | 4 +- src/lib/actions/sandbox/snapshot.ts | 6 +- src/lib/domain/backup-failure.test.ts | 42 ++++++++ src/lib/domain/backup-failure.ts | 50 +++++++++ src/lib/state/sandbox.ts | 55 +--------- 7 files changed, 137 insertions(+), 150 deletions(-) create mode 100644 src/lib/domain/backup-failure.test.ts create mode 100644 src/lib/domain/backup-failure.ts diff --git a/src/lib/actions/maintenance.test.ts b/src/lib/actions/maintenance.test.ts index 102a5a9bf69..31eeff77171 100644 --- a/src/lib/actions/maintenance.test.ts +++ b/src/lib/actions/maintenance.test.ts @@ -16,13 +16,9 @@ const mocks = vi.hoisted(() => ({ vi.mock("../state/registry", () => ({ listSandboxes: mocks.listSandboxes, })); -vi.mock("../state/sandbox", async (importOriginal) => ({ +vi.mock("../state/sandbox", () => ({ backupSandboxState: mocks.backupSandboxState, BackupResult: {}, - // Real formatter so the backup-failed message tests exercise the actual - // per-dir cause rendering (#6455). - formatFailedBackupItems: (await importOriginal()) - .formatFailedBackupItems, })); vi.mock("../openshell-sandbox-list", () => ({ captureSandboxListWithGatewayPreflightOrExit: mocks.captureSandboxListWithGatewayPreflightOrExit, @@ -43,6 +39,7 @@ vi.mock("../credentials/store", () => ({ vi.mock("../domain/lifecycle/options", () => ({ normalizeGarbageCollectImagesOptions: (o: unknown) => o || {}, })); + // ../domain/maintenance/images is left unmocked so the gc tests run the real // orphan-detection helpers and can assert on gc's actual output. @@ -117,28 +114,45 @@ describe("backupAll", () => { expect(mocks.backupSandboxState).not.toHaveBeenCalled(); }); - it("backs up only sandboxes reported Ready by OpenShell", async () => { + it("preserves retry counters when ready sandboxes have mixed backup outcomes (#6455)", async () => { mocks.listSandboxes.mockReturnValue({ - sandboxes: [{ name: "sb-good" }, { name: "sb-stopped" }], + sandboxes: [{ name: "sb-bad" }, { name: "sb-good" }, { name: "sb-stopped" }], defaultSandbox: null, }); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["sb-good"])); - mocks.backupSandboxState.mockReturnValue({ - success: true, - backedUpDirs: ["workspace"], - failedDirs: [], - backedUpFiles: [], - failedFiles: [], - manifest: { backupPath: "/backups/sb-good/timestamp" }, - }); + mocks.parseReadySandboxNames.mockReturnValue(new Set(["sb-bad", "sb-good"])); + mocks.backupSandboxState.mockImplementation((name: string) => + name === "sb-bad" + ? { + success: false, + backedUpDirs: [], + failedDirs: ["identity"], + failedDirReasons: { identity: "permission denied" }, + backedUpFiles: [], + failedFiles: ["settings.json"], + } + : { + success: true, + backedUpDirs: ["workspace"], + failedDirs: [], + backedUpFiles: [], + failedFiles: [], + manifest: { backupPath: "/backups/sb-good/timestamp" }, + }, + ); const logSpy = vi.spyOn(console, "log").mockImplementation(() => undefined); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); + vi.spyOn(process, "exit").mockImplementation(() => { + throw new Error("exit:1"); + }); - await backupAll(); + await expect(backupAll()).rejects.toThrow("exit:1"); - expect(mocks.backupSandboxState).toHaveBeenCalledOnce(); - expect(mocks.backupSandboxState).toHaveBeenCalledWith("sb-good"); - expect(logSpy.mock.calls.flat().join("\n")).toContain("Skipping 'sb-stopped' (not running)"); - logSpy.mockRestore(); + const logOutput = logSpy.mock.calls.flat().join("\n"); + expect(logOutput).toContain("Skipping 'sb-stopped' (not running)"); + expect(logOutput).toContain("1 backed up, 1 failed, 1 skipped"); + expect(errorSpy.mock.calls.flat().join("\n")).toContain( + "backup failed (identity (permission denied), settings.json)", + ); }); it("fails installer-strict backup when a registered sandbox is not Ready (#6114)", async () => { @@ -388,77 +402,6 @@ describe("backupAll", () => { expect(exitSpy).toHaveBeenCalledWith(1); }); - it("names the per-dir failure cause in the backup-failed message (#6455)", async () => { - mocks.listSandboxes.mockReturnValue({ - sandboxes: [{ name: "clone-test" }], - defaultSandbox: null, - }); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["clone-test"])); - mocks.captureSandboxListWithGatewayPreflightOrExit.mockResolvedValue({ - status: 0, - output: "clone-test\n", - }); - mocks.backupSandboxState.mockReturnValue({ - success: false, - backedUpDirs: [], - failedDirs: ["identity", "devices", "credentials"], - failedDirReasons: { - identity: "permission denied", - devices: "permission denied", - credentials: "absent after extraction", - }, - backedUpFiles: [], - failedFiles: [], - }); - vi.spyOn(console, "log").mockImplementation(() => undefined); - const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); - const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { - throw new Error(`exit:${code}`); - }) as never); - - await expect(backupAll()).rejects.toThrow("exit:1"); - - const errorOutput = errorSpy.mock.calls.map((c) => c[0]).join("\n"); - expect(errorOutput).toContain( - "backup failed (identity (permission denied), devices (permission denied), credentials (absent after extraction))", - ); - - errorSpy.mockRestore(); - exitSpy.mockRestore(); - }); - - it("renders the backup-failed message unchanged when no failure causes are recorded", async () => { - mocks.listSandboxes.mockReturnValue({ - sandboxes: [{ name: "sb-bad" }], - defaultSandbox: null, - }); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["sb-bad"])); - mocks.captureSandboxListWithGatewayPreflightOrExit.mockResolvedValue({ - status: 0, - output: "sb-bad\n", - }); - mocks.backupSandboxState.mockReturnValue({ - success: false, - backedUpDirs: [], - failedDirs: ["memories"], - backedUpFiles: [], - failedFiles: ["settings.json"], - }); - vi.spyOn(console, "log").mockImplementation(() => undefined); - const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); - const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { - throw new Error(`exit:${code}`); - }) as never); - - await expect(backupAll()).rejects.toThrow("exit:1"); - - const errorOutput = errorSpy.mock.calls.map((c) => c[0]).join("\n"); - expect(errorOutput).toContain("backup failed (memories, settings.json)"); - - errorSpy.mockRestore(); - exitSpy.mockRestore(); - }); - it.each([ ["standalone backup", "", true], ["installer-strict backup", "1", false], diff --git a/src/lib/actions/maintenance.ts b/src/lib/actions/maintenance.ts index 6d47723786d..7d27e42bb3d 100644 --- a/src/lib/actions/maintenance.ts +++ b/src/lib/actions/maintenance.ts @@ -4,6 +4,7 @@ import { dockerListImagesFormat, dockerRmi } from "../adapters/docker"; import { CLI_NAME } from "../cli/branding"; import { prompt as askPrompt } from "../credentials/store"; +import { formatFailedBackupItems } from "../domain/backup-failure"; import { type GarbageCollectImagesOptions, normalizeGarbageCollectImagesOptions, @@ -113,7 +114,7 @@ export async function backupAll(): Promise { } unreachableRunning++; } - const failedItems = sandboxState.formatFailedBackupItems( + const failedItems = formatFailedBackupItems( [...result.failedDirs, ...result.failedFiles], result.failedDirReasons, ); diff --git a/src/lib/actions/sandbox/snapshot.test.ts b/src/lib/actions/sandbox/snapshot.test.ts index 51bc7147985..738d0a4dd8b 100644 --- a/src/lib/actions/sandbox/snapshot.test.ts +++ b/src/lib/actions/sandbox/snapshot.test.ts @@ -228,11 +228,9 @@ vi.mock("../../state/registry", () => ({ updateSandbox: updateSandboxMock, })); -vi.mock("../../state/sandbox", async (importOriginal) => ({ +vi.mock("../../state/sandbox", () => ({ backupSandboxState: backupSandboxStateMock, findBackup: findBackupMock, - formatFailedBackupItems: (await importOriginal()) - .formatFailedBackupItems, getLatestBackup: getLatestBackupMock, listBackups: listBackupsMock, restoreSandboxState: restoreSandboxStateMock, diff --git a/src/lib/actions/sandbox/snapshot.ts b/src/lib/actions/sandbox/snapshot.ts index 76c8cea4e49..dadd4f95daf 100644 --- a/src/lib/actions/sandbox/snapshot.ts +++ b/src/lib/actions/sandbox/snapshot.ts @@ -12,6 +12,7 @@ import { import { OPENSHELL_PROBE_TIMEOUT_MS } from "../../adapters/openshell/timeouts"; import { CLI_NAME } from "../../cli/branding"; import { prompt as askPrompt } from "../../credentials/store"; +import { formatFailedBackupItems } from "../../domain/backup-failure"; import { getSandboxDeleteOutcome } from "../../domain/sandbox/destroy"; import { checkGatewayRouteCompatibility, @@ -545,10 +546,7 @@ function runSnapshotCreate( } else { console.error(" Snapshot failed."); if (result.failedDirs.length > 0) { - const failedDirs = sandboxState.formatFailedBackupItems( - result.failedDirs, - result.failedDirReasons, - ); + const failedDirs = formatFailedBackupItems(result.failedDirs, result.failedDirReasons); console.error(` Failed directories: ${failedDirs}`); } if (result.failedFiles.length > 0) { diff --git a/src/lib/domain/backup-failure.test.ts b/src/lib/domain/backup-failure.test.ts new file mode 100644 index 00000000000..4290929c603 --- /dev/null +++ b/src/lib/domain/backup-failure.test.ts @@ -0,0 +1,42 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; +import { + BACKUP_FAILURE_ABSENT_AFTER_EXTRACTION, + BACKUP_FAILURE_PERMISSION_DENIED, + BACKUP_FAILURE_TAR_READ_ERROR, + classifyFailedDirsFromTarStderr, + formatFailedBackupItems, +} from "./backup-failure"; + +describe("backup failure diagnostics", () => { + it("classifies permission and generic tar read errors by directory", () => { + const failures = classifyFailedDirsFromTarStderr( + [ + "tar: agents/main/session.json: Cannot read: Input/output error", + "tar: workspace/marker.txt: Cannot read: Input/output error", + "tar: agents/main/session.json: Cannot open: Permission denied", + "tar: unrelated/file: Cannot read: Input/output error", + ].join("\n"), + ["agents", "agents/main", "workspace"], + ); + + expect(Object.fromEntries(failures)).toEqual({ + "agents/main": BACKUP_FAILURE_PERMISSION_DENIED, + workspace: BACKUP_FAILURE_TAR_READ_ERROR, + }); + }); + + it("renders known reasons while preserving uncategorized items", () => { + expect( + formatFailedBackupItems(["identity", "credentials", "settings.json"], { + credentials: BACKUP_FAILURE_ABSENT_AFTER_EXTRACTION, + identity: BACKUP_FAILURE_PERMISSION_DENIED, + }), + ).toBe("identity (permission denied), credentials (absent after extraction), settings.json"); + expect(formatFailedBackupItems(["memories", "settings.json"], undefined)).toBe( + "memories, settings.json", + ); + }); +}); diff --git a/src/lib/domain/backup-failure.ts b/src/lib/domain/backup-failure.ts new file mode 100644 index 00000000000..fe86abef3e0 --- /dev/null +++ b/src/lib/domain/backup-failure.ts @@ -0,0 +1,50 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +/** Failure cause: tar reported "Permission denied" while reading the dir. */ +export const BACKUP_FAILURE_PERMISSION_DENIED = "permission denied"; +/** Failure cause: tar reported other read errors for the dir. */ +export const BACKUP_FAILURE_TAR_READ_ERROR = "tar read error"; +/** Failure cause: tar succeeded but the dir never materialized on the host. */ +export const BACKUP_FAILURE_ABSENT_AFTER_EXTRACTION = "absent after extraction"; + +export function classifyFailedDirsFromTarStderr( + stderr: string, + existingDirs: readonly string[], +): Map { + const failed = new Map(); + const dirs = [...existingDirs].sort((a, b) => b.length - a.length); + for (const rawLine of stderr.split(/\r?\n/)) { + const line = rawLine.trim(); + if (!line.startsWith("tar: ")) continue; + const message = line.slice("tar: ".length); + for (const dirName of dirs) { + if ( + message === dirName || + message.startsWith(`${dirName}:`) || + message.startsWith(`${dirName}/`) + ) { + // "permission denied" is the more actionable cause — keep it even if + // other read errors were attributed to the same dir first. + const reason = message.includes("Permission denied") + ? BACKUP_FAILURE_PERMISSION_DENIED + : BACKUP_FAILURE_TAR_READ_ERROR; + if (reason === BACKUP_FAILURE_PERMISSION_DENIED || !failed.has(dirName)) { + failed.set(dirName, reason); + } + break; + } + } + } + return failed; +} + +/** Render failed items with any known per-directory cause. */ +export function formatFailedBackupItems( + failedItems: readonly string[], + reasons: Readonly> | undefined, +): string { + return failedItems + .map((item) => (reasons?.[item] ? `${item} (${reasons[item]})` : item)) + .join(", "); +} diff --git a/src/lib/state/sandbox.ts b/src/lib/state/sandbox.ts index 582a0288ac6..7a5968a9ed5 100644 --- a/src/lib/state/sandbox.ts +++ b/src/lib/state/sandbox.ts @@ -31,6 +31,10 @@ import { OPENSHELL_PROBE_TIMEOUT_MS } from "../adapters/openshell/timeouts.js"; import type { AgentStateFile } from "../agent/defs.js"; import { loadAgent } from "../agent/defs.js"; import { isObjectRecord, type UnknownRecord } from "../core/json-types.js"; +import { + BACKUP_FAILURE_ABSENT_AFTER_EXTRACTION, + classifyFailedDirsFromTarStderr, +} from "../domain/backup-failure.js"; import { shellQuote } from "../runner.js"; import { createTempSshConfig } from "../sandbox/temp-ssh-config.js"; import { isSensitiveFile, sanitizeConfigFile } from "../security/credential-filter.js"; @@ -736,55 +740,6 @@ function stateFileRemotePath(dir: string, filePath: string): string { return `${dir.replace(/\/+$/, "")}/${filePath}`; } -/** Failure cause: tar reported "Permission denied" while reading the dir. */ -export const BACKUP_FAILURE_PERMISSION_DENIED = "permission denied"; -/** Failure cause: tar reported other read errors for the dir. */ -export const BACKUP_FAILURE_TAR_READ_ERROR = "tar read error"; -/** Failure cause: tar succeeded but the dir never materialized on the host. */ -export const BACKUP_FAILURE_ABSENT_AFTER_EXTRACTION = "absent after extraction"; - -function failedDirsFromTarStderr(stderr: string, existingDirs: string[]): Map { - const failed = new Map(); - const dirs = [...existingDirs].sort((a, b) => b.length - a.length); - for (const rawLine of stderr.split(/\r?\n/)) { - const line = rawLine.trim(); - if (!line.startsWith("tar: ")) continue; - const message = line.slice("tar: ".length); - for (const dirName of dirs) { - if ( - message === dirName || - message.startsWith(`${dirName}:`) || - message.startsWith(`${dirName}/`) - ) { - // "permission denied" is the more actionable cause — keep it even if - // other read errors were attributed to the same dir first. - const reason = message.includes("Permission denied") - ? BACKUP_FAILURE_PERMISSION_DENIED - : BACKUP_FAILURE_TAR_READ_ERROR; - if (reason === BACKUP_FAILURE_PERMISSION_DENIED || !failed.has(dirName)) { - failed.set(dirName, reason); - } - break; - } - } - } - return failed; -} - -/** - * Render failed dirs/files for user-facing backup failure messages, - * appending the known per-dir cause: "identity (permission denied)". - * Items without a recorded cause render unchanged. - */ -export function formatFailedBackupItems( - failedItems: string[], - reasons: Record | undefined, -): string { - return failedItems - .map((item) => (reasons?.[item] ? `${item} (${reasons[item]})` : item)) - .join(", "); -} - const SQLITE_BACKUP_PY = [ "import sqlite3, sys", "src, dst = sys.argv[1], sys.argv[2]", @@ -1380,7 +1335,7 @@ export function backupSandboxState(sandboxName: string, options: BackupOptions = } } } else { - const tarFailedDirs = failedDirsFromTarStderr( + const tarFailedDirs = classifyFailedDirsFromTarStderr( result.stderr?.toString() || "", existingDirs, );