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
37 changes: 37 additions & 0 deletions src/lib/actions/uninstall/run-plan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -582,4 +582,41 @@ describe("uninstall run plan", () => {
expect(warnings).toContain("Failed to disable /swapfile; skipping swap cleanup.");
expect(logs).not.toContain("Swap file removed");
});

it("#3456 sub-bug #4: gateway destroy no-op uses the 'already removed' wording, not 'Destroyed ... skipped'", () => {
// When `openshell gateway destroy -g nemoclaw` returns non-zero (gateway
// already gone), the previous code printed `Destroyed gateway 'nemoclaw'
// skipped` — self-contradictory. The fix routes this branch to an onSkip
// message that describes the actual state.
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) => {
// The openshell gateway destroy command no-ops when the gateway is
// already gone — return non-zero to exercise the onSkip branch.
if (command === "openshell" && args[0] === "gateway" && args[1] === "destroy") {
return notFound();
}
if (args[0] === "-c") return ok("/fake/bin/tool\n");
return ok();
},
runDocker: () => ok(""),
},
);

expect(result.exitCode).toBe(0);
expect(warnings.join("\n")).toContain("Gateway 'nemoclaw' already removed or unreachable");
expect(`${warnings.join("\n")}\n${logs.join("\n")}`).not.toContain(
"Destroyed gateway 'nemoclaw' skipped",
);
});
});
34 changes: 25 additions & 9 deletions src/lib/actions/uninstall/run-plan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,24 @@ function confirm(options: UninstallRunOptions, runtime: UninstallRuntime): boole
return false;
}

function runOptional(runtime: UninstallRuntime, description: string, command: string, args: string[]): void {
function runOptional(
runtime: UninstallRuntime,
description: string,
command: string,
args: string[],
opts: { onSkip?: string } = {},
): void {
const result = runtime.run(command, args, { env: runtime.env, stdio: "ignore" });
if (result.status === 0) runtime.log(description);
else runtime.warn(`${description} skipped`);
if (result.status === 0) {
runtime.log(description);
return;
}
// #3456 sub-bug #4: when the destroy/delete call no-ops (target already
// gone), printing `<description> skipped` was self-contradictory — e.g.
// "Destroyed gateway 'nemoclaw' skipped" suggested the gateway was both
// destroyed AND skipped. Callers that care can pass a `onSkip` message
// describing the actual state (target absent or unreachable).
runtime.warn(opts.onSkip ?? `${description} skipped`);
}

function stopHelperServices(paths: UninstallPaths, runtime: UninstallRuntime): void {
Expand Down Expand Up @@ -390,12 +404,14 @@ function removeOpenShellResources(options: UninstallRunOptions, runtime: Uninsta
for (const provider of NEMOCLAW_PROVIDERS) {
runOptional(runtime, `Deleted provider '${provider}'`, "openshell", ["provider", "delete", provider]);
}
runOptional(runtime, `Destroyed gateway '${options.gatewayName || "nemoclaw"}'`, "openshell", [
"gateway",
"destroy",
"-g",
options.gatewayName || "nemoclaw",
]);
const gatewayLabel = options.gatewayName || "nemoclaw";
runOptional(
runtime,
`Destroyed gateway '${gatewayLabel}'`,
"openshell",
["gateway", "destroy", "-g", gatewayLabel],
{ onSkip: `Gateway '${gatewayLabel}' already removed or unreachable` },
);
}

function removeAliases(paths: UninstallPaths, runtime: UninstallRuntime): void {
Expand Down
5 changes: 2 additions & 3 deletions src/lib/onboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ import type {
} from "./onboard/types";
import { listChannels } from "./sandbox/channels";
import { streamGatewayStart } from "./onboard/gateway";
import { reportGpuPassthroughRecovery } from "./onboard/gpu-recovery";
import type { StreamSandboxCreateResult } from "./sandbox/create-stream";
import type { SandboxEntry } from "./state/registry";
import type { BackupResult } from "./state/sandbox";
Expand Down Expand Up @@ -10343,9 +10344,7 @@ async function onboard(opts: OnboardOptions = {}): Promise<void> {
const gpuOutput = String(gpuCheck.stdout || "").trim();
const gatewayHasGpu = gpuCheck.status === 0 && gpuOutput !== "null" && gpuOutput !== "[]";
if (!gatewayHasGpu) {
console.error(" Existing gateway was started without GPU passthrough.");
console.error(" To enable GPU, destroy the existing sandbox and gateway, then re-onboard:");
console.error(` nemoclaw <name> destroy --yes && nemoclaw onboard --gpu`);
reportGpuPassthroughRecovery(console.error);
process.exit(1);
}
}
Expand Down
92 changes: 92 additions & 0 deletions src/lib/onboard/gpu-recovery.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

/**
* Tests for the GPU-passthrough mismatch recovery hint (#3456 sub-bug #3).
*
* The hint replaces a hard-coded line that printed a literal `<name>`
* placeholder and assumed at least one sandbox was registered — which broke
* the install-loop recovery flow when the registry was empty (the State A /
* State B dead loop the reporter hit on six Linux hosts).
*/

import { describe, expect, it, vi } from "vitest";
import { gpuPassthroughRecoveryLines, reportGpuPassthroughRecovery } from "./gpu-recovery";

describe("gpuPassthroughRecoveryLines", () => {
it("never emits a literal `<name>` placeholder for any input", () => {
for (const names of [null, [], ["alpha"], ["alpha", "beta"], ["alpha", "beta", "gamma"]]) {
const lines = gpuPassthroughRecoveryLines(names);
expect(lines.join("\n")).not.toMatch(/<name>/);
}
});

it("suggests `nemoclaw uninstall` when no sandboxes are registered (null input)", () => {
const lines = gpuPassthroughRecoveryLines(null);
const joined = lines.join("\n");
expect(joined).toContain("Existing gateway was started without GPU passthrough");
expect(joined).toContain("nemoclaw uninstall");
expect(joined).toContain("nemoclaw onboard --gpu");
// Must NOT suggest the destroy form — there is nothing to destroy.
expect(joined).not.toMatch(/nemoclaw [a-z-]+ destroy/);
});

it("suggests `nemoclaw uninstall` when no sandboxes are registered (empty array)", () => {
const lines = gpuPassthroughRecoveryLines([]);
expect(lines.join("\n")).toContain("nemoclaw uninstall");
expect(lines.join("\n")).not.toMatch(/nemoclaw [a-z-]+ destroy/);
});

it("suggests destroy for a single registered sandbox with --cleanup-gateway", () => {
const lines = gpuPassthroughRecoveryLines(["my-assistant"]);
const joined = lines.join("\n");
expect(joined).toContain("nemoclaw my-assistant destroy --yes --cleanup-gateway");
expect(joined).toContain("nemoclaw onboard --gpu");
// The single-sandbox form must not suggest uninstall — destroy is enough.
expect(joined).not.toContain("nemoclaw uninstall");
});

it("lists every registered sandbox and only appends --cleanup-gateway to the last", () => {
const lines = gpuPassthroughRecoveryLines(["alpha", "beta", "gamma"]);
const joined = lines.join("\n");
expect(joined).toContain("nemoclaw alpha destroy --yes");
expect(joined).toContain("nemoclaw beta destroy --yes");
expect(joined).toContain("nemoclaw gamma destroy --yes --cleanup-gateway");
// Only one --cleanup-gateway across all rows.
expect(joined.match(/--cleanup-gateway/g) ?? []).toHaveLength(1);
// alpha/beta lines must NOT have --cleanup-gateway.
const alphaLine = lines.find((line) => line.includes("nemoclaw alpha destroy"));
const betaLine = lines.find((line) => line.includes("nemoclaw beta destroy"));
expect(alphaLine).not.toContain("--cleanup-gateway");
expect(betaLine).not.toContain("--cleanup-gateway");
});

it("filters out empty/whitespace names defensively", () => {
// Belt-and-suspenders: if registry.listSandboxes() ever returns a row with
// an empty name, we shouldn't render `nemoclaw destroy --yes` (the very
// bug shape this fix exists to prevent).
const lines = gpuPassthroughRecoveryLines(["", " ", "real"]);
const joined = lines.join("\n");
expect(joined).toContain("nemoclaw real destroy --yes --cleanup-gateway");
// No double-spaced "nemoclaw destroy" rendering.
expect(joined).not.toMatch(/nemoclaw\s{2,}destroy/);
});
});

describe("reportGpuPassthroughRecovery", () => {
it("emits the empty-registry path when loadNames returns no names", () => {
const emit = vi.fn();
reportGpuPassthroughRecovery(emit, () => []);
const joined = emit.mock.calls.map((c) => c[0]).join("\n");
expect(joined).toContain("nemoclaw uninstall");
expect(joined).not.toMatch(/<name>/);
});

it("emits the multi-sandbox path when loadNames returns several names", () => {
const emit = vi.fn();
reportGpuPassthroughRecovery(emit, () => ["alpha", "beta"]);
const joined = emit.mock.calls.map((c) => c[0]).join("\n");
expect(joined).toContain("nemoclaw alpha destroy --yes");
expect(joined).toContain("nemoclaw beta destroy --yes --cleanup-gateway");
});
});
91 changes: 91 additions & 0 deletions src/lib/onboard/gpu-recovery.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

/**
* Recovery hint emitted when an onboard run finds the reusable gateway was
* started without GPU passthrough but the current run requested it.
*
* Before #3456 this was a hard-coded `nemoclaw <name> destroy --yes` line
* with a literal `<name>` placeholder — not actionable when the registry was
* empty (the State A / State B dead loop the reporter hit on six Linux
* hosts). This helper renders the right shape based on what's actually
* registered AND owns the registry lookup, so the onboard.ts callsite stays
* a single call (also keeps onboard.ts inside its size budget).
*/

import * as registry from "../state/registry";

/**
* Returns the multi-line recovery hint for the GPU-passthrough mismatch
* branch in onboard. Caller is expected to emit each line on its own line
* via `console.error` / `runtime.log`.
*
* Empty / null input means no sandboxes are registered locally; we suggest
* `nemoclaw uninstall` because there is nothing for `nemoclaw <name>
* destroy` to act on. A single registered sandbox gets one destroy line
* with `--cleanup-gateway` so the gateway also goes away (otherwise destroy
* preserves the shared gateway by default — see v0.0.39 release notes).
* Multiple sandboxes get one destroy line each; only the last carries
* `--cleanup-gateway` so the gateway lives until every sandbox is gone.
*/
export function gpuPassthroughRecoveryLines(names: readonly string[] | null): string[] {
const cleanNames = (names ?? []).map((n) => n.trim()).filter((n) => n.length > 0);

if (cleanNames.length === 0) {
return [
" Existing gateway was started without GPU passthrough.",
" No sandboxes are registered, so there is nothing for `nemoclaw destroy` to act on.",
" Clear the stale gateway state and re-onboard with GPU enabled:",
" nemoclaw uninstall && nemoclaw onboard --gpu",
];
}

if (cleanNames.length === 1) {
return [
" Existing gateway was started without GPU passthrough.",
" To enable GPU, destroy the existing sandbox and gateway, then re-onboard:",
` nemoclaw ${cleanNames[0]} destroy --yes --cleanup-gateway && nemoclaw onboard --gpu`,
];
}

const lastIdx = cleanNames.length - 1;
const destroyLines = cleanNames.map((name, idx) =>
idx === lastIdx
? ` nemoclaw ${name} destroy --yes --cleanup-gateway && nemoclaw onboard --gpu`
: ` nemoclaw ${name} destroy --yes`,
);

return [
" Existing gateway was started without GPU passthrough.",
" To enable GPU, destroy each registered sandbox and the gateway, then re-onboard:",
...destroyLines,
];
}

/**
* Read registered sandbox names with a graceful empty-list fallback when the
* registry can't be opened. Extracted so the onboard callsite stays a single
* line and so unit tests can inject their own list.
*/
export function getRegisteredSandboxNamesForGpuRecovery(): string[] {
try {
return registry
.listSandboxes()
.sandboxes.map((s) => s.name)
.filter(Boolean);
} catch {
return [];
}
}

/**
* Emit the GPU-passthrough mismatch recovery hint to `emit` (typically
* `console.error`). `loadNames` is injectable for tests; the production
* default reads the on-disk sandbox registry.
*/
export function reportGpuPassthroughRecovery(
emit: (line: string) => void,
loadNames: () => string[] = getRegisteredSandboxNamesForGpuRecovery,
): void {
for (const line of gpuPassthroughRecoveryLines(loadNames())) emit(line);
}
Loading