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
6 changes: 5 additions & 1 deletion docs/manage-sandboxes/update-sandboxes.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,11 @@ If the installed OpenShell release cannot retire its gateway through a supported

After the host CLI and OpenShell update, the installer runs `$$nemoclaw upgrade-sandboxes --auto` to reconcile the existing sandboxes.

If an existing sandbox is not Ready, the automatic path requires a validated latest backup whose sandbox and agent identity match the registry and positive evidence that NemoClaw managed the image.
During installer-driven recovery, each stale or non-Ready sandbox requires a validated latest backup.
The backup's sandbox and agent identities must match the registry.
The registry must also contain positive evidence that NemoClaw managed the sandbox image.
If the replacement gateway reports a stale sandbox as Ready or Running, the installer reuses the validated pre-upgrade backup.
It does not attempt another backup from the replaced legacy runtime.
Comment on lines +81 to +85

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

State the managed-image confirmation exception.

Line 83 says that positive registry evidence is always required. The recovery code also permits an exact-name pre-fingerprint OpenClaw or Hermes confirmation when that registry evidence is absent. This conflicts with lines 87-92 and can make the supported recovery path appear unavailable.

Proposed fix
- The registry must also contain positive evidence that NemoClaw managed the sandbox image.
+ The registry must contain positive evidence that NemoClaw managed the sandbox image, unless a listed pre-fingerprint OpenClaw or Hermes entry has explicit managed-image confirmation.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
During installer-driven recovery, each stale or non-Ready sandbox requires a validated latest backup.
The backup's sandbox and agent identities must match the registry.
The registry must also contain positive evidence that NemoClaw managed the sandbox image.
If the replacement gateway reports a stale sandbox as Ready or Running, the installer reuses the validated pre-upgrade backup.
It does not attempt another backup from the replaced legacy runtime.
During installer-driven recovery, each stale or non-Ready sandbox requires a validated latest backup.
The backup's sandbox and agent identities must match the registry.
The registry must contain positive evidence that NemoClaw managed the sandbox image, unless a listed pre-fingerprint OpenClaw or Hermes entry has explicit managed-image confirmation.
If the replacement gateway reports a stale sandbox as Ready or Running, the installer reuses the validated pre-upgrade backup.
It does not attempt another backup from the replaced legacy runtime.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/manage-sandboxes/update-sandboxes.mdx` around lines 81 - 85, Update the
recovery requirements in the paragraph beginning “During installer-driven
recovery” to state that registry evidence of NemoClaw-managed image ownership is
required unless the exact-name pre-fingerprint OpenClaw or Hermes confirmation
exception applies. Align the wording with the recovery behavior described in the
surrounding lines so this supported fallback remains explicitly available.


<AgentOnly variant="openclaw,hermes">
For a listed pre-fingerprint OpenClaw or Hermes registry entry, you can provide that evidence through the installer's explicit managed-image confirmation.
Expand Down
3 changes: 3 additions & 0 deletions scripts/install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2848,6 +2848,9 @@ preinstall_backup_and_retire_legacy_gateway() {
fi
error "Pre-upgrade backup stopped the installer. Resolve every reported sandbox backup failure or skipped sandbox using the CLI output above, then rerun the installer."
fi
# The replacement gateway may report legacy rows as Ready even when their
# state is no longer inspectable. Reuse this validated backup for every stale
# or non-Ready recreate instead of attempting a second live backup.
export NEMOCLAW_RESTORE_LATEST_BACKUP_ON_RECREATE=1

# Retire a backed-up gateway before install-openshell replaces an out-of-range
Expand Down
44 changes: 39 additions & 5 deletions src/lib/actions/upgrade-sandboxes-recovery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ describe("upgrade-sandboxes prepared backup recovery (#6114)", () => {
);
});

it("forwards two stale shared-route sandboxes through upgrade-sandboxes --auto (#7615, #7798)", async () => {
it("restores stale live sandboxes from the validated pre-upgrade backup (#7615, #7798)", async () => {
const names = ["alpha", "beta"];
const harness = createRecoveryHarness(names, {
liveOutput: names.map((name) => `${name} Ready`).join("\n"),
Expand All @@ -316,11 +316,11 @@ describe("upgrade-sandboxes prepared backup recovery (#6114)", () => {
await expect(harness.upgradeSandboxes(["--auto"])).resolves.toBeUndefined();

expect(harness.rebuildSpy).toHaveBeenNthCalledWith(1, "alpha", ["--yes"], {
recoveryManifest: undefined,
recoveryManifest: expect.objectContaining({ sandboxName: "alpha" }),
throwOnError: true,
});
expect(harness.rebuildSpy).toHaveBeenNthCalledWith(2, "beta", ["--yes"], {
recoveryManifest: undefined,
recoveryManifest: expect.objectContaining({ sandboxName: "beta" }),
throwOnError: true,
});
expect(harness.rebuildSpy).toHaveBeenCalledTimes(2);
Expand Down Expand Up @@ -828,7 +828,7 @@ describe("upgrade-sandboxes prepared backup recovery (#6114)", () => {
});
});

it("attempts both a live stale rebuild and a prepared non-Ready recovery", async () => {
it("uses prepared recovery for both stale live and non-Ready sandboxes", async () => {
const harness = createRecoveryHarness(["stale-box", "recovery-box"], {
liveOutput: "stale-box Ready\nrecovery-box Error",
staleNames: ["stale-box"],
Expand All @@ -839,14 +839,48 @@ describe("upgrade-sandboxes prepared backup recovery (#6114)", () => {
expect(harness.rebuildSpy).toHaveBeenCalledTimes(2);
expect(harness.rebuildSpy).toHaveBeenNthCalledWith(1, "stale-box", ["--yes"], {
throwOnError: true,
recoveryManifest: undefined,
recoveryManifest: expect.objectContaining({ sandboxName: "stale-box" }),
});
expect(harness.rebuildSpy).toHaveBeenNthCalledWith(2, "recovery-box", ["--yes"], {
throwOnError: true,
recoveryManifest: expect.objectContaining({ sandboxName: "recovery-box" }),
});
});

it("takes a fresh backup for stale live sandboxes outside installer restore intent", async () => {
const harness = createRecoveryHarness(["stale-box"], {
liveOutput: "stale-box Ready",
staleNames: ["stale-box"],
});
vi.stubEnv("NEMOCLAW_RESTORE_LATEST_BACKUP_ON_RECREATE", "0");

await expect(harness.upgradeSandboxes({ auto: true })).resolves.toBeUndefined();

expect(harness.latestBackupSpy).not.toHaveBeenCalled();
expect(harness.rebuildSpy).toHaveBeenCalledWith("stale-box", ["--yes"], {
throwOnError: true,
recoveryManifest: undefined,
});
});

it("fails closed when a stale live sandbox has no validated pre-upgrade backup", async () => {
const harness = createRecoveryHarness(["stale-box"], {
latestBackup: null,
liveOutput: "stale-box Ready",
staleNames: ["stale-box"],
});
vi.spyOn(process, "exit").mockImplementation(((code?: number) => {
throw new Error(`process.exit(${code})`);
}) as never);

await expect(harness.upgradeSandboxes({ auto: true })).rejects.toThrow("process.exit(1)");

expect(harness.rebuildSpy).not.toHaveBeenCalled();
expect(console.error).toHaveBeenCalledWith(
expect.stringContaining("no validated pre-upgrade backup was found"),
);
});

it("fails closed for a live Error sandbox with no latest backup", async () => {
const harness = createRecoveryHarness(["broken-box"], {
latestBackup: null,
Expand Down
69 changes: 42 additions & 27 deletions src/lib/actions/upgrade-sandboxes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,22 +153,24 @@ function confirmedLegacyManagedRecoveryNames(): Set<string> {
}
}

// Under installer restore intent, a registry sandbox the selected gateway does
// not report Ready/Running is eligible for prepared-backup recovery only when
// its persisted binding resolves to that selected gateway, whether the gateway
// observes it in a non-Ready phase or it is absent. Observation alone is
// insufficient: a sandbox bound to a different recorded gateway may be Ready
// there, so recovering it would clobber a healthy sandbox.
// Under installer restore intent, a registry sandbox is eligible for prepared-
// backup recovery only when its persisted binding resolves to the selected
// gateway. Ready/Running sandboxes are eligible only when upgrade classification
// also proves them stale; non-Ready or absent sandboxes remain eligible because
// the replaced gateway may expose legacy state optimistically or not at all.
// Observation alone is insufficient: a sandbox bound to a different recorded
// gateway may be Ready there, so recovering it would clobber a healthy sandbox.
// resolveSandboxGatewayName throws on an invalid persisted
// binding — report that fixed, sanitized condition and treat it as ineligible so
// a corrupted registry row never drives a recreate. Remove this guard only when
// every registry write path validates gateway bindings before persistence.
function isPreparedRecoveryCandidate(
sandbox: registry.SandboxEntry,
liveNames: Set<string>,
staleLiveNames: Set<string>,
selectedGatewayName: string,
): boolean {
if (liveNames.has(sandbox.name)) return false;
if (liveNames.has(sandbox.name) && !staleLiveNames.has(sandbox.name)) return false;
try {
return resolveSandboxGatewayName(sandbox) === selectedGatewayName;
} catch {
Expand Down Expand Up @@ -302,8 +304,9 @@ export async function upgradeSandboxes(
});
const liveNames = parseReadySandboxNames(liveResult.output || "");
// Sandboxes the selected gateway observes in a non-Ready phase. Absence from
// the selected gateway is handled by isPreparedRecoveryCandidate, which recovers
// an absent sandbox only when it resolves to the selected gateway.
// the selected gateway and stale Ready/Running rows are handled by
// isPreparedRecoveryCandidate, which recovers them only when they resolve to
// the selected gateway.
const nonReadyLiveNames = new Set(
parseLiveSandboxEntries(liveResult.output || "")
.filter(
Expand All @@ -322,14 +325,15 @@ export async function upgradeSandboxes(
{ currentNemoclawVersion: resolveCurrentNemoclawVersion() },
);

// Source boundary (#6114): a v0.0.55/legacy-OpenShell install can leave its
// already-registered sandboxes in Provisioning/Error after the host upgrade.
// That state comes from the already-installed legacy CLI/gateway and cannot be
// prevented at its source by this candidate. install.sh exports this signal only
// after the current CLI completes a strict backup, or after an operator asserts
// prepared upgrade state. Pre-fingerprint OpenClaw/Hermes rows require a separate,
// exact-name confirmation that they used a managed image; custom-image evidence
// still fails closed.
// Source boundary (#6114): a legacy OpenShell install can leave its already-
// registered sandboxes in Provisioning/Error after the host upgrade, or the
// replacement gateway can report a stale row as Ready even though its legacy
// state is no longer inspectable. That state comes from the already-installed
// legacy CLI/gateway and cannot be prevented at its source by this candidate.
// install.sh exports this signal only after the current CLI completes a strict
// backup, or after an operator asserts prepared upgrade state. Pre-fingerprint
// OpenClaw/Hermes rows require a separate, exact-name confirmation that they
// used a managed image; custom-image evidence still fails closed.
// upgrade-sandboxes-recovery.test.ts and
// install-preexisting-sandbox-recovery.test.ts guard the handoff. Remove this
// bridge with onboard's matching consumer once prepared-backup installer recovery
Expand All @@ -351,14 +355,20 @@ export async function upgradeSandboxes(
// reconnected mid-run, so neither recovery candidates nor orphans.
const becameReadyNames = new Set<string>();
if (recoverPreparedBackups) {
const staleLiveNames = new Set(
stale.filter((sandbox) => sandbox.running).map((sandbox) => sandbox.name),
);
const gatewayEligible = sandboxes.filter((sandbox) =>
isPreparedRecoveryCandidate(sandbox, liveNames, selectedGatewayName),
isPreparedRecoveryCandidate(sandbox, liveNames, staleLiveNames, selectedGatewayName),
);
const staleLiveCandidates = gatewayEligible.filter((sandbox) =>
staleLiveNames.has(sandbox.name),
);
const nonReadyCandidates = gatewayEligible.filter((sandbox) =>
nonReadyLiveNames.has(sandbox.name),
);
const absentCandidates = gatewayEligible.filter(
(sandbox) => !nonReadyLiveNames.has(sandbox.name),
(sandbox) => !staleLiveNames.has(sandbox.name) && !nonReadyLiveNames.has(sandbox.name),
);
const confirmedAbsentCandidates = await confirmAbsentRecoveryCandidates(
absentCandidates,
Expand All @@ -369,7 +379,11 @@ export async function upgradeSandboxes(
for (const sandbox of absentCandidates) {
if (!confirmedAbsentNames.has(sandbox.name)) becameReadyNames.add(sandbox.name);
}
recoveryCandidates = [...nonReadyCandidates, ...confirmedAbsentCandidates];
recoveryCandidates = [
...staleLiveCandidates,
...nonReadyCandidates,
...confirmedAbsentCandidates,
];
}
const backupRecoveryAssessments = recoveryCandidates.map((sandbox) =>
prepareBackupRecovery(
Expand Down Expand Up @@ -433,7 +447,7 @@ export async function upgradeSandboxes(
console.log(`\n ${B}Prepared backup recovery:${R}`);
for (const recovery of preparedRecoveries) {
console.log(
` ${recovery.sandbox.name} ${D}${recovery.manifest.timestamp}${R} (non-Ready)`,
` ${recovery.sandbox.name} ${D}${recovery.manifest.timestamp}${R} (pre-upgrade backup)`,
);
// #7073: the validated manifest records the agent-specific managed state
// root restored for this sandbox. Warn before the destructive recreate so
Expand Down Expand Up @@ -461,13 +475,11 @@ export async function upgradeSandboxes(
}
if (preparedRecoveries.length > 0) {
console.log(
` ${preparedRecoveries.length} non-Ready sandbox(es) have a validated pre-upgrade backup.`,
` ${preparedRecoveries.length} sandbox(es) have a validated pre-upgrade backup.`,
);
}
if (rejectedRecoveries.length > 0) {
console.log(
` ${rejectedRecoveries.length} non-Ready sandbox(es) cannot be recovered automatically.`,
);
console.log(` ${rejectedRecoveries.length} sandbox(es) cannot be recovered automatically.`);
}
// Check mode must agree with auto mode on the orphan diagnosis (#6520).
printOrphanedRegistrySandboxes(unobservedOwnGatewaySandboxes);
Expand All @@ -476,6 +488,9 @@ export async function upgradeSandboxes(
}

const { rebuildable, stopped } = splitRebuildableSandboxes(stale);
const ordinaryRebuildable = rebuildable.filter(
(sandbox) => !assessedRecoveryNames.has(sandbox.name),
);
const notObservedReadyOrNonReady = stopped.filter(
(sandbox) => !assessedRecoveryNames.has(sandbox.name),
);
Expand All @@ -485,7 +500,7 @@ export async function upgradeSandboxes(
);
}
if (
rebuildable.length === 0 &&
ordinaryRebuildable.length === 0 &&
preparedRecoveries.length === 0 &&
rejectedRecoveries.length === 0
) {
Expand All @@ -498,7 +513,7 @@ export async function upgradeSandboxes(
let failed = rejectedRecoveries.length;
const recoveredNames = new Set<string>();
const work = [
...rebuildable.map((sandbox) => ({ sandbox, manifest: null })),
...ordinaryRebuildable.map((sandbox) => ({ sandbox, manifest: null })),
...preparedRecoveries.map((recovery) => ({
sandbox: { name: recovery.sandbox.name },
manifest: recovery.manifest,
Expand Down
16 changes: 16 additions & 0 deletions test/e2e/live/openshell-gateway-upgrade-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { reviewedOldInstallerProfile } from "./openshell-gateway-upgrade-old-ins

const NON_INTERACTIVE_INSTALLER_ARGS = ["--non-interactive", "--yes-i-accept-third-party-software"];
const GATEWAY_VOLUME_PREFIX = "openshell-cluster-nemoclaw";
const LEGACY_GATEWAY_DOCKER_NETWORK = "openshell-cluster-nemoclaw";

export interface LegacyGatewayUpgradeFixture {
nemoclawRef: string;
Expand Down Expand Up @@ -70,6 +71,21 @@ export function currentNemoclawUpgradeRef(env: NodeJS.ProcessEnv): string {
return "HEAD";
}

export function legacyGatewayUpgradeDockerNetwork(nemoclawRef: string): string | undefined {
switch (nemoclawRef) {
case "v0.0.36":
// This cluster-era gateway names its bridge after the gateway; newer
// Docker gateways use the host fixture's openshell-docker default.
return LEGACY_GATEWAY_DOCKER_NETWORK;
case "v0.0.55":
case "v0.0.74":
case "v0.0.89":
return undefined;
default:
throw new Error(`Unsupported gateway-upgrade network fixture: ${nemoclawRef}`);
}
}

export function throwGatewayUpgradeSetupFailures(
results: readonly PromiseSettledResult<unknown>[],
): void {
Expand Down
2 changes: 2 additions & 0 deletions test/e2e/live/openshell-gateway-upgrade.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import {
currentGatewayUpgradeInstallerArgs,
currentNemoclawUpgradeRef,
expectedLegacyRegistryMetadata,
legacyGatewayUpgradeDockerNetwork,
oldGatewayUpgradeInstallerArgs,
throwGatewayUpgradeSetupFailures,
upgradeGatewayCleanupScript,
Expand Down Expand Up @@ -1194,6 +1195,7 @@ runLinuxOpenShellGatewayUpgrade(
firewallSetup = registerOpenShellHostMockFirewall({
cleanup,
host,
networkName: legacyGatewayUpgradeDockerNetwork(OLD_NEMOCLAW_REF),
port: Number(new URL(fake.baseUrl).port),
});
} catch (error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
currentGatewayUpgradeInstallerArgs,
currentNemoclawUpgradeRef,
expectedLegacyRegistryMetadata,
legacyGatewayUpgradeDockerNetwork,
oldGatewayUpgradeInstallerArgs,
throwGatewayUpgradeSetupFailures,
upgradeGatewayCleanupScript,
Expand Down Expand Up @@ -102,6 +103,16 @@ describe("OpenShell gateway upgrade workflow boundary", () => {
expect(currentNemoclawUpgradeRef({})).toBe("HEAD");
});

it("targets the Docker network created by each historical gateway fixture", () => {
expect(legacyGatewayUpgradeDockerNetwork("v0.0.36")).toBe("openshell-cluster-nemoclaw");
for (const nemoclawRef of ["v0.0.55", "v0.0.74", "v0.0.89"]) {
expect(legacyGatewayUpgradeDockerNetwork(nemoclawRef)).toBeUndefined();
}
expect(() => legacyGatewayUpgradeDockerNetwork("v0.0.90")).toThrow(
/Unsupported gateway-upgrade network fixture/,
);
});

it("accepts successful legacy install and firewall setup results (#8696)", () => {
expect(() =>
throwGatewayUpgradeSetupFailures([
Expand Down
Loading