From af2dcfb16c6566f7adc835b47d0a19c5d58d09c0 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Mon, 10 Aug 2026 04:41:39 -0700 Subject: [PATCH] fix(sandbox): repair legacy gateway upgrade recovery Signed-off-by: Apurv Kumaria --- docs/manage-sandboxes/update-sandboxes.mdx | 6 +- scripts/install.sh | 3 + .../upgrade-sandboxes-recovery.test.ts | 44 ++++++++++-- src/lib/actions/upgrade-sandboxes.ts | 69 +++++++++++-------- .../live/openshell-gateway-upgrade-helpers.ts | 16 +++++ .../live/openshell-gateway-upgrade.test.ts | 2 + ...-gateway-upgrade-workflow-boundary.test.ts | 11 +++ 7 files changed, 118 insertions(+), 33 deletions(-) diff --git a/docs/manage-sandboxes/update-sandboxes.mdx b/docs/manage-sandboxes/update-sandboxes.mdx index 2cd65c5ab22..2a617391f87 100644 --- a/docs/manage-sandboxes/update-sandboxes.mdx +++ b/docs/manage-sandboxes/update-sandboxes.mdx @@ -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. For a listed pre-fingerprint OpenClaw or Hermes registry entry, you can provide that evidence through the installer's explicit managed-image confirmation. diff --git a/scripts/install.sh b/scripts/install.sh index 4566500864a..e60a7e5bf1a 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -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 diff --git a/src/lib/actions/upgrade-sandboxes-recovery.test.ts b/src/lib/actions/upgrade-sandboxes-recovery.test.ts index 92ebd4e7ba6..5757e9310c5 100644 --- a/src/lib/actions/upgrade-sandboxes-recovery.test.ts +++ b/src/lib/actions/upgrade-sandboxes-recovery.test.ts @@ -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"), @@ -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); @@ -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"], @@ -839,7 +839,7 @@ 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, @@ -847,6 +847,40 @@ describe("upgrade-sandboxes prepared backup recovery (#6114)", () => { }); }); + 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, diff --git a/src/lib/actions/upgrade-sandboxes.ts b/src/lib/actions/upgrade-sandboxes.ts index acffb7b641a..34ea14c9c72 100644 --- a/src/lib/actions/upgrade-sandboxes.ts +++ b/src/lib/actions/upgrade-sandboxes.ts @@ -153,12 +153,13 @@ function confirmedLegacyManagedRecoveryNames(): Set { } } -// 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 @@ -166,9 +167,10 @@ function confirmedLegacyManagedRecoveryNames(): Set { function isPreparedRecoveryCandidate( sandbox: registry.SandboxEntry, liveNames: Set, + staleLiveNames: Set, 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 { @@ -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( @@ -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 @@ -351,14 +355,20 @@ export async function upgradeSandboxes( // reconnected mid-run, so neither recovery candidates nor orphans. const becameReadyNames = new Set(); 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, @@ -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( @@ -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 @@ -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); @@ -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), ); @@ -485,7 +500,7 @@ export async function upgradeSandboxes( ); } if ( - rebuildable.length === 0 && + ordinaryRebuildable.length === 0 && preparedRecoveries.length === 0 && rejectedRecoveries.length === 0 ) { @@ -498,7 +513,7 @@ export async function upgradeSandboxes( let failed = rejectedRecoveries.length; const recoveredNames = new Set(); 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, diff --git a/test/e2e/live/openshell-gateway-upgrade-helpers.ts b/test/e2e/live/openshell-gateway-upgrade-helpers.ts index 56a8a060a4c..704f25c8bca 100644 --- a/test/e2e/live/openshell-gateway-upgrade-helpers.ts +++ b/test/e2e/live/openshell-gateway-upgrade-helpers.ts @@ -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; @@ -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[], ): void { diff --git a/test/e2e/live/openshell-gateway-upgrade.test.ts b/test/e2e/live/openshell-gateway-upgrade.test.ts index 47e906fa97f..61a7a62ade5 100644 --- a/test/e2e/live/openshell-gateway-upgrade.test.ts +++ b/test/e2e/live/openshell-gateway-upgrade.test.ts @@ -46,6 +46,7 @@ import { currentGatewayUpgradeInstallerArgs, currentNemoclawUpgradeRef, expectedLegacyRegistryMetadata, + legacyGatewayUpgradeDockerNetwork, oldGatewayUpgradeInstallerArgs, throwGatewayUpgradeSetupFailures, upgradeGatewayCleanupScript, @@ -1194,6 +1195,7 @@ runLinuxOpenShellGatewayUpgrade( firewallSetup = registerOpenShellHostMockFirewall({ cleanup, host, + networkName: legacyGatewayUpgradeDockerNetwork(OLD_NEMOCLAW_REF), port: Number(new URL(fake.baseUrl).port), }); } catch (error) { diff --git a/test/e2e/support/openshell-gateway-upgrade-workflow-boundary.test.ts b/test/e2e/support/openshell-gateway-upgrade-workflow-boundary.test.ts index d79dcf7280f..a67488ea573 100644 --- a/test/e2e/support/openshell-gateway-upgrade-workflow-boundary.test.ts +++ b/test/e2e/support/openshell-gateway-upgrade-workflow-boundary.test.ts @@ -18,6 +18,7 @@ import { currentGatewayUpgradeInstallerArgs, currentNemoclawUpgradeRef, expectedLegacyRegistryMetadata, + legacyGatewayUpgradeDockerNetwork, oldGatewayUpgradeInstallerArgs, throwGatewayUpgradeSetupFailures, upgradeGatewayCleanupScript, @@ -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([