Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 1 addition & 1 deletion src/lib/onboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2692,7 +2692,7 @@ async function createSandboxWithBaseImageResolution(
console.warn(` Warning: failed to remove old sandbox image '${previousEntry.imageTag}'.`);
}
}
registry.removeSandbox(sandboxName);
sandboxLifecycle.removeSandboxUnlessSessionReservation(previousEntry, sandboxName);
}

// Stage build context — use the custom Dockerfile path when provided,
Expand Down
2 changes: 2 additions & 0 deletions src/lib/onboard/machine/handlers/provider-inference.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,7 @@ describe("handleProviderInferenceState", () => {
credentialEnv: "COMPATIBLE_API_KEY",
preferredInferenceApi: "openai-completions",
gatewayName: "nemoclaw",
reservationSessionId: expect.any(String),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
});
});

Expand Down Expand Up @@ -1055,6 +1056,7 @@ describe("handleProviderInferenceState", () => {
credentialEnv: "NVIDIA_INFERENCE_API_KEY",
preferredInferenceApi: null,
gatewayName: "nemoclaw",
reservationSessionId: expect.any(String),
});
});

Expand Down
3 changes: 3 additions & 0 deletions src/lib/onboard/machine/handlers/provider-inference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ export interface ProviderInferenceStateOptions<Gpu, Agent, Host> {
credentialEnv: string | null;
preferredInferenceApi: string | null;
gatewayName: string;
reservationSessionId?: string;
},
): boolean;
registryUpdateSandbox(sandboxName: string, updates: { nimContainer?: string | null }): void;
Expand Down Expand Up @@ -636,6 +637,7 @@ export async function handleProviderInferenceState<Gpu, Agent, Host>({
credentialEnv,
preferredInferenceApi,
gatewayName,
reservationSessionId: session?.sessionId,
})
: null;
return { reupserted, reserved };
Expand Down Expand Up @@ -670,6 +672,7 @@ export async function handleProviderInferenceState<Gpu, Agent, Host>({
credentialEnv,
preferredInferenceApi,
gatewayName,
reservationSessionId: session?.sessionId,
});
});
if (!reserved) {
Expand Down
10 changes: 10 additions & 0 deletions src/lib/onboard/sandbox-lifecycle.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,20 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

import * as onboardSession from "../state/onboard-session";
import type { SandboxEntry, SandboxMcpState } from "../state/registry";
import * as registry from "../state/registry";
import type { SelectionDrift } from "./selection-drift";

export function removeSandboxUnlessSessionReservation(
entry: SandboxEntry | null,
sandboxName: string,
): void {
if (!registry.isPendingReservationForSession(entry, onboardSession.loadSession()?.sessionId)) {
registry.removeSandbox(sandboxName);
}
}

export interface SandboxLifecycleDeps {
runCaptureOpenshell(args: string[], opts?: Record<string, unknown>): string | null;
fetchGatewayAuthTokenFromSandbox(sandboxName: string): string | null;
Expand Down
81 changes: 81 additions & 0 deletions src/lib/state/registry-route-reservation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,85 @@ describe("sandbox inference route reservation", () => {
await fs.rm(home, { recursive: true, force: true });
}
});

it("stamps the owning onboard session on the reservation (#6562)", async () => {
const home = await fs.mkdtemp(path.join(os.tmpdir(), "nemoclaw-route-reservation-"));
vi.stubEnv("HOME", home);
vi.resetModules();
try {
const registry = await import("./registry");
registry.reserveSandboxInferenceRoute("alpha", {
provider: "compatible-endpoint",
model: "model-a",
endpointUrl: "https://api.example.test/v1",
credentialEnv: "CUSTOM_API_KEY",
preferredInferenceApi: "openai-responses",
gatewayName: "nemoclaw-9090",
reservationSessionId: "session-owner",
});

expect(registry.getSandbox("alpha")).toMatchObject({
pendingRouteReservation: true,
reservationSessionId: "session-owner",
});
} finally {
await fs.rm(home, { recursive: true, force: true });
}
});
});

describe("pending reservation ownership (#6562)", () => {
afterEach(() => {
vi.unstubAllEnvs();
vi.resetModules();
});

it("keeps the reserving session's row but treats another session's as abandoned", async () => {
const home = await fs.mkdtemp(path.join(os.tmpdir(), "nemoclaw-route-ownership-"));
vi.stubEnv("HOME", home);
vi.resetModules();
try {
const registry = await import("./registry");
registry.reserveSandboxInferenceRoute("alpha", {
provider: "compatible-endpoint",
model: "model-a",
endpointUrl: "https://api.example.test/v1",
credentialEnv: "CUSTOM_API_KEY",
preferredInferenceApi: "openai-responses",
gatewayName: "nemoclaw-9090",
reservationSessionId: "session-owner",
});
const reserved = registry.getSandbox("alpha");

expect(registry.isPendingReservationForSession(reserved, "session-owner")).toBe(true);
expect(registry.isPendingReservationForSession(reserved, "session-other")).toBe(false);
expect(registry.isPendingReservationForSession(reserved, null)).toBe(false);
expect(registry.isPendingReservationForSession(reserved, undefined)).toBe(false);
} finally {
await fs.rm(home, { recursive: true, force: true });
}
});

it("never preserves a fully registered sandbox or a missing row (#6562)", async () => {
const home = await fs.mkdtemp(path.join(os.tmpdir(), "nemoclaw-route-ownership-"));
vi.stubEnv("HOME", home);
vi.resetModules();
try {
const registry = await import("./registry");
registry.registerSandbox({
name: "beta",
provider: "nvidia-prod",
model: "model-a",
gatewayName: "nemoclaw",
gatewayPort: 8080,
});

expect(
registry.isPendingReservationForSession(registry.getSandbox("beta"), "session-owner"),
).toBe(false);
expect(registry.isPendingReservationForSession(null, "session-owner")).toBe(false);
} finally {
await fs.rm(home, { recursive: true, force: true });
}
});
});
15 changes: 15 additions & 0 deletions src/lib/state/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ export interface SandboxEntry extends Partial<InferenceSelection> {
name: string;
/** Route-only placeholder created before sandbox creation; never eligible as the default. */
pendingRouteReservation?: true;
/** Onboard session that owns a pending reservation, so resume preserves its own row while abandoned reservations stay reconcilable. */
reservationSessionId?: string;
createdAt?: string;
gpuEnabled?: boolean;
hostGpuDetected?: boolean;
Expand Down Expand Up @@ -553,6 +555,7 @@ type SandboxInferenceRouteReservation = Pick<
"provider" | "model" | "endpointUrl" | "credentialEnv" | "preferredInferenceApi"
> & {
gatewayName: string;
reservationSessionId?: string;
};

/**
Expand All @@ -571,6 +574,7 @@ export function reserveSandboxInferenceRoute(
data.sandboxes[name] = {
...(existing ?? { name, pendingRouteReservation: true as const }),
pendingRouteReservation: true,
reservationSessionId: route.reservationSessionId ?? existing?.reservationSessionId,
provider: normalized.provider,
model: normalized.model,
endpointUrl: normalized.endpointUrl,
Expand All @@ -584,6 +588,17 @@ export function reserveSandboxInferenceRoute(
});
}

export function isPendingReservationForSession(
entry: SandboxEntry | null,
sessionId: string | null | undefined,
): boolean {
return (
entry?.pendingRouteReservation === true &&
Boolean(sessionId) &&
entry.reservationSessionId === sessionId
);
}

export function updateSandbox(name: string, updates: Partial<SandboxEntry>): boolean {
return withLock(() => {
const data = load();
Expand Down
150 changes: 150 additions & 0 deletions test/onboard-reservation-recreate.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

import assert from "node:assert/strict";
import { spawnSync } from "node:child_process";
import fs from "node:fs";
import os from "node:os";
import path from "node:path";
import { describe, it } from "vitest";
import { writeOkOpenshell } from "./helpers/onboard-openshell-fixture";

const repoRoot = path.join(import.meta.dirname, "..");
const onboardScriptMocksPath = JSON.stringify(
path.join(repoRoot, "test", "helpers", "onboard-script-mocks.cjs"),
);

describe("onboard sandbox recreate reservation safety", () => {
it("preserves a current-session pending route reservation across a not-ready recreate (#6562)", {
timeout: 60_000,
}, async () => {
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-onboard-reservation-survives-"));
const fakeBin = path.join(tmpDir, "bin");
const scriptPath = path.join(tmpDir, "reservation-survives.js");
const onboardPath = JSON.stringify(path.join(repoRoot, "src", "lib", "onboard.ts"));
const runnerPath = JSON.stringify(path.join(repoRoot, "src", "lib", "runner.ts"));
const registryPath = JSON.stringify(path.join(repoRoot, "src", "lib", "state", "registry.ts"));
const onboardSessionPath = JSON.stringify(
path.join(repoRoot, "src", "lib", "state", "onboard-session.ts"),
);

fs.mkdirSync(fakeBin, { recursive: true });
writeOkOpenshell(fakeBin);

const script = String.raw`
const runner = require(${runnerPath});
const _n = (c) => (Array.isArray(c) ? c.join(" ") : String(c)).replace(/'/g, "");
const registry = require(${registryPath});
const onboardSession = require(${onboardSessionPath});
const childProcess = require("node:child_process");
const { EventEmitter } = require("node:events");

const events = [];
let sandboxDeleted = false;
runner.run = (command) => {
const cmd = _n(command);
events.push({ kind: "run", cmd });
if (cmd.includes("sandbox delete")) sandboxDeleted = true;
return { status: 0 };
};
runner.runCapture = (command) => {
const cmd = _n(command);
if (cmd.includes("sandbox get my-assistant")) return "my-assistant";
if (cmd.includes("sandbox list")) {
return sandboxDeleted ? "my-assistant Ready" : "my-assistant NotReady";
}
if (cmd.includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running";
{
const mockedCapture = require(${onboardScriptMocksPath}).mockOnboardRunCapture(command, {
defaultCurlOutput: "ok",
});
if (mockedCapture !== null) return mockedCapture;
}
return "";
};

onboardSession.loadSession = () => ({ sessionId: "session-owner" });

registry.getSandbox = () => ({
name: "my-assistant",
gpuEnabled: false,
pendingRouteReservation: true,
reservationSessionId: "session-owner",
});
registry.registerSandbox = () => true;
registry.updateSandbox = () => true;
registry.setDefault = () => true;
registry.removeSandbox = (name) => {
events.push({ kind: "removeSandbox", name });
return true;
};

const preflight = require(${JSON.stringify(path.join(repoRoot, "src", "lib", "onboard", "preflight.ts"))});
preflight.checkPortAvailable = async () => ({ ok: true });

childProcess.spawn = (...args) => {
const child = new EventEmitter();
child.stdout = new EventEmitter();
child.stderr = new EventEmitter();
child.unref = () => {};
child.pid = 4246;
events.push({ kind: "spawn", cmd: _n([args[0], ...(Array.isArray(args[1]) ? args[1] : [])]) });
process.nextTick(() => {
child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n"));
child.emit("close", 0);
});
return child;
};

const { createSandbox } = require(${onboardPath});

(async () => {
process.env.OPENSHELL_GATEWAY = "nemoclaw";
process.env.NEMOCLAW_RECREATE_SANDBOX = "1";
process.env.NEMOCLAW_RECREATE_WITHOUT_BACKUP = "1";
const sandboxName = await createSandbox(null, "gpt-5.4", "nvidia-prod", null, "my-assistant");
console.log(JSON.stringify({ sandboxName, events }));
})().catch((error) => {
console.error(error);
process.exit(1);
});
`;
fs.writeFileSync(scriptPath, script);

const result = spawnSync(process.execPath, [scriptPath], {
cwd: repoRoot,
encoding: "utf-8",
env: {
...process.env,
HOME: tmpDir,
PATH: `${fakeBin}:${process.env.PATH || ""}`,
NEMOCLAW_NON_INTERACTIVE: "1",
},
});

assert.equal(result.status, 0, result.stderr);
const payloadLine = result.stdout
.trim()
.split("\n")
.slice()
.reverse()
.find((line) => line.startsWith("{") && line.endsWith("}"));
assert.ok(payloadLine, `expected JSON payload in stdout:\n${result.stdout}`);
const payload = JSON.parse(payloadLine);
assert.equal(payload.sandboxName, "my-assistant");

const events = payload.events as Array<{ kind: string; cmd?: string; name?: string }>;
const removedReservation = events.some(
(e) => e.kind === "removeSandbox" && e.name === "my-assistant",
);
assert.equal(
removedReservation,
false,
"must not delete the current session's pending route reservation during recreate",
);
assert.ok(
events.some((e) => e.kind === "run" && (e.cmd || "").includes("sandbox delete")),
"should still delete the not-ready gateway sandbox before rebuilding",
);
});
});
Loading