Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 34 additions & 16 deletions src/lib/actions/maintenance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,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.

Expand Down Expand Up @@ -113,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 () => {
Expand Down
8 changes: 6 additions & 2 deletions src/lib/actions/maintenance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -113,8 +114,11 @@ export async function backupAll(): Promise<void> {
}
unreachableRunning++;
}
const failedItems = [...result.failedDirs, ...result.failedFiles];
console.error(` ${RD}✗${R} ${sb.name}: backup failed (${failedItems.join(", ")})`);
const failedItems = formatFailedBackupItems(
[...result.failedDirs, ...result.failedFiles],
result.failedDirReasons,
);
console.error(` ${RD}✗${R} ${sb.name}: backup failed (${failedItems})`);
failed++;
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/lib/actions/sandbox/snapshot.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1474,6 +1474,7 @@ describe("runSandboxSnapshot", () => {
backupSandboxStateMock.mockReturnValue({
success: false,
failedDirs: ["workspace", "skills"],
failedDirReasons: { workspace: "permission denied" },
failedFiles: ["openclaw.json"],
});
const consoleError = vi.spyOn(console, "error").mockImplementation(() => {});
Expand All @@ -1486,7 +1487,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");
});
});
4 changes: 3 additions & 1 deletion src/lib/actions/sandbox/snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -545,7 +546,8 @@ function runSnapshotCreate(
} else {
console.error(" Snapshot failed.");
if (result.failedDirs.length > 0) {
console.error(` Failed directories: ${result.failedDirs.join(", ")}`);
const failedDirs = formatFailedBackupItems(result.failedDirs, result.failedDirReasons);
console.error(` Failed directories: ${failedDirs}`);
}
if (result.failedFiles.length > 0) {
console.error(` Failed files: ${result.failedFiles.join(", ")}`);
Expand Down
42 changes: 42 additions & 0 deletions src/lib/domain/backup-failure.test.ts
Original file line number Diff line number Diff line change
@@ -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",
);
});
});
50 changes: 50 additions & 0 deletions src/lib/domain/backup-failure.ts
Original file line number Diff line number Diff line change
@@ -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<string, string> {
const failed = new Map<string, string>();
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<Record<string, string>> | undefined,
): string {
return failedItems
.map((item) => (reasons?.[item] ? `${item} (${reasons[item]})` : item))
.join(", ");
}
43 changes: 19 additions & 24 deletions src/lib/state/sandbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -136,6 +140,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<string, string>;
// 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;
Expand Down Expand Up @@ -730,27 +740,6 @@ function stateFileRemotePath(dir: string, filePath: string): string {
return `${dir.replace(/\/+$/, "")}/${filePath}`;
}

function failedDirsFromTarStderr(stderr: string, existingDirs: string[]): Set<string> {
const failed = new Set<string>();
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}/`)
) {
failed.add(dirName);
break;
}
}
}
return failed;
}

const SQLITE_BACKUP_PY = [
"import sqlite3, sys",
"src, dst = sys.argv[1], sys.argv[2]",
Expand Down Expand Up @@ -1128,6 +1117,7 @@ export function backupSandboxState(sandboxName: string, options: BackupOptions =

const backedUpDirs: string[] = [];
const failedDirs: string[] = [];
const failedDirReasons: Record<string, string> = {};
const backedUpFiles: string[] = [];
const failedFiles: string[] = [];
let unreachable = false;
Expand Down Expand Up @@ -1341,10 +1331,11 @@ 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 {
const tarFailedDirs = failedDirsFromTarStderr(
const tarFailedDirs = classifyFailedDirsFromTarStderr(
result.stderr?.toString() || "",
existingDirs,
);
Expand All @@ -1355,12 +1346,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);
}
Expand Down Expand Up @@ -1425,6 +1419,7 @@ export function backupSandboxState(sandboxName: string, options: BackupOptions =
manifest,
backedUpDirs,
failedDirs,
...(Object.keys(failedDirReasons).length > 0 ? { failedDirReasons } : {}),
backedUpFiles,
failedFiles,
};
Expand Down
21 changes: 15 additions & 6 deletions test/snapshot.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand All @@ -590,22 +591,26 @@ process.exit(0);

const backup = sandboxState.backupSandboxState("alpha");
expect(backup.success).toBe(false);
expect(backup.failedDirs).toEqual(["agents"]);
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")
.trim()
.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'");
Expand Down Expand Up @@ -960,6 +965,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 {
Expand Down
Loading