From 4e5c7b75e5bc7f2572e9e4108ae0b2080e8cf716 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Fri, 10 Jul 2026 05:49:55 +0000 Subject: [PATCH 1/4] fix(onboard): keep pending route reservation through not-ready recreate When an interrupted onboard left a partial sandbox live on the gateway, the not-ready recreate removed the pending inference-route row before the long rebuild, so a second interruption stranded `--resume` with "route reservation disappeared". The create path finalises that row in place via updateSandbox and never re-registers it, so the reservation must survive the recreate. Stamp the reservation with the owning onboard session id and skip the pre-rebuild registry removal for the current session's pending reservation; abandoned reservations from other sessions still prune. Signed-off-by: Tinson Lai --- src/lib/onboard.ts | 9 +- .../machine/handlers/provider-inference.ts | 3 + .../state/registry-route-reservation.test.ts | 81 ++++++++++ src/lib/state/registry.ts | 15 ++ test/onboard-reservation-recreate.test.ts | 150 ++++++++++++++++++ 5 files changed, 257 insertions(+), 1 deletion(-) create mode 100644 test/onboard-reservation-recreate.test.ts diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 68403a1dc2c..67e1ea14a97 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -2692,7 +2692,14 @@ async function createSandboxWithBaseImageResolution( console.warn(` Warning: failed to remove old sandbox image '${previousEntry.imageTag}'.`); } } - registry.removeSandbox(sandboxName); + if ( + !registry.isPendingReservationForSession( + previousEntry, + onboardSession.loadSession()?.sessionId, + ) + ) { + registry.removeSandbox(sandboxName); + } } // Stage build context — use the custom Dockerfile path when provided, diff --git a/src/lib/onboard/machine/handlers/provider-inference.ts b/src/lib/onboard/machine/handlers/provider-inference.ts index 37e1ec4020a..b525d0f46db 100644 --- a/src/lib/onboard/machine/handlers/provider-inference.ts +++ b/src/lib/onboard/machine/handlers/provider-inference.ts @@ -172,6 +172,7 @@ export interface ProviderInferenceStateOptions { credentialEnv: string | null; preferredInferenceApi: string | null; gatewayName: string; + reservationSessionId?: string; }, ): boolean; registryUpdateSandbox(sandboxName: string, updates: { nimContainer?: string | null }): void; @@ -636,6 +637,7 @@ export async function handleProviderInferenceState({ credentialEnv, preferredInferenceApi, gatewayName, + reservationSessionId: session?.sessionId, }) : null; return { reupserted, reserved }; @@ -670,6 +672,7 @@ export async function handleProviderInferenceState({ credentialEnv, preferredInferenceApi, gatewayName, + reservationSessionId: session?.sessionId, }); }); if (!reserved) { diff --git a/src/lib/state/registry-route-reservation.test.ts b/src/lib/state/registry-route-reservation.test.ts index 7293c4175ab..ddb25495f1e 100644 --- a/src/lib/state/registry-route-reservation.test.ts +++ b/src/lib/state/registry-route-reservation.test.ts @@ -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 }); + } + }); }); diff --git a/src/lib/state/registry.ts b/src/lib/state/registry.ts index a3647e77486..7ed38deecd1 100644 --- a/src/lib/state/registry.ts +++ b/src/lib/state/registry.ts @@ -89,6 +89,8 @@ export interface SandboxEntry extends Partial { 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; @@ -553,6 +555,7 @@ type SandboxInferenceRouteReservation = Pick< "provider" | "model" | "endpointUrl" | "credentialEnv" | "preferredInferenceApi" > & { gatewayName: string; + reservationSessionId?: string; }; /** @@ -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, @@ -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): boolean { return withLock(() => { const data = load(); diff --git a/test/onboard-reservation-recreate.test.ts b/test/onboard-reservation-recreate.test.ts new file mode 100644 index 00000000000..bc66763e12f --- /dev/null +++ b/test/onboard-reservation-recreate.test.ts @@ -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", + ); + }); +}); From f8798f249a9413382b73657d38718b1161aa4bb9 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Fri, 10 Jul 2026 08:28:49 +0000 Subject: [PATCH 2/4] refactor(onboard): extract reservation recreate guard for net-neutral entrypoint Move the pending-reservation removal guard out of the onboard.ts entrypoint into sandbox-lifecycle.removeSandboxUnlessSessionReservation so the codebase-growth guardrail stays satisfied, and assert the new reservationSessionId stamp in the provider-inference route reservation tests. Signed-off-by: Tinson Lai --- src/lib/onboard.ts | 9 +-------- .../machine/handlers/provider-inference.test.ts | 2 ++ src/lib/onboard/sandbox-lifecycle.ts | 10 ++++++++++ 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 67e1ea14a97..5721a618d96 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -2692,14 +2692,7 @@ async function createSandboxWithBaseImageResolution( console.warn(` Warning: failed to remove old sandbox image '${previousEntry.imageTag}'.`); } } - if ( - !registry.isPendingReservationForSession( - previousEntry, - onboardSession.loadSession()?.sessionId, - ) - ) { - registry.removeSandbox(sandboxName); - } + sandboxLifecycle.removeSandboxUnlessSessionReservation(previousEntry, sandboxName); } // Stage build context — use the custom Dockerfile path when provided, diff --git a/src/lib/onboard/machine/handlers/provider-inference.test.ts b/src/lib/onboard/machine/handlers/provider-inference.test.ts index 4c70a01e50f..7d11ebe80cf 100644 --- a/src/lib/onboard/machine/handlers/provider-inference.test.ts +++ b/src/lib/onboard/machine/handlers/provider-inference.test.ts @@ -383,6 +383,7 @@ describe("handleProviderInferenceState", () => { credentialEnv: "COMPATIBLE_API_KEY", preferredInferenceApi: "openai-completions", gatewayName: "nemoclaw", + reservationSessionId: expect.any(String), }); }); @@ -1055,6 +1056,7 @@ describe("handleProviderInferenceState", () => { credentialEnv: "NVIDIA_INFERENCE_API_KEY", preferredInferenceApi: null, gatewayName: "nemoclaw", + reservationSessionId: expect.any(String), }); }); diff --git a/src/lib/onboard/sandbox-lifecycle.ts b/src/lib/onboard/sandbox-lifecycle.ts index 88e392cd662..08e9732ddb2 100644 --- a/src/lib/onboard/sandbox-lifecycle.ts +++ b/src/lib/onboard/sandbox-lifecycle.ts @@ -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 | null; fetchGatewayAuthTokenFromSandbox(sandboxName: string): string | null; From 0642e00ddbc40ea7b4959be30ac49c02e3474b33 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Fri, 10 Jul 2026 10:27:25 +0000 Subject: [PATCH 3/4] fix(onboard): stamp session id on the initial inference route reservation Signed-off-by: Tinson Lai --- .../handlers/provider-inference.test.ts | 38 ++++++++++-- .../machine/handlers/provider-inference.ts | 4 ++ .../setup-inference-route-containment.test.ts | 60 +++++++++++++++++++ src/lib/onboard/setup-inference.ts | 1 + 4 files changed, 97 insertions(+), 6 deletions(-) diff --git a/src/lib/onboard/machine/handlers/provider-inference.test.ts b/src/lib/onboard/machine/handlers/provider-inference.test.ts index 7d11ebe80cf..db04ea35b90 100644 --- a/src/lib/onboard/machine/handlers/provider-inference.test.ts +++ b/src/lib/onboard/machine/handlers/provider-inference.test.ts @@ -57,6 +57,7 @@ describe("handleProviderInferenceState", () => { gatewayName: "nemoclaw", allowToolsIncompatible: false, preferredInferenceApi: "openai-responses", + reservationSessionId: expect.any(String), }, ); expect(calls.deleteEnv).toHaveBeenCalledWith("NVIDIA_INFERENCE_API_KEY"); @@ -125,6 +126,7 @@ describe("handleProviderInferenceState", () => { gatewayName: "nemoclaw", allowToolsIncompatible: false, preferredInferenceApi: "openai-completions", + reservationSessionId: expect.any(String), }, ); expect(result.preferredInferenceApi).toBe("openai-completions"); @@ -167,6 +169,7 @@ describe("handleProviderInferenceState", () => { gatewayName: "nemoclaw", allowToolsIncompatible: false, preferredInferenceApi: "openai-completions", + reservationSessionId: expect.any(String), }, ); expect(calls.complete).toHaveBeenCalledWith( @@ -223,6 +226,7 @@ describe("handleProviderInferenceState", () => { gatewayName: "nemoclaw-9090", allowToolsIncompatible: false, preferredInferenceApi: "openai-completions", + reservationSessionId: expect.any(String), }, ); }); @@ -348,6 +352,8 @@ describe("handleProviderInferenceState", () => { preferredInferenceApi: "openai-completions", }); const { deps, calls } = createDeps({ isInferenceRouteReady: vi.fn(() => true) }); + const rebuiltSession = createSession({ sessionId: "rebuild-session-id" }); + calls.complete.mockResolvedValueOnce(rebuiltSession); const result = await handleProviderInferenceState({ ...baseOptions(deps, session), @@ -383,7 +389,7 @@ describe("handleProviderInferenceState", () => { credentialEnv: "COMPATIBLE_API_KEY", preferredInferenceApi: "openai-completions", gatewayName: "nemoclaw", - reservationSessionId: expect.any(String), + reservationSessionId: rebuiltSession.sessionId, }); }); @@ -680,7 +686,11 @@ describe("handleProviderInferenceState", () => { "COMPATIBLE_API_KEY", null, [], - { gatewayName: "nemoclaw", allowToolsIncompatible: false }, + { + gatewayName: "nemoclaw", + allowToolsIncompatible: false, + reservationSessionId: expect.any(String), + }, ); }); @@ -716,7 +726,11 @@ describe("handleProviderInferenceState", () => { "COMPATIBLE_API_KEY", null, [], - { gatewayName: "nemoclaw", allowToolsIncompatible: false }, + { + gatewayName: "nemoclaw", + allowToolsIncompatible: false, + reservationSessionId: expect.any(String), + }, ); }); @@ -753,7 +767,11 @@ describe("handleProviderInferenceState", () => { "COMPATIBLE_API_KEY", null, [], - { gatewayName: "nemoclaw", allowToolsIncompatible: false }, + { + gatewayName: "nemoclaw", + allowToolsIncompatible: false, + reservationSessionId: expect.any(String), + }, ); expect(calls.log).toHaveBeenCalledWith( " [resume] Refreshing compatible-endpoint inference route for messaging.", @@ -831,6 +849,7 @@ describe("handleProviderInferenceState", () => { skipHostInferenceSmoke: true, reuseGatewayCredentialWithoutLocalKey: true, preferredInferenceApi: "openai-completions", + reservationSessionId: expect.any(String), }, ); expect(calls.log).toHaveBeenCalledWith( @@ -920,7 +939,11 @@ describe("handleProviderInferenceState", () => { "COMPATIBLE_API_KEY", null, [], - { gatewayName: "nemoclaw", allowToolsIncompatible: false }, + { + gatewayName: "nemoclaw", + allowToolsIncompatible: false, + reservationSessionId: expect.any(String), + }, ); expect(calls.log).toHaveBeenCalledWith( " [resume] Refreshing compatible-endpoint inference route for messaging.", @@ -1029,6 +1052,8 @@ describe("handleProviderInferenceState", () => { isInferenceRouteReady: vi.fn(() => true), withGatewayRouteMutationLock, }); + const rebuiltSession = createSession({ sessionId: "router-rebuild-session-id" }); + calls.complete.mockResolvedValueOnce(rebuiltSession); calls.reconcileRouter.mockImplementation(async () => { expect(insideGatewayLock).toBe(true); }); @@ -1056,7 +1081,7 @@ describe("handleProviderInferenceState", () => { credentialEnv: "NVIDIA_INFERENCE_API_KEY", preferredInferenceApi: null, gatewayName: "nemoclaw", - reservationSessionId: expect.any(String), + reservationSessionId: rebuiltSession.sessionId, }); }); @@ -1173,6 +1198,7 @@ describe("handleProviderInferenceState", () => { gatewayName: "nemoclaw", allowToolsIncompatible: true, preferredInferenceApi: "openai-responses", + reservationSessionId: expect.any(String), }, ); }); diff --git a/src/lib/onboard/machine/handlers/provider-inference.ts b/src/lib/onboard/machine/handlers/provider-inference.ts index b525d0f46db..bc53c2c701c 100644 --- a/src/lib/onboard/machine/handlers/provider-inference.ts +++ b/src/lib/onboard/machine/handlers/provider-inference.ts @@ -34,6 +34,8 @@ export interface ProviderInferenceSetupOptions { preferredInferenceApi?: string | null; /** Public addresses approved for custom endpoint host probes. */ endpointPinnedAddresses?: readonly string[]; + /** Onboard session that owns the route reservation this setup creates. */ + reservationSessionId?: string; } export interface ProviderSelectionResult { @@ -554,6 +556,7 @@ export async function handleProviderInferenceState({ : {}), ...(preferredInferenceApi ? { preferredInferenceApi } : {}), ...(endpointPinnedAddresses ? { endpointPinnedAddresses } : {}), + reservationSessionId: session?.sessionId, }; await deps.startRecordedStep("inference", { provider, model }); inferenceResult = await withInferenceTrace( @@ -741,6 +744,7 @@ export async function handleProviderInferenceState({ ...(reuseGatewayCredentialWithoutLocalKey ? { reuseGatewayCredentialWithoutLocalKey } : {}), ...(preferredInferenceApi ? { preferredInferenceApi } : {}), ...(endpointPinnedAddresses ? { endpointPinnedAddresses } : {}), + reservationSessionId: session?.sessionId, }; await deps.startRecordedStep("inference", { provider, model }); inferenceResult = await withInferenceTrace( diff --git a/src/lib/onboard/setup-inference-route-containment.test.ts b/src/lib/onboard/setup-inference-route-containment.test.ts index 07b9c959a5a..9db47479837 100644 --- a/src/lib/onboard/setup-inference-route-containment.test.ts +++ b/src/lib/onboard/setup-inference-route-containment.test.ts @@ -234,4 +234,64 @@ describe("onboard shared gateway route containment", () => { expect(reservations).toHaveLength(1); expect(exitProcess).toHaveBeenCalledWith(1); }); + + it("stamps the owning onboard session on the initial route reservation (#6562)", async () => { + const reservations: SandboxEntry[] = []; + const updateSandbox = vi.fn( + (name: string, route: Parameters[1]) => { + reservations.push({ name, ...route }); + return true; + }, + ); + const setupInference = createSetupInference({ + checkGatewayRouteCompatibility: vi.fn(() => ({ ok: true as const })), + withSandboxMutationLock: async (_name: string, operation: () => Promise | T) => + await operation(), + withGatewayRouteMutationLock: async (_name: string, operation: () => Promise | T) => + await operation(), + step: vi.fn(), + getGatewayName: () => "nemoclaw", + runOpenshell: vi.fn(() => ({ status: 0 })), + updateSandbox, + upsertProvider: vi.fn(() => ({ ok: true })), + verifyInferenceRoute: vi.fn(), + verifyOnboardInferenceSmoke: vi.fn(), + isNonInteractive: () => true, + hermesProviderAuth: { HERMES_PROVIDER_NAME: "hermes-provider" }, + isRoutedInferenceProvider: () => true, + reconcileModelRouter: vi.fn(async () => undefined), + routedInference: { + upsertRoutedProvider: vi.fn(() => ({ + ok: true, + endpointUrl: "http://router.test/v1", + result: { ok: true }, + })), + }, + hydrateCredentialEnv: vi.fn(() => "secret"), + redact: (value: string) => value, + compactText: (value: string) => value, + log: vi.fn(), + error: vi.fn(), + exitProcess: vi.fn((code: number): never => { + throw new Error(`exit ${code}`); + }), + } as unknown as SetupInferenceDeps); + + await expect( + setupInference( + "gamma", + "model-c", + "router-c", + "http://router-c.test/v1", + "ROUTER_KEY", + null, + [], + { skipHostInferenceSmoke: true, reservationSessionId: "session-gamma" }, + ), + ).resolves.toEqual({ ok: true }); + + expect(reservations).toEqual([ + expect.objectContaining({ name: "gamma", reservationSessionId: "session-gamma" }), + ]); + }); }); diff --git a/src/lib/onboard/setup-inference.ts b/src/lib/onboard/setup-inference.ts index cd0c3cb2e17..3c18708702a 100644 --- a/src/lib/onboard/setup-inference.ts +++ b/src/lib/onboard/setup-inference.ts @@ -282,6 +282,7 @@ export function createSetupInference( credentialEnv, preferredInferenceApi: options.preferredInferenceApi ?? null, gatewayName, + reservationSessionId: options.reservationSessionId, }); routeReserved = reserved; return reserved; From a5a33d67c9e4aa128b05b4ab7678da7d342c8143 Mon Sep 17 00:00:00 2001 From: Charan Jagwani Date: Fri, 10 Jul 2026 10:03:57 -0700 Subject: [PATCH 4/4] test(onboard): cover reservation ownership boundaries Signed-off-by: Charan Jagwani --- .../onboard/machine/core-flow-phases.test.ts | 6 +- .../handlers/provider-inference.test.ts | 39 ++++++++---- src/lib/onboard/sandbox-lifecycle.test.ts | 60 +++++++++++++++++-- .../state/registry-route-reservation.test.ts | 39 ++++++++++++ test/onboard-reservation-recreate.test.ts | 32 ++++++++-- 5 files changed, 152 insertions(+), 24 deletions(-) diff --git a/src/lib/onboard/machine/core-flow-phases.test.ts b/src/lib/onboard/machine/core-flow-phases.test.ts index 7fc4b99ee45..c3efd474638 100644 --- a/src/lib/onboard/machine/core-flow-phases.test.ts +++ b/src/lib/onboard/machine/core-flow-phases.test.ts @@ -360,7 +360,11 @@ describe("core onboard flow phases", () => { "HERMES_API_KEY", "api_key", ["nous-web"], - { gatewayName: "nemoclaw", allowToolsIncompatible: false }, + { + gatewayName: "nemoclaw", + allowToolsIncompatible: false, + reservationSessionId: session.sessionId, + }, ); expect(result.context.hermesToolGateways).toEqual(["nous-web"]); diff --git a/src/lib/onboard/machine/handlers/provider-inference.test.ts b/src/lib/onboard/machine/handlers/provider-inference.test.ts index db04ea35b90..51c62e1fd38 100644 --- a/src/lib/onboard/machine/handlers/provider-inference.test.ts +++ b/src/lib/onboard/machine/handlers/provider-inference.test.ts @@ -26,8 +26,10 @@ import { describe("handleProviderInferenceState", () => { it("runs provider selection and inference setup on a fresh flow", async () => { const { deps, calls } = createDeps(); + const session = createSession(); + calls.complete.mockResolvedValue(session); - const result = await handleProviderInferenceState(baseOptions(deps)); + const result = await handleProviderInferenceState(baseOptions(deps, session)); expect(calls.startStep).toHaveBeenNthCalledWith(1, "provider_selection"); expect(calls.setupNim).toHaveBeenCalledWith( @@ -57,7 +59,7 @@ describe("handleProviderInferenceState", () => { gatewayName: "nemoclaw", allowToolsIncompatible: false, preferredInferenceApi: "openai-responses", - reservationSessionId: expect.any(String), + reservationSessionId: session.sessionId, }, ); expect(calls.deleteEnv).toHaveBeenCalledWith("NVIDIA_INFERENCE_API_KEY"); @@ -98,9 +100,11 @@ describe("handleProviderInferenceState", () => { preferredInferenceApi: "anthropic-messages", })); const { deps, calls } = createDeps({ setupNim }); + const session = createSession(); + calls.complete.mockResolvedValue(session); const result = await handleProviderInferenceState({ - ...baseOptions(deps), + ...baseOptions(deps, session), agent: { name: "hermes" }, sandboxName: "hermes-custom", }); @@ -126,7 +130,7 @@ describe("handleProviderInferenceState", () => { gatewayName: "nemoclaw", allowToolsIncompatible: false, preferredInferenceApi: "openai-completions", - reservationSessionId: expect.any(String), + reservationSessionId: session.sessionId, }, ); expect(result.preferredInferenceApi).toBe("openai-completions"); @@ -143,6 +147,7 @@ describe("handleProviderInferenceState", () => { preferredInferenceApi: "anthropic-messages", }); const { deps, calls } = createDeps({ isInferenceRouteReady: vi.fn(() => true) }); + calls.complete.mockResolvedValue(session); const result = await handleProviderInferenceState({ ...baseOptions(deps, session), @@ -169,7 +174,7 @@ describe("handleProviderInferenceState", () => { gatewayName: "nemoclaw", allowToolsIncompatible: false, preferredInferenceApi: "openai-completions", - reservationSessionId: expect.any(String), + reservationSessionId: session.sessionId, }, ); expect(calls.complete).toHaveBeenCalledWith( @@ -194,6 +199,7 @@ describe("handleProviderInferenceState", () => { isInferenceRouteReady: vi.fn(() => true), isResumeProviderSurfaceReady: surfaceReady, }); + calls.complete.mockResolvedValue(session); await handleProviderInferenceState({ ...baseOptions(deps, session), @@ -226,7 +232,7 @@ describe("handleProviderInferenceState", () => { gatewayName: "nemoclaw-9090", allowToolsIncompatible: false, preferredInferenceApi: "openai-completions", - reservationSessionId: expect.any(String), + reservationSessionId: session.sessionId, }, ); }); @@ -669,6 +675,7 @@ describe("handleProviderInferenceState", () => { credentialEnv: "COMPATIBLE_API_KEY", })), }); + calls.complete.mockResolvedValue(session); await handleProviderInferenceState({ ...baseOptions(deps, session), @@ -689,7 +696,7 @@ describe("handleProviderInferenceState", () => { { gatewayName: "nemoclaw", allowToolsIncompatible: false, - reservationSessionId: expect.any(String), + reservationSessionId: session.sessionId, }, ); }); @@ -709,6 +716,7 @@ describe("handleProviderInferenceState", () => { credentialEnv: "COMPATIBLE_API_KEY", })), }); + calls.complete.mockResolvedValue(session); await handleProviderInferenceState({ ...baseOptions(deps, session), @@ -729,7 +737,7 @@ describe("handleProviderInferenceState", () => { { gatewayName: "nemoclaw", allowToolsIncompatible: false, - reservationSessionId: expect.any(String), + reservationSessionId: session.sessionId, }, ); }); @@ -746,6 +754,7 @@ describe("handleProviderInferenceState", () => { hydrateCredentialEnv: vi.fn(() => "host-key"), isInferenceRouteReady: vi.fn(() => true), }); + calls.complete.mockResolvedValue(session); await handleProviderInferenceState({ ...baseOptions(deps, session), @@ -770,7 +779,7 @@ describe("handleProviderInferenceState", () => { { gatewayName: "nemoclaw", allowToolsIncompatible: false, - reservationSessionId: expect.any(String), + reservationSessionId: session.sessionId, }, ); expect(calls.log).toHaveBeenCalledWith( @@ -827,6 +836,7 @@ describe("handleProviderInferenceState", () => { hydrateCredentialEnv: vi.fn(() => null), isInferenceRouteReady: vi.fn(() => true), }); + calls.complete.mockResolvedValue(session); await handleProviderInferenceState({ ...baseOptions(deps, session), @@ -849,7 +859,7 @@ describe("handleProviderInferenceState", () => { skipHostInferenceSmoke: true, reuseGatewayCredentialWithoutLocalKey: true, preferredInferenceApi: "openai-completions", - reservationSessionId: expect.any(String), + reservationSessionId: session.sessionId, }, ); expect(calls.log).toHaveBeenCalledWith( @@ -923,6 +933,7 @@ describe("handleProviderInferenceState", () => { hydrateCredentialEnv: vi.fn(() => "nvapi-test"), isInferenceRouteReady: vi.fn(() => true), }); + calls.complete.mockResolvedValue(session); await handleProviderInferenceState({ ...baseOptions(deps, session), @@ -942,7 +953,7 @@ describe("handleProviderInferenceState", () => { { gatewayName: "nemoclaw", allowToolsIncompatible: false, - reservationSessionId: expect.any(String), + reservationSessionId: session.sessionId, }, ); expect(calls.log).toHaveBeenCalledWith( @@ -1183,8 +1194,10 @@ describe("handleProviderInferenceState", () => { allowToolsIncompatible: true, })); const { deps, calls } = createDeps({ setupNim }); + const session = createSession(); + calls.complete.mockResolvedValue(session); - await handleProviderInferenceState(baseOptions(deps)); + await handleProviderInferenceState(baseOptions(deps, session)); expect(calls.setupInference).toHaveBeenCalledWith( "my-assistant", @@ -1198,7 +1211,7 @@ describe("handleProviderInferenceState", () => { gatewayName: "nemoclaw", allowToolsIncompatible: true, preferredInferenceApi: "openai-responses", - reservationSessionId: expect.any(String), + reservationSessionId: session.sessionId, }, ); }); diff --git a/src/lib/onboard/sandbox-lifecycle.test.ts b/src/lib/onboard/sandbox-lifecycle.test.ts index ec8c8485dc9..4fcae628cff 100644 --- a/src/lib/onboard/sandbox-lifecycle.test.ts +++ b/src/lib/onboard/sandbox-lifecycle.test.ts @@ -9,13 +9,65 @@ const registryState = vi.hoisted(() => ({ removeSandbox: vi.fn(), sandbox: null as SandboxEntry | null, })); +const onboardSessionState = vi.hoisted(() => ({ sessionId: "session-owner" as string | null })); -vi.mock("../state/registry", () => ({ - getSandbox: () => registryState.sandbox, - removeSandbox: registryState.removeSandbox, +vi.mock("../state/registry", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + getSandbox: () => registryState.sandbox, + removeSandbox: registryState.removeSandbox, + }; +}); +vi.mock("../state/onboard-session", () => ({ + loadSession: () => + onboardSessionState.sessionId === null ? null : { sessionId: onboardSessionState.sessionId }, })); -import { createSandboxLifecycleHelpers } from "./sandbox-lifecycle"; +import { + createSandboxLifecycleHelpers, + removeSandboxUnlessSessionReservation, +} from "./sandbox-lifecycle"; + +describe("sandbox recreate reservation ownership", () => { + beforeEach(() => { + registryState.removeSandbox.mockReset(); + onboardSessionState.sessionId = "session-owner"; + }); + + it("preserves a pending reservation owned by the active session (#6562)", () => { + removeSandboxUnlessSessionReservation( + { + name: "alpha", + pendingRouteReservation: true, + reservationSessionId: "session-owner", + }, + "alpha", + ); + + expect(registryState.removeSandbox).not.toHaveBeenCalled(); + }); + + it.each([ + { + label: "foreign-session", + entry: { + name: "alpha", + pendingRouteReservation: true, + reservationSessionId: "session-other", + }, + }, + { + label: "unstamped", + entry: { name: "alpha", pendingRouteReservation: true }, + }, + ] as const)("removes a $label pending reservation before recreation (#6562)", ({ entry }) => { + removeSandboxUnlessSessionReservation(entry, "alpha"); + + expect(registryState.removeSandbox).toHaveBeenCalledOnce(); + expect(registryState.removeSandbox).toHaveBeenCalledWith("alpha"); + }); +}); describe("sandbox lifecycle MCP destroy boundaries", () => { beforeEach(() => { diff --git a/src/lib/state/registry-route-reservation.test.ts b/src/lib/state/registry-route-reservation.test.ts index ddb25495f1e..f8222999ec3 100644 --- a/src/lib/state/registry-route-reservation.test.ts +++ b/src/lib/state/registry-route-reservation.test.ts @@ -110,6 +110,45 @@ describe("sandbox inference route reservation", () => { await fs.rm(home, { recursive: true, force: true }); } }); + + it("transfers reservation ownership when a new session retargets the route (#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", + reservationSessionId: "session-old", + }); + + registry.reserveSandboxInferenceRoute("alpha", { + provider: "compatible-endpoint", + model: "model-b", + endpointUrl: "https://api.example.test/v1", + credentialEnv: "CUSTOM_API_KEY", + preferredInferenceApi: "openai-responses", + gatewayName: "nemoclaw", + reservationSessionId: "session-new", + }); + + const reserved = registry.getSandbox("alpha"); + expect(reserved).toMatchObject({ + model: "model-b", + pendingRouteReservation: true, + reservationSessionId: "session-new", + }); + expect(registry.isPendingReservationForSession(reserved, "session-new")).toBe(true); + expect(registry.isPendingReservationForSession(reserved, "session-old")).toBe(false); + } finally { + await fs.rm(home, { recursive: true, force: true }); + } + }); }); describe("pending reservation ownership (#6562)", () => { diff --git a/test/onboard-reservation-recreate.test.ts b/test/onboard-reservation-recreate.test.ts index bc66763e12f..72b8d78885b 100644 --- a/test/onboard-reservation-recreate.test.ts +++ b/test/onboard-reservation-recreate.test.ts @@ -15,9 +15,26 @@ const onboardScriptMocksPath = JSON.stringify( ); describe("onboard sandbox recreate reservation safety", () => { - it("preserves a current-session pending route reservation across a not-ready recreate (#6562)", { - timeout: 60_000, - }, async () => { + it.each([ + { + name: "preserves a current-session pending route reservation across a not-ready recreate", + reservationSessionId: "session-owner", + expectedRemoval: false, + }, + { + name: "removes a foreign-session pending route reservation before a not-ready recreate", + reservationSessionId: "session-other", + expectedRemoval: true, + }, + { + name: "removes an unstamped pending route reservation before a not-ready recreate", + reservationSessionId: null, + expectedRemoval: true, + }, + ] as const)("$name (#6562)", { timeout: 60_000 }, async ({ + reservationSessionId, + expectedRemoval, + }) => { 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"); @@ -65,11 +82,12 @@ runner.runCapture = (command) => { onboardSession.loadSession = () => ({ sessionId: "session-owner" }); +const reservationSessionId = ${JSON.stringify(reservationSessionId)}; registry.getSandbox = () => ({ name: "my-assistant", gpuEnabled: false, pendingRouteReservation: true, - reservationSessionId: "session-owner", + ...(reservationSessionId === null ? {} : { reservationSessionId }), }); registry.registerSandbox = () => true; registry.updateSandbox = () => true; @@ -139,8 +157,10 @@ const { createSandbox } = require(${onboardPath}); ); assert.equal( removedReservation, - false, - "must not delete the current session's pending route reservation during recreate", + expectedRemoval, + expectedRemoval + ? "must delete abandoned pending route reservations during recreate" + : "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")),