diff --git a/ci/platform-matrix.json b/ci/platform-matrix.json index 5f8a476ba59..52f737a7fc1 100644 --- a/ci/platform-matrix.json +++ b/ci/platform-matrix.json @@ -31,7 +31,7 @@ "status": "tested", "prd_priority": "P0", "ci_tested": true, - "notes": "Primary tested path. Ubuntu 24.04 is the validated distro in production source (`DEFAULT_COMPAT_IMAGE` in `src/lib/onboard/docker-driver-gateway-compat.ts:11` and the preflight tests pin 24.04 only); the installer's package-manager probes assume apt-get. Other distros (Ubuntu 22.04, Fedora, Rocky, Alma, NixOS, Arch) may work but are not validated." + "notes": "Primary tested path. Ubuntu 24.04 is the validated distro in production source (`DEFAULT_COMPAT_IMAGE` in `src/lib/onboard/docker-driver-gateway-compat.ts:16` and the preflight tests pin 24.04 only); the installer's package-manager probes assume apt-get. Other distros (Ubuntu 22.04, Fedora, Rocky, Alma, NixOS, Arch) may work but are not validated." }, { "name": "macOS (Apple Silicon)", diff --git a/docs/get-started/prerequisites.mdx b/docs/get-started/prerequisites.mdx index c0044cd6c19..47a2513a5e6 100644 --- a/docs/get-started/prerequisites.mdx +++ b/docs/get-started/prerequisites.mdx @@ -91,7 +91,7 @@ The table comes from [`ci/platform-matrix.json`](https://github.com/NVIDIA/NemoC {/* platform-matrix:begin */} | OS | Container runtime | Status | Notes | |----|-------------------|--------|-------| -| Linux | Docker | Tested | Primary tested path. Ubuntu 24.04 is the validated distro in production source (`DEFAULT_COMPAT_IMAGE` in `src/lib/onboard/docker-driver-gateway-compat.ts:11` and the preflight tests pin 24.04 only); the installer's package-manager probes assume apt-get. Other distros (Ubuntu 22.04, Fedora, Rocky, Alma, NixOS, Arch) may work but are not validated. | +| Linux | Docker | Tested | Primary tested path. Ubuntu 24.04 is the validated distro in production source (`DEFAULT_COMPAT_IMAGE` in `src/lib/onboard/docker-driver-gateway-compat.ts:16` and the preflight tests pin 24.04 only); the installer's package-manager probes assume apt-get. Other distros (Ubuntu 22.04, Fedora, Rocky, Alma, NixOS, Arch) may work but are not validated. | | macOS (Apple Silicon) | Colima, Docker Desktop | Tested with limitations | Start the container runtime (Colima or Docker Desktop) before running the installer. Homebrew Colima users must install both Colima and the Docker CLI (`brew install colima docker`) before `docker info` can work. Xcode Command Line Tools (`xcode-select --install`) are typically required for Node native modules during install. NemoClaw recommends them but does not enforce them during preflight. | | DGX Spark | Docker | Tested | Use the standard installer and `$$nemoclaw onboard`. For an end-to-end walkthrough with local inference, see the [NVIDIA Spark playbook](https://build.nvidia.com/spark/nemoclaw). | | Windows WSL2 | Docker Desktop (WSL backend) | Tested with limitations | Requires WSL2 with Docker Desktop backend. | diff --git a/docs/get-started/quickstart.mdx b/docs/get-started/quickstart.mdx index 99203ec601d..4ba7663eb68 100644 --- a/docs/get-started/quickstart.mdx +++ b/docs/get-started/quickstart.mdx @@ -120,6 +120,7 @@ At any prompt, press Enter to accept the default shown in `[brackets]`, type `ba If registered sandboxes already exist, the installer prepares the current NemoClaw CLI without replacing OpenShell, then requires a fresh backup of every registered sandbox before it changes the gateway. After the host upgrade, it runs `nemoclaw upgrade-sandboxes --auto` to rebuild stale sandboxes and restore validated backups for registered sandboxes that are not Ready. Successful recovery completes the existing-sandbox upgrade and skips generic onboarding, so the installer does not create an extra sandbox or ask for a new provider credential. +If the recovery pass exits 0 but a recorded sandbox is not found on its own recorded gateway (for example after `nemoclaw uninstall` removed the gateway and Docker image while preserving `sandboxes.json`), the installer finishes with `Installation completed with warnings` and remediation guidance instead of claiming the sandbox was recovered. For pre-fingerprint OpenClaw and Hermes registry entries, the installer asks you to confirm that every listed sandbox used a NemoClaw-managed image before it permits recovery onto the current managed image. In non-interactive runs, set `NEMOCLAW_CONFIRM_LEGACY_MANAGED_RECREATE` to the exact JSON array of names printed by the installer, such as `["my-assistant","preserve-hermes"]`, only after you verify every named sandbox used a managed image. Legacy managed-image confirmation never overrides recorded custom-image evidence. diff --git a/docs/manage-sandboxes/lifecycle.mdx b/docs/manage-sandboxes/lifecycle.mdx index 4c314c40b79..01d3819bedf 100644 --- a/docs/manage-sandboxes/lifecycle.mdx +++ b/docs/manage-sandboxes/lifecycle.mdx @@ -382,6 +382,7 @@ In a non-interactive run, set `NEMOCLAW_CONFIRM_LEGACY_MANAGED_RECREATE` to the Legacy managed-image confirmation never overrides recorded custom-image evidence. A custom OpenClaw sandbox can be recovered only when the selected validated backup independently carries complete authoritative image-plugin provenance; otherwise recovery stops before deletion. The installer attempts every eligible recovery, exits with a nonzero status if any recovery fails, and skips generic onboarding after successful recovery. +A third outcome exists: when a recorded sandbox is not observed in any phase on its own recorded gateway (typically because a prior uninstall removed the gateway and Docker image while preserving `sandboxes.json`), the recovery pass exits 0 but reports the sandbox as not found rather than recovered, and the installer finishes with `Installation completed with warnings` plus remediation guidance (`$$nemoclaw destroy`, then `$$nemoclaw onboard`) instead of claiming success. For manual upgrade flows, create a snapshot first and then run the update or rebuild command you need: ```bash @@ -421,6 +422,7 @@ For non-interactive runs (`--yes`, `NEMOCLAW_NON_INTERACTIVE=1`, or a non-TTY sh `--yes` stays non-destructive by design. It only acknowledges the global confirmation prompt and never purges preserved user data on its own. Full purge always requires an explicit `--destroy-user-data` or the matching env var, so existing automation using `--yes` retains its safe behaviour. +Preserving `sandboxes.json` does not preserve the gateway registration, provider registrations, or Docker image its recorded sandboxes depend on; uninstall warns that those records cannot be recovered automatically on reinstall, and the remediation is `$$nemoclaw destroy` followed by `$$nemoclaw onboard`. For a full host-side file reference, see [Host Files and State](../reference/host-files-and-state). Refer to the [Commands reference](../reference/commands#$$nemoclaw-uninstall) for the full preservation contract. diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index 773ff48565c..7a3ce0d6b58 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -2412,8 +2412,10 @@ NemoClaw resolves the digest of `ghcr.io/nvidia/nemoclaw/sandbox-base:latest` fr Sandboxes that match the current digest are left alone. NemoClaw also checks the build fingerprint recorded on each managed sandbox image. A sandbox needs upgrade when its agent version is stale, when its recorded NemoClaw image fingerprint differs from the running CLI, or both. +When the target version is older than the recorded one (for example after reinstalling with an older `NEMOCLAW_INSTALL_TAG`), the stale listing marks the change with a `(downgrade)` suffix instead of framing it as a routine upgrade. Custom Dockerfile sandboxes are not classified by image drift because rebuilding them onto the default image would drop the custom image. Legacy sandboxes without a recorded fingerprint opt into this check after their next rebuild. +A recorded sandbox that is not observed in any phase on its own recorded gateway is reported as not found there, with remediation guidance — this typically means its gateway registration or Docker image was removed (for example by `$$nemoclaw uninstall`, which preserves `sandboxes.json` but removes both). ```bash $$nemoclaw upgrade-sandboxes [--check] [--auto] [--yes|-y] @@ -2965,6 +2967,11 @@ Decision matrix: The preserved entries survive uninstall as inert files on disk. Reinstall NemoClaw and re-onboard the sandbox before `$$nemoclaw snapshot restore` can use them. +Preserving `sandboxes.json` does not make the recorded sandboxes recoverable on their own: uninstall removes the gateway registration, provider registrations, and Docker image those records depend on. +Uninstall warns about this at preserve time. +After reinstalling, the installer reports such records as not found on their recorded gateway instead of claiming they were recovered; run `$$nemoclaw destroy` to clear a stranded record, then `$$nemoclaw onboard` to rebuild it. +Pass `--destroy-user-data` at uninstall time if you prefer to purge the registry along with its dependencies. + #### `$$nemoclaw uninstall` vs. the hosted `uninstall.sh` Both forms execute the same `uninstall.sh` with the same flags, but differ in where the script comes from and how much they trust the network. diff --git a/docs/reference/host-files-and-state.mdx b/docs/reference/host-files-and-state.mdx index 334791ca8be..c8fd969d652 100644 --- a/docs/reference/host-files-and-state.mdx +++ b/docs/reference/host-files-and-state.mdx @@ -51,6 +51,7 @@ If you see `registry.json` in older tests, notes, or discussions, treat it as le `$$nemoclaw uninstall --yes` removes active NemoClaw runtime resources but preserves the user data needed for recovery by default. Preserved entries include `rebuild-backups/`, `backups/`, and `sandboxes.json`. +Preserved `sandboxes.json` records are not automatically recoverable after reinstall, because uninstall removes the gateway registration, provider registrations, and Docker image they reference; uninstall warns about this at preserve time, and a later reinstall reports such records as not found on their recorded gateway with `$$nemoclaw destroy` / `$$nemoclaw onboard` remediation. Interactive uninstall prompts before removing preserved state. For non-interactive runs, pass `--destroy-user-data` only when you accept losing local registry metadata and backups. diff --git a/docs/reference/platform-support.mdx b/docs/reference/platform-support.mdx index 33270549c3c..1d12cfbbbd5 100644 --- a/docs/reference/platform-support.mdx +++ b/docs/reference/platform-support.mdx @@ -78,7 +78,7 @@ For the onboarding-time supported set without deferred rows, refer to [Prerequis {/* platform-matrix-full:begin */} | OS | Container runtime | Status | PRD priority | CI | Notes | |----|-------------------|--------|--------------|----|-------| -| Linux | Docker | Tested | P0 | Yes | Primary tested path. Ubuntu 24.04 is the validated distro in production source (`DEFAULT_COMPAT_IMAGE` in `src/lib/onboard/docker-driver-gateway-compat.ts:11` and the preflight tests pin 24.04 only); the installer's package-manager probes assume apt-get. Other distros (Ubuntu 22.04, Fedora, Rocky, Alma, NixOS, Arch) may work but are not validated. | +| Linux | Docker | Tested | P0 | Yes | Primary tested path. Ubuntu 24.04 is the validated distro in production source (`DEFAULT_COMPAT_IMAGE` in `src/lib/onboard/docker-driver-gateway-compat.ts:16` and the preflight tests pin 24.04 only); the installer's package-manager probes assume apt-get. Other distros (Ubuntu 22.04, Fedora, Rocky, Alma, NixOS, Arch) may work but are not validated. | | macOS (Apple Silicon) | Colima, Docker Desktop | Tested with limitations | P0 | Yes | Start the container runtime (Colima or Docker Desktop) before running the installer. Homebrew Colima users must install both Colima and the Docker CLI (`brew install colima docker`) before `docker info` can work. Xcode Command Line Tools (`xcode-select --install`) are typically required for Node native modules during install. NemoClaw recommends them but does not enforce them during preflight. | | DGX Spark | Docker | Tested | P1 | Yes | Use the standard installer and `$$nemoclaw onboard`. For an end-to-end walkthrough with local inference, see the [NVIDIA Spark playbook](https://build.nvidia.com/spark/nemoclaw). | | Windows WSL2 | Docker Desktop (WSL backend) | Tested with limitations | P1 | No | Requires WSL2 with Docker Desktop backend. | diff --git a/scripts/install.sh b/scripts/install.sh index c82dcc476cd..d6def9c521c 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -536,8 +536,13 @@ print_done() { # #5735: do not claim a clean install when the automatic upgrade of a # pre-existing sandbox failed (it may have been destroyed before its recreate # failed). Surface an explicit incomplete/recovery status instead. + # #6520: same when recovery exited 0 but recorded sandboxes were not found + # on their own recorded gateway — they were not recovered, so the install is + # not clean either. if [[ "${_UPGRADE_SANDBOXES_FAILED:-false}" == true ]]; then warn "=== Installation completed with warnings ===" + elif [[ "${_PREEXISTING_SANDBOX_ORPHANED:-false}" == true ]]; then + warn "=== Installation completed with warnings ===" else info "=== Installation complete ===" fi @@ -545,14 +550,27 @@ print_done() { printf " ${C_GREEN}${C_BOLD}%s${C_RESET} ${C_DIM}(%ss)${C_RESET}\n" "$_CLI_DISPLAY" "$elapsed" printf "\n" if [[ "${_PREEXISTING_SANDBOX_RECOVERY_RAN:-false}" == true ]]; then - printf " ${C_GREEN}Existing sandboxes were recovered and upgraded.${C_RESET}\n" + if [[ "${_PREEXISTING_SANDBOX_ORPHANED:-false}" == true ]]; then + # #6520: recovery exited 0 but recorded sandboxes were not found on + # their own recorded gateway; do not report them as recovered, and give + # a concrete remediation path instead. + printf " ${C_YELLOW}Some recorded sandboxes were not found on their recorded gateway and were not recovered.${C_RESET}\n" + printf " ${C_YELLOW}Their gateway registration or Docker image may have been removed (see the recovery notes above).${C_RESET}\n" + printf " ${C_DIM}Clear a stranded sandbox with '%s destroy', then rebuild it with '%s onboard'.${C_RESET}\n" "$_CLI_BIN" "$_CLI_BIN" + else + printf " ${C_GREEN}Existing sandboxes were recovered and upgraded.${C_RESET}\n" + fi if [[ "$_needs_cli_refresh" == true ]]; then printf " ${C_YELLOW}%s installed, but this shell needs PATH refresh before '%s' will run.${C_RESET}\n" "$_CLI_DISPLAY" "$_CLI_BIN" printf "\n" printf " ${C_GREEN}For this terminal:${C_RESET}\n" print_cli_path_refresh_actions fi - printf " ${C_DIM}No new sandbox onboarding was needed.${C_RESET}\n" + if [[ "${_PREEXISTING_SANDBOX_ORPHANED:-false}" == true ]]; then + printf " ${C_DIM}Generic onboarding was skipped because recorded sandboxes exist.${C_RESET}\n" + else + printf " ${C_DIM}No new sandbox onboarding was needed.${C_RESET}\n" + fi elif [[ "$ONBOARD_RAN" == true ]]; then local agent_name agent_name="$(resolve_onboarded_agent)" @@ -925,6 +943,11 @@ ONBOARD_RAN=false _CLI_PATH="" _PREEXISTING_SANDBOX_COUNT=0 _PREEXISTING_SANDBOX_RECOVERY_RAN=false +# #6520: set when the automatic recovery pass exited 0 but skipped recorded +# sandboxes it could not observe on the selected gateway (e.g. their gateway +# and Docker image were removed by a prior uninstall while sandboxes.json was +# preserved). The final summary must not claim those sandboxes were recovered. +_PREEXISTING_SANDBOX_ORPHANED=false _LEGACY_MANAGED_RECOVERY_NAMES_JSON="[]" # #5735: set when automatic recovery/upgrade of pre-existing sandboxes # reported a failure. A failed/destructive rebuild must not be reported as a @@ -2262,12 +2285,45 @@ recover_preexisting_sandboxes_before_onboard() { # pre-upgrade backup signal is present, the CLI also recovers registered # non-Ready sandboxes from their validated latest backup. It attempts every # eligible sandbox before returning non-zero for any failure. - if NEMOCLAW_CONFIRMED_LEGACY_MANAGED_SANDBOXES="${_LEGACY_MANAGED_RECOVERY_NAMES_JSON:-[]}" \ - "$cli_runner" upgrade-sandboxes --auto 2>&1; then + # + # #6520: mirror the CLI output into a temp log (while still streaming it) so + # the installer can tell "recovered" apart from "exited 0 but recorded + # sandboxes are unrecoverable" — e.g. after `nemoclaw uninstall` removed the + # gateway and Docker image a preserved sandboxes.json still references. The + # CLI emits a dedicated orphan marker only for sandboxes absent from their + # own recorded gateway (never for sandboxes bound to another live gateway or + # ones that reconnect mid-run); keep the grep in sync with the "recorded + # sandbox(es) were not found on their recorded gateway" line in + # src/lib/actions/upgrade-sandboxes.ts. + local recovery_log="" + recovery_log="$(mktemp "${TMPDIR:-/tmp}/nemoclaw-recovery-XXXXXX" 2>/dev/null)" || recovery_log="" + local recovery_status=0 + if [ -n "$recovery_log" ]; then + _cleanup_files+=("$recovery_log") + if NEMOCLAW_CONFIRMED_LEGACY_MANAGED_SANDBOXES="${_LEGACY_MANAGED_RECOVERY_NAMES_JSON:-[]}" \ + "$cli_runner" upgrade-sandboxes --auto 2>&1 | tee "$recovery_log"; then + recovery_status=0 + else + # pipefail: take the CLI's own status, not tee's — a log-write failure + # (e.g. ENOSPC on TMPDIR) must not convert a successful recovery into + # the #5735 failure path. + recovery_status=${PIPESTATUS[0]} + fi + else + NEMOCLAW_CONFIRMED_LEGACY_MANAGED_SANDBOXES="${_LEGACY_MANAGED_RECOVERY_NAMES_JSON:-[]}" \ + "$cli_runner" upgrade-sandboxes --auto 2>&1 || recovery_status=$? + fi + if [ "$recovery_status" -eq 0 ]; then _PREEXISTING_SANDBOX_RECOVERY_RAN=true + if [ -n "$recovery_log" ] \ + && grep -Fq "recorded sandbox(es) were not found on their recorded gateway" "$recovery_log"; then + _PREEXISTING_SANDBOX_ORPHANED=true + fi + rm -f "$recovery_log" 2>/dev/null || true return 0 fi + rm -f "$recovery_log" 2>/dev/null || true _UPGRADE_SANDBOXES_FAILED=true warn "One or more existing sandboxes could not be recovered automatically." warn "Generic onboarding will not run; review the affected sandbox and preserved backup diagnostics above." @@ -2858,7 +2914,12 @@ main() { return 1 fi if [[ "${_PREEXISTING_SANDBOX_RECOVERY_RAN:-false}" == true ]]; then - info "Existing sandboxes recovered; skipping generic onboarding." + if [[ "${_PREEXISTING_SANDBOX_ORPHANED:-false}" == true ]]; then + # #6520: do not claim recovery when recorded sandboxes are stranded. + warn "Some recorded sandboxes could not be recovered; skipping generic onboarding." + else + info "Existing sandboxes recovered; skipping generic onboarding." + fi else run_onboard || error "Onboarding did not complete successfully." ONBOARD_RAN=true diff --git a/src/lib/actions/uninstall/run-plan-preserved-registry.test.ts b/src/lib/actions/uninstall/run-plan-preserved-registry.test.ts new file mode 100644 index 00000000000..e4e45886067 --- /dev/null +++ b/src/lib/actions/uninstall/run-plan-preserved-registry.test.ts @@ -0,0 +1,151 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { describe, expect, it, vi } from "vitest"; + +import { type RunResult, runUninstallPlan, type UninstallRunDeps } from "./run-plan"; + +function ok(stdout = ""): RunResult { + return { status: 0, stdout, stderr: "" }; +} + +function notFound(): RunResult { + return { status: 1, stdout: "", stderr: "" }; +} + +function setupStateDir(): { tmpHome: string; stateDir: string } { + const tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-uninstall-registry-")); + const stateDir = path.join(tmpHome, ".nemoclaw"); + fs.mkdirSync(path.join(stateDir, "rebuild-backups"), { recursive: true }); + fs.mkdirSync(path.join(stateDir, "backups"), { recursive: true }); + fs.writeFileSync(path.join(stateDir, "sandboxes.json"), "[]"); + return { tmpHome, stateDir }; +} + +function preserveCaseDeps( + tmpHome: string, + logs: string[], + warnings: string[], + opts: { envOverrides?: Record } = {}, +): UninstallRunDeps { + return { + commandExists: () => false, + env: { + HOME: tmpHome, + NEMOCLAW_NON_INTERACTIVE: "", + NEMOCLAW_UNINSTALL_DESTROY_USER_DATA: "", + ...(opts.envOverrides ?? {}), + } as NodeJS.ProcessEnv, + error: (line) => warnings.push(line), + existsSync: (target: string) => target.startsWith(tmpHome) && fs.existsSync(target), + isTty: false, + log: (line) => logs.push(line), + run: vi.fn(() => ok()), + runDocker: () => ok(""), + }; +} + +describe("uninstall messaging for a preserved-but-orphaned sandbox registry (#6520)", () => { + it("uses the 'already removed' wording for provider and sandbox delete no-ops", () => { + // Same defect family as the gateway wording fix (#3456 sub-bug 4): when + // `openshell provider delete ` or `openshell sandbox delete --all` + // no-ops (target already gone), `Deleted provider 'X' skipped` reads as if + // the deletion both happened and was skipped. + const warnings: string[] = []; + const logs: string[] = []; + const result = runUninstallPlan( + { assumeYes: true, deleteModels: false, keepOpenShell: true }, + { + commandExists: (command) => command !== "docker" && command !== "pgrep", + env: { HOME: "/home/test", TMPDIR: "/tmp/test" } as NodeJS.ProcessEnv, + error: (line) => warnings.push(line), + existsSync: () => false, + isTty: false, + log: (line) => logs.push(line), + rmSync: vi.fn(), + run: (command, args) => + command === "openshell" ? notFound() : args[0] === "-c" ? ok("/fake/bin/tool\n") : ok(), + runDocker: () => ok(""), + }, + ); + + expect(result.exitCode).toBe(0); + const combined = `${warnings.join("\n")}\n${logs.join("\n")}`; + expect(warnings.join("\n")).toContain("Provider 'nvidia-nim' already removed or unreachable"); + expect(warnings.join("\n")).toContain("OpenShell sandboxes already removed or unreachable"); + expect(combined).not.toContain("Deleted provider 'nvidia-nim' skipped"); + expect(combined).not.toContain("Deleted all OpenShell sandboxes skipped"); + }); + + it("warns that preserved sandboxes.json cannot be auto-recovered after uninstall removes its dependencies", () => { + // Uninstall keeps sandboxes.json but removes the gateway, provider + // registrations, and Docker image its recorded sandboxes depend on. Say + // so at the moment the preserve choice is made, with a remediation path, + // instead of letting a later reinstall report false success. + const { tmpHome } = setupStateDir(); + try { + const logs: string[] = []; + const warnings: string[] = []; + const result = runUninstallPlan( + { assumeYes: true, deleteModels: false, keepOpenShell: true }, + preserveCaseDeps(tmpHome, logs, warnings), + ); + + expect(result.exitCode).toBe(0); + const joined = warnings.join("\n"); + expect(joined).toContain("sandboxes.json"); + expect(joined).toContain("cannot be recovered automatically"); + expect(joined).toContain("--destroy-user-data"); + } finally { + fs.rmSync(tmpHome, { recursive: true, force: true }); + } + }); + + it("warns on the interactive keep path when the purge prompt is declined", () => { + const { tmpHome } = setupStateDir(); + try { + const logs: string[] = []; + const warnings: string[] = []; + // First reply confirms the uninstall itself; the empty second reply + // declines the purge prompt, keeping user data. + const replies = ["yes", ""]; + const result = runUninstallPlan( + { assumeYes: false, deleteModels: false, keepOpenShell: true }, + { + ...preserveCaseDeps(tmpHome, logs, warnings), + isTty: true, + readLine: () => replies.shift() ?? null, + }, + ); + + expect(result.exitCode).toBe(0); + expect(logs).toContain("Keeping user data."); + expect(warnings.join("\n")).toContain("cannot be recovered automatically"); + } finally { + fs.rmSync(tmpHome, { recursive: true, force: true }); + } + }); + + it("does not warn about unrecoverable sandboxes when user data is purged", () => { + const { tmpHome } = setupStateDir(); + try { + const logs: string[] = []; + const warnings: string[] = []; + const result = runUninstallPlan( + { assumeYes: true, deleteModels: false, keepOpenShell: true }, + preserveCaseDeps(tmpHome, logs, warnings, { + envOverrides: { NEMOCLAW_UNINSTALL_DESTROY_USER_DATA: "1" }, + }), + ); + + expect(result.exitCode).toBe(0); + expect(warnings.join("\n")).not.toContain("cannot be recovered automatically"); + } finally { + fs.rmSync(tmpHome, { recursive: true, force: true }); + } + }); +}); diff --git a/src/lib/actions/uninstall/run-plan.ts b/src/lib/actions/uninstall/run-plan.ts index b4b3e115037..1a6b1e1a2e2 100644 --- a/src/lib/actions/uninstall/run-plan.ts +++ b/src/lib/actions/uninstall/run-plan.ts @@ -16,6 +16,12 @@ import { NEMOCLAW_PROVIDERS, type UninstallPaths, } from "../../domain/uninstall/paths"; +import { + gatewayDestroySkipMessage, + OPENSHELL_SANDBOXES_DELETE_SKIP_MESSAGE, + preservedRegistryUnrecoverableWarnings, + providerDeleteSkipMessage, +} from "../../domain/uninstall/messaging"; import { buildUninstallPlan, type UninstallPlan } from "../../domain/uninstall/plan"; import { stopHostGatewayProcesses } from "../../onboard/host-gateway-process"; import { isModelRouterCommandLineForPort } from "../../onboard/model-router-process"; @@ -601,17 +607,25 @@ function removeOpenShellResources(options: UninstallRunOptions, runtime: Uninsta runtime.warn("openshell not found; skipping gateway/provider/sandbox cleanup."); return; } - runOptional(runtime, "Deleted all OpenShell sandboxes", "openshell", [ - "sandbox", - "delete", - "--all", - ]); + // #6520 sub-bug: a no-op delete must not print `Deleted … skipped`; + // wording lives in domain/uninstall/messaging.ts. + runOptional( + runtime, + "Deleted all OpenShell sandboxes", + "openshell", + ["sandbox", "delete", "--all"], + { + onSkip: OPENSHELL_SANDBOXES_DELETE_SKIP_MESSAGE, + }, + ); for (const provider of NEMOCLAW_PROVIDERS) { - runOptional(runtime, `Deleted provider '${provider}'`, "openshell", [ - "provider", - "delete", - provider, - ]); + runOptional( + runtime, + `Deleted provider '${provider}'`, + "openshell", + ["provider", "delete", provider], + { onSkip: providerDeleteSkipMessage(provider) }, + ); } const gatewayLabel = options.gatewayName || "nemoclaw"; runOptional( @@ -619,7 +633,7 @@ function removeOpenShellResources(options: UninstallRunOptions, runtime: Uninsta `Destroyed gateway '${gatewayLabel}'`, "openshell", ["gateway", "destroy", "-g", gatewayLabel], - { onSkip: `Gateway '${gatewayLabel}' already removed or unreachable` }, + { onSkip: gatewayDestroySkipMessage(gatewayLabel) }, ); } @@ -813,6 +827,19 @@ function detectPreservableEntries(paths: UninstallPaths, runtime: UninstallRunti ); } +// #6520: wording lives in domain/uninstall/messaging.ts; empty when +// sandboxes.json is not being preserved. +function warnPreservedRegistryUnrecoverable( + preservable: readonly string[], + runtime: UninstallRuntime, +): void { + for (const line of preservedRegistryUnrecoverableWarnings( + preservable, + runtimeBranding(runtime).cli, + )) + runtime.warn(line); +} + function resolvePreserveSet( paths: UninstallPaths, options: UninstallRunOptions, @@ -837,6 +864,7 @@ function resolvePreserveSet( runtime.log( " Pass --destroy-user-data (or set NEMOCLAW_UNINSTALL_DESTROY_USER_DATA=1) to purge user data on uninstall.", ); + warnPreservedRegistryUnrecoverable(preservable, runtime); return PRESERVED_USER_DATA_ENTRIES; } runtime.log(`The following user data under ${paths.nemoclawStateDir} is preserved by default:`); @@ -848,6 +876,7 @@ function resolvePreserveSet( return []; } runtime.log("Keeping user data."); + warnPreservedRegistryUnrecoverable(preservable, runtime); return PRESERVED_USER_DATA_ENTRIES; } diff --git a/src/lib/actions/upgrade-sandboxes-recovery.test.ts b/src/lib/actions/upgrade-sandboxes-recovery.test.ts index d2cb0cd0782..309fbfd61ec 100644 --- a/src/lib/actions/upgrade-sandboxes-recovery.test.ts +++ b/src/lib/actions/upgrade-sandboxes-recovery.test.ts @@ -342,6 +342,137 @@ describe("upgrade-sandboxes prepared backup recovery (#6114)", () => { expect(exitSpy).not.toHaveBeenCalled(); }); + it("flags an own-gateway orphan with the dedicated marker and remediation when backup recovery is unavailable (#6520)", async () => { + // The direct #6520 repro: `nemoclaw uninstall --yes` preserves + // sandboxes.json but removes the gateway and Docker image; a same-version + // reinstall classifies the recorded sandbox "current", so staleness never + // fires. The orphan marker must fire anyway — it is derived from + // registry-vs-live observation, not version classification. + const harness = createRecoveryHarness(["my-assistant"], { + liveOutput: "other-box Ready", + }); + vi.stubEnv("NEMOCLAW_RESTORE_LATEST_BACKUP_ON_RECREATE", "0"); + + await expect(harness.upgradeSandboxes({ auto: true })).resolves.toBeUndefined(); + + expect(harness.rebuildSpy).not.toHaveBeenCalled(); + expect(console.log).not.toHaveBeenCalledWith(" All sandboxes are up to date."); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining( + "1 recorded sandbox(es) were not found on their recorded gateway: my-assistant", + ), + ); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining("cannot be recovered automatically"), + ); + expect(console.log).toHaveBeenCalledWith(expect.stringContaining("destroy` to clear")); + expect(console.log).toHaveBeenCalledWith(expect.stringContaining("onboard` to rebuild")); + }); + + it("does not double-report an unknown-version orphan under the Unknown version list (#6520)", async () => { + // An orphan with no cached or probeable version would otherwise land in + // the "Unknown version" bucket ("start them and rerun") AND the orphan + // block (destroy/onboard) — conflicting guidance for the same record. + const harness = createRecoveryHarness(["my-assistant"], { + liveOutput: "other-box Ready", + }); + vi.stubEnv("NEMOCLAW_RESTORE_LATEST_BACKUP_ON_RECREATE", "0"); + vi.spyOn(sandboxVersion, "checkAgentVersion").mockReturnValue({ + sandboxVersion: null, + expectedVersion: "2026.5.27", + isStale: false, + verificationFailed: true, + detectionMethod: "registry", + }); + + await expect(harness.upgradeSandboxes({ auto: true })).resolves.toBeUndefined(); + + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining("were not found on their recorded gateway: my-assistant"), + ); + expect(console.log).not.toHaveBeenCalledWith(expect.stringContaining("Unknown version")); + }); + + it("prints the orphan diagnosis in --check mode so check and auto agree (#6520)", async () => { + const harness = createRecoveryHarness(["my-assistant"], { + liveOutput: "other-box Ready", + latestBackup: null, + staleNames: ["my-assistant"], + }); + vi.stubEnv("NEMOCLAW_RESTORE_LATEST_BACKUP_ON_RECREATE", "0"); + + await expect(harness.upgradeSandboxes({ check: true })).resolves.toBeUndefined(); + + expect(harness.rebuildSpy).not.toHaveBeenCalled(); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining("were not found on their recorded gateway: my-assistant"), + ); + }); + + it("also flags a stale own-gateway orphan alongside the generic skip line (#6520)", async () => { + // The versioned-reinstall repro (v0.0.77 sandbox, v0.0.76 tag): the + // sandbox is stale+stopped, prints the generic skip line, and must ALSO + // be flagged as an orphan since its own gateway does not observe it. + const harness = createRecoveryHarness(["my-assistant"], { + liveOutput: "other-box Ready", + latestBackup: null, + staleNames: ["my-assistant"], + }); + vi.stubEnv("NEMOCLAW_RESTORE_LATEST_BACKUP_ON_RECREATE", "0"); + + await expect(harness.upgradeSandboxes({ auto: true })).resolves.toBeUndefined(); + + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining("Skipping 1 sandbox(es) not observed on the selected gateway"), + ); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining("were not found on their recorded gateway: my-assistant"), + ); + }); + + it("does not flag a sandbox bound to another live gateway as orphaned (#6520)", async () => { + const harness = createRecoveryHarness(["registered-elsewhere"], { + gatewayNames: { "registered-elsewhere": "gateway-b" }, + liveOutput: "selected-box Ready", + }); + vi.stubEnv("NEMOCLAW_RESTORE_LATEST_BACKUP_ON_RECREATE", "0"); + + await expect(harness.upgradeSandboxes({ auto: true })).resolves.toBeUndefined(); + + expect(console.log).not.toHaveBeenCalledWith( + expect.stringContaining("were not found on their recorded gateway"), + ); + expect(console.log).toHaveBeenCalledWith(" All sandboxes are up to date."); + }); + + it("does not flag a sandbox that becomes Ready on the confirming listing as orphaned (#6520)", async () => { + const harness = createRecoveryHarness(["reconnecting-box"], { + staleNames: ["reconnecting-box"], + }); + harness.liveListSpy + .mockResolvedValueOnce({ status: 0, output: "other-box Ready" }) + .mockResolvedValueOnce({ status: 0, output: "reconnecting-box Ready" }); + + await expect(harness.upgradeSandboxes({ auto: true })).resolves.toBeUndefined(); + + expect(console.log).not.toHaveBeenCalledWith( + expect.stringContaining("were not found on their recorded gateway"), + ); + }); + + it("does not flag an absent sandbox that prepared-backup recovery restores as orphaned (#6520)", async () => { + const harness = createRecoveryHarness(["orphaned-box"], { + liveOutput: "other-box Ready", + }); + + await expect(harness.upgradeSandboxes({ auto: true })).resolves.toBeUndefined(); + + expect(harness.rebuildSpy).toHaveBeenCalled(); + expect(console.log).not.toHaveBeenCalledWith( + expect.stringContaining("were not found on their recorded gateway"), + ); + }); + it("recovers a registered sandbox absent from the selected gateway when it resolves to the selected gateway", async () => { const harness = createRecoveryHarness(["orphaned-box"], { liveOutput: "other-box Ready", diff --git a/src/lib/actions/upgrade-sandboxes.ts b/src/lib/actions/upgrade-sandboxes.ts index 38b7a080d1c..30de8d955c0 100644 --- a/src/lib/actions/upgrade-sandboxes.ts +++ b/src/lib/actions/upgrade-sandboxes.ts @@ -10,11 +10,16 @@ import { normalizeUpgradeSandboxesOptions, type UpgradeSandboxesOptions, } from "../domain/lifecycle/options"; +import { + classifyOrphanedRegistrySandboxes, + orphanedRegistryRemediation, + orphanedRegistrySummary, +} from "../domain/maintenance/orphan-detection"; import { classifyUpgradeableSandboxes, + describeStaleUpgrade, shouldSkipUpgradeConfirmation, splitRebuildableSandboxes, - type UpgradeSandboxCandidate, } from "../domain/maintenance/upgrade"; import { resolveGatewayName, resolveSandboxGatewayName } from "../onboard/gateway-binding"; import { captureSandboxListWithGatewayPreflightOrExit } from "../openshell-sandbox-list"; @@ -69,24 +74,11 @@ function resolveCurrentNemoclawVersion(): string | null { } } -/** - * Build a human-readable description of why a sandbox needs rebuilding, covering - * an outdated agent version, NemoClaw image/build drift, or both (#5026). - */ -function describeStaleUpgrade(s: UpgradeSandboxCandidate): string { - const reasons = s.reasons ?? []; - const parts: string[] = []; - if (reasons.includes("agent-version")) { - parts.push(`v${s.current || "?"} → v${s.expected}`); - } else if (reasons.includes("image-drift") && s.current) { - // Agent version is current; make clear it is the NemoClaw image that drifted. - parts.push(`v${s.current} unchanged`); - } - if (reasons.includes("image-drift")) { - const from = s.imageCurrent ? `v${s.imageCurrent}` : "unknown build"; - parts.push(`NemoClaw image ${from} → v${s.imageExpected}`); - } - return parts.join("; "); +// Rendering over domain/maintenance/orphan-detection.ts (#6520). +function printOrphanedRegistrySandboxes(orphans: registry.SandboxEntry[]): void { + if (orphans.length === 0) return; + console.log(` ${YW}${orphanedRegistrySummary(orphans.map((sandbox) => sandbox.name))}${R}`); + console.log(` ${D}${orphanedRegistryRemediation(CLI_NAME)}${R}`); } type PreparedBackupRecovery = { @@ -283,6 +275,9 @@ export async function upgradeSandboxes( confirmedLegacyManagedNames.delete(name); } let recoveryCandidates: registry.SandboxEntry[] = []; + // Absent candidates the confirming second listing observed as Ready: + // reconnected mid-run, so neither recovery candidates nor orphans. + const becameReadyNames = new Set(); if (recoverPreparedBackups) { const gatewayEligible = sandboxes.filter((sandbox) => isPreparedRecoveryCandidate(sandbox, liveNames, selectedGatewayName), @@ -297,6 +292,10 @@ export async function upgradeSandboxes( absentCandidates, selectedGatewayName, ); + const confirmedAbsentNames = new Set(confirmedAbsentCandidates.map((s) => s.name)); + for (const sandbox of absentCandidates) { + if (!confirmedAbsentNames.has(sandbox.name)) becameReadyNames.add(sandbox.name); + } recoveryCandidates = [...nonReadyCandidates, ...confirmedAbsentCandidates]; } const backupRecoveryAssessments = recoveryCandidates.map((sandbox) => @@ -314,12 +313,31 @@ export async function upgradeSandboxes( backupRecoveryAssessments.map((candidate) => candidate.sandbox.name), ); + // #6520: see domain/maintenance/orphan-detection.ts; recovered sandboxes + // are excluded at print time. + const unobservedOwnGatewaySandboxes = classifyOrphanedRegistrySandboxes(sandboxes, { + observedNames: new Set([...liveNames, ...nonReadyLiveNames]), + reconnectedNames: becameReadyNames, + selectedGatewayName, + resolveGatewayBinding: resolveSandboxGatewayName, + }); + // An orphan's version is unknown because the sandbox is gone, not because a + // probe is pending — listing it under "Unknown version" with start-and-rerun + // guidance would contradict the orphan block's remediation. Stale orphans + // stay in the stale list: their version drift is real information. + const orphanNames = new Set(unobservedOwnGatewaySandboxes.map((sandbox) => sandbox.name)); + const unknownWithoutOrphans = unknown.filter((sandbox) => !orphanNames.has(sandbox.name)); + if ( stale.length === 0 && - unknown.length === 0 && + unknownWithoutOrphans.length === 0 && preparedRecoveries.length === 0 && rejectedRecoveries.length === 0 ) { + if (unobservedOwnGatewaySandboxes.length > 0) { + printOrphanedRegistrySandboxes(unobservedOwnGatewaySandboxes); + return; + } console.log(" All sandboxes are up to date."); return; } @@ -331,9 +349,9 @@ export async function upgradeSandboxes( console.log(` ${s.name} ${describeStaleUpgrade(s)} (${status})`); } } - if (unknown.length > 0) { + if (unknownWithoutOrphans.length > 0) { console.log(`\n ${YW}Unknown version:${R}`); - for (const s of unknown) { + for (const s of unknownWithoutOrphans) { const status = s.running ? `${G}running${R}` : `${D}stopped${R}`; console.log(` ${s.name} v? → v${s.expected} (${status})`); } @@ -356,9 +374,9 @@ export async function upgradeSandboxes( if (checkOnly) { if (stale.length > 0) console.log(` ${stale.length} sandbox(es) need upgrading.`); - if (unknown.length > 0) { + if (unknownWithoutOrphans.length > 0) { console.log( - ` ${unknown.length} sandbox(es) could not be version-checked; start them and rerun, or rebuild manually.`, + ` ${unknownWithoutOrphans.length} sandbox(es) could not be version-checked; start them and rerun, or rebuild manually.`, ); } if (preparedRecoveries.length > 0) { @@ -371,6 +389,8 @@ export async function upgradeSandboxes( ` ${rejectedRecoveries.length} non-Ready sandbox(es) cannot be recovered automatically.`, ); } + // Check mode must agree with auto mode on the orphan diagnosis (#6520). + printOrphanedRegistrySandboxes(unobservedOwnGatewaySandboxes); console.log(` Run \`${CLI_NAME} upgrade-sandboxes\` to rebuild them.`); return; } @@ -389,12 +409,14 @@ export async function upgradeSandboxes( preparedRecoveries.length === 0 && rejectedRecoveries.length === 0 ) { + printOrphanedRegistrySandboxes(unobservedOwnGatewaySandboxes); console.log(" No running stale sandboxes to rebuild."); return; } let rebuilt = 0; let failed = rejectedRecoveries.length; + const recoveredNames = new Set(); const work = [ ...rebuildable.map((sandbox) => ({ sandbox, manifest: null })), ...preparedRecoveries.map((recovery) => ({ @@ -424,6 +446,7 @@ export async function upgradeSandboxes( : {}), }); rebuilt++; + recoveredNames.add(sandbox.name); } catch (err) { const errorMessage = err instanceof Error ? err.message : String(err); const verb = manifest ? "recover" : "rebuild"; @@ -433,6 +456,9 @@ export async function upgradeSandboxes( } console.log(""); + printOrphanedRegistrySandboxes( + unobservedOwnGatewaySandboxes.filter((sandbox) => !recoveredNames.has(sandbox.name)), + ); if (rebuilt > 0) console.log(` ${G}✓${R} ${rebuilt} sandbox(es) rebuilt.`); if (failed > 0) console.log(` ${YW}⚠${R} ${failed} sandbox(es) failed — see errors above.`); if (failed > 0) process.exit(1); diff --git a/src/lib/domain/maintenance/orphan-detection.test.ts b/src/lib/domain/maintenance/orphan-detection.test.ts new file mode 100644 index 00000000000..f9e86e18c85 --- /dev/null +++ b/src/lib/domain/maintenance/orphan-detection.test.ts @@ -0,0 +1,67 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; + +import { + classifyOrphanedRegistrySandboxes, + ORPHANED_SANDBOX_MARKER, + orphanedRegistryRemediation, + orphanedRegistrySummary, +} from "./orphan-detection"; + +type Entry = { name: string; gatewayName?: string }; + +function classify(sandboxes: Entry[], overrides: { observed?: string[]; reconnected?: string[] }) { + return classifyOrphanedRegistrySandboxes(sandboxes, { + observedNames: new Set(overrides.observed ?? []), + reconnectedNames: new Set(overrides.reconnected ?? []), + selectedGatewayName: "nemoclaw", + resolveGatewayBinding: (sandbox) => sandbox.gatewayName ?? "nemoclaw", + }); +} + +describe("classifyOrphanedRegistrySandboxes (#6520)", () => { + it("flags an own-gateway sandbox unobserved in any phase", () => { + expect(classify([{ name: "my-assistant" }], {})).toEqual([{ name: "my-assistant" }]); + }); + + it("excludes sandboxes the selected gateway observes", () => { + expect(classify([{ name: "my-assistant" }], { observed: ["my-assistant"] })).toEqual([]); + }); + + it("excludes sandboxes the confirming second listing observed", () => { + expect(classify([{ name: "my-assistant" }], { reconnected: ["my-assistant"] })).toEqual([]); + }); + + it("excludes sandboxes bound to a different gateway", () => { + expect(classify([{ name: "elsewhere", gatewayName: "gateway-b" }], {})).toEqual([]); + }); + + it("never classifies a corrupted binding as an orphan", () => { + expect( + classifyOrphanedRegistrySandboxes([{ name: "tampered" }], { + observedNames: new Set(), + reconnectedNames: new Set(), + selectedGatewayName: "nemoclaw", + resolveGatewayBinding: () => { + throw new Error("invalid persisted binding"); + }, + }), + ).toEqual([]); + }); +}); + +describe("orphaned-registry messaging (#6520)", () => { + it("renders the summary from the install.sh grep marker", () => { + const summary = orphanedRegistrySummary(["a", "b"]); + expect(summary).toBe(`2 ${ORPHANED_SANDBOX_MARKER}: a, b.`); + }); + + it("names concrete remediation commands", () => { + const remediation = orphanedRegistryRemediation("nemoclaw"); + expect(remediation).toContain("cannot be recovered automatically"); + expect(remediation).toContain("`nemoclaw destroy` to clear"); + expect(remediation).toContain("`nemoclaw onboard` to rebuild"); + }); +}); diff --git a/src/lib/domain/maintenance/orphan-detection.ts b/src/lib/domain/maintenance/orphan-detection.ts new file mode 100644 index 00000000000..dbe8698eae5 --- /dev/null +++ b/src/lib/domain/maintenance/orphan-detection.ts @@ -0,0 +1,62 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +// #6520: a recorded sandbox the selected gateway does not observe in any +// phase, while its persisted binding resolves to that same gateway, has +// nowhere else to be running — its gateway registration and Docker image were +// likely removed (`nemoclaw uninstall` preserves sandboxes.json but deletes +// both). The signal is independent of version classification: a same-version +// reinstall leaves such an orphan classified "current" and a missing cached +// version leaves it "unknown", so staleness alone cannot carry it. + +/** + * Marker prefix for the orphan summary line. install.sh greps this to keep + * its final install summary honest — keep the grep in scripts/install.sh in + * sync. The bash harness in test/install-orphaned-sandbox-recovery.test.ts + * builds its stub output from this constant and drives the real install.sh + * grep, so drift on either side fails that suite. + */ +export const ORPHANED_SANDBOX_MARKER = + "recorded sandbox(es) were not found on their recorded gateway"; + +export interface OrphanClassificationOptions { + /** Names the selected gateway observes in any phase. */ + observedNames: ReadonlySet; + /** Names a confirming second listing observed as Ready mid-run. */ + reconnectedNames: ReadonlySet; + selectedGatewayName: string; + /** Resolve a sandbox's persisted gateway binding; may throw when invalid. */ + resolveGatewayBinding: (sandbox: Entry) => string; +} + +/** + * Registry sandboxes that are unobserved on the selected gateway while their + * persisted binding resolves to that same gateway. Sandboxes bound to a + * different gateway are excluded (they may be healthy there), as are ones a + * confirming second listing observed. Callers additionally exclude sandboxes + * a prepared-backup recovery restores. + */ +export function classifyOrphanedRegistrySandboxes( + sandboxes: readonly Entry[], + options: OrphanClassificationOptions, +): Entry[] { + return sandboxes.filter((sandbox) => { + if (options.observedNames.has(sandbox.name)) return false; + if (options.reconnectedNames.has(sandbox.name)) return false; + try { + return options.resolveGatewayBinding(sandbox) === options.selectedGatewayName; + } catch { + // Invalid persisted binding — surfaced by the recovery-candidate guard; + // never classify a corrupted row as an orphan here. + return false; + } + }); +} + +export function orphanedRegistrySummary(names: readonly string[]): string { + return `${names.length} ${ORPHANED_SANDBOX_MARKER}: ${names.join(", ")}.`; +} + +export function orphanedRegistryRemediation(cliName: string): string { + return `Their gateway registration or Docker image may have been removed (for example by \`${cliName} uninstall\`), so they cannot be recovered automatically — run \`${cliName} destroy\` to clear a stranded record, then \`${cliName} onboard\` to rebuild it.`; +} diff --git a/src/lib/domain/maintenance/upgrade.test.ts b/src/lib/domain/maintenance/upgrade.test.ts index ed68d8722ec..d37c690c017 100644 --- a/src/lib/domain/maintenance/upgrade.test.ts +++ b/src/lib/domain/maintenance/upgrade.test.ts @@ -5,6 +5,7 @@ import { describe, expect, it } from "vitest"; import { classifyUpgradeableSandboxes, + describeStaleUpgrade, isNemoclawImageStale, type SandboxVersionCheck, shouldSkipUpgradeConfirmation, @@ -195,3 +196,59 @@ describe("upgrade sandboxes helpers", () => { }); }); }); + +describe("describeStaleUpgrade version-change labeling (#6520)", () => { + it("labels a NemoClaw image downgrade explicitly instead of framing it as a routine upgrade", () => { + expect( + describeStaleUpgrade({ + name: "my-assistant", + running: false, + current: "2026.6.10", + expected: "2026.6.10", + reasons: ["image-drift"], + imageCurrent: "0.0.77", + imageExpected: "0.0.76", + }), + ).toBe("v2026.6.10 unchanged; NemoClaw image v0.0.77 → v0.0.76 (downgrade)"); + }); + + it("labels an agent-version downgrade explicitly", () => { + expect( + describeStaleUpgrade({ + name: "my-assistant", + running: false, + current: "2026.6.10", + expected: "2026.5.1", + reasons: ["agent-version"], + }), + ).toBe("v2026.6.10 → v2026.5.1 (downgrade)"); + }); + + it("keeps upgrade descriptions unchanged", () => { + expect( + describeStaleUpgrade({ + name: "my-assistant", + running: false, + current: "2026.6.10", + expected: "2026.6.10", + reasons: ["image-drift"], + imageCurrent: "0.0.76", + imageExpected: "0.0.77", + }), + ).toBe("v2026.6.10 unchanged; NemoClaw image v0.0.76 → v0.0.77"); + }); + + it("does not label a downgrade when the recorded image build is unknown", () => { + expect( + describeStaleUpgrade({ + name: "my-assistant", + running: false, + current: "2026.6.10", + expected: "2026.6.10", + reasons: ["image-drift"], + imageCurrent: null, + imageExpected: "0.0.76", + }), + ).toBe("v2026.6.10 unchanged; NemoClaw image unknown build → v0.0.76"); + }); +}); diff --git a/src/lib/domain/maintenance/upgrade.ts b/src/lib/domain/maintenance/upgrade.ts index 007ed28f106..974ab1890d2 100644 --- a/src/lib/domain/maintenance/upgrade.ts +++ b/src/lib/domain/maintenance/upgrade.ts @@ -139,3 +139,51 @@ export function splitRebuildableSandboxes(stale: UpgradeSandboxCandidate[]): { } return { rebuildable, stopped }; } + +/** + * Compare two dotted version strings numerically, segment by segment. + * Canonical home of the comparison used by gateway compatibility checks and + * upgrade display; `onboard/docker-driver-gateway-compat` re-exports it for + * its existing consumers. + */ +export function compareDottedVersions(a: string, b: string): number { + const left = a.split(".").map((part) => Number.parseInt(part, 10) || 0); + const right = b.split(".").map((part) => Number.parseInt(part, 10) || 0); + const len = Math.max(left.length, right.length); + for (let i = 0; i < len; i += 1) { + const delta = (left[i] ?? 0) - (right[i] ?? 0); + if (delta !== 0) return delta; + } + return 0; +} + +// #6520: a requested version below the recorded one is a downgrade (e.g. +// reinstalling with an older NEMOCLAW_INSTALL_TAG than the build that created +// the sandbox); call it out instead of framing it as a routine upgrade step. +function downgradeSuffix(current?: string | null, expected?: string | null): string { + if (!current || !expected) return ""; + return compareDottedVersions(expected, current) < 0 ? " (downgrade)" : ""; +} + +/** + * Build a human-readable description of why a sandbox needs rebuilding, + * covering an outdated agent version, NemoClaw image/build drift, or both + * (#5026), and labeling version regressions explicitly (#6520). + */ +export function describeStaleUpgrade(s: UpgradeSandboxCandidate): string { + const reasons = s.reasons ?? []; + const parts: string[] = []; + if (reasons.includes("agent-version")) { + parts.push(`v${s.current || "?"} → v${s.expected}${downgradeSuffix(s.current, s.expected)}`); + } else if (reasons.includes("image-drift") && s.current) { + // Agent version is current; make clear it is the NemoClaw image that drifted. + parts.push(`v${s.current} unchanged`); + } + if (reasons.includes("image-drift")) { + const from = s.imageCurrent ? `v${s.imageCurrent}` : "unknown build"; + parts.push( + `NemoClaw image ${from} → v${s.imageExpected}${downgradeSuffix(s.imageCurrent, s.imageExpected)}`, + ); + } + return parts.join("; "); +} diff --git a/src/lib/domain/uninstall/messaging.test.ts b/src/lib/domain/uninstall/messaging.test.ts new file mode 100644 index 00000000000..b17b689f041 --- /dev/null +++ b/src/lib/domain/uninstall/messaging.test.ts @@ -0,0 +1,44 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; + +import { + gatewayDestroySkipMessage, + OPENSHELL_SANDBOXES_DELETE_SKIP_MESSAGE, + preservedRegistryUnrecoverableWarnings, + providerDeleteSkipMessage, +} from "./messaging"; + +describe("uninstall no-op delete wording (#6520, #3456)", () => { + it("describes the actual state instead of 'Deleted … skipped'", () => { + expect(OPENSHELL_SANDBOXES_DELETE_SKIP_MESSAGE).toBe( + "OpenShell sandboxes already removed or unreachable", + ); + expect(providerDeleteSkipMessage("nvidia-nim")).toBe( + "Provider 'nvidia-nim' already removed or unreachable", + ); + expect(gatewayDestroySkipMessage("nemoclaw")).toBe( + "Gateway 'nemoclaw' already removed or unreachable", + ); + }); +}); + +describe("preservedRegistryUnrecoverableWarnings (#6520)", () => { + it("warns with remediation when sandboxes.json is preserved", () => { + const lines = preservedRegistryUnrecoverableWarnings( + ["rebuild-backups", "backups", "sandboxes.json"], + "nemoclaw", + ); + const joined = lines.join("\n"); + expect(joined).toContain("sandboxes.json"); + expect(joined).toContain("cannot be recovered automatically"); + expect(joined).toContain("'nemoclaw destroy'"); + expect(joined).toContain("'nemoclaw onboard'"); + expect(joined).toContain("--destroy-user-data"); + }); + + it("stays silent when sandboxes.json is not being preserved", () => { + expect(preservedRegistryUnrecoverableWarnings(["rebuild-backups"], "nemoclaw")).toEqual([]); + }); +}); diff --git a/src/lib/domain/uninstall/messaging.ts b/src/lib/domain/uninstall/messaging.ts new file mode 100644 index 00000000000..338ec937ce6 --- /dev/null +++ b/src/lib/domain/uninstall/messaging.ts @@ -0,0 +1,37 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +// User-facing uninstall messaging whose wording is load-bearing (#6520, +// #3456 sub-bug 4): a no-op delete must describe the actual state instead of +// the self-contradictory `Deleted … skipped`, and preserving sandboxes.json +// must say up front that the kept registry is not recoverable on its own. + +export const OPENSHELL_SANDBOXES_DELETE_SKIP_MESSAGE = + "OpenShell sandboxes already removed or unreachable"; + +export function providerDeleteSkipMessage(provider: string): string { + return `Provider '${provider}' already removed or unreachable`; +} + +export function gatewayDestroySkipMessage(gatewayLabel: string): string { + return `Gateway '${gatewayLabel}' already removed or unreachable`; +} + +/** + * Warnings shown when uninstall preserves sandboxes.json: uninstall removes + * the gateway registration, provider registrations, and Docker image the + * recorded sandboxes depend on, so a later reinstall cannot bring them back + * on its own. Say so at the moment the preserve choice is made instead of + * letting the reinstall report false success while silently orphaning the + * user's sandbox (#6520). Empty when sandboxes.json is not being preserved. + */ +export function preservedRegistryUnrecoverableWarnings( + preservable: readonly string[], + cliName: string, +): string[] { + if (!preservable.includes("sandboxes.json")) return []; + return [ + "Preserved sandboxes.json references the gateway, provider registrations, and Docker image this uninstall removes — its recorded sandboxes cannot be recovered automatically on reinstall.", + `After reinstalling, run '${cliName} destroy' to clear a stranded sandbox and '${cliName} onboard' to rebuild it, or rerun uninstall with --destroy-user-data to purge the registry.`, + ]; +} diff --git a/src/lib/onboard/docker-driver-gateway-compat.ts b/src/lib/onboard/docker-driver-gateway-compat.ts index f3d5f1355d0..72547785298 100644 --- a/src/lib/onboard/docker-driver-gateway-compat.ts +++ b/src/lib/onboard/docker-driver-gateway-compat.ts @@ -6,8 +6,13 @@ import fs from "node:fs"; import path from "node:path"; import { dockerForceRm } from "../adapters/docker"; +import { compareDottedVersions } from "../domain/maintenance/upgrade"; import type { DockerDriverGatewayLaunch } from "./docker-driver-gateway-launch"; +// Canonical implementation moved to domain/maintenance/upgrade.ts (#6520); +// re-exported here for existing consumers. +export { compareDottedVersions }; + const DEFAULT_COMPAT_IMAGE = "ubuntu:24.04@sha256:786a8b558f7be160c6c8c4a54f9a57274f3b4fb1491cf65146521ae77ff1dc54"; const DEFAULT_COMPAT_CONTAINER_NAME = "nemoclaw-openshell-gateway"; @@ -26,17 +31,6 @@ type ContainerizedGatewayLaunchOptions = { reason?: string; }; -export function compareDottedVersions(a: string, b: string): number { - const left = a.split(".").map((part) => Number.parseInt(part, 10) || 0); - const right = b.split(".").map((part) => Number.parseInt(part, 10) || 0); - const len = Math.max(left.length, right.length); - for (let i = 0; i < len; i += 1) { - const delta = (left[i] ?? 0) - (right[i] ?? 0); - if (delta !== 0) return delta; - } - return 0; -} - export function maxDottedVersion(versions: string[]): string | null { return versions.reduce( (max, version) => (!max || compareDottedVersions(version, max) > 0 ? version : max), diff --git a/test/install-orphaned-sandbox-recovery.test.ts b/test/install-orphaned-sandbox-recovery.test.ts new file mode 100644 index 00000000000..6721187f659 --- /dev/null +++ b/test/install-orphaned-sandbox-recovery.test.ts @@ -0,0 +1,178 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { spawnSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { describe, expect, it } from "vitest"; + +import { ORPHANED_SANDBOX_MARKER } from "../src/lib/domain/maintenance/orphan-detection"; + +const INSTALLER_PAYLOAD = path.join(import.meta.dirname, "..", "scripts", "install.sh"); + +// The installer greps the dedicated orphan marker (emitted only for sandboxes +// absent from their OWN recorded gateway) to keep its final summary honest +// when the CLI exits 0 without recovering them (#6520). The stub line is +// built from the CLI's exported marker constant and drives the real +// install.sh grep below, so a rewording on either side fails this suite. The +// generic multi-gateway skip line must NOT trip the flag: a sandbox healthy +// on another live gateway is not an orphan. +const ORPHAN_LINE = ` 1 ${ORPHANED_SANDBOX_MARKER}: my-assistant.`; +const LEGACY_SKIP_LINE = + " Skipping 1 sandbox(es) not observed on the selected gateway — verify their recorded gateway or start them first."; +const NO_REBUILD_LINE = " No running stale sandboxes to rebuild."; +const REBUILT_LINE = " ✓ 1 sandbox(es) rebuilt."; + +function installerTestEnv(home: string): Record { + return { + HOME: home, + PATH: process.env.PATH ?? "/usr/bin:/bin", + }; +} + +function runRecoveryClassification( + outputLines: string[], + exitCode: number, +): { output: string; cleanup: () => void } { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-install-orphan-")); + const outFile = path.join(tmp, "cli-output.txt"); + fs.writeFileSync(outFile, outputLines.length > 0 ? `${outputLines.join("\n")}\n` : ""); + const stubBin = path.join(tmp, "stub-cli"); + fs.writeFileSync( + stubBin, + `#!/usr/bin/env bash\ncat ${JSON.stringify(outFile)}\nexit ${exitCode}\n`, + { mode: 0o755 }, + ); + + const snippet = ` + set -e + source "${INSTALLER_PAYLOAD}" >/dev/null 2>&1 || true + info() { :; } + warn() { :; } + _PREEXISTING_SANDBOX_COUNT=1 + recover_preexisting_sandboxes_before_onboard "${stubBin}" >/dev/null 2>&1 || true + echo "recovery_ran=\${_PREEXISTING_SANDBOX_RECOVERY_RAN:-unset}" + echo "orphaned=\${_PREEXISTING_SANDBOX_ORPHANED:-unset}" + echo "failed=\${_UPGRADE_SANDBOXES_FAILED:-unset}" + `; + + const result = spawnSync("bash", ["-c", snippet], { + encoding: "utf-8", + env: installerTestEnv(tmp), + }); + return { + output: `${result.stdout}\n${result.stderr}`, + cleanup: () => fs.rmSync(tmp, { recursive: true, force: true }), + }; +} + +function runPrintDone(flags: { recoveryRan: string; orphaned: string }): { + output: string; + cleanup: () => void; +} { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-install-printdone-")); + const snippet = ` + set -e + source "${INSTALLER_PAYLOAD}" >/dev/null 2>&1 || true + needs_shell_reload() { return 1; } + _INSTALL_START=0 + _CLI_DISPLAY="NemoClaw" + _CLI_BIN="nemoclaw" + ONBOARD_RAN=false + _PREEXISTING_SANDBOX_RECOVERY_RAN=${flags.recoveryRan} + _PREEXISTING_SANDBOX_ORPHANED=${flags.orphaned} + _UPGRADE_SANDBOXES_FAILED=false + print_done 2>&1 + `; + + const result = spawnSync("bash", ["-c", snippet], { + encoding: "utf-8", + env: installerTestEnv(tmp), + }); + return { + output: `${result.stdout}\n${result.stderr}`, + cleanup: () => fs.rmSync(tmp, { recursive: true, force: true }), + }; +} + +describe("install.sh recovery outcome classification (#6520)", () => { + it("marks the run orphaned when the CLI reports sandboxes not found on their recorded gateway", () => { + const { output, cleanup } = runRecoveryClassification([ORPHAN_LINE, NO_REBUILD_LINE], 0); + try { + expect(output).toContain("recovery_ran=true"); + expect(output).toContain("orphaned=true"); + expect(output).toContain("failed=false"); + } finally { + cleanup(); + } + }); + + it("does not mark the run orphaned for the generic multi-gateway skip line", () => { + // A sandbox bound to another live gateway prints the legacy skip line and + // is legitimately left alone — the install summary must stay clean. + const { output, cleanup } = runRecoveryClassification([LEGACY_SKIP_LINE, NO_REBUILD_LINE], 0); + try { + expect(output).toContain("recovery_ran=true"); + expect(output).toContain("orphaned=false"); + } finally { + cleanup(); + } + }); + + it("does not mark the run orphaned when sandboxes were actually rebuilt", () => { + const { output, cleanup } = runRecoveryClassification([REBUILT_LINE], 0); + try { + expect(output).toContain("recovery_ran=true"); + expect(output).toContain("orphaned=false"); + } finally { + cleanup(); + } + }); + + it("marks the run orphaned when some sandboxes rebuilt and others were orphaned", () => { + const { output, cleanup } = runRecoveryClassification([ORPHAN_LINE, REBUILT_LINE], 0); + try { + expect(output).toContain("recovery_ran=true"); + expect(output).toContain("orphaned=true"); + } finally { + cleanup(); + } + }); + + it("still marks a non-zero recovery exit as failed, not orphaned", () => { + const { output, cleanup } = runRecoveryClassification([], 1); + try { + expect(output).toContain("recovery_ran=false"); + expect(output).toContain("failed=true"); + } finally { + cleanup(); + } + }); +}); + +describe("install.sh print_done honesty for orphaned sandboxes (#6520)", () => { + it("does not claim sandboxes were recovered when recovery skipped them", () => { + const { output, cleanup } = runPrintDone({ recoveryRan: "true", orphaned: "true" }); + try { + expect(output).toContain("completed with warnings"); + expect(output).not.toContain("Existing sandboxes were recovered and upgraded."); + expect(output).not.toContain("No new sandbox onboarding was needed."); + expect(output).toContain("nemoclaw destroy"); + expect(output).toContain("nemoclaw onboard"); + } finally { + cleanup(); + } + }); + + it("keeps the recovered-and-upgraded summary when nothing was skipped", () => { + const { output, cleanup } = runPrintDone({ recoveryRan: "true", orphaned: "false" }); + try { + expect(output).toContain("=== Installation complete ==="); + expect(output).toContain("Existing sandboxes were recovered and upgraded."); + } finally { + cleanup(); + } + }); +});