From 6737a6229a21db222e3d6ba37d3bd1a8e4d5d822 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Mon, 1 Jun 2026 04:00:20 +0000 Subject: [PATCH 01/27] fix(onboard): refuse gateway recreate when live sandboxes exist Signed-off-by: Tinson Lai --- docs/reference/troubleshooting.mdx | 13 ++- src/lib/onboard.ts | 12 +- ...preflight-gateway-cleanup-decision.test.ts | 110 +++++++++++++++--- .../preflight-gateway-cleanup-decision.ts | 95 ++++++++++++++- src/lib/state/gateway.ts | 24 ++++ test/gateway-state.test.ts | 38 ++++++ 6 files changed, 268 insertions(+), 24 deletions(-) diff --git a/docs/reference/troubleshooting.mdx b/docs/reference/troubleshooting.mdx index b89ebeac30a..5a4909f9ad2 100644 --- a/docs/reference/troubleshooting.mdx +++ b/docs/reference/troubleshooting.mdx @@ -214,6 +214,14 @@ or Ollama proxy ports: $ NEMOCLAW_GATEWAY_PORT=8990 nemoclaw onboard ``` +`NEMOCLAW_GATEWAY_PORT` relocates the singleton OpenShell gateway — it does +not spawn a second gateway alongside the existing one. Changing the port while +a sandbox is live triggers a destructive gateway recreate, so onboarding now +refuses the change while any sandbox is in `Ready` or `Running` state and +prints the names you need to stop first. Concurrent NemoClaw instances on a +single host are tracked in +[#3053](https://github.com/NVIDIA/NemoClaw/issues/3053). + Remote/headless hosts can bind the OpenShell gateway to all IPv4 interfaces: ```console @@ -227,7 +235,10 @@ See [Environment Variables](/reference/commands#environment-variables) for the f ### Running multiple sandboxes simultaneously -Each sandbox requires its own dashboard port. +Multiple sandboxes share a single OpenShell gateway on the same host. Each +sandbox gets its own dashboard port and SSH tunnel; the gateway, gateway port +(`NEMOCLAW_GATEWAY_PORT`, default `8080`), and cluster container are shared. + If you onboard a second sandbox without overriding the port, onboarding uses the next free port in the `18789` to `18799` range. `onboard` checks `openshell forward list` before starting a new forward, so a second onboard cannot silently take over the first sandbox's port. diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 504004f75fd..505ad8d1803 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -437,7 +437,7 @@ const { preflightDashboardPortRangeAvailability, } = require("./onboard/dashboard-port") as typeof import("./onboard/dashboard-port"); const { destroyGatewayForReuse } = require("./onboard/gateway-cleanup") as typeof import("./onboard/gateway-cleanup"); -const { applyPreflightGatewayCleanup } = +const { runPreflightGatewayCleanup } = require("./onboard/preflight-gateway-cleanup-decision") as typeof import("./onboard/preflight-gateway-cleanup-decision"); const { verifyGatewayContainerRunning } = require("./onboard/gateway-container-running") as typeof import("./onboard/gateway-container-running"); @@ -1989,13 +1989,13 @@ async function preflight( exitProcess: (code) => process.exit(code), }); - gatewayReuseState = applyPreflightGatewayCleanup({ + gatewayReuseState = runPreflightGatewayCleanup({ gatewayReuseState, - isDockerDriverGatewayEnabled: isLinuxDockerDriverGatewayEnabled(), - cliDisplayName: cliDisplayName(), - dashboardPort: DASHBOARD_PORT, - log: console.log, + isLinuxDockerDriverGatewayEnabled, + runCaptureOpenshell, runOpenshell, + cliName, + cliDisplayName, destroyGateway, destroyGatewayForReuse, }); diff --git a/src/lib/onboard/preflight-gateway-cleanup-decision.test.ts b/src/lib/onboard/preflight-gateway-cleanup-decision.test.ts index b2e8fad1dec..6386fe3f9e5 100644 --- a/src/lib/onboard/preflight-gateway-cleanup-decision.test.ts +++ b/src/lib/onboard/preflight-gateway-cleanup-decision.test.ts @@ -7,58 +7,97 @@ import type { GatewayReuseState } from "../state/gateway"; import { PREFLIGHT_DEFERRED_RECREATE_MESSAGE, + PREFLIGHT_LIVE_SANDBOX_REFUSAL_HEADER, applyPreflightGatewayCleanup, preflightGatewayCleanupDecision, } from "./preflight-gateway-cleanup-decision"; describe("preflightGatewayCleanupDecision", () => { - it("defers when state is stale and Docker-driver gateway is enabled", () => { + it("defers when state is stale, Docker-driver gateway is enabled, and no live sandboxes", () => { expect( preflightGatewayCleanupDecision({ gatewayReuseState: "stale", isDockerDriverGatewayEnabled: true, + liveSandboxNames: [], }), ).toBe("defer"); }); - it("defers when state is active-unnamed and Docker-driver gateway is enabled", () => { + it("defers when state is active-unnamed and no live sandboxes", () => { expect( preflightGatewayCleanupDecision({ gatewayReuseState: "active-unnamed", isDockerDriverGatewayEnabled: true, + liveSandboxNames: [], }), ).toBe("defer"); }); + it("refuses when Docker-driver path would destroy live sandboxes", () => { + expect( + preflightGatewayCleanupDecision({ + gatewayReuseState: "stale", + isDockerDriverGatewayEnabled: true, + liveSandboxNames: ["sandbox-a"], + }), + ).toBe("refuse"); + expect( + preflightGatewayCleanupDecision({ + gatewayReuseState: "active-unnamed", + isDockerDriverGatewayEnabled: true, + liveSandboxNames: ["sandbox-a", "sandbox-b"], + }), + ).toBe("refuse"); + }); + it("destroys legacy gateway in preflight when Docker-driver gateway is not enabled", () => { expect( preflightGatewayCleanupDecision({ gatewayReuseState: "stale", isDockerDriverGatewayEnabled: false, + liveSandboxNames: [], }), ).toBe("destroy-legacy"); expect( preflightGatewayCleanupDecision({ gatewayReuseState: "active-unnamed", isDockerDriverGatewayEnabled: false, + liveSandboxNames: [], + }), + ).toBe("destroy-legacy"); + }); + + it("destroys legacy gateway even with live sandboxes when Docker-driver gateway is not enabled", () => { + // Legacy package-managed gateway path is unaffected by the live-sandbox + // guard — that path destroys/restarts the gateway process without + // touching the openshell-cluster-* container. + expect( + preflightGatewayCleanupDecision({ + gatewayReuseState: "stale", + isDockerDriverGatewayEnabled: false, + liveSandboxNames: ["sandbox-a"], }), ).toBe("destroy-legacy"); }); - it("returns noop for non-stale states regardless of driver", () => { + it("returns noop for non-stale states regardless of driver or sandbox set", () => { for (const state of ["healthy", "missing", "foreign-active"] as const) { - expect( - preflightGatewayCleanupDecision({ - gatewayReuseState: state, - isDockerDriverGatewayEnabled: true, - }), - ).toBe("noop"); - expect( - preflightGatewayCleanupDecision({ - gatewayReuseState: state, - isDockerDriverGatewayEnabled: false, - }), - ).toBe("noop"); + for (const liveSandboxNames of [[], ["sandbox-a"]]) { + expect( + preflightGatewayCleanupDecision({ + gatewayReuseState: state, + isDockerDriverGatewayEnabled: true, + liveSandboxNames, + }), + ).toBe("noop"); + expect( + preflightGatewayCleanupDecision({ + gatewayReuseState: state, + isDockerDriverGatewayEnabled: false, + liveSandboxNames, + }), + ).toBe("noop"); + } } }); }); @@ -67,6 +106,7 @@ describe("applyPreflightGatewayCleanup", () => { function makeDeps(overrides: { gatewayReuseState: GatewayReuseState; isDockerDriverGatewayEnabled: boolean; + liveSandboxNames?: readonly string[]; }) { const log = vi.fn(); const runOpenshell = vi.fn(() => ({ status: 0 })); @@ -81,21 +121,28 @@ describe("applyPreflightGatewayCleanup", () => { destroy(); return "missing"; }); + const exitProcess = vi.fn((_code: number) => { + throw new Error("exit"); + }) as unknown as (code: number) => never; return { deps: { gatewayReuseState: overrides.gatewayReuseState, isDockerDriverGatewayEnabled: overrides.isDockerDriverGatewayEnabled, cliDisplayName: "NemoClaw", + cliCommandName: "nemoclaw", dashboardPort: 8081, + liveSandboxNames: overrides.liveSandboxNames ?? [], log, runOpenshell, destroyGateway, destroyGatewayForReuse, + exitProcess, }, log, runOpenshell, destroyGateway, destroyGatewayForReuse, + exitProcess, }; } @@ -107,6 +154,7 @@ describe("applyPreflightGatewayCleanup", () => { expect(ctx.destroyGateway).not.toHaveBeenCalled(); expect(ctx.destroyGatewayForReuse).not.toHaveBeenCalled(); expect(ctx.runOpenshell).not.toHaveBeenCalled(); + expect(ctx.exitProcess).not.toHaveBeenCalled(); }); it("destroys the legacy gateway and stops the dashboard forward on the non-Docker-driver path", () => { @@ -119,6 +167,37 @@ describe("applyPreflightGatewayCleanup", () => { }); expect(ctx.destroyGatewayForReuse).toHaveBeenCalledTimes(1); expect(ctx.destroyGateway).toHaveBeenCalledTimes(1); + expect(ctx.exitProcess).not.toHaveBeenCalled(); + }); + + it("refuses with structured guidance when a live sandbox is at risk on the Docker-driver path", () => { + const ctx = makeDeps({ + gatewayReuseState: "stale", + isDockerDriverGatewayEnabled: true, + liveSandboxNames: ["sandbox-a", "sandbox-b"], + }); + expect(() => applyPreflightGatewayCleanup(ctx.deps)).toThrow("exit"); + const logged = ctx.log.mock.calls.map(([line]) => line).join("\n"); + expect(logged).toContain(PREFLIGHT_LIVE_SANDBOX_REFUSAL_HEADER); + expect(logged).toContain("Live sandbox(es): sandbox-a, sandbox-b"); + expect(logged).toContain("nemoclaw sandbox-a stop"); + expect(logged).toContain("nemoclaw sandbox-b stop"); + expect(logged).toContain("NEMOCLAW_GATEWAY_PORT"); + expect(logged).toContain("#3053"); + expect(ctx.exitProcess).toHaveBeenCalledWith(1); + expect(ctx.destroyGateway).not.toHaveBeenCalled(); + expect(ctx.destroyGatewayForReuse).not.toHaveBeenCalled(); + }); + + it("does not refuse the legacy non-Docker-driver path even when live sandboxes exist", () => { + const ctx = makeDeps({ + gatewayReuseState: "stale", + isDockerDriverGatewayEnabled: false, + liveSandboxNames: ["sandbox-a"], + }); + const next = applyPreflightGatewayCleanup(ctx.deps); + expect(next).toBe("missing"); + expect(ctx.exitProcess).not.toHaveBeenCalled(); }); it("is a no-op for healthy / missing / foreign-active states", () => { @@ -130,6 +209,7 @@ describe("applyPreflightGatewayCleanup", () => { expect(ctx.destroyGateway).not.toHaveBeenCalled(); expect(ctx.destroyGatewayForReuse).not.toHaveBeenCalled(); expect(ctx.runOpenshell).not.toHaveBeenCalled(); + expect(ctx.exitProcess).not.toHaveBeenCalled(); } }); }); diff --git a/src/lib/onboard/preflight-gateway-cleanup-decision.ts b/src/lib/onboard/preflight-gateway-cleanup-decision.ts index d704bc925ea..5211ba3c4f2 100644 --- a/src/lib/onboard/preflight-gateway-cleanup-decision.ts +++ b/src/lib/onboard/preflight-gateway-cleanup-decision.ts @@ -1,20 +1,43 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import type { GatewayReuseState } from "../state/gateway"; +import { DASHBOARD_PORT } from "../core/ports"; +import { listLiveSandboxNames, type GatewayReuseState } from "../state/gateway"; -export type PreflightGatewayCleanupAction = "defer" | "destroy-legacy" | "noop"; +export type PreflightGatewayCleanupAction = + | "defer" + | "destroy-legacy" + | "refuse" + | "noop"; export const PREFLIGHT_DEFERRED_RECREATE_MESSAGE = " ⚠ Gateway will be recreated when sandbox creation starts — this will affect running sandboxes."; +export const PREFLIGHT_LIVE_SANDBOX_REFUSAL_HEADER = + " ✗ Refusing to recreate gateway: live sandbox(es) would be destroyed."; + +// Decision for the preflight gateway cleanup step. Returns: +// - "refuse" — drift would trigger a destructive gateway recreate +// while one or more sandboxes are live (Ready/Running). +// The singleton-gateway design (`GATEWAY_NAME = "nemoclaw"`) +// means the shared cluster container holds every sandbox, +// so recreating the gateway SIGKILLs them. See #4422. +// - "defer" — Docker-driver path: postpone the recreate to step [2/8] +// when no live sandboxes are at risk. +// - "destroy-legacy" — pre-Docker-driver path: destroy immediately so the +// port frees up for the upcoming port-availability checks. +// - "noop" — recorded state needs no preflight cleanup. export function preflightGatewayCleanupDecision(opts: { gatewayReuseState: GatewayReuseState; isDockerDriverGatewayEnabled: boolean; + liveSandboxNames: readonly string[]; }): PreflightGatewayCleanupAction { if (opts.gatewayReuseState !== "stale" && opts.gatewayReuseState !== "active-unnamed") { return "noop"; } + if (opts.isDockerDriverGatewayEnabled && opts.liveSandboxNames.length > 0) { + return "refuse"; + } return opts.isDockerDriverGatewayEnabled ? "defer" : "destroy-legacy"; } @@ -22,7 +45,9 @@ export interface PreflightGatewayCleanupDeps { gatewayReuseState: GatewayReuseState; isDockerDriverGatewayEnabled: boolean; cliDisplayName: string; + cliCommandName: string; dashboardPort: number; + liveSandboxNames: readonly string[]; log: (line: string) => void; runOpenshell: (args: string[], options: { ignoreError: true }) => unknown; destroyGateway: () => boolean; @@ -31,6 +56,46 @@ export interface PreflightGatewayCleanupDeps { successMessage: string, failureMessage: string, ) => GatewayReuseState; + exitProcess: (code: number) => never; +} + +export interface RunPreflightGatewayCleanupDeps { + gatewayReuseState: GatewayReuseState; + isLinuxDockerDriverGatewayEnabled: () => boolean; + runCaptureOpenshell: (args: string[], options: { ignoreError: true }) => string; + runOpenshell: (args: string[], options: { ignoreError: true }) => unknown; + cliName: () => string; + cliDisplayName: () => string; + destroyGateway: () => boolean; + destroyGatewayForReuse: ( + destroy: () => boolean, + successMessage: string, + failureMessage: string, + ) => GatewayReuseState; +} + +// Convenience wrapper for the onboard call site: fetches the live-sandbox set +// from `openshell sandbox list`, resolves CLI display/command names, and wires +// `process.exit` for the refuse path. Tests target `applyPreflightGatewayCleanup` +// directly for fine-grained dependency injection. +export function runPreflightGatewayCleanup( + deps: RunPreflightGatewayCleanupDeps, +): GatewayReuseState { + return applyPreflightGatewayCleanup({ + gatewayReuseState: deps.gatewayReuseState, + isDockerDriverGatewayEnabled: deps.isLinuxDockerDriverGatewayEnabled(), + cliDisplayName: deps.cliDisplayName(), + cliCommandName: deps.cliName(), + dashboardPort: DASHBOARD_PORT, + liveSandboxNames: listLiveSandboxNames( + deps.runCaptureOpenshell(["sandbox", "list"], { ignoreError: true }), + ), + log: console.log, + runOpenshell: deps.runOpenshell, + destroyGateway: deps.destroyGateway, + destroyGatewayForReuse: deps.destroyGatewayForReuse, + exitProcess: (code) => process.exit(code), + }); } export function applyPreflightGatewayCleanup( @@ -39,7 +104,33 @@ export function applyPreflightGatewayCleanup( const action = preflightGatewayCleanupDecision({ gatewayReuseState: deps.gatewayReuseState, isDockerDriverGatewayEnabled: deps.isDockerDriverGatewayEnabled, + liveSandboxNames: deps.liveSandboxNames, }); + if (action === "refuse") { + const names = deps.liveSandboxNames.join(", "); + deps.log(PREFLIGHT_LIVE_SANDBOX_REFUSAL_HEADER); + deps.log(` Live sandbox(es): ${names}`); + deps.log( + " Recreating the gateway here would SIGKILL the running sandbox container(s)", + ); + deps.log(" and leave them in Phase=Error."); + deps.log(""); + deps.log(" Resolve with one of:"); + for (const name of deps.liveSandboxNames) { + deps.log(` - ${deps.cliCommandName} ${name} stop`); + } + deps.log( + " - Onboard with the existing NEMOCLAW_GATEWAY_PORT (do not change it).", + ); + deps.log(""); + deps.log( + " Concurrent NemoClaw instances on a single host are tracked in #3053;", + ); + deps.log( + " this refusal protects existing sandboxes until that support lands.", + ); + deps.exitProcess(1); + } if (action === "defer") { deps.log(PREFLIGHT_DEFERRED_RECREATE_MESSAGE); return deps.gatewayReuseState; diff --git a/src/lib/state/gateway.ts b/src/lib/state/gateway.ts index 7d06f98fc85..c45e62e1dbb 100644 --- a/src/lib/state/gateway.ts +++ b/src/lib/state/gateway.ts @@ -57,6 +57,30 @@ export function isSandboxReady(output: string, sandboxName: string): boolean { return (cols.includes("Ready") || cols.includes("Running")) && !cols.includes("NotReady"); } +/** + * Enumerate sandbox names from `openshell sandbox list` whose state column + * indicates a live workload ("Ready" or "Running" and not "NotReady"). Used + * by the preflight cleanup decision to refuse a gateway-recreate that would + * SIGKILL live sandbox containers — the singleton-gateway design means + * recreating the gateway destroys the shared `openshell-cluster-*` container + * holding every sandbox. See #4422. + */ +export function listLiveSandboxNames(output: string): string[] { + if (typeof output !== "string") return []; + const clean = stripAnsi(output); + const names: string[] = []; + for (const line of clean.split("\n")) { + const cols = line.trim().split(/\s+/); + if (cols.length < 2) continue; + const name = cols[0]; + if (!name) continue; + if ((cols.includes("Ready") || cols.includes("Running")) && !cols.includes("NotReady")) { + names.push(name); + } + } + return names; +} + /** * Determine whether stale NemoClaw gateway output indicates a previous * session that should be cleaned up before the port preflight check. diff --git a/test/gateway-state.test.ts b/test/gateway-state.test.ts index 7b564e77a12..0090c44aaa6 100644 --- a/test/gateway-state.test.ts +++ b/test/gateway-state.test.ts @@ -14,6 +14,7 @@ import { hasStaleGateway, hasActiveGatewayInfo, getReportedGatewayName, + listLiveSandboxNames, shouldSelectNamedGatewayForReuse, parseSandboxPhase, } from "../src/lib/state/gateway.js"; @@ -224,6 +225,43 @@ describe("isGatewayHealthy", () => { }); }); +describe("listLiveSandboxNames", () => { + it("returns names of Ready and Running sandboxes only", () => { + const output = [ + "NAME STATUS AGE", + "sandbox-a Ready 5m", + "sandbox-b Running 2m", + "sandbox-c Provisioning 10s", + "sandbox-d NotReady 1m", + "sandbox-e Failed 30s", + ].join("\n"); + expect(listLiveSandboxNames(output)).toEqual(["sandbox-a", "sandbox-b"]); + }); + + it("treats NotReady as not live even when Ready also appears on the row", () => { + // Defensive: prefer the negative signal so a sandbox in transitional state + // isn't counted as live and accidentally blocks a legitimate recreate. + const output = "sandbox-x Ready NotReady 3m"; + expect(listLiveSandboxNames(output)).toEqual([]); + }); + + it("strips ANSI escapes before parsing", () => { + const output = + "\x1b[1mNAME STATUS AGE\x1b[0m\nsandbox-a \x1b[32mReady\x1b[0m 5m"; + expect(listLiveSandboxNames(output)).toEqual(["sandbox-a"]); + }); + + it("returns an empty array for empty / non-string input", () => { + expect(listLiveSandboxNames("")).toEqual([]); + expect(listLiveSandboxNames(undefined as unknown as string)).toEqual([]); + }); + + it("returns an empty array when no rows match", () => { + const output = ["NAME STATUS AGE", "sandbox-a Provisioning 10s"].join("\n"); + expect(listLiveSandboxNames(output)).toEqual([]); + }); +}); + describe("parseSandboxPhase", () => { it("extracts Ready phase from sandbox get output", () => { const output = ["Sandbox:", "", " Id: abc", " Name: my-assistant", " Phase: Ready"].join( From ab5648a2f091b6796a0a5285cfe70e42f81ea48d Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Mon, 1 Jun 2026 05:09:53 +0000 Subject: [PATCH 02/27] fixup: address review (extract live-row helper, wire-up test, docs, comments) Signed-off-by: Tinson Lai --- docs/reference/commands.mdx | 1 + ...preflight-gateway-cleanup-decision.test.ts | 80 +++++++++++++++++++ .../preflight-gateway-cleanup-decision.ts | 2 +- src/lib/state/gateway.ts | 21 ++--- 4 files changed, 93 insertions(+), 11 deletions(-) diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index d1a45c5a4e7..08a17b4d783 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -1325,6 +1325,7 @@ All ports must be non-privileged integers between 1024 and 65535. If a port value is not a valid integer or falls outside the allowed range, the CLI exits with an error. `NEMOCLAW_GATEWAY_PORT` also cannot overlap the configured dashboard, vLLM, Ollama, or Ollama proxy ports, and cannot use the dashboard auto-allocation range `18789` through `18799` or the default inference/proxy ports `8000`, `11434`, and `11435`. +`NEMOCLAW_GATEWAY_PORT` relocates the singleton OpenShell gateway; it does not spawn a second gateway alongside an existing one. Onboarding refuses to change the port while any sandbox is `Ready` or `Running` and prints the names you must stop first. See [troubleshooting](/reference/troubleshooting#port-conflicts) for the recovery steps and [#3053](https://github.com/NVIDIA/NemoClaw/issues/3053) for concurrent-instance support. On non-WSL hosts, `NEMOCLAW_OLLAMA_PORT` and `NEMOCLAW_OLLAMA_PROXY_PORT` must be different. If you run Ollama on port 11435, set `NEMOCLAW_OLLAMA_PROXY_PORT` to another free port before onboarding. diff --git a/src/lib/onboard/preflight-gateway-cleanup-decision.test.ts b/src/lib/onboard/preflight-gateway-cleanup-decision.test.ts index 6386fe3f9e5..f17cdfb5a8c 100644 --- a/src/lib/onboard/preflight-gateway-cleanup-decision.test.ts +++ b/src/lib/onboard/preflight-gateway-cleanup-decision.test.ts @@ -10,6 +10,7 @@ import { PREFLIGHT_LIVE_SANDBOX_REFUSAL_HEADER, applyPreflightGatewayCleanup, preflightGatewayCleanupDecision, + runPreflightGatewayCleanup, } from "./preflight-gateway-cleanup-decision"; describe("preflightGatewayCleanupDecision", () => { @@ -213,3 +214,82 @@ describe("applyPreflightGatewayCleanup", () => { } }); }); + +describe("runPreflightGatewayCleanup", () => { + function makeDeps(overrides: { + gatewayReuseState: GatewayReuseState; + isDockerDriverGatewayEnabled: boolean; + sandboxListOutput?: string; + }) { + const runCaptureOpenshell = vi.fn((args: string[], _opts: { ignoreError: true }) => { + expect(args).toEqual(["sandbox", "list"]); + return overrides.sandboxListOutput ?? ""; + }); + const runOpenshell = vi.fn(() => ({ status: 0 })); + const destroyGateway = vi.fn(() => true); + const destroyGatewayForReuse = vi.fn< + ( + destroy: () => boolean, + success: string, + failure: string, + ) => GatewayReuseState + >(() => "missing"); + return { + runCaptureOpenshell, + runOpenshell, + destroyGateway, + destroyGatewayForReuse, + deps: { + gatewayReuseState: overrides.gatewayReuseState, + isLinuxDockerDriverGatewayEnabled: () => overrides.isDockerDriverGatewayEnabled, + runCaptureOpenshell, + runOpenshell, + cliName: () => "nemoclaw", + cliDisplayName: () => "NemoClaw", + destroyGateway, + destroyGatewayForReuse, + }, + }; + } + + it("queries sandbox list and defers when no live sandboxes exist on the Docker-driver path", () => { + const ctx = makeDeps({ + gatewayReuseState: "stale", + isDockerDriverGatewayEnabled: true, + sandboxListOutput: "NAME STATUS AGE\nsandbox-a Provisioning 10s\n", + }); + const exitSpy = vi.spyOn(process, "exit").mockImplementation((() => { + throw new Error("unexpected exit"); + }) as never); + try { + const next = runPreflightGatewayCleanup(ctx.deps); + expect(next).toBe("stale"); + expect(ctx.runCaptureOpenshell).toHaveBeenCalledTimes(1); + expect(ctx.destroyGateway).not.toHaveBeenCalled(); + expect(exitSpy).not.toHaveBeenCalled(); + } finally { + exitSpy.mockRestore(); + } + }); + + it("refuses through process.exit(1) when sandbox list reports a live sandbox", () => { + const ctx = makeDeps({ + gatewayReuseState: "stale", + isDockerDriverGatewayEnabled: true, + sandboxListOutput: "NAME STATUS AGE\nsandbox-a Ready 5m\n", + }); + const exitSpy = vi.spyOn(process, "exit").mockImplementation((() => { + throw new Error("exit"); + }) as never); + try { + expect(() => runPreflightGatewayCleanup(ctx.deps)).toThrow("exit"); + expect(exitSpy).toHaveBeenCalledWith(1); + expect(ctx.runCaptureOpenshell).toHaveBeenCalledWith(["sandbox", "list"], { + ignoreError: true, + }); + expect(ctx.destroyGateway).not.toHaveBeenCalled(); + } finally { + exitSpy.mockRestore(); + } + }); +}); diff --git a/src/lib/onboard/preflight-gateway-cleanup-decision.ts b/src/lib/onboard/preflight-gateway-cleanup-decision.ts index 5211ba3c4f2..91e47eabe74 100644 --- a/src/lib/onboard/preflight-gateway-cleanup-decision.ts +++ b/src/lib/onboard/preflight-gateway-cleanup-decision.ts @@ -21,7 +21,7 @@ export const PREFLIGHT_LIVE_SANDBOX_REFUSAL_HEADER = // while one or more sandboxes are live (Ready/Running). // The singleton-gateway design (`GATEWAY_NAME = "nemoclaw"`) // means the shared cluster container holds every sandbox, -// so recreating the gateway SIGKILLs them. See #4422. +// so recreating the gateway SIGKILLs them. // - "defer" — Docker-driver path: postpone the recreate to step [2/8] // when no live sandboxes are at risk. // - "destroy-legacy" — pre-Docker-driver path: destroy immediately so the diff --git a/src/lib/state/gateway.ts b/src/lib/state/gateway.ts index c45e62e1dbb..7afa90fa3bd 100644 --- a/src/lib/state/gateway.ts +++ b/src/lib/state/gateway.ts @@ -51,19 +51,22 @@ export function parseSandboxStatus(output: string, sandboxName: string): string * sandbox stays in "Running" phase which is functionally equivalent to * "Ready" — the agent is live and the gateway is reachable inside. */ +function isLiveSandboxRow(cols: readonly string[]): boolean { + return (cols.includes("Ready") || cols.includes("Running")) && !cols.includes("NotReady"); +} + export function isSandboxReady(output: string, sandboxName: string): boolean { const cols = parseSandboxRow(output, sandboxName); if (!cols) return false; - return (cols.includes("Ready") || cols.includes("Running")) && !cols.includes("NotReady"); + return isLiveSandboxRow(cols); } /** - * Enumerate sandbox names from `openshell sandbox list` whose state column - * indicates a live workload ("Ready" or "Running" and not "NotReady"). Used - * by the preflight cleanup decision to refuse a gateway-recreate that would - * SIGKILL live sandbox containers — the singleton-gateway design means - * recreating the gateway destroys the shared `openshell-cluster-*` container - * holding every sandbox. See #4422. + * Enumerate sandbox names from `openshell sandbox list` output whose state + * column indicates a live workload — "Ready" or "Running" and not "NotReady". + * The singleton-gateway design means recreating the gateway destroys the + * shared `openshell-cluster-*` container holding every sandbox, so the + * preflight cleanup decision uses this set to refuse destructive recreates. */ export function listLiveSandboxNames(output: string): string[] { if (typeof output !== "string") return []; @@ -74,9 +77,7 @@ export function listLiveSandboxNames(output: string): string[] { if (cols.length < 2) continue; const name = cols[0]; if (!name) continue; - if ((cols.includes("Ready") || cols.includes("Running")) && !cols.includes("NotReady")) { - names.push(name); - } + if (isLiveSandboxRow(cols)) names.push(name); } return names; } From 84bdda87bcbbd3ada9ef5238c80fd8755cba2dfb Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Mon, 1 Jun 2026 07:01:41 +0000 Subject: [PATCH 03/27] fix(onboard): narrow refuse-recreate to confirmed stale drift only Signed-off-by: Tinson Lai --- ...preflight-gateway-cleanup-decision.test.ts | 18 +++++++++++++-- .../preflight-gateway-cleanup-decision.ts | 22 +++++++++++++------ 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/src/lib/onboard/preflight-gateway-cleanup-decision.test.ts b/src/lib/onboard/preflight-gateway-cleanup-decision.test.ts index f17cdfb5a8c..bd29f8814ff 100644 --- a/src/lib/onboard/preflight-gateway-cleanup-decision.test.ts +++ b/src/lib/onboard/preflight-gateway-cleanup-decision.test.ts @@ -34,7 +34,7 @@ describe("preflightGatewayCleanupDecision", () => { ).toBe("defer"); }); - it("refuses when Docker-driver path would destroy live sandboxes", () => { + it("refuses on confirmed drift (stale) when live sandboxes exist", () => { expect( preflightGatewayCleanupDecision({ gatewayReuseState: "stale", @@ -44,13 +44,27 @@ describe("preflightGatewayCleanupDecision", () => { ).toBe("refuse"); expect( preflightGatewayCleanupDecision({ - gatewayReuseState: "active-unnamed", + gatewayReuseState: "stale", isDockerDriverGatewayEnabled: true, liveSandboxNames: ["sandbox-a", "sandbox-b"], }), ).toBe("refuse"); }); + it("defers on active-unnamed even with live sandboxes so the port-availability check can run", () => { + // `active-unnamed` means there is an endpoint without a named-gateway + // metadata entry; the port loop may still fail on its own (e.g. host + // listener squatting on the configured gateway port), so deferring lets + // that diagnostic fire instead of pre-empting with the refuse message. + expect( + preflightGatewayCleanupDecision({ + gatewayReuseState: "active-unnamed", + isDockerDriverGatewayEnabled: true, + liveSandboxNames: ["sandbox-a"], + }), + ).toBe("defer"); + }); + it("destroys legacy gateway in preflight when Docker-driver gateway is not enabled", () => { expect( preflightGatewayCleanupDecision({ diff --git a/src/lib/onboard/preflight-gateway-cleanup-decision.ts b/src/lib/onboard/preflight-gateway-cleanup-decision.ts index 91e47eabe74..ed75bb60432 100644 --- a/src/lib/onboard/preflight-gateway-cleanup-decision.ts +++ b/src/lib/onboard/preflight-gateway-cleanup-decision.ts @@ -17,13 +17,17 @@ export const PREFLIGHT_LIVE_SANDBOX_REFUSAL_HEADER = " ✗ Refusing to recreate gateway: live sandbox(es) would be destroyed."; // Decision for the preflight gateway cleanup step. Returns: -// - "refuse" — drift would trigger a destructive gateway recreate -// while one or more sandboxes are live (Ready/Running). -// The singleton-gateway design (`GATEWAY_NAME = "nemoclaw"`) -// means the shared cluster container holds every sandbox, -// so recreating the gateway SIGKILLs them. +// - "refuse" — confirmed drift would trigger a destructive gateway +// recreate while one or more sandboxes are live +// (Ready/Running). The singleton-gateway design +// (`GATEWAY_NAME = "nemoclaw"`) means the shared cluster +// container holds every sandbox, so recreating the +// gateway SIGKILLs them. Only "stale" reflects confirmed +// drift; "active-unnamed" defers so the port-availability +// check can fire its own diagnostic if applicable. // - "defer" — Docker-driver path: postpone the recreate to step [2/8] -// when no live sandboxes are at risk. +// when no live sandboxes are at risk, or when the +// reuse state hasn't confirmed a destructive recreate. // - "destroy-legacy" — pre-Docker-driver path: destroy immediately so the // port frees up for the upcoming port-availability checks. // - "noop" — recorded state needs no preflight cleanup. @@ -35,7 +39,11 @@ export function preflightGatewayCleanupDecision(opts: { if (opts.gatewayReuseState !== "stale" && opts.gatewayReuseState !== "active-unnamed") { return "noop"; } - if (opts.isDockerDriverGatewayEnabled && opts.liveSandboxNames.length > 0) { + if ( + opts.isDockerDriverGatewayEnabled && + opts.gatewayReuseState === "stale" && + opts.liveSandboxNames.length > 0 + ) { return "refuse"; } return opts.isDockerDriverGatewayEnabled ? "defer" : "destroy-legacy"; From 0ef1f56470297468a85768165430568b21f4ad4c Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Mon, 1 Jun 2026 08:51:10 +0000 Subject: [PATCH 04/27] refactor(state): introduce getGatewayName resolver for parallel-gateway groundwork Signed-off-by: Tinson Lai --- docs/reference/commands.mdx | 1 - docs/reference/troubleshooting.mdx | 13 +- src/lib/onboard.ts | 14 +- ...preflight-gateway-cleanup-decision.test.ts | 204 ++---------------- .../preflight-gateway-cleanup-decision.ts | 103 +-------- src/lib/state/gateway-name.ts | 23 ++ src/lib/state/gateway.ts | 33 +-- test/gateway-state.test.ts | 38 ---- test/state-gateway-name.test.ts | 29 +++ 9 files changed, 83 insertions(+), 375 deletions(-) create mode 100644 src/lib/state/gateway-name.ts create mode 100644 test/state-gateway-name.test.ts diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index 08a17b4d783..d1a45c5a4e7 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -1325,7 +1325,6 @@ All ports must be non-privileged integers between 1024 and 65535. If a port value is not a valid integer or falls outside the allowed range, the CLI exits with an error. `NEMOCLAW_GATEWAY_PORT` also cannot overlap the configured dashboard, vLLM, Ollama, or Ollama proxy ports, and cannot use the dashboard auto-allocation range `18789` through `18799` or the default inference/proxy ports `8000`, `11434`, and `11435`. -`NEMOCLAW_GATEWAY_PORT` relocates the singleton OpenShell gateway; it does not spawn a second gateway alongside an existing one. Onboarding refuses to change the port while any sandbox is `Ready` or `Running` and prints the names you must stop first. See [troubleshooting](/reference/troubleshooting#port-conflicts) for the recovery steps and [#3053](https://github.com/NVIDIA/NemoClaw/issues/3053) for concurrent-instance support. On non-WSL hosts, `NEMOCLAW_OLLAMA_PORT` and `NEMOCLAW_OLLAMA_PROXY_PORT` must be different. If you run Ollama on port 11435, set `NEMOCLAW_OLLAMA_PROXY_PORT` to another free port before onboarding. diff --git a/docs/reference/troubleshooting.mdx b/docs/reference/troubleshooting.mdx index 5a4909f9ad2..b89ebeac30a 100644 --- a/docs/reference/troubleshooting.mdx +++ b/docs/reference/troubleshooting.mdx @@ -214,14 +214,6 @@ or Ollama proxy ports: $ NEMOCLAW_GATEWAY_PORT=8990 nemoclaw onboard ``` -`NEMOCLAW_GATEWAY_PORT` relocates the singleton OpenShell gateway — it does -not spawn a second gateway alongside the existing one. Changing the port while -a sandbox is live triggers a destructive gateway recreate, so onboarding now -refuses the change while any sandbox is in `Ready` or `Running` state and -prints the names you need to stop first. Concurrent NemoClaw instances on a -single host are tracked in -[#3053](https://github.com/NVIDIA/NemoClaw/issues/3053). - Remote/headless hosts can bind the OpenShell gateway to all IPv4 interfaces: ```console @@ -235,10 +227,7 @@ See [Environment Variables](/reference/commands#environment-variables) for the f ### Running multiple sandboxes simultaneously -Multiple sandboxes share a single OpenShell gateway on the same host. Each -sandbox gets its own dashboard port and SSH tunnel; the gateway, gateway port -(`NEMOCLAW_GATEWAY_PORT`, default `8080`), and cluster container are shared. - +Each sandbox requires its own dashboard port. If you onboard a second sandbox without overriding the port, onboarding uses the next free port in the `18789` to `18799` range. `onboard` checks `openshell forward list` before starting a new forward, so a second onboard cannot silently take over the first sandbox's port. diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 505ad8d1803..2d05ef2902d 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -437,7 +437,7 @@ const { preflightDashboardPortRangeAvailability, } = require("./onboard/dashboard-port") as typeof import("./onboard/dashboard-port"); const { destroyGatewayForReuse } = require("./onboard/gateway-cleanup") as typeof import("./onboard/gateway-cleanup"); -const { runPreflightGatewayCleanup } = +const { applyPreflightGatewayCleanup } = require("./onboard/preflight-gateway-cleanup-decision") as typeof import("./onboard/preflight-gateway-cleanup-decision"); const { verifyGatewayContainerRunning } = require("./onboard/gateway-container-running") as typeof import("./onboard/gateway-container-running"); @@ -571,7 +571,7 @@ const USE_COLOR = !process.env.NO_COLOR && !!process.stdout.isTTY; const DIM = USE_COLOR ? "\x1b[2m" : ""; const RESET = USE_COLOR ? "\x1b[0m" : ""; let OPENSHELL_BIN: string | null = null; -const GATEWAY_NAME = "nemoclaw"; +const GATEWAY_NAME = gatewayState.getGatewayName(GATEWAY_PORT); const OPENCLAW_LAUNCH_AGENT_PLIST = "~/Library/LaunchAgents/ai.openclaw.gateway.plist"; const BRAVE_SEARCH_HELP_URL = "https://brave.com/search/api/"; @@ -1989,13 +1989,13 @@ async function preflight( exitProcess: (code) => process.exit(code), }); - gatewayReuseState = runPreflightGatewayCleanup({ + gatewayReuseState = applyPreflightGatewayCleanup({ gatewayReuseState, - isLinuxDockerDriverGatewayEnabled, - runCaptureOpenshell, + isDockerDriverGatewayEnabled: isLinuxDockerDriverGatewayEnabled(), + cliDisplayName: cliDisplayName(), + dashboardPort: DASHBOARD_PORT, + log: console.log, runOpenshell, - cliName, - cliDisplayName, destroyGateway, destroyGatewayForReuse, }); diff --git a/src/lib/onboard/preflight-gateway-cleanup-decision.test.ts b/src/lib/onboard/preflight-gateway-cleanup-decision.test.ts index bd29f8814ff..b2e8fad1dec 100644 --- a/src/lib/onboard/preflight-gateway-cleanup-decision.test.ts +++ b/src/lib/onboard/preflight-gateway-cleanup-decision.test.ts @@ -7,60 +7,25 @@ import type { GatewayReuseState } from "../state/gateway"; import { PREFLIGHT_DEFERRED_RECREATE_MESSAGE, - PREFLIGHT_LIVE_SANDBOX_REFUSAL_HEADER, applyPreflightGatewayCleanup, preflightGatewayCleanupDecision, - runPreflightGatewayCleanup, } from "./preflight-gateway-cleanup-decision"; describe("preflightGatewayCleanupDecision", () => { - it("defers when state is stale, Docker-driver gateway is enabled, and no live sandboxes", () => { + it("defers when state is stale and Docker-driver gateway is enabled", () => { expect( preflightGatewayCleanupDecision({ gatewayReuseState: "stale", isDockerDriverGatewayEnabled: true, - liveSandboxNames: [], }), ).toBe("defer"); }); - it("defers when state is active-unnamed and no live sandboxes", () => { + it("defers when state is active-unnamed and Docker-driver gateway is enabled", () => { expect( preflightGatewayCleanupDecision({ gatewayReuseState: "active-unnamed", isDockerDriverGatewayEnabled: true, - liveSandboxNames: [], - }), - ).toBe("defer"); - }); - - it("refuses on confirmed drift (stale) when live sandboxes exist", () => { - expect( - preflightGatewayCleanupDecision({ - gatewayReuseState: "stale", - isDockerDriverGatewayEnabled: true, - liveSandboxNames: ["sandbox-a"], - }), - ).toBe("refuse"); - expect( - preflightGatewayCleanupDecision({ - gatewayReuseState: "stale", - isDockerDriverGatewayEnabled: true, - liveSandboxNames: ["sandbox-a", "sandbox-b"], - }), - ).toBe("refuse"); - }); - - it("defers on active-unnamed even with live sandboxes so the port-availability check can run", () => { - // `active-unnamed` means there is an endpoint without a named-gateway - // metadata entry; the port loop may still fail on its own (e.g. host - // listener squatting on the configured gateway port), so deferring lets - // that diagnostic fire instead of pre-empting with the refuse message. - expect( - preflightGatewayCleanupDecision({ - gatewayReuseState: "active-unnamed", - isDockerDriverGatewayEnabled: true, - liveSandboxNames: ["sandbox-a"], }), ).toBe("defer"); }); @@ -70,49 +35,30 @@ describe("preflightGatewayCleanupDecision", () => { preflightGatewayCleanupDecision({ gatewayReuseState: "stale", isDockerDriverGatewayEnabled: false, - liveSandboxNames: [], }), ).toBe("destroy-legacy"); expect( preflightGatewayCleanupDecision({ gatewayReuseState: "active-unnamed", isDockerDriverGatewayEnabled: false, - liveSandboxNames: [], - }), - ).toBe("destroy-legacy"); - }); - - it("destroys legacy gateway even with live sandboxes when Docker-driver gateway is not enabled", () => { - // Legacy package-managed gateway path is unaffected by the live-sandbox - // guard — that path destroys/restarts the gateway process without - // touching the openshell-cluster-* container. - expect( - preflightGatewayCleanupDecision({ - gatewayReuseState: "stale", - isDockerDriverGatewayEnabled: false, - liveSandboxNames: ["sandbox-a"], }), ).toBe("destroy-legacy"); }); - it("returns noop for non-stale states regardless of driver or sandbox set", () => { + it("returns noop for non-stale states regardless of driver", () => { for (const state of ["healthy", "missing", "foreign-active"] as const) { - for (const liveSandboxNames of [[], ["sandbox-a"]]) { - expect( - preflightGatewayCleanupDecision({ - gatewayReuseState: state, - isDockerDriverGatewayEnabled: true, - liveSandboxNames, - }), - ).toBe("noop"); - expect( - preflightGatewayCleanupDecision({ - gatewayReuseState: state, - isDockerDriverGatewayEnabled: false, - liveSandboxNames, - }), - ).toBe("noop"); - } + expect( + preflightGatewayCleanupDecision({ + gatewayReuseState: state, + isDockerDriverGatewayEnabled: true, + }), + ).toBe("noop"); + expect( + preflightGatewayCleanupDecision({ + gatewayReuseState: state, + isDockerDriverGatewayEnabled: false, + }), + ).toBe("noop"); } }); }); @@ -121,7 +67,6 @@ describe("applyPreflightGatewayCleanup", () => { function makeDeps(overrides: { gatewayReuseState: GatewayReuseState; isDockerDriverGatewayEnabled: boolean; - liveSandboxNames?: readonly string[]; }) { const log = vi.fn(); const runOpenshell = vi.fn(() => ({ status: 0 })); @@ -136,28 +81,21 @@ describe("applyPreflightGatewayCleanup", () => { destroy(); return "missing"; }); - const exitProcess = vi.fn((_code: number) => { - throw new Error("exit"); - }) as unknown as (code: number) => never; return { deps: { gatewayReuseState: overrides.gatewayReuseState, isDockerDriverGatewayEnabled: overrides.isDockerDriverGatewayEnabled, cliDisplayName: "NemoClaw", - cliCommandName: "nemoclaw", dashboardPort: 8081, - liveSandboxNames: overrides.liveSandboxNames ?? [], log, runOpenshell, destroyGateway, destroyGatewayForReuse, - exitProcess, }, log, runOpenshell, destroyGateway, destroyGatewayForReuse, - exitProcess, }; } @@ -169,7 +107,6 @@ describe("applyPreflightGatewayCleanup", () => { expect(ctx.destroyGateway).not.toHaveBeenCalled(); expect(ctx.destroyGatewayForReuse).not.toHaveBeenCalled(); expect(ctx.runOpenshell).not.toHaveBeenCalled(); - expect(ctx.exitProcess).not.toHaveBeenCalled(); }); it("destroys the legacy gateway and stops the dashboard forward on the non-Docker-driver path", () => { @@ -182,37 +119,6 @@ describe("applyPreflightGatewayCleanup", () => { }); expect(ctx.destroyGatewayForReuse).toHaveBeenCalledTimes(1); expect(ctx.destroyGateway).toHaveBeenCalledTimes(1); - expect(ctx.exitProcess).not.toHaveBeenCalled(); - }); - - it("refuses with structured guidance when a live sandbox is at risk on the Docker-driver path", () => { - const ctx = makeDeps({ - gatewayReuseState: "stale", - isDockerDriverGatewayEnabled: true, - liveSandboxNames: ["sandbox-a", "sandbox-b"], - }); - expect(() => applyPreflightGatewayCleanup(ctx.deps)).toThrow("exit"); - const logged = ctx.log.mock.calls.map(([line]) => line).join("\n"); - expect(logged).toContain(PREFLIGHT_LIVE_SANDBOX_REFUSAL_HEADER); - expect(logged).toContain("Live sandbox(es): sandbox-a, sandbox-b"); - expect(logged).toContain("nemoclaw sandbox-a stop"); - expect(logged).toContain("nemoclaw sandbox-b stop"); - expect(logged).toContain("NEMOCLAW_GATEWAY_PORT"); - expect(logged).toContain("#3053"); - expect(ctx.exitProcess).toHaveBeenCalledWith(1); - expect(ctx.destroyGateway).not.toHaveBeenCalled(); - expect(ctx.destroyGatewayForReuse).not.toHaveBeenCalled(); - }); - - it("does not refuse the legacy non-Docker-driver path even when live sandboxes exist", () => { - const ctx = makeDeps({ - gatewayReuseState: "stale", - isDockerDriverGatewayEnabled: false, - liveSandboxNames: ["sandbox-a"], - }); - const next = applyPreflightGatewayCleanup(ctx.deps); - expect(next).toBe("missing"); - expect(ctx.exitProcess).not.toHaveBeenCalled(); }); it("is a no-op for healthy / missing / foreign-active states", () => { @@ -224,86 +130,6 @@ describe("applyPreflightGatewayCleanup", () => { expect(ctx.destroyGateway).not.toHaveBeenCalled(); expect(ctx.destroyGatewayForReuse).not.toHaveBeenCalled(); expect(ctx.runOpenshell).not.toHaveBeenCalled(); - expect(ctx.exitProcess).not.toHaveBeenCalled(); - } - }); -}); - -describe("runPreflightGatewayCleanup", () => { - function makeDeps(overrides: { - gatewayReuseState: GatewayReuseState; - isDockerDriverGatewayEnabled: boolean; - sandboxListOutput?: string; - }) { - const runCaptureOpenshell = vi.fn((args: string[], _opts: { ignoreError: true }) => { - expect(args).toEqual(["sandbox", "list"]); - return overrides.sandboxListOutput ?? ""; - }); - const runOpenshell = vi.fn(() => ({ status: 0 })); - const destroyGateway = vi.fn(() => true); - const destroyGatewayForReuse = vi.fn< - ( - destroy: () => boolean, - success: string, - failure: string, - ) => GatewayReuseState - >(() => "missing"); - return { - runCaptureOpenshell, - runOpenshell, - destroyGateway, - destroyGatewayForReuse, - deps: { - gatewayReuseState: overrides.gatewayReuseState, - isLinuxDockerDriverGatewayEnabled: () => overrides.isDockerDriverGatewayEnabled, - runCaptureOpenshell, - runOpenshell, - cliName: () => "nemoclaw", - cliDisplayName: () => "NemoClaw", - destroyGateway, - destroyGatewayForReuse, - }, - }; - } - - it("queries sandbox list and defers when no live sandboxes exist on the Docker-driver path", () => { - const ctx = makeDeps({ - gatewayReuseState: "stale", - isDockerDriverGatewayEnabled: true, - sandboxListOutput: "NAME STATUS AGE\nsandbox-a Provisioning 10s\n", - }); - const exitSpy = vi.spyOn(process, "exit").mockImplementation((() => { - throw new Error("unexpected exit"); - }) as never); - try { - const next = runPreflightGatewayCleanup(ctx.deps); - expect(next).toBe("stale"); - expect(ctx.runCaptureOpenshell).toHaveBeenCalledTimes(1); - expect(ctx.destroyGateway).not.toHaveBeenCalled(); - expect(exitSpy).not.toHaveBeenCalled(); - } finally { - exitSpy.mockRestore(); - } - }); - - it("refuses through process.exit(1) when sandbox list reports a live sandbox", () => { - const ctx = makeDeps({ - gatewayReuseState: "stale", - isDockerDriverGatewayEnabled: true, - sandboxListOutput: "NAME STATUS AGE\nsandbox-a Ready 5m\n", - }); - const exitSpy = vi.spyOn(process, "exit").mockImplementation((() => { - throw new Error("exit"); - }) as never); - try { - expect(() => runPreflightGatewayCleanup(ctx.deps)).toThrow("exit"); - expect(exitSpy).toHaveBeenCalledWith(1); - expect(ctx.runCaptureOpenshell).toHaveBeenCalledWith(["sandbox", "list"], { - ignoreError: true, - }); - expect(ctx.destroyGateway).not.toHaveBeenCalled(); - } finally { - exitSpy.mockRestore(); } }); }); diff --git a/src/lib/onboard/preflight-gateway-cleanup-decision.ts b/src/lib/onboard/preflight-gateway-cleanup-decision.ts index ed75bb60432..d704bc925ea 100644 --- a/src/lib/onboard/preflight-gateway-cleanup-decision.ts +++ b/src/lib/onboard/preflight-gateway-cleanup-decision.ts @@ -1,51 +1,20 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { DASHBOARD_PORT } from "../core/ports"; -import { listLiveSandboxNames, type GatewayReuseState } from "../state/gateway"; +import type { GatewayReuseState } from "../state/gateway"; -export type PreflightGatewayCleanupAction = - | "defer" - | "destroy-legacy" - | "refuse" - | "noop"; +export type PreflightGatewayCleanupAction = "defer" | "destroy-legacy" | "noop"; export const PREFLIGHT_DEFERRED_RECREATE_MESSAGE = " ⚠ Gateway will be recreated when sandbox creation starts — this will affect running sandboxes."; -export const PREFLIGHT_LIVE_SANDBOX_REFUSAL_HEADER = - " ✗ Refusing to recreate gateway: live sandbox(es) would be destroyed."; - -// Decision for the preflight gateway cleanup step. Returns: -// - "refuse" — confirmed drift would trigger a destructive gateway -// recreate while one or more sandboxes are live -// (Ready/Running). The singleton-gateway design -// (`GATEWAY_NAME = "nemoclaw"`) means the shared cluster -// container holds every sandbox, so recreating the -// gateway SIGKILLs them. Only "stale" reflects confirmed -// drift; "active-unnamed" defers so the port-availability -// check can fire its own diagnostic if applicable. -// - "defer" — Docker-driver path: postpone the recreate to step [2/8] -// when no live sandboxes are at risk, or when the -// reuse state hasn't confirmed a destructive recreate. -// - "destroy-legacy" — pre-Docker-driver path: destroy immediately so the -// port frees up for the upcoming port-availability checks. -// - "noop" — recorded state needs no preflight cleanup. export function preflightGatewayCleanupDecision(opts: { gatewayReuseState: GatewayReuseState; isDockerDriverGatewayEnabled: boolean; - liveSandboxNames: readonly string[]; }): PreflightGatewayCleanupAction { if (opts.gatewayReuseState !== "stale" && opts.gatewayReuseState !== "active-unnamed") { return "noop"; } - if ( - opts.isDockerDriverGatewayEnabled && - opts.gatewayReuseState === "stale" && - opts.liveSandboxNames.length > 0 - ) { - return "refuse"; - } return opts.isDockerDriverGatewayEnabled ? "defer" : "destroy-legacy"; } @@ -53,9 +22,7 @@ export interface PreflightGatewayCleanupDeps { gatewayReuseState: GatewayReuseState; isDockerDriverGatewayEnabled: boolean; cliDisplayName: string; - cliCommandName: string; dashboardPort: number; - liveSandboxNames: readonly string[]; log: (line: string) => void; runOpenshell: (args: string[], options: { ignoreError: true }) => unknown; destroyGateway: () => boolean; @@ -64,46 +31,6 @@ export interface PreflightGatewayCleanupDeps { successMessage: string, failureMessage: string, ) => GatewayReuseState; - exitProcess: (code: number) => never; -} - -export interface RunPreflightGatewayCleanupDeps { - gatewayReuseState: GatewayReuseState; - isLinuxDockerDriverGatewayEnabled: () => boolean; - runCaptureOpenshell: (args: string[], options: { ignoreError: true }) => string; - runOpenshell: (args: string[], options: { ignoreError: true }) => unknown; - cliName: () => string; - cliDisplayName: () => string; - destroyGateway: () => boolean; - destroyGatewayForReuse: ( - destroy: () => boolean, - successMessage: string, - failureMessage: string, - ) => GatewayReuseState; -} - -// Convenience wrapper for the onboard call site: fetches the live-sandbox set -// from `openshell sandbox list`, resolves CLI display/command names, and wires -// `process.exit` for the refuse path. Tests target `applyPreflightGatewayCleanup` -// directly for fine-grained dependency injection. -export function runPreflightGatewayCleanup( - deps: RunPreflightGatewayCleanupDeps, -): GatewayReuseState { - return applyPreflightGatewayCleanup({ - gatewayReuseState: deps.gatewayReuseState, - isDockerDriverGatewayEnabled: deps.isLinuxDockerDriverGatewayEnabled(), - cliDisplayName: deps.cliDisplayName(), - cliCommandName: deps.cliName(), - dashboardPort: DASHBOARD_PORT, - liveSandboxNames: listLiveSandboxNames( - deps.runCaptureOpenshell(["sandbox", "list"], { ignoreError: true }), - ), - log: console.log, - runOpenshell: deps.runOpenshell, - destroyGateway: deps.destroyGateway, - destroyGatewayForReuse: deps.destroyGatewayForReuse, - exitProcess: (code) => process.exit(code), - }); } export function applyPreflightGatewayCleanup( @@ -112,33 +39,7 @@ export function applyPreflightGatewayCleanup( const action = preflightGatewayCleanupDecision({ gatewayReuseState: deps.gatewayReuseState, isDockerDriverGatewayEnabled: deps.isDockerDriverGatewayEnabled, - liveSandboxNames: deps.liveSandboxNames, }); - if (action === "refuse") { - const names = deps.liveSandboxNames.join(", "); - deps.log(PREFLIGHT_LIVE_SANDBOX_REFUSAL_HEADER); - deps.log(` Live sandbox(es): ${names}`); - deps.log( - " Recreating the gateway here would SIGKILL the running sandbox container(s)", - ); - deps.log(" and leave them in Phase=Error."); - deps.log(""); - deps.log(" Resolve with one of:"); - for (const name of deps.liveSandboxNames) { - deps.log(` - ${deps.cliCommandName} ${name} stop`); - } - deps.log( - " - Onboard with the existing NEMOCLAW_GATEWAY_PORT (do not change it).", - ); - deps.log(""); - deps.log( - " Concurrent NemoClaw instances on a single host are tracked in #3053;", - ); - deps.log( - " this refusal protects existing sandboxes until that support lands.", - ); - deps.exitProcess(1); - } if (action === "defer") { deps.log(PREFLIGHT_DEFERRED_RECREATE_MESSAGE); return deps.gatewayReuseState; diff --git a/src/lib/state/gateway-name.ts b/src/lib/state/gateway-name.ts new file mode 100644 index 00000000000..9441fc08561 --- /dev/null +++ b/src/lib/state/gateway-name.ts @@ -0,0 +1,23 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +/** + * NemoClaw's OpenShell gateway name resolver. + * + * NemoClaw currently runs a singleton gateway: every onboard uses the literal + * `"nemoclaw"` name regardless of which port the gateway binds to. That + * invariant makes concurrent NemoClaw instances on a single host impossible + * — changing `NEMOCLAW_GATEWAY_PORT` relocates the singleton instead of + * spawning a second instance. Tracked in NemoClaw#3053. + * + * This module owns the canonical name and exposes a `port`-aware resolver so + * follow-up work can derive per-port names (e.g. `"nemoclaw-8081"`) without + * touching every call site again. Until that work lands, `getGatewayName` + * returns the singleton name for every port. + */ + +export const DEFAULT_GATEWAY_NAME = "nemoclaw"; + +export function getGatewayName(_port: number): string { + return DEFAULT_GATEWAY_NAME; +} diff --git a/src/lib/state/gateway.ts b/src/lib/state/gateway.ts index 7afa90fa3bd..fa5b6896047 100644 --- a/src/lib/state/gateway.ts +++ b/src/lib/state/gateway.ts @@ -8,7 +8,11 @@ * returns a typed result — no I/O, no side effects. */ -const GATEWAY_NAME = "nemoclaw"; +import { DEFAULT_GATEWAY_NAME, getGatewayName } from "./gateway-name"; + +export { DEFAULT_GATEWAY_NAME, getGatewayName }; + +const GATEWAY_NAME = DEFAULT_GATEWAY_NAME; const ANSI_RE = /\x1b\[[0-9;]*m/g; @@ -51,35 +55,10 @@ export function parseSandboxStatus(output: string, sandboxName: string): string * sandbox stays in "Running" phase which is functionally equivalent to * "Ready" — the agent is live and the gateway is reachable inside. */ -function isLiveSandboxRow(cols: readonly string[]): boolean { - return (cols.includes("Ready") || cols.includes("Running")) && !cols.includes("NotReady"); -} - export function isSandboxReady(output: string, sandboxName: string): boolean { const cols = parseSandboxRow(output, sandboxName); if (!cols) return false; - return isLiveSandboxRow(cols); -} - -/** - * Enumerate sandbox names from `openshell sandbox list` output whose state - * column indicates a live workload — "Ready" or "Running" and not "NotReady". - * The singleton-gateway design means recreating the gateway destroys the - * shared `openshell-cluster-*` container holding every sandbox, so the - * preflight cleanup decision uses this set to refuse destructive recreates. - */ -export function listLiveSandboxNames(output: string): string[] { - if (typeof output !== "string") return []; - const clean = stripAnsi(output); - const names: string[] = []; - for (const line of clean.split("\n")) { - const cols = line.trim().split(/\s+/); - if (cols.length < 2) continue; - const name = cols[0]; - if (!name) continue; - if (isLiveSandboxRow(cols)) names.push(name); - } - return names; + return (cols.includes("Ready") || cols.includes("Running")) && !cols.includes("NotReady"); } /** diff --git a/test/gateway-state.test.ts b/test/gateway-state.test.ts index 0090c44aaa6..7b564e77a12 100644 --- a/test/gateway-state.test.ts +++ b/test/gateway-state.test.ts @@ -14,7 +14,6 @@ import { hasStaleGateway, hasActiveGatewayInfo, getReportedGatewayName, - listLiveSandboxNames, shouldSelectNamedGatewayForReuse, parseSandboxPhase, } from "../src/lib/state/gateway.js"; @@ -225,43 +224,6 @@ describe("isGatewayHealthy", () => { }); }); -describe("listLiveSandboxNames", () => { - it("returns names of Ready and Running sandboxes only", () => { - const output = [ - "NAME STATUS AGE", - "sandbox-a Ready 5m", - "sandbox-b Running 2m", - "sandbox-c Provisioning 10s", - "sandbox-d NotReady 1m", - "sandbox-e Failed 30s", - ].join("\n"); - expect(listLiveSandboxNames(output)).toEqual(["sandbox-a", "sandbox-b"]); - }); - - it("treats NotReady as not live even when Ready also appears on the row", () => { - // Defensive: prefer the negative signal so a sandbox in transitional state - // isn't counted as live and accidentally blocks a legitimate recreate. - const output = "sandbox-x Ready NotReady 3m"; - expect(listLiveSandboxNames(output)).toEqual([]); - }); - - it("strips ANSI escapes before parsing", () => { - const output = - "\x1b[1mNAME STATUS AGE\x1b[0m\nsandbox-a \x1b[32mReady\x1b[0m 5m"; - expect(listLiveSandboxNames(output)).toEqual(["sandbox-a"]); - }); - - it("returns an empty array for empty / non-string input", () => { - expect(listLiveSandboxNames("")).toEqual([]); - expect(listLiveSandboxNames(undefined as unknown as string)).toEqual([]); - }); - - it("returns an empty array when no rows match", () => { - const output = ["NAME STATUS AGE", "sandbox-a Provisioning 10s"].join("\n"); - expect(listLiveSandboxNames(output)).toEqual([]); - }); -}); - describe("parseSandboxPhase", () => { it("extracts Ready phase from sandbox get output", () => { const output = ["Sandbox:", "", " Id: abc", " Name: my-assistant", " Phase: Ready"].join( diff --git a/test/state-gateway-name.test.ts b/test/state-gateway-name.test.ts new file mode 100644 index 00000000000..49fc0e234c0 --- /dev/null +++ b/test/state-gateway-name.test.ts @@ -0,0 +1,29 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; + +import { DEFAULT_GATEWAY_NAME, getGatewayName } from "../src/lib/state/gateway-name.js"; + +describe("DEFAULT_GATEWAY_NAME", () => { + it("exposes the canonical singleton gateway name 'nemoclaw'", () => { + expect(DEFAULT_GATEWAY_NAME).toBe("nemoclaw"); + }); +}); + +describe("getGatewayName", () => { + it("returns the singleton name for the default 8080 gateway port", () => { + expect(getGatewayName(8080)).toBe(DEFAULT_GATEWAY_NAME); + }); + + it("returns the singleton name for non-default ports until per-port names land (NemoClaw#3053)", () => { + // Today NemoClaw runs a single gateway regardless of which port is + // configured. The follow-up work that flips this resolver to per-port + // names so concurrent NemoClaw instances can coexist on a single host is + // tracked in NemoClaw#3053. Lock the current behaviour so the call-site + // refactor lands first without behavioural drift. + expect(getGatewayName(8081)).toBe(DEFAULT_GATEWAY_NAME); + expect(getGatewayName(8990)).toBe(DEFAULT_GATEWAY_NAME); + expect(getGatewayName(65535)).toBe(DEFAULT_GATEWAY_NAME); + }); +}); From a1c55fecdb28bd62a3c436410c4773629228c2f4 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Mon, 1 Jun 2026 08:59:33 +0000 Subject: [PATCH 05/27] feat(registry): track per-sandbox gateway name with singleton backfill Signed-off-by: Tinson Lai --- src/lib/state/registry.ts | 20 ++++++++++++++++++++ test/registry.test.ts | 23 +++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/src/lib/state/registry.ts b/src/lib/state/registry.ts index 037c99f79f8..ffa6cfca6e7 100644 --- a/src/lib/state/registry.ts +++ b/src/lib/state/registry.ts @@ -6,6 +6,7 @@ import path from "node:path"; import { ensureConfigDir, readConfigFile, writeConfigFile } from "./config-io"; import { isErrnoException } from "../core/errno"; +import { DEFAULT_GATEWAY_NAME } from "./gateway-name"; import type { MessagingChannelConfig } from "../messaging-channel-config"; export interface CustomPolicyEntry { @@ -44,6 +45,13 @@ export interface SandboxEntry { hermesDashboardTui?: boolean; disabledChannels?: string[]; dashboardPort?: number | null; + /** + * OpenShell gateway name this sandbox is bound to. Optional for backward + * compatibility — legacy entries created before per-sandbox gateway tracking + * resolve to {@link DEFAULT_GATEWAY_NAME} via {@link getSandboxGatewayName}. + * Tracked in NemoClaw#3053; currently every sandbox uses the singleton name. + */ + gatewayName?: string; } export interface SandboxRegistry { @@ -183,6 +191,17 @@ export function getSandbox(name: string): SandboxEntry | null { return data.sandboxes[name] || null; } +/** + * Resolve the OpenShell gateway name a sandbox is bound to, backfilling the + * singleton {@link DEFAULT_GATEWAY_NAME} for legacy entries that predate the + * `gatewayName` field. Callers should prefer this over reading + * `entry.gatewayName` directly so the backfill stays in one place. + */ +export function getSandboxGatewayName(name: string): string { + const entry = getSandbox(name); + return entry?.gatewayName || DEFAULT_GATEWAY_NAME; +} + export function getDefault(): string | null { const data = load(); if (data.defaultSandbox && data.sandboxes[data.defaultSandbox]) { @@ -232,6 +251,7 @@ export function registerSandbox(entry: SandboxEntry): void { ? [...entry.disabledChannels] : undefined, dashboardPort: entry.dashboardPort ?? undefined, + gatewayName: entry.gatewayName || undefined, }; if (!data.defaultSandbox) { data.defaultSandbox = entry.name; diff --git a/test/registry.test.ts b/test/registry.test.ts index b2b8aee847b..192e784404c 100644 --- a/test/registry.test.ts +++ b/test/registry.test.ts @@ -501,4 +501,27 @@ describe("advisory file locking", () => { expect(sandboxes).toHaveLength(0); expect(defaultSandbox).toBe(null); }); + + it("persists gatewayName when supplied at registration", () => { + registry.registerSandbox({ name: "alpha", gatewayName: "nemoclaw" }); + const sb = registry.getSandbox("alpha"); + expect(sb.gatewayName).toBe("nemoclaw"); + }); + + it("getSandboxGatewayName backfills the singleton name for legacy entries", () => { + // Legacy entries created before per-sandbox gateway tracking lack the + // `gatewayName` field; the accessor must transparently return the + // singleton default so callers never see undefined. NemoClaw#3053. + registry.registerSandbox({ name: "legacy" }); + expect(registry.getSandboxGatewayName("legacy")).toBe("nemoclaw"); + }); + + it("getSandboxGatewayName returns the persisted value when present", () => { + registry.registerSandbox({ name: "alpha", gatewayName: "nemoclaw-8081" }); + expect(registry.getSandboxGatewayName("alpha")).toBe("nemoclaw-8081"); + }); + + it("getSandboxGatewayName falls back to the singleton for unknown sandbox names", () => { + expect(registry.getSandboxGatewayName("does-not-exist")).toBe("nemoclaw"); + }); }); From fa4550762968788c27181380a13ab8983833ee9c Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Mon, 1 Jun 2026 09:25:15 +0000 Subject: [PATCH 06/27] refactor(state): single source for gateway name + tighten registry accessor Signed-off-by: Tinson Lai --- src/lib/actions/sandbox/connect.ts | 2 +- src/lib/actions/sandbox/doctor.ts | 2 +- src/lib/actions/sandbox/snapshot.ts | 2 +- src/lib/adapters/openshell/gateway-drift.ts | 3 +-- src/lib/domain/uninstall/paths.ts | 4 +++- src/lib/onboard.ts | 2 +- src/lib/resources-cmd.ts | 3 +-- src/lib/state/gateway-name.ts | 4 ++-- src/lib/state/gateway.ts | 14 ++++++-------- src/lib/state/registry.ts | 18 +++++++++++------- test/registry.test.ts | 6 +++--- test/state-gateway-name.test.ts | 9 ++++----- 12 files changed, 35 insertions(+), 34 deletions(-) diff --git a/src/lib/actions/sandbox/connect.ts b/src/lib/actions/sandbox/connect.ts index 236f18f4302..9ca345ce342 100644 --- a/src/lib/actions/sandbox/connect.ts +++ b/src/lib/actions/sandbox/connect.ts @@ -43,7 +43,7 @@ import { shouldApplyVmDnsMonkeypatch, } from "./vm-dns-monkeypatch"; -const NEMOCLAW_GATEWAY_NAME = "nemoclaw"; +import { DEFAULT_GATEWAY_NAME as NEMOCLAW_GATEWAY_NAME } from "../../state/gateway-name"; export type SandboxConnectOptions = { probeOnly?: boolean; diff --git a/src/lib/actions/sandbox/doctor.ts b/src/lib/actions/sandbox/doctor.ts index 8555b8a2e5b..b22b0cfaec9 100644 --- a/src/lib/actions/sandbox/doctor.ts +++ b/src/lib/actions/sandbox/doctor.ts @@ -30,7 +30,7 @@ import * as shields from "../../shields"; import { buildStatusCommandDeps } from "../../status-command-deps"; import { B, D, G, R, RD, YW } from "../../cli/terminal-style"; -const NEMOCLAW_GATEWAY_NAME = "nemoclaw"; +import { DEFAULT_GATEWAY_NAME as NEMOCLAW_GATEWAY_NAME } from "../../state/gateway-name"; type DoctorStatus = "ok" | "warn" | "fail" | "info"; diff --git a/src/lib/actions/sandbox/snapshot.ts b/src/lib/actions/sandbox/snapshot.ts index 8a122540877..adeaa608372 100644 --- a/src/lib/actions/sandbox/snapshot.ts +++ b/src/lib/actions/sandbox/snapshot.ts @@ -27,7 +27,7 @@ const B = useColor ? "\x1b[1m" : ""; const D = useColor ? "\x1b[2m" : ""; const R = useColor ? "\x1b[0m" : ""; -const NEMOCLAW_GATEWAY_NAME = "nemoclaw"; +import { DEFAULT_GATEWAY_NAME as NEMOCLAW_GATEWAY_NAME } from "../../state/gateway-name"; export type SnapshotRequest = | { kind: "help" } diff --git a/src/lib/adapters/openshell/gateway-drift.ts b/src/lib/adapters/openshell/gateway-drift.ts index b0e7e1e7def..9589b284938 100644 --- a/src/lib/adapters/openshell/gateway-drift.ts +++ b/src/lib/adapters/openshell/gateway-drift.ts @@ -4,13 +4,12 @@ import { CLI_DISPLAY_NAME, CLI_NAME } from "../../cli/branding"; import { isOpenShellProtobufSchemaMismatch } from "../../runtime-recovery"; import { isGatewayHealthy } from "../../state/gateway"; +import { DEFAULT_GATEWAY_NAME } from "../../state/gateway-name"; import { dockerContainerInspectFormat } from "../docker"; import { stripAnsi } from "./client"; import { captureOpenshell, getInstalledOpenshellVersionOrNull } from "./runtime"; import { OPENSHELL_PROBE_TIMEOUT_MS } from "./timeouts"; -const DEFAULT_GATEWAY_NAME = "nemoclaw"; - export type GatewayClusterImageDrift = { containerName: string; currentImage: string; diff --git a/src/lib/domain/uninstall/paths.ts b/src/lib/domain/uninstall/paths.ts index 32e8602ca37..0368460cb04 100644 --- a/src/lib/domain/uninstall/paths.ts +++ b/src/lib/domain/uninstall/paths.ts @@ -3,7 +3,9 @@ import path from "node:path"; -export const DEFAULT_GATEWAY_NAME = "nemoclaw"; +import { DEFAULT_GATEWAY_NAME } from "../../state/gateway-name"; + +export { DEFAULT_GATEWAY_NAME }; export const NEMOCLAW_PROVIDERS = [ "nvidia-nim", "vllm-local", diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 2d05ef2902d..6db0caa20d9 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -6624,7 +6624,7 @@ async function onboard(opts: OnboardOptions = {}): Promise { session = onboardSession.saveSession( onboardSession.createSession({ mode: isNonInteractive() ? "non-interactive" : "interactive", - metadata: { gatewayName: "nemoclaw", fromDockerfile: fromDockerfile || null }, + metadata: { gatewayName: GATEWAY_NAME, fromDockerfile: fromDockerfile || null }, }), ); } diff --git a/src/lib/resources-cmd.ts b/src/lib/resources-cmd.ts index 1d87d896a98..eee38a77f1b 100644 --- a/src/lib/resources-cmd.ts +++ b/src/lib/resources-cmd.ts @@ -16,8 +16,7 @@ import { spawnSync, execSync } from "child_process"; import * as YAML from "yaml"; import { dockerSpawnSync } from "./adapters/docker"; - -const GATEWAY_NAME = "nemoclaw"; +import { DEFAULT_GATEWAY_NAME as GATEWAY_NAME } from "./state/gateway-name"; function getGatewayContainer(): string { return `openshell-cluster-${GATEWAY_NAME}`; diff --git a/src/lib/state/gateway-name.ts b/src/lib/state/gateway-name.ts index 9441fc08561..28f5b38589b 100644 --- a/src/lib/state/gateway-name.ts +++ b/src/lib/state/gateway-name.ts @@ -7,8 +7,8 @@ * NemoClaw currently runs a singleton gateway: every onboard uses the literal * `"nemoclaw"` name regardless of which port the gateway binds to. That * invariant makes concurrent NemoClaw instances on a single host impossible - * — changing `NEMOCLAW_GATEWAY_PORT` relocates the singleton instead of - * spawning a second instance. Tracked in NemoClaw#3053. + * — changing the gateway port relocates the singleton instead of spawning a + * second instance. * * This module owns the canonical name and exposes a `port`-aware resolver so * follow-up work can derive per-port names (e.g. `"nemoclaw-8081"`) without diff --git a/src/lib/state/gateway.ts b/src/lib/state/gateway.ts index fa5b6896047..268d2f2af23 100644 --- a/src/lib/state/gateway.ts +++ b/src/lib/state/gateway.ts @@ -12,8 +12,6 @@ import { DEFAULT_GATEWAY_NAME, getGatewayName } from "./gateway-name"; export { DEFAULT_GATEWAY_NAME, getGatewayName }; -const GATEWAY_NAME = DEFAULT_GATEWAY_NAME; - const ANSI_RE = /\x1b\[[0-9;]*m/g; function stripAnsi(value: string): string { @@ -69,7 +67,7 @@ export function hasStaleGateway(gwInfoOutput: string): boolean { const clean = typeof gwInfoOutput === "string" ? stripAnsi(gwInfoOutput) : ""; return ( clean.length > 0 && - clean.includes(`Gateway: ${GATEWAY_NAME}`) && + clean.includes(`Gateway: ${DEFAULT_GATEWAY_NAME}`) && !clean.includes("No gateway metadata found") ); } @@ -101,7 +99,7 @@ export function hasActiveGatewayInfo(activeGatewayInfoOutput = ""): boolean { ); } -export function isSelectedGateway(statusOutput = "", gatewayName = GATEWAY_NAME): boolean { +export function isSelectedGateway(statusOutput = "", gatewayName = DEFAULT_GATEWAY_NAME): boolean { return getReportedGatewayName(statusOutput) === gatewayName; } @@ -117,12 +115,12 @@ export function isGatewayHealthy( const activeInfo = hasActiveGatewayInfo(activeGatewayInfoOutput); // Primary path: status reports connected and gateway name matches - if (connected && activeGatewayName === GATEWAY_NAME) return true; + if (connected && activeGatewayName === DEFAULT_GATEWAY_NAME) return true; // Fallback: status is empty (ARM64/non-TTY) but gateway info confirms // the named gateway exists and has an active endpoint const statusEmpty = typeof statusOutput === 'string' && stripAnsi(statusOutput).trim().length === 0; - if (statusEmpty && namedGatewayKnown && activeInfo && activeGatewayName === GATEWAY_NAME) return true; + if (statusEmpty && namedGatewayKnown && activeInfo && activeGatewayName === DEFAULT_GATEWAY_NAME) return true; return false; } @@ -139,10 +137,10 @@ export function getGatewayReuseState( const activeGatewayName = getReportedGatewayName(statusOutput) || getReportedGatewayName(activeGatewayInfoOutput); const activeInfo = hasActiveGatewayInfo(activeGatewayInfoOutput); - if (connected && activeGatewayName === GATEWAY_NAME) { + if (connected && activeGatewayName === DEFAULT_GATEWAY_NAME) { return "active-unnamed"; } - if ((connected || activeInfo) && activeGatewayName && activeGatewayName !== GATEWAY_NAME) { + if ((connected || activeInfo) && activeGatewayName && activeGatewayName !== DEFAULT_GATEWAY_NAME) { return "foreign-active"; } if (hasStaleGateway(gwInfoOutput)) { diff --git a/src/lib/state/registry.ts b/src/lib/state/registry.ts index ffa6cfca6e7..77f529f64c8 100644 --- a/src/lib/state/registry.ts +++ b/src/lib/state/registry.ts @@ -49,7 +49,8 @@ export interface SandboxEntry { * OpenShell gateway name this sandbox is bound to. Optional for backward * compatibility — legacy entries created before per-sandbox gateway tracking * resolve to {@link DEFAULT_GATEWAY_NAME} via {@link getSandboxGatewayName}. - * Tracked in NemoClaw#3053; currently every sandbox uses the singleton name. + * Currently every sandbox uses the singleton name; the field exists so + * follow-up work can record per-port gateway names without a schema change. */ gatewayName?: string; } @@ -192,14 +193,17 @@ export function getSandbox(name: string): SandboxEntry | null { } /** - * Resolve the OpenShell gateway name a sandbox is bound to, backfilling the - * singleton {@link DEFAULT_GATEWAY_NAME} for legacy entries that predate the - * `gatewayName` field. Callers should prefer this over reading - * `entry.gatewayName` directly so the backfill stays in one place. + * Resolve the OpenShell gateway name a sandbox is bound to. Returns the + * persisted value when set, or backfills the singleton + * {@link DEFAULT_GATEWAY_NAME} for legacy entries that predate the + * `gatewayName` field. Returns `null` when the sandbox does not exist so the + * caller surfaces a clean lookup failure instead of silently treating a typo + * as the default gateway. */ -export function getSandboxGatewayName(name: string): string { +export function getSandboxGatewayName(name: string): string | null { const entry = getSandbox(name); - return entry?.gatewayName || DEFAULT_GATEWAY_NAME; + if (!entry) return null; + return entry.gatewayName || DEFAULT_GATEWAY_NAME; } export function getDefault(): string | null { diff --git a/test/registry.test.ts b/test/registry.test.ts index 192e784404c..7fd4b3b1b97 100644 --- a/test/registry.test.ts +++ b/test/registry.test.ts @@ -511,7 +511,7 @@ describe("advisory file locking", () => { it("getSandboxGatewayName backfills the singleton name for legacy entries", () => { // Legacy entries created before per-sandbox gateway tracking lack the // `gatewayName` field; the accessor must transparently return the - // singleton default so callers never see undefined. NemoClaw#3053. + // singleton default so callers never see undefined. registry.registerSandbox({ name: "legacy" }); expect(registry.getSandboxGatewayName("legacy")).toBe("nemoclaw"); }); @@ -521,7 +521,7 @@ describe("advisory file locking", () => { expect(registry.getSandboxGatewayName("alpha")).toBe("nemoclaw-8081"); }); - it("getSandboxGatewayName falls back to the singleton for unknown sandbox names", () => { - expect(registry.getSandboxGatewayName("does-not-exist")).toBe("nemoclaw"); + it("getSandboxGatewayName returns null for unknown sandbox names so callers surface the lookup failure", () => { + expect(registry.getSandboxGatewayName("does-not-exist")).toBeNull(); }); }); diff --git a/test/state-gateway-name.test.ts b/test/state-gateway-name.test.ts index 49fc0e234c0..89b2da342a6 100644 --- a/test/state-gateway-name.test.ts +++ b/test/state-gateway-name.test.ts @@ -16,12 +16,11 @@ describe("getGatewayName", () => { expect(getGatewayName(8080)).toBe(DEFAULT_GATEWAY_NAME); }); - it("returns the singleton name for non-default ports until per-port names land (NemoClaw#3053)", () => { + it("returns the singleton name for non-default ports until per-port names land", () => { // Today NemoClaw runs a single gateway regardless of which port is - // configured. The follow-up work that flips this resolver to per-port - // names so concurrent NemoClaw instances can coexist on a single host is - // tracked in NemoClaw#3053. Lock the current behaviour so the call-site - // refactor lands first without behavioural drift. + // configured. Lock the current behaviour so the call-site refactor lands + // first without behavioural drift; the follow-up resolver swap can flip + // the per-port branch in one place. expect(getGatewayName(8081)).toBe(DEFAULT_GATEWAY_NAME); expect(getGatewayName(8990)).toBe(DEFAULT_GATEWAY_NAME); expect(getGatewayName(65535)).toBe(DEFAULT_GATEWAY_NAME); From fdddf5366f0aa16d42abbb0ccbddc137e84e11f2 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Mon, 1 Jun 2026 09:54:44 +0000 Subject: [PATCH 07/27] refactor(state): replace remaining hard-coded gateway names with DEFAULT_GATEWAY_NAME Signed-off-by: Tinson Lai --- src/lib/actions/sandbox/connect.ts | 3 +-- src/lib/actions/sandbox/destroy.ts | 2 +- src/lib/actions/sandbox/doctor.ts | 3 +-- src/lib/actions/sandbox/gateway-state.ts | 5 +++-- src/lib/actions/sandbox/snapshot.ts | 3 +-- src/lib/gateway-runtime-action.ts | 19 ++++++++++--------- src/lib/inference/live.ts | 3 ++- src/lib/state/onboard-session.ts | 5 +++-- 8 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/lib/actions/sandbox/connect.ts b/src/lib/actions/sandbox/connect.ts index 9ca345ce342..7de7d0ce7de 100644 --- a/src/lib/actions/sandbox/connect.ts +++ b/src/lib/actions/sandbox/connect.ts @@ -37,14 +37,13 @@ import { import { getNamedGatewayLifecycleState } from "../../gateway-runtime-action"; import { runSetupDnsProxy } from "../dns"; import { ensureLiveSandboxOrExit, printGatewayLifecycleHint } from "./gateway-state"; +import { DEFAULT_GATEWAY_NAME as NEMOCLAW_GATEWAY_NAME } from "../../state/gateway-name"; import { checkAndRecoverSandboxProcesses } from "./process-recovery"; import { applyOpenShellVmDnsMonkeypatch, shouldApplyVmDnsMonkeypatch, } from "./vm-dns-monkeypatch"; -import { DEFAULT_GATEWAY_NAME as NEMOCLAW_GATEWAY_NAME } from "../../state/gateway-name"; - export type SandboxConnectOptions = { probeOnly?: boolean; }; diff --git a/src/lib/actions/sandbox/destroy.ts b/src/lib/actions/sandbox/destroy.ts index 7e061f8d415..7fa82adea79 100644 --- a/src/lib/actions/sandbox/destroy.ts +++ b/src/lib/actions/sandbox/destroy.ts @@ -23,6 +23,7 @@ import { stopStaleDashboardListeners } from "../../onboard/stale-gateway-cleanup import { stopHostGatewayProcesses } from "../../onboard/host-gateway-process"; import { parseLiveSandboxNames } from "../../runtime-recovery"; import { killTimer as defaultKillShieldsTimer } from "../../shields/timer-control"; +import { DEFAULT_GATEWAY_NAME as NEMOCLAW_GATEWAY_NAME } from "../../state/gateway-name"; import type { Session } from "../../state/onboard-session"; import * as onboardSession from "../../state/onboard-session"; import { resolveNemoclawStateDir } from "../../state/paths"; @@ -78,7 +79,6 @@ type RemoveShieldsStateDeps = { warn?: (message: string) => void; }; -const NEMOCLAW_GATEWAY_NAME = "nemoclaw"; const DASHBOARD_FORWARD_PORT = String(DASHBOARD_PORT); function cleanupGatewayAfterLastSandbox(): void { diff --git a/src/lib/actions/sandbox/doctor.ts b/src/lib/actions/sandbox/doctor.ts index b22b0cfaec9..0179c5cad8e 100644 --- a/src/lib/actions/sandbox/doctor.ts +++ b/src/lib/actions/sandbox/doctor.ts @@ -27,11 +27,10 @@ import { ROOT } from "../../runner"; import { parseLiveSandboxNames } from "../../runtime-recovery"; import * as sandboxVersion from "../../sandbox/version"; import * as shields from "../../shields"; +import { DEFAULT_GATEWAY_NAME as NEMOCLAW_GATEWAY_NAME } from "../../state/gateway-name"; import { buildStatusCommandDeps } from "../../status-command-deps"; import { B, D, G, R, RD, YW } from "../../cli/terminal-style"; -import { DEFAULT_GATEWAY_NAME as NEMOCLAW_GATEWAY_NAME } from "../../state/gateway-name"; - type DoctorStatus = "ok" | "warn" | "fail" | "info"; export type DoctorCheck = { diff --git a/src/lib/actions/sandbox/gateway-state.ts b/src/lib/actions/sandbox/gateway-state.ts index 5f77b0b27f0..ddf4ff4ce5f 100644 --- a/src/lib/actions/sandbox/gateway-state.ts +++ b/src/lib/actions/sandbox/gateway-state.ts @@ -8,6 +8,7 @@ import path from "node:path"; import { CLI_DISPLAY_NAME, CLI_NAME } from "../../cli/branding"; import { parseSandboxPhase } from "../../state/gateway"; +import { DEFAULT_GATEWAY_NAME } from "../../state/gateway-name"; import { getNamedGatewayLifecycleState, recoverNamedGatewayRuntime, @@ -215,7 +216,7 @@ export function reconcileMissingAgainstNamedGateway( ): SandboxGatewayState { const lifecycle = getNamedGatewayLifecycleState(); if (lifecycle.state === "connected_other") { - runOpenshell(["gateway", "select", "nemoclaw"], { + runOpenshell(["gateway", "select", DEFAULT_GATEWAY_NAME], { ignoreError: true, timeout: OPENSHELL_OPERATION_TIMEOUT_MS, }); @@ -257,7 +258,7 @@ export function printWrongGatewayActiveGuidance( activeGateway: string | null | undefined, writer: (message: string) => void = console.error, ): void { - const other = activeGateway && activeGateway !== "nemoclaw" ? activeGateway : "another gateway"; + const other = activeGateway && activeGateway !== DEFAULT_GATEWAY_NAME ? activeGateway : "another gateway"; writer( ` Sandbox '${sandboxName}' is registered against the ${CLI_DISPLAY_NAME} gateway, but the currently active OpenShell gateway is '${other}'. Your sandbox has NOT been removed.`, ); diff --git a/src/lib/actions/sandbox/snapshot.ts b/src/lib/actions/sandbox/snapshot.ts index adeaa608372..950b2f18dd8 100644 --- a/src/lib/actions/sandbox/snapshot.ts +++ b/src/lib/actions/sandbox/snapshot.ts @@ -14,6 +14,7 @@ import { ROOT, run, shellQuote, validateName } from "../../runner"; import { parseLiveSandboxNames } from "../../runtime-recovery"; import { isShieldsDown } from "../../shields"; import { isGatewayHealthy } from "../../state/gateway"; +import { DEFAULT_GATEWAY_NAME as NEMOCLAW_GATEWAY_NAME } from "../../state/gateway-name"; import type { SandboxEntry } from "../../state/registry"; import * as registry from "../../state/registry"; import * as sandboxState from "../../state/sandbox"; @@ -27,8 +28,6 @@ const B = useColor ? "\x1b[1m" : ""; const D = useColor ? "\x1b[2m" : ""; const R = useColor ? "\x1b[0m" : ""; -import { DEFAULT_GATEWAY_NAME as NEMOCLAW_GATEWAY_NAME } from "../../state/gateway-name"; - export type SnapshotRequest = | { kind: "help" } | { kind: "create"; name?: string } diff --git a/src/lib/gateway-runtime-action.ts b/src/lib/gateway-runtime-action.ts index bfcd9b57135..48a91743001 100644 --- a/src/lib/gateway-runtime-action.ts +++ b/src/lib/gateway-runtime-action.ts @@ -7,9 +7,10 @@ const { startGatewayForRecovery } = require("./onboard") as { import { OPENSHELL_OPERATION_TIMEOUT_MS, OPENSHELL_PROBE_TIMEOUT_MS } from "./adapters/openshell/timeouts"; import { stripAnsi } from "./adapters/openshell/client"; import { captureOpenshell, runOpenshell } from "./adapters/openshell/runtime"; +import { DEFAULT_GATEWAY_NAME } from "./state/gateway-name"; function hasNamedGateway(output = ""): boolean { - return stripAnsi(output).includes("Gateway: nemoclaw"); + return stripAnsi(output).includes(`Gateway: ${DEFAULT_GATEWAY_NAME}`); } function getActiveGatewayName(output = ""): string | null { @@ -19,7 +20,7 @@ function getActiveGatewayName(output = ""): string | null { export function getNamedGatewayLifecycleState() { const status = captureOpenshell(["status"], { timeout: OPENSHELL_PROBE_TIMEOUT_MS }); - const gatewayInfo = captureOpenshell(["gateway", "info", "-g", "nemoclaw"], { + const gatewayInfo = captureOpenshell(["gateway", "info", "-g", DEFAULT_GATEWAY_NAME], { timeout: OPENSHELL_PROBE_TIMEOUT_MS, }); const cleanStatus = stripAnsi(status.output); @@ -29,7 +30,7 @@ export function getNamedGatewayLifecycleState() { const refusing = /Connection refused|client error \(Connect\)|tcp connect error/i.test( cleanStatus, ); - if (connected && activeGateway === "nemoclaw" && named) { + if (connected && activeGateway === DEFAULT_GATEWAY_NAME && named) { return { state: "healthy_named", status: status.output, @@ -37,7 +38,7 @@ export function getNamedGatewayLifecycleState() { activeGateway, }; } - if (activeGateway === "nemoclaw" && named && refusing) { + if (activeGateway === DEFAULT_GATEWAY_NAME && named && refusing) { return { state: "named_unreachable", status: status.output, @@ -45,7 +46,7 @@ export function getNamedGatewayLifecycleState() { activeGateway, }; } - if (activeGateway === "nemoclaw" && named) { + if (activeGateway === DEFAULT_GATEWAY_NAME && named) { return { state: "named_unhealthy", status: status.output, @@ -93,13 +94,13 @@ export async function recoverNamedGatewayRuntime(options: RecoverNamedGatewayRun return { recovered: false, before, after: before, attempted: false }; } - runOpenshell(["gateway", "select", "nemoclaw"], { + runOpenshell(["gateway", "select", DEFAULT_GATEWAY_NAME], { ignoreError: true, timeout: OPENSHELL_OPERATION_TIMEOUT_MS, }); let after = getNamedGatewayLifecycleState(); if (after.state === "healthy_named") { - process.env.OPENSHELL_GATEWAY = "nemoclaw"; + process.env.OPENSHELL_GATEWAY = DEFAULT_GATEWAY_NAME; return { recovered: true, before, after, attempted: true, via: "select" }; } @@ -115,13 +116,13 @@ export async function recoverNamedGatewayRuntime(options: RecoverNamedGatewayRun // Fall through to the lifecycle re-check below so we preserve the // existing recovery result shape and emit the correct classification. } - runOpenshell(["gateway", "select", "nemoclaw"], { + runOpenshell(["gateway", "select", DEFAULT_GATEWAY_NAME], { ignoreError: true, timeout: OPENSHELL_OPERATION_TIMEOUT_MS, }); after = getNamedGatewayLifecycleState(); if (after.state === "healthy_named") { - process.env.OPENSHELL_GATEWAY = "nemoclaw"; + process.env.OPENSHELL_GATEWAY = DEFAULT_GATEWAY_NAME; return { recovered: true, before, after, attempted: true, via: "start" }; } } diff --git a/src/lib/inference/live.ts b/src/lib/inference/live.ts index 5cb3e51b961..e08e3ec2dfe 100644 --- a/src/lib/inference/live.ts +++ b/src/lib/inference/live.ts @@ -3,6 +3,7 @@ import type { CaptureOpenshellResult } from "../adapters/openshell/client"; import { stripAnsi } from "../adapters/openshell/client"; +import { DEFAULT_GATEWAY_NAME } from "../state/gateway-name"; import { parseGatewayInference, type GatewayInference } from "./config"; type CaptureLiveInference = ( @@ -26,7 +27,7 @@ export function getLiveGatewayInference( opts: { timeout?: number } = {}, ): LiveGatewayInferenceResult { const attempts = [ - ["inference", "get", "-g", "nemoclaw"], + ["inference", "get", "-g", DEFAULT_GATEWAY_NAME], ["inference", "get"], ]; let last: LiveGatewayInferenceResult = { diff --git a/src/lib/state/onboard-session.ts b/src/lib/state/onboard-session.ts index 26cbf083539..84639e1833a 100644 --- a/src/lib/state/onboard-session.ts +++ b/src/lib/state/onboard-session.ts @@ -26,6 +26,7 @@ import { import { isOnboardMachineState } from "../onboard/machine/transitions"; import type { OnboardMachineState } from "../onboard/machine/types"; import { redactSensitiveText, redactUrl } from "../security/redact"; +import { DEFAULT_GATEWAY_NAME } from "./gateway-name"; export const SESSION_VERSION = 1; export const MACHINE_SNAPSHOT_VERSION = 1; @@ -308,7 +309,7 @@ function parseWechatConfig(value: unknown): WechatConfig | null { function parseSessionMetadata(value: SessionJsonValue | undefined): SessionMetadata | undefined { if (!isObject(value)) return undefined; return { - gatewayName: readString(value.gatewayName) ?? "nemoclaw", + gatewayName: readString(value.gatewayName) ?? DEFAULT_GATEWAY_NAME, fromDockerfile: readString(value.fromDockerfile), }; } @@ -490,7 +491,7 @@ export function createSession(overrides: Partial = {}): Session { telegramConfig: parseTelegramConfig(overrides.telegramConfig), wechatConfig: parseWechatConfig(overrides.wechatConfig), metadata: { - gatewayName: overrides.metadata?.gatewayName ?? "nemoclaw", + gatewayName: overrides.metadata?.gatewayName ?? DEFAULT_GATEWAY_NAME, fromDockerfile: overrides.metadata?.fromDockerfile ?? null, }, machine: parseMachineSnapshot(overrides.machine as SessionJsonValue | undefined) ?? From 48e58ae6fd800b4610f2099f9b671827a750930c Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Mon, 1 Jun 2026 10:19:05 +0000 Subject: [PATCH 08/27] refactor(state): persist + validate gatewayName at registry boundary Signed-off-by: Tinson Lai --- src/lib/actions/inference-set.ts | 3 ++- src/lib/actions/uninstall/run-plan.ts | 4 ++-- src/lib/onboard.ts | 2 +- src/lib/onboard/docker-gpu-patch.ts | 3 ++- src/lib/onboard/sandbox-registry-metadata.ts | 13 ++++++++-- src/lib/state/registry.ts | 3 +++ test/registry.test.ts | 25 ++++++++++++++++++++ 7 files changed, 46 insertions(+), 7 deletions(-) diff --git a/src/lib/actions/inference-set.ts b/src/lib/actions/inference-set.ts index f84a9ff1874..c5282b376fd 100644 --- a/src/lib/actions/inference-set.ts +++ b/src/lib/actions/inference-set.ts @@ -19,6 +19,7 @@ import { writeSandboxConfig, } from "../sandbox/config"; import { appendAuditEntry } from "../shields/audit"; +import { DEFAULT_GATEWAY_NAME } from "../state/gateway-name"; import * as onboardSession from "../state/onboard-session"; import * as registry from "../state/registry"; import type { SandboxEntry } from "../state/registry"; @@ -300,7 +301,7 @@ function openshellInferenceSetArgs(options: { "inference", "set", "-g", - "nemoclaw", + DEFAULT_GATEWAY_NAME, "--provider", options.provider, "--model", diff --git a/src/lib/actions/uninstall/run-plan.ts b/src/lib/actions/uninstall/run-plan.ts index d0ce76169b5..1859e5125ae 100644 --- a/src/lib/actions/uninstall/run-plan.ts +++ b/src/lib/actions/uninstall/run-plan.ts @@ -9,7 +9,7 @@ import path from "node:path"; import { dockerSpawnSync } from "../../adapters/docker/exec"; import { getAgentBranding, type AgentBranding } from "../../cli/branding"; import { sleepMs } from "../../core/wait"; -import { defaultUninstallPaths, NEMOCLAW_OLLAMA_MODELS, NEMOCLAW_PROVIDERS, type UninstallPaths } from "../../domain/uninstall/paths"; +import { DEFAULT_GATEWAY_NAME, defaultUninstallPaths, NEMOCLAW_OLLAMA_MODELS, NEMOCLAW_PROVIDERS, type UninstallPaths } from "../../domain/uninstall/paths"; import { buildUninstallPlan, type UninstallPlan } from "../../domain/uninstall/plan"; import { stopHostGatewayProcesses } from "../../onboard/host-gateway-process"; import { stopStaleDashboardListeners } from "../../onboard/stale-gateway-cleanup"; @@ -470,7 +470,7 @@ function removeOpenShellResources(options: UninstallRunOptions, runtime: Uninsta for (const provider of NEMOCLAW_PROVIDERS) { runOptional(runtime, `Deleted provider '${provider}'`, "openshell", ["provider", "delete", provider]); } - const gatewayLabel = options.gatewayName || "nemoclaw"; + const gatewayLabel = options.gatewayName || DEFAULT_GATEWAY_NAME; runOptional( runtime, `Destroyed gateway '${gatewayLabel}'`, diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 6db0caa20d9..d8908bce345 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -3797,7 +3797,7 @@ async function createSandbox( // openshell tags images with seconds; buildId is ms. Parse actual tag from output. Fixes #2672. const resolvedImageTag = resolveSandboxImageTagFromCreateOutput(createResult.output, buildId); - const sandboxRuntimeFields = getSandboxRuntimeRegistryFields(effectiveSandboxGpuConfig); + const sandboxRuntimeFields = getSandboxRuntimeRegistryFields(effectiveSandboxGpuConfig, GATEWAY_NAME); registry.registerSandbox({ name: sandboxName, model: model || null, diff --git a/src/lib/onboard/docker-gpu-patch.ts b/src/lib/onboard/docker-gpu-patch.ts index bc3546291d3..feb0a1e431b 100644 --- a/src/lib/onboard/docker-gpu-patch.ts +++ b/src/lib/onboard/docker-gpu-patch.ts @@ -14,6 +14,7 @@ import { dockerRunDetached, dockerStop, } from "../adapters/docker"; +import { DEFAULT_GATEWAY_NAME } from "../state/gateway-name"; import { envInt } from "./env"; export const OPENSHELL_MANAGED_BY_LABEL = "openshell.ai/managed-by"; @@ -1253,7 +1254,7 @@ export function collectDockerGpuPatchDiagnostics( const captures: Array<[string, string[]]> = [ ["openshell-sandbox-get.txt", ["sandbox", "get", sandboxName]], ["openshell-sandbox-list.txt", ["sandbox", "list"]], - ["openshell-logs.txt", ["doctor", "logs", "--name", "nemoclaw"]], + ["openshell-logs.txt", ["doctor", "logs", "--name", DEFAULT_GATEWAY_NAME]], ]; for (const [fileName, args] of captures) { try { diff --git a/src/lib/onboard/sandbox-registry-metadata.ts b/src/lib/onboard/sandbox-registry-metadata.ts index c6a69ba7103..9d531fa4b28 100644 --- a/src/lib/onboard/sandbox-registry-metadata.ts +++ b/src/lib/onboard/sandbox-registry-metadata.ts @@ -14,7 +14,10 @@ export interface SandboxRegistryMetadataDeps { } export interface SandboxRegistryMetadataHelpers { - getSandboxRuntimeRegistryFields(config: SandboxGpuConfig): Pick< + getSandboxRuntimeRegistryFields( + config: SandboxGpuConfig, + gatewayName?: string, + ): Pick< SandboxEntry, | "gpuEnabled" | "hostGpuDetected" @@ -23,6 +26,7 @@ export interface SandboxRegistryMetadataHelpers { | "sandboxGpuDevice" | "openshellDriver" | "openshellVersion" + | "gatewayName" >; hasSandboxGpuDrift(sandboxName: string, config: SandboxGpuConfig): boolean; updateReusedSandboxMetadata( @@ -39,7 +43,10 @@ export interface SandboxRegistryMetadataHelpers { export function createSandboxRegistryMetadataHelpers( deps: SandboxRegistryMetadataDeps, ): SandboxRegistryMetadataHelpers { - function getSandboxRuntimeRegistryFields(config: SandboxGpuConfig): Pick< + function getSandboxRuntimeRegistryFields( + config: SandboxGpuConfig, + gatewayName?: string, + ): Pick< SandboxEntry, | "gpuEnabled" | "hostGpuDetected" @@ -48,6 +55,7 @@ export function createSandboxRegistryMetadataHelpers( | "sandboxGpuDevice" | "openshellDriver" | "openshellVersion" + | "gatewayName" > { // OpenShell's Docker-driver gateway always starts with OPENSHELL_DRIVERS=docker, // including on macOS arm64 (#3454). Recording "vm" for darwin here makes later @@ -63,6 +71,7 @@ export function createSandboxRegistryMetadataHelpers( openshellVersion: deps.getInstalledOpenshellVersion( deps.runCaptureOpenshell(["--version"], { ignoreError: true }), ), + ...(gatewayName ? { gatewayName } : {}), }; } diff --git a/src/lib/state/registry.ts b/src/lib/state/registry.ts index 77f529f64c8..665ca257a5e 100644 --- a/src/lib/state/registry.ts +++ b/src/lib/state/registry.ts @@ -8,6 +8,7 @@ import { ensureConfigDir, readConfigFile, writeConfigFile } from "./config-io"; import { isErrnoException } from "../core/errno"; import { DEFAULT_GATEWAY_NAME } from "./gateway-name"; import type { MessagingChannelConfig } from "../messaging-channel-config"; +import { validateName } from "../runner"; export interface CustomPolicyEntry { name: string; @@ -216,6 +217,7 @@ export function getDefault(): string | null { } export function registerSandbox(entry: SandboxEntry): void { + if (entry.gatewayName) validateName(entry.gatewayName, "gatewayName"); withLock(() => { const data = load(); data.sandboxes[entry.name] = { @@ -265,6 +267,7 @@ export function registerSandbox(entry: SandboxEntry): void { } export function updateSandbox(name: string, updates: Partial): boolean { + if (updates.gatewayName) validateName(updates.gatewayName, "gatewayName"); return withLock(() => { const data = load(); if (!data.sandboxes[name]) return false; diff --git a/test/registry.test.ts b/test/registry.test.ts index 7fd4b3b1b97..98e5663ed09 100644 --- a/test/registry.test.ts +++ b/test/registry.test.ts @@ -524,4 +524,29 @@ describe("advisory file locking", () => { it("getSandboxGatewayName returns null for unknown sandbox names so callers surface the lookup failure", () => { expect(registry.getSandboxGatewayName("does-not-exist")).toBeNull(); }); + + it("rejects malformed gatewayName at the registry boundary", () => { + // gatewayName is fed into openshell CLI args and Docker container names. + // Reject anything the existing name policy refuses so future lifecycle + // consumers can trust the stored value. + expect(() => + registry.registerSandbox({ name: "alpha", gatewayName: "../escape" }), + ).toThrow(/gatewayName/); + expect(() => + registry.registerSandbox({ name: "alpha", gatewayName: "has space" }), + ).toThrow(/gatewayName/); + expect(() => + registry.registerSandbox({ name: "alpha", gatewayName: ";rm" }), + ).toThrow(/gatewayName/); + }); + + it("rejects malformed gatewayName in updateSandbox too", () => { + registry.registerSandbox({ name: "alpha", gatewayName: "nemoclaw" }); + expect(() => registry.updateSandbox("alpha", { gatewayName: "../escape" })).toThrow( + /gatewayName/, + ); + // Sanity check: a valid update still succeeds. + expect(registry.updateSandbox("alpha", { gatewayName: "nemoclaw-8081" })).toBe(true); + expect(registry.getSandboxGatewayName("alpha")).toBe("nemoclaw-8081"); + }); }); From b9e28babae3df68b1c8ac6ee2c8379bd28e33449 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Mon, 1 Jun 2026 10:32:50 +0000 Subject: [PATCH 09/27] fix(state): inline gatewayName validation to drop runner dep on platform Signed-off-by: Tinson Lai --- src/lib/state/registry.ts | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/src/lib/state/registry.ts b/src/lib/state/registry.ts index 665ca257a5e..0cf9af901e7 100644 --- a/src/lib/state/registry.ts +++ b/src/lib/state/registry.ts @@ -8,7 +8,25 @@ import { ensureConfigDir, readConfigFile, writeConfigFile } from "./config-io"; import { isErrnoException } from "../core/errno"; import { DEFAULT_GATEWAY_NAME } from "./gateway-name"; import type { MessagingChannelConfig } from "../messaging-channel-config"; -import { validateName } from "../runner"; +import { + NAME_ALLOWED_FORMAT, + NAME_MAX_LENGTH, + NAME_VALID_PATTERN, +} from "../name-validation"; + +function validateGatewayNameField(value: string): void { + if (typeof value !== "string" || value.length === 0) { + throw new Error(`gatewayName is required. Allowed format: ${NAME_ALLOWED_FORMAT}.`); + } + if (value.length > NAME_MAX_LENGTH) { + throw new Error( + `gatewayName too long (max ${NAME_MAX_LENGTH} chars). Allowed format: ${NAME_ALLOWED_FORMAT}.`, + ); + } + if (!NAME_VALID_PATTERN.test(value)) { + throw new Error(`Invalid gatewayName: '${value}'. Allowed format: ${NAME_ALLOWED_FORMAT}.`); + } +} export interface CustomPolicyEntry { name: string; @@ -217,7 +235,7 @@ export function getDefault(): string | null { } export function registerSandbox(entry: SandboxEntry): void { - if (entry.gatewayName) validateName(entry.gatewayName, "gatewayName"); + if (entry.gatewayName) validateGatewayNameField(entry.gatewayName); withLock(() => { const data = load(); data.sandboxes[entry.name] = { @@ -267,7 +285,7 @@ export function registerSandbox(entry: SandboxEntry): void { } export function updateSandbox(name: string, updates: Partial): boolean { - if (updates.gatewayName) validateName(updates.gatewayName, "gatewayName"); + if (updates.gatewayName) validateGatewayNameField(updates.gatewayName); return withLock(() => { const data = load(); if (!data.sandboxes[name]) return false; From 7cf4f48e5a187be7d71f50334fb1716d93fdd892 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Mon, 1 Jun 2026 11:23:22 +0000 Subject: [PATCH 10/27] refactor(state): default getSandboxGatewayName + migrate reused entries + stricter writes Signed-off-by: Tinson Lai --- src/lib/onboard/sandbox-registry-metadata.ts | 9 ++++ src/lib/state/registry.ts | 43 ++++++++++++----- test/registry.test.ts | 51 ++++++++++++++++++-- 3 files changed, 88 insertions(+), 15 deletions(-) diff --git a/src/lib/onboard/sandbox-registry-metadata.ts b/src/lib/onboard/sandbox-registry-metadata.ts index 9d531fa4b28..e6ef31cef4d 100644 --- a/src/lib/onboard/sandbox-registry-metadata.ts +++ b/src/lib/onboard/sandbox-registry-metadata.ts @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import type { AgentDefinition } from "../agent/defs"; +import { DEFAULT_GATEWAY_NAME } from "../state/gateway-name"; import type { SandboxEntry } from "../state/registry"; import * as registry from "../state/registry"; import { getSandboxAgentRegistryFields } from "./sandbox-agent"; @@ -97,11 +98,19 @@ export function createSandboxRegistryMetadataHelpers( const existingEntry = registry.getSandbox(sandboxName); const agentVersionKnown = existingEntry?.agentVersion !== null; const selectionUpdates = selectionVerified ? { model, provider } : {}; + // Migrate legacy reused entries that predate per-sandbox gateway tracking + // by recording the singleton gateway name on first reuse. When existing + // entries already carry a binding, preserve it untouched. + const gatewayMigration = + existingEntry && existingEntry.gatewayName === undefined + ? { gatewayName: DEFAULT_GATEWAY_NAME } + : {}; registry.updateSandbox(sandboxName, { ...selectionUpdates, dashboardPort, ...getSandboxAgentRegistryFields(agent, agentVersionKnown), ...(sandboxGpuConfig ? getSandboxRuntimeRegistryFields(sandboxGpuConfig) : {}), + ...gatewayMigration, }); registry.setDefault(sandboxName); } diff --git a/src/lib/state/registry.ts b/src/lib/state/registry.ts index 0cf9af901e7..d26afe57a66 100644 --- a/src/lib/state/registry.ts +++ b/src/lib/state/registry.ts @@ -212,17 +212,37 @@ export function getSandbox(name: string): SandboxEntry | null { } /** - * Resolve the OpenShell gateway name a sandbox is bound to. Returns the - * persisted value when set, or backfills the singleton - * {@link DEFAULT_GATEWAY_NAME} for legacy entries that predate the - * `gatewayName` field. Returns `null` when the sandbox does not exist so the - * caller surfaces a clean lookup failure instead of silently treating a typo - * as the default gateway. + * Resolve the OpenShell gateway name a sandbox is bound to. Always returns a + * usable gateway name so callers do not need null-handling. Falls back to + * {@link DEFAULT_GATEWAY_NAME} when the sandbox is missing, when the entry + * predates the `gatewayName` field (legacy backfill), or when the persisted + * value fails validation (defense-in-depth against corrupt on-disk state). + * Missing sandboxes and legacy entries log at info level so unexpected + * fallbacks remain observable; corrupt values log at warning level. */ -export function getSandboxGatewayName(name: string): string | null { +export function getSandboxGatewayName(name: string): string { const entry = getSandbox(name); - if (!entry) return null; - return entry.gatewayName || DEFAULT_GATEWAY_NAME; + if (!entry) { + console.log( + ` Gateway-name lookup for unknown sandbox '${name}' resolved to '${DEFAULT_GATEWAY_NAME}'.`, + ); + return DEFAULT_GATEWAY_NAME; + } + if (entry.gatewayName === undefined) { + console.log( + ` Sandbox '${name}' has no recorded gatewayName; using '${DEFAULT_GATEWAY_NAME}' from the singleton default.`, + ); + return DEFAULT_GATEWAY_NAME; + } + try { + validateGatewayNameField(entry.gatewayName); + return entry.gatewayName; + } catch { + console.warn( + ` Sandbox '${name}' has an invalid recorded gatewayName; falling back to '${DEFAULT_GATEWAY_NAME}'.`, + ); + return DEFAULT_GATEWAY_NAME; + } } export function getDefault(): string | null { @@ -235,7 +255,7 @@ export function getDefault(): string | null { } export function registerSandbox(entry: SandboxEntry): void { - if (entry.gatewayName) validateGatewayNameField(entry.gatewayName); + if (entry.gatewayName !== undefined) validateGatewayNameField(entry.gatewayName); withLock(() => { const data = load(); data.sandboxes[entry.name] = { @@ -285,7 +305,8 @@ export function registerSandbox(entry: SandboxEntry): void { } export function updateSandbox(name: string, updates: Partial): boolean { - if (updates.gatewayName) validateGatewayNameField(updates.gatewayName); + if (Object.prototype.hasOwnProperty.call(updates, "gatewayName")) + validateGatewayNameField(updates.gatewayName as string); return withLock(() => { const data = load(); if (!data.sandboxes[name]) return false; diff --git a/test/registry.test.ts b/test/registry.test.ts index 98e5663ed09..305025bead3 100644 --- a/test/registry.test.ts +++ b/test/registry.test.ts @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { describe, it, expect, beforeEach } from "vitest"; +import { describe, it, expect, beforeEach, vi } from "vitest"; import fs from "node:fs"; import path from "node:path"; import os from "node:os"; @@ -521,8 +521,42 @@ describe("advisory file locking", () => { expect(registry.getSandboxGatewayName("alpha")).toBe("nemoclaw-8081"); }); - it("getSandboxGatewayName returns null for unknown sandbox names so callers surface the lookup failure", () => { - expect(registry.getSandboxGatewayName("does-not-exist")).toBeNull(); + it("getSandboxGatewayName falls back to the singleton default for unknown sandbox names and emits an info log", () => { + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + try { + expect(registry.getSandboxGatewayName("does-not-exist")).toBe("nemoclaw"); + expect(logSpy).toHaveBeenCalled(); + expect( + logSpy.mock.calls.some(([msg]: unknown[]) => + typeof msg === "string" && msg.includes("unknown sandbox 'does-not-exist'"), + ), + ).toBe(true); + } finally { + logSpy.mockRestore(); + } + }); + + it("getSandboxGatewayName falls back with a warning when the persisted value is invalid", () => { + // Corrupt on-disk state: caller hand-edited sandboxes.json or a future + // version persisted an invalid value. Defense-in-depth — return the + // singleton default rather than feeding the bad value to lifecycle code. + const corrupt = JSON.stringify({ + sandboxes: { alpha: { name: "alpha", gatewayName: "../escape" } }, + defaultSandbox: "alpha", + }); + fs.writeFileSync(regFile, corrupt); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + expect(registry.getSandboxGatewayName("alpha")).toBe("nemoclaw"); + expect(warnSpy).toHaveBeenCalled(); + expect( + warnSpy.mock.calls.some(([msg]: unknown[]) => + typeof msg === "string" && msg.includes("invalid recorded gatewayName"), + ), + ).toBe(true); + } finally { + warnSpy.mockRestore(); + } }); it("rejects malformed gatewayName at the registry boundary", () => { @@ -538,13 +572,22 @@ describe("advisory file locking", () => { expect(() => registry.registerSandbox({ name: "alpha", gatewayName: ";rm" }), ).toThrow(/gatewayName/); + expect(() => + registry.registerSandbox({ name: "alpha", gatewayName: "" }), + ).toThrow(/gatewayName/); }); - it("rejects malformed gatewayName in updateSandbox too", () => { + it("rejects malformed and empty gatewayName in updateSandbox too", () => { registry.registerSandbox({ name: "alpha", gatewayName: "nemoclaw" }); expect(() => registry.updateSandbox("alpha", { gatewayName: "../escape" })).toThrow( /gatewayName/, ); + // Explicit empty string is a deliberate write — reject it so the field + // cannot be cleared into an invalid state. Persisted absence is fine; the + // accessor backfills the singleton default for legacy entries. + expect(() => registry.updateSandbox("alpha", { gatewayName: "" })).toThrow( + /gatewayName/, + ); // Sanity check: a valid update still succeeds. expect(registry.updateSandbox("alpha", { gatewayName: "nemoclaw-8081" })).toBe(true); expect(registry.getSandboxGatewayName("alpha")).toBe("nemoclaw-8081"); From 8b085b1e0ce9c4e6a06032533d776875ccf17e6a Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Mon, 1 Jun 2026 11:46:24 +0000 Subject: [PATCH 11/27] refactor(state): return null for unknown and corrupt gatewayName lookups, migrate via port resolver Signed-off-by: Tinson Lai --- src/lib/onboard/sandbox-registry-metadata.ts | 6 ++--- src/lib/state/registry.ts | 28 ++++++++++---------- test/registry.test.ts | 13 ++++----- 3 files changed, 24 insertions(+), 23 deletions(-) diff --git a/src/lib/onboard/sandbox-registry-metadata.ts b/src/lib/onboard/sandbox-registry-metadata.ts index e6ef31cef4d..fa9b015d985 100644 --- a/src/lib/onboard/sandbox-registry-metadata.ts +++ b/src/lib/onboard/sandbox-registry-metadata.ts @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import type { AgentDefinition } from "../agent/defs"; -import { DEFAULT_GATEWAY_NAME } from "../state/gateway-name"; +import { getGatewayName } from "../state/gateway-name"; import type { SandboxEntry } from "../state/registry"; import * as registry from "../state/registry"; import { getSandboxAgentRegistryFields } from "./sandbox-agent"; @@ -99,11 +99,11 @@ export function createSandboxRegistryMetadataHelpers( const agentVersionKnown = existingEntry?.agentVersion !== null; const selectionUpdates = selectionVerified ? { model, provider } : {}; // Migrate legacy reused entries that predate per-sandbox gateway tracking - // by recording the singleton gateway name on first reuse. When existing + // by recording the port-resolved gateway name on first reuse. When existing // entries already carry a binding, preserve it untouched. const gatewayMigration = existingEntry && existingEntry.gatewayName === undefined - ? { gatewayName: DEFAULT_GATEWAY_NAME } + ? { gatewayName: getGatewayName(dashboardPort) } : {}; registry.updateSandbox(sandboxName, { ...selectionUpdates, diff --git a/src/lib/state/registry.ts b/src/lib/state/registry.ts index d26afe57a66..93b491b6347 100644 --- a/src/lib/state/registry.ts +++ b/src/lib/state/registry.ts @@ -212,21 +212,21 @@ export function getSandbox(name: string): SandboxEntry | null { } /** - * Resolve the OpenShell gateway name a sandbox is bound to. Always returns a - * usable gateway name so callers do not need null-handling. Falls back to - * {@link DEFAULT_GATEWAY_NAME} when the sandbox is missing, when the entry - * predates the `gatewayName` field (legacy backfill), or when the persisted - * value fails validation (defense-in-depth against corrupt on-disk state). - * Missing sandboxes and legacy entries log at info level so unexpected - * fallbacks remain observable; corrupt values log at warning level. + * Resolve the OpenShell gateway name a sandbox is bound to. Returns `null` + * for unknown sandboxes (so callers cannot transitively act on the singleton + * with a stale or mistyped name) and for entries whose persisted value fails + * validation (defense-in-depth against corrupt on-disk state). For sandboxes + * that exist but predate the `gatewayName` field, falls back to + * {@link DEFAULT_GATEWAY_NAME} as a legacy backfill. Unknown sandboxes and + * legacy entries log at info level so unexpected fallbacks remain + * observable; corrupt values log at warning level since they indicate + * tampering or schema drift. */ -export function getSandboxGatewayName(name: string): string { +export function getSandboxGatewayName(name: string): string | null { const entry = getSandbox(name); if (!entry) { - console.log( - ` Gateway-name lookup for unknown sandbox '${name}' resolved to '${DEFAULT_GATEWAY_NAME}'.`, - ); - return DEFAULT_GATEWAY_NAME; + console.log(` Gateway-name lookup for unknown sandbox '${name}' returned null.`); + return null; } if (entry.gatewayName === undefined) { console.log( @@ -239,9 +239,9 @@ export function getSandboxGatewayName(name: string): string { return entry.gatewayName; } catch { console.warn( - ` Sandbox '${name}' has an invalid recorded gatewayName; falling back to '${DEFAULT_GATEWAY_NAME}'.`, + ` Sandbox '${name}' has an invalid recorded gatewayName; returning null.`, ); - return DEFAULT_GATEWAY_NAME; + return null; } } diff --git a/test/registry.test.ts b/test/registry.test.ts index 305025bead3..b5d7f7299d3 100644 --- a/test/registry.test.ts +++ b/test/registry.test.ts @@ -521,10 +521,10 @@ describe("advisory file locking", () => { expect(registry.getSandboxGatewayName("alpha")).toBe("nemoclaw-8081"); }); - it("getSandboxGatewayName falls back to the singleton default for unknown sandbox names and emits an info log", () => { + it("getSandboxGatewayName returns null for unknown sandbox names so callers cannot transitively act on the singleton", () => { const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); try { - expect(registry.getSandboxGatewayName("does-not-exist")).toBe("nemoclaw"); + expect(registry.getSandboxGatewayName("does-not-exist")).toBeNull(); expect(logSpy).toHaveBeenCalled(); expect( logSpy.mock.calls.some(([msg]: unknown[]) => @@ -536,10 +536,11 @@ describe("advisory file locking", () => { } }); - it("getSandboxGatewayName falls back with a warning when the persisted value is invalid", () => { + it("getSandboxGatewayName returns null with a warning when the persisted value is invalid", () => { // Corrupt on-disk state: caller hand-edited sandboxes.json or a future - // version persisted an invalid value. Defense-in-depth — return the - // singleton default rather than feeding the bad value to lifecycle code. + // version persisted an invalid value. Defense-in-depth — return null so + // lifecycle code refuses rather than transitively act on the singleton + // with a bad persisted name. const corrupt = JSON.stringify({ sandboxes: { alpha: { name: "alpha", gatewayName: "../escape" } }, defaultSandbox: "alpha", @@ -547,7 +548,7 @@ describe("advisory file locking", () => { fs.writeFileSync(regFile, corrupt); const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); try { - expect(registry.getSandboxGatewayName("alpha")).toBe("nemoclaw"); + expect(registry.getSandboxGatewayName("alpha")).toBeNull(); expect(warnSpy).toHaveBeenCalled(); expect( warnSpy.mock.calls.some(([msg]: unknown[]) => From ca876d67e5045a9d72451bae567dfe6a3e0f999c Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Mon, 1 Jun 2026 12:21:49 +0000 Subject: [PATCH 12/27] refactor(state): route accessor diagnostics to stderr and cover reuse migration Signed-off-by: Tinson Lai --- .../onboard/sandbox-registry-metadata.test.ts | 76 ++++++++++++++++++- src/lib/state/registry.ts | 11 ++- test/registry.test.ts | 54 ++++++------- 3 files changed, 108 insertions(+), 33 deletions(-) diff --git a/src/lib/onboard/sandbox-registry-metadata.test.ts b/src/lib/onboard/sandbox-registry-metadata.test.ts index 1935956f36c..52df86bc18b 100644 --- a/src/lib/onboard/sandbox-registry-metadata.test.ts +++ b/src/lib/onboard/sandbox-registry-metadata.test.ts @@ -1,13 +1,29 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { afterEach, describe, expect, it } from "vitest"; +import fs from "node:fs"; +import { createRequire } from "node:module"; +import os from "node:os"; +import path from "node:path"; +import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +// Use a temp HOME so tests do not touch the real ~/.nemoclaw registry. +// HOME must be set before loading the registry module (it reads HOME at +// require time), so we use createRequire instead of a static import. +const tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-meta-")); +const originalHome = process.env.HOME; +process.env.HOME = tmpHome; + // Import the compiled module: sandbox-registry-metadata.ts pulls in state/registry, // which transitively requires the JS-only `./platform` helper that vitest cannot // resolve from TS source. Same pattern as `vm-dns-monkeypatch.test.ts`. import { createSandboxRegistryMetadataHelpers } from "../../../dist/lib/onboard/sandbox-registry-metadata"; import type { SandboxGpuConfig } from "./sandbox-gpu-mode"; +const require = createRequire(import.meta.url); +const registry = require("../../../dist/lib/state/registry"); +const regFile = path.join(tmpHome, ".nemoclaw", "sandboxes.json"); + const ORIGINAL_PLATFORM = Object.getOwnPropertyDescriptor(process, "platform"); function setPlatform(platform: NodeJS.Platform): void { @@ -37,6 +53,12 @@ const GPU_OFF: SandboxGpuConfig = { errors: [], }; +afterAll(() => { + if (originalHome === undefined) delete process.env.HOME; + else process.env.HOME = originalHome; + fs.rmSync(tmpHome, { recursive: true, force: true }); +}); + describe("getSandboxRuntimeRegistryFields openshellDriver", () => { afterEach(restorePlatform); @@ -67,3 +89,55 @@ describe("getSandboxRuntimeRegistryFields openshellDriver", () => { expect(fields.openshellDriver).toBe("kubernetes"); }); }); + +describe("getSandboxRuntimeRegistryFields gatewayName", () => { + it("omits gatewayName when no name is supplied so the reused path can preserve existing bindings", () => { + const helpers = makeHelpers({ dockerDriverEnabled: true }); + const fields = helpers.getSandboxRuntimeRegistryFields(GPU_OFF); + expect(fields.gatewayName).toBeUndefined(); + }); + + it("emits gatewayName when supplied so fresh onboard registrations record the binding", () => { + const helpers = makeHelpers({ dockerDriverEnabled: true }); + const fields = helpers.getSandboxRuntimeRegistryFields(GPU_OFF, "nemoclaw"); + expect(fields.gatewayName).toBe("nemoclaw"); + }); +}); + +describe("updateReusedSandboxMetadata gatewayName migration", () => { + let warnSpy: ReturnType; + + beforeEach(() => { + if (fs.existsSync(regFile)) fs.unlinkSync(regFile); + warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + }); + + afterEach(() => { + warnSpy.mockRestore(); + }); + + it("backfills gatewayName via the port-resolved name on first reuse of a legacy entry", () => { + // Legacy entry written before per-sandbox gateway tracking lacks the + // field; reuse must record the active singleton name so future lifecycle + // callers can resolve a stable binding. + registry.registerSandbox({ name: "legacy", model: "m", provider: "p" }); + expect(registry.getSandbox("legacy").gatewayName).toBeUndefined(); + + const helpers = makeHelpers({ dockerDriverEnabled: true }); + helpers.updateReusedSandboxMetadata("legacy", null, "m2", "p2", 8081); + + expect(registry.getSandbox("legacy").gatewayName).toBe("nemoclaw"); + }); + + it("preserves an existing gatewayName binding on reuse", () => { + // Once a sandbox carries an explicit binding, reuse must not overwrite it + // — that protects per-sandbox bindings from being clobbered by the active + // singleton when follow-up PRs flip the resolver to per-port names. + registry.registerSandbox({ name: "alpha", gatewayName: "nemoclaw-8081" }); + + const helpers = makeHelpers({ dockerDriverEnabled: true }); + helpers.updateReusedSandboxMetadata("alpha", null, "m", "p", 8090); + + expect(registry.getSandbox("alpha").gatewayName).toBe("nemoclaw-8081"); + }); +}); diff --git a/src/lib/state/registry.ts b/src/lib/state/registry.ts index 93b491b6347..4c54ac6ae33 100644 --- a/src/lib/state/registry.ts +++ b/src/lib/state/registry.ts @@ -217,19 +217,18 @@ export function getSandbox(name: string): SandboxEntry | null { * with a stale or mistyped name) and for entries whose persisted value fails * validation (defense-in-depth against corrupt on-disk state). For sandboxes * that exist but predate the `gatewayName` field, falls back to - * {@link DEFAULT_GATEWAY_NAME} as a legacy backfill. Unknown sandboxes and - * legacy entries log at info level so unexpected fallbacks remain - * observable; corrupt values log at warning level since they indicate - * tampering or schema drift. + * {@link DEFAULT_GATEWAY_NAME} as a legacy backfill. All diagnostics are + * written to `stderr` via `console.warn` so JSON / non-interactive callers + * keep stdout clean. */ export function getSandboxGatewayName(name: string): string | null { const entry = getSandbox(name); if (!entry) { - console.log(` Gateway-name lookup for unknown sandbox '${name}' returned null.`); + console.warn(` Gateway-name lookup for unknown sandbox '${name}' returned null.`); return null; } if (entry.gatewayName === undefined) { - console.log( + console.warn( ` Sandbox '${name}' has no recorded gatewayName; using '${DEFAULT_GATEWAY_NAME}' from the singleton default.`, ); return DEFAULT_GATEWAY_NAME; diff --git a/test/registry.test.ts b/test/registry.test.ts index b5d7f7299d3..119648935ca 100644 --- a/test/registry.test.ts +++ b/test/registry.test.ts @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { describe, it, expect, beforeEach, vi } from "vitest"; +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import fs from "node:fs"; import path from "node:path"; import os from "node:os"; @@ -502,6 +502,19 @@ describe("advisory file locking", () => { expect(defaultSandbox).toBe(null); }); +}); + +describe("gatewayName persistence and resolution", () => { + // Silence accessor diagnostics globally for this block; specific tests that + // assert on log content install their own targeted spies. + let warnSpy: ReturnType; + beforeEach(() => { + warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + }); + afterEach(() => { + warnSpy.mockRestore(); + }); + it("persists gatewayName when supplied at registration", () => { registry.registerSandbox({ name: "alpha", gatewayName: "nemoclaw" }); const sb = registry.getSandbox("alpha"); @@ -522,18 +535,13 @@ describe("advisory file locking", () => { }); it("getSandboxGatewayName returns null for unknown sandbox names so callers cannot transitively act on the singleton", () => { - const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); - try { - expect(registry.getSandboxGatewayName("does-not-exist")).toBeNull(); - expect(logSpy).toHaveBeenCalled(); - expect( - logSpy.mock.calls.some(([msg]: unknown[]) => - typeof msg === "string" && msg.includes("unknown sandbox 'does-not-exist'"), - ), - ).toBe(true); - } finally { - logSpy.mockRestore(); - } + expect(registry.getSandboxGatewayName("does-not-exist")).toBeNull(); + expect(warnSpy).toHaveBeenCalled(); + expect( + warnSpy.mock.calls.some(([msg]: unknown[]) => + typeof msg === "string" && msg.includes("unknown sandbox 'does-not-exist'"), + ), + ).toBe(true); }); it("getSandboxGatewayName returns null with a warning when the persisted value is invalid", () => { @@ -546,18 +554,13 @@ describe("advisory file locking", () => { defaultSandbox: "alpha", }); fs.writeFileSync(regFile, corrupt); - const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); - try { - expect(registry.getSandboxGatewayName("alpha")).toBeNull(); - expect(warnSpy).toHaveBeenCalled(); - expect( - warnSpy.mock.calls.some(([msg]: unknown[]) => - typeof msg === "string" && msg.includes("invalid recorded gatewayName"), - ), - ).toBe(true); - } finally { - warnSpy.mockRestore(); - } + expect(registry.getSandboxGatewayName("alpha")).toBeNull(); + expect(warnSpy).toHaveBeenCalled(); + expect( + warnSpy.mock.calls.some(([msg]: unknown[]) => + typeof msg === "string" && msg.includes("invalid recorded gatewayName"), + ), + ).toBe(true); }); it("rejects malformed gatewayName at the registry boundary", () => { @@ -589,7 +592,6 @@ describe("advisory file locking", () => { expect(() => registry.updateSandbox("alpha", { gatewayName: "" })).toThrow( /gatewayName/, ); - // Sanity check: a valid update still succeeds. expect(registry.updateSandbox("alpha", { gatewayName: "nemoclaw-8081" })).toBe(true); expect(registry.getSandboxGatewayName("alpha")).toBe("nemoclaw-8081"); }); From 6bb247f188e7947acff2c83595b53ef85796471a Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Mon, 1 Jun 2026 12:54:45 +0000 Subject: [PATCH 13/27] test(state): load registry after HOME mutation and ensure regfile parent dir Signed-off-by: Tinson Lai --- .../onboard/sandbox-registry-metadata.test.ts | 27 ++++++++++--------- test/registry.test.ts | 4 +++ 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/lib/onboard/sandbox-registry-metadata.test.ts b/src/lib/onboard/sandbox-registry-metadata.test.ts index 52df86bc18b..46137fa486a 100644 --- a/src/lib/onboard/sandbox-registry-metadata.test.ts +++ b/src/lib/onboard/sandbox-registry-metadata.test.ts @@ -6,22 +6,23 @@ import { createRequire } from "node:module"; import os from "node:os"; import path from "node:path"; import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { SandboxGpuConfig } from "./sandbox-gpu-mode"; -// Use a temp HOME so tests do not touch the real ~/.nemoclaw registry. -// HOME must be set before loading the registry module (it reads HOME at -// require time), so we use createRequire instead of a static import. +// Use a temp HOME so tests do not touch the real ~/.nemoclaw registry. Both +// the helper and the registry modules read HOME at require time, so HOME must +// be set before they load. Static ESM imports are hoisted ahead of any module +// body statement, so both modules must be loaded via `createRequire` after +// the HOME mutation runs. Same pattern as `vm-dns-monkeypatch.test.ts`. const tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-meta-")); const originalHome = process.env.HOME; process.env.HOME = tmpHome; -// Import the compiled module: sandbox-registry-metadata.ts pulls in state/registry, -// which transitively requires the JS-only `./platform` helper that vitest cannot -// resolve from TS source. Same pattern as `vm-dns-monkeypatch.test.ts`. -import { createSandboxRegistryMetadataHelpers } from "../../../dist/lib/onboard/sandbox-registry-metadata"; -import type { SandboxGpuConfig } from "./sandbox-gpu-mode"; - const require = createRequire(import.meta.url); -const registry = require("../../../dist/lib/state/registry"); +const registry: typeof import("../state/registry") = require( + "../../../dist/lib/state/registry", +); +const { createSandboxRegistryMetadataHelpers }: typeof import("./sandbox-registry-metadata") = + require("../../../dist/lib/onboard/sandbox-registry-metadata"); const regFile = path.join(tmpHome, ".nemoclaw", "sandboxes.json"); const ORIGINAL_PLATFORM = Object.getOwnPropertyDescriptor(process, "platform"); @@ -121,12 +122,12 @@ describe("updateReusedSandboxMetadata gatewayName migration", () => { // field; reuse must record the active singleton name so future lifecycle // callers can resolve a stable binding. registry.registerSandbox({ name: "legacy", model: "m", provider: "p" }); - expect(registry.getSandbox("legacy").gatewayName).toBeUndefined(); + expect(registry.getSandbox("legacy")?.gatewayName).toBeUndefined(); const helpers = makeHelpers({ dockerDriverEnabled: true }); helpers.updateReusedSandboxMetadata("legacy", null, "m2", "p2", 8081); - expect(registry.getSandbox("legacy").gatewayName).toBe("nemoclaw"); + expect(registry.getSandbox("legacy")?.gatewayName).toBe("nemoclaw"); }); it("preserves an existing gatewayName binding on reuse", () => { @@ -138,6 +139,6 @@ describe("updateReusedSandboxMetadata gatewayName migration", () => { const helpers = makeHelpers({ dockerDriverEnabled: true }); helpers.updateReusedSandboxMetadata("alpha", null, "m", "p", 8090); - expect(registry.getSandbox("alpha").gatewayName).toBe("nemoclaw-8081"); + expect(registry.getSandbox("alpha")?.gatewayName).toBe("nemoclaw-8081"); }); }); diff --git a/test/registry.test.ts b/test/registry.test.ts index 119648935ca..f4348c57570 100644 --- a/test/registry.test.ts +++ b/test/registry.test.ts @@ -19,6 +19,10 @@ const registry = require("../dist/lib/state/registry"); const regFile = path.join(tmpDir, ".nemoclaw", "sandboxes.json"); beforeEach(() => { + // Ensure the registry parent dir exists so tests that write `regFile` + // directly (e.g. the corrupt-state path) do not race ahead of the lazy + // dir creation done by `ensureConfigDir` on the first registry write. + fs.mkdirSync(path.dirname(regFile), { recursive: true }); if (fs.existsSync(regFile)) fs.unlinkSync(regFile); }); From 8fc38b06c00e56bf308f24574b5f6f23cc90e52f Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Mon, 1 Jun 2026 14:17:35 +0000 Subject: [PATCH 14/27] refactor(state): relocate gateway-name accessor and route remaining singleton literals through resolver Signed-off-by: Tinson Lai --- src/lib/credentials/command-support.ts | 3 +- src/lib/inventory/index.ts | 3 +- src/lib/onboard/sandbox-registry-metadata.ts | 6 ++ src/lib/state/gateway-name.ts | 78 +++++++++++++++++++- src/lib/state/registry.ts | 58 +-------------- src/lib/tunnel/services.ts | 3 +- 6 files changed, 91 insertions(+), 60 deletions(-) diff --git a/src/lib/credentials/command-support.ts b/src/lib/credentials/command-support.ts index dd9c7ab4212..ec8e29173b8 100644 --- a/src/lib/credentials/command-support.ts +++ b/src/lib/credentials/command-support.ts @@ -3,6 +3,7 @@ import { recoverNamedGatewayRuntime } from "../actions/global"; import { CLI_DISPLAY_NAME, CLI_NAME } from "../cli/branding"; +import { DEFAULT_GATEWAY_NAME } from "../state/gateway-name"; // Suffixes that mark per-sandbox messaging integrations in the gateway's // provider list. These are managed by `channels`, not `credentials`. @@ -34,7 +35,7 @@ export function credentialsGatewayRecoveryFailureLines(kind: "query" | "reach"): const action = kind === "query" ? "query" : "reach"; return [ ` Could not ${action} the ${CLI_DISPLAY_NAME} OpenShell gateway. Is it running?`, - ` Run 'openshell gateway start --name nemoclaw' or '${CLI_NAME} onboard' first.`, + ` Run 'openshell gateway start --name ${DEFAULT_GATEWAY_NAME}' or '${CLI_NAME} onboard' first.`, ]; } diff --git a/src/lib/inventory/index.ts b/src/lib/inventory/index.ts index 5f34b08d2f1..186b1df6c7a 100644 --- a/src/lib/inventory/index.ts +++ b/src/lib/inventory/index.ts @@ -4,6 +4,7 @@ import { CLI_NAME } from "../cli/branding"; import type { GatewayInference } from "../inference/config"; import { redactFull } from "../security/redact"; +import { DEFAULT_GATEWAY_NAME } from "../state/gateway-name"; export interface SandboxEntry { name: string; @@ -453,7 +454,7 @@ export function showStatusCommand(deps: ShowStatusCommandDeps): void { const detail = health.reason ? ` (${health.reason})` : ""; log(` gateway: down [${health.state}]${detail}`); log( - ` Run 'openshell gateway start --name nemoclaw' or 'nemoclaw onboard --resume' to recover.`, + ` Run 'openshell gateway start --name ${DEFAULT_GATEWAY_NAME}' or 'nemoclaw onboard --resume' to recover.`, ); process.exitCode = 1; } diff --git a/src/lib/onboard/sandbox-registry-metadata.ts b/src/lib/onboard/sandbox-registry-metadata.ts index fa9b015d985..5fe657ab5c7 100644 --- a/src/lib/onboard/sandbox-registry-metadata.ts +++ b/src/lib/onboard/sandbox-registry-metadata.ts @@ -101,6 +101,12 @@ export function createSandboxRegistryMetadataHelpers( // Migrate legacy reused entries that predate per-sandbox gateway tracking // by recording the port-resolved gateway name on first reuse. When existing // entries already carry a binding, preserve it untouched. + // + // Removal boundary: drop this branch once every on-disk registry has been + // migrated through at least one onboard reuse (paired with the legacy + // backfill in `getSandboxGatewayName`). A future PR introducing a + // registry schema version field is the recommended trigger for that + // cleanup. const gatewayMigration = existingEntry && existingEntry.gatewayName === undefined ? { gatewayName: getGatewayName(dashboardPort) } diff --git a/src/lib/state/gateway-name.ts b/src/lib/state/gateway-name.ts index 28f5b38589b..117ec038d83 100644 --- a/src/lib/state/gateway-name.ts +++ b/src/lib/state/gateway-name.ts @@ -1,6 +1,14 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import * as registry from "./registry"; + +import { + NAME_ALLOWED_FORMAT, + NAME_MAX_LENGTH, + NAME_VALID_PATTERN, +} from "../name-validation"; + /** * NemoClaw's OpenShell gateway name resolver. * @@ -10,10 +18,12 @@ * — changing the gateway port relocates the singleton instead of spawning a * second instance. * - * This module owns the canonical name and exposes a `port`-aware resolver so + * This module owns the canonical name, exposes a `port`-aware resolver so * follow-up work can derive per-port names (e.g. `"nemoclaw-8081"`) without - * touching every call site again. Until that work lands, `getGatewayName` - * returns the singleton name for every port. + * touching every call site again, validates persisted gateway names at the + * registry boundary, and exposes a sandbox-scoped accessor with legacy and + * defense-in-depth fallbacks. Until the per-port flip lands, + * `getGatewayName` returns the singleton name for every port. */ export const DEFAULT_GATEWAY_NAME = "nemoclaw"; @@ -21,3 +31,65 @@ export const DEFAULT_GATEWAY_NAME = "nemoclaw"; export function getGatewayName(_port: number): string { return DEFAULT_GATEWAY_NAME; } + +/** + * Validate a persisted `gatewayName` against the same RFC 1123-derived rules + * as sandbox/instance names. Throws when the value is empty, too long, or + * uses disallowed characters. Used both at the registry write boundary and + * by {@link getSandboxGatewayName} as a defense-in-depth read-side check. + */ +export function validateGatewayName(value: string): void { + if (typeof value !== "string" || value.length === 0) { + throw new Error(`gatewayName is required. Allowed format: ${NAME_ALLOWED_FORMAT}.`); + } + if (value.length > NAME_MAX_LENGTH) { + throw new Error( + `gatewayName too long (max ${NAME_MAX_LENGTH} chars). Allowed format: ${NAME_ALLOWED_FORMAT}.`, + ); + } + if (!NAME_VALID_PATTERN.test(value)) { + throw new Error(`Invalid gatewayName: '${value}'. Allowed format: ${NAME_ALLOWED_FORMAT}.`); + } +} + +/** + * Resolve the OpenShell gateway name a sandbox is bound to. Returns `null` + * for unknown sandboxes (so callers cannot transitively act on the singleton + * with a stale or mistyped name) and for entries whose persisted value fails + * validation (defense-in-depth against corrupt on-disk state). For sandboxes + * that exist but predate the `gatewayName` field, falls back to + * {@link DEFAULT_GATEWAY_NAME} as a legacy backfill. All diagnostics are + * written to `stderr` via `console.warn` so JSON / non-interactive callers + * keep stdout clean. + * + * Removal boundary: the legacy-backfill branch (and the matching reuse-time + * backfill in `updateReusedSandboxMetadata`) exists so registries written + * before per-sandbox gateway tracking remain usable. Both fallbacks can be + * dropped once `getGatewayName(port)` returns a per-port name AND every + * on-disk registry has been migrated through at least one `nemoclaw onboard` + * — fresh registrations now write `gatewayName` and the reuse path migrates + * legacy entries on first touch. A future PR that introduces a registry + * schema version field is the recommended trigger for that cleanup. + */ +export function getSandboxGatewayName(name: string): string | null { + const entry = registry.getSandbox(name); + if (!entry) { + console.warn(` Gateway-name lookup for unknown sandbox '${name}' returned null.`); + return null; + } + if (entry.gatewayName === undefined) { + console.warn( + ` Sandbox '${name}' has no recorded gatewayName; using '${DEFAULT_GATEWAY_NAME}' from the singleton default.`, + ); + return DEFAULT_GATEWAY_NAME; + } + try { + validateGatewayName(entry.gatewayName); + return entry.gatewayName; + } catch { + console.warn( + ` Sandbox '${name}' has an invalid recorded gatewayName; returning null.`, + ); + return null; + } +} diff --git a/src/lib/state/registry.ts b/src/lib/state/registry.ts index 4c54ac6ae33..62a1c4985be 100644 --- a/src/lib/state/registry.ts +++ b/src/lib/state/registry.ts @@ -6,27 +6,10 @@ import path from "node:path"; import { ensureConfigDir, readConfigFile, writeConfigFile } from "./config-io"; import { isErrnoException } from "../core/errno"; -import { DEFAULT_GATEWAY_NAME } from "./gateway-name"; +import { DEFAULT_GATEWAY_NAME, validateGatewayName } from "./gateway-name"; import type { MessagingChannelConfig } from "../messaging-channel-config"; -import { - NAME_ALLOWED_FORMAT, - NAME_MAX_LENGTH, - NAME_VALID_PATTERN, -} from "../name-validation"; -function validateGatewayNameField(value: string): void { - if (typeof value !== "string" || value.length === 0) { - throw new Error(`gatewayName is required. Allowed format: ${NAME_ALLOWED_FORMAT}.`); - } - if (value.length > NAME_MAX_LENGTH) { - throw new Error( - `gatewayName too long (max ${NAME_MAX_LENGTH} chars). Allowed format: ${NAME_ALLOWED_FORMAT}.`, - ); - } - if (!NAME_VALID_PATTERN.test(value)) { - throw new Error(`Invalid gatewayName: '${value}'. Allowed format: ${NAME_ALLOWED_FORMAT}.`); - } -} +export { getSandboxGatewayName } from "./gateway-name"; export interface CustomPolicyEntry { name: string; @@ -211,39 +194,6 @@ export function getSandbox(name: string): SandboxEntry | null { return data.sandboxes[name] || null; } -/** - * Resolve the OpenShell gateway name a sandbox is bound to. Returns `null` - * for unknown sandboxes (so callers cannot transitively act on the singleton - * with a stale or mistyped name) and for entries whose persisted value fails - * validation (defense-in-depth against corrupt on-disk state). For sandboxes - * that exist but predate the `gatewayName` field, falls back to - * {@link DEFAULT_GATEWAY_NAME} as a legacy backfill. All diagnostics are - * written to `stderr` via `console.warn` so JSON / non-interactive callers - * keep stdout clean. - */ -export function getSandboxGatewayName(name: string): string | null { - const entry = getSandbox(name); - if (!entry) { - console.warn(` Gateway-name lookup for unknown sandbox '${name}' returned null.`); - return null; - } - if (entry.gatewayName === undefined) { - console.warn( - ` Sandbox '${name}' has no recorded gatewayName; using '${DEFAULT_GATEWAY_NAME}' from the singleton default.`, - ); - return DEFAULT_GATEWAY_NAME; - } - try { - validateGatewayNameField(entry.gatewayName); - return entry.gatewayName; - } catch { - console.warn( - ` Sandbox '${name}' has an invalid recorded gatewayName; returning null.`, - ); - return null; - } -} - export function getDefault(): string | null { const data = load(); if (data.defaultSandbox && data.sandboxes[data.defaultSandbox]) { @@ -254,7 +204,7 @@ export function getDefault(): string | null { } export function registerSandbox(entry: SandboxEntry): void { - if (entry.gatewayName !== undefined) validateGatewayNameField(entry.gatewayName); + if (entry.gatewayName !== undefined) validateGatewayName(entry.gatewayName); withLock(() => { const data = load(); data.sandboxes[entry.name] = { @@ -305,7 +255,7 @@ export function registerSandbox(entry: SandboxEntry): void { export function updateSandbox(name: string, updates: Partial): boolean { if (Object.prototype.hasOwnProperty.call(updates, "gatewayName")) - validateGatewayNameField(updates.gatewayName as string); + validateGatewayName(updates.gatewayName as string); return withLock(() => { const data = load(); if (!data.sandboxes[name]) return false; diff --git a/src/lib/tunnel/services.ts b/src/lib/tunnel/services.ts index dbee2081c98..5039d379c38 100644 --- a/src/lib/tunnel/services.ts +++ b/src/lib/tunnel/services.ts @@ -21,6 +21,7 @@ import { renderBox } from "../cli/banner"; import { AGENT_PRODUCT_NAME, CLI_DISPLAY_NAME, CLI_NAME } from "../cli/branding"; import { isRecord } from "../core/json-types"; import { DASHBOARD_PORT } from "../core/ports"; +import { DEFAULT_GATEWAY_NAME } from "../state/gateway-name"; import { buildSubprocessEnv } from "../subprocess-env"; // --------------------------------------------------------------------------- @@ -466,7 +467,7 @@ export function stopSandboxChannels(sandboxName: string): void { reportStopResult(fallbackResult); } -const GATEWAY_CLUSTER_CONTAINER = "openshell-cluster-nemoclaw"; +const GATEWAY_CLUSTER_CONTAINER = `openshell-cluster-${DEFAULT_GATEWAY_NAME}`; const GATEWAY_STOP_SCRIPT = String.raw` set -eu From 2d1292f32642ff7f1a464b955b3bf185497ce259 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Mon, 1 Jun 2026 14:31:14 +0000 Subject: [PATCH 15/27] refactor(state): route gateway-state recovery hints and regex through DEFAULT_GATEWAY_NAME Signed-off-by: Tinson Lai --- src/lib/actions/sandbox/gateway-state.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/lib/actions/sandbox/gateway-state.ts b/src/lib/actions/sandbox/gateway-state.ts index ddf4ff4ce5f..49f9052d76f 100644 --- a/src/lib/actions/sandbox/gateway-state.ts +++ b/src/lib/actions/sandbox/gateway-state.ts @@ -263,7 +263,7 @@ export function printWrongGatewayActiveGuidance( ` Sandbox '${sandboxName}' is registered against the ${CLI_DISPLAY_NAME} gateway, but the currently active OpenShell gateway is '${other}'. Your sandbox has NOT been removed.`, ); writer(" Switch gateways and retry:"); - writer(" openshell gateway select nemoclaw"); + writer(` openshell gateway select ${DEFAULT_GATEWAY_NAME}`); writer(` Then re-run: ${CLI_NAME} ${sandboxName} connect`); } @@ -279,7 +279,7 @@ export function printGatewayLifecycleHint( ` The selected ${CLI_DISPLAY_NAME} gateway is no longer configured or its metadata/runtime has been lost.`, ); writer( - " Start the gateway again with `openshell gateway start --name nemoclaw` before expecting existing sandboxes to reconnect.", + ` Start the gateway again with \`openshell gateway start --name ${DEFAULT_GATEWAY_NAME}\` before expecting existing sandboxes to reconnect.`, ); writer( " If the gateway has to be rebuilt from scratch, recreate the affected sandbox afterward.", @@ -288,14 +288,14 @@ export function printGatewayLifecycleHint( } if ( /Connection refused|client error \(Connect\)|tcp connect error/i.test(cleanOutput) && - /Gateway:\s+nemoclaw/i.test(cleanOutput) + new RegExp(`Gateway:\\s+${DEFAULT_GATEWAY_NAME}`, "i").test(cleanOutput) ) { writer( - " The selected NemoClaw gateway exists in metadata, but its API is refusing connections after restart.", + ` The selected ${CLI_DISPLAY_NAME} gateway exists in metadata, but its API is refusing connections after restart.`, ); writer(" This usually means the gateway runtime did not come back cleanly after the restart."); writer( - " Retry `openshell gateway start --name nemoclaw`; if it stays in this state, rebuild the gateway before expecting existing sandboxes to reconnect.", + ` Retry \`openshell gateway start --name ${DEFAULT_GATEWAY_NAME}\`; if it stays in this state, rebuild the gateway before expecting existing sandboxes to reconnect.`, ); return; } @@ -364,7 +364,7 @@ export async function getReconciledSandboxGatewayState( } if ( /Connection refused|client error \(Connect\)|tcp connect error/i.test(latestStatus) && - /Gateway:\s+nemoclaw/i.test(latestStatus) + new RegExp(`Gateway:\\s+${DEFAULT_GATEWAY_NAME}`, "i").test(latestStatus) ) { return { state: "gateway_unreachable_after_restart", @@ -475,7 +475,7 @@ export async function ensureLiveSandboxOrExit( console.error(lookup.output); } console.error( - " Retry `openshell gateway start --name nemoclaw` and verify `openshell status` is healthy before reconnecting.", + ` Retry \`openshell gateway start --name ${DEFAULT_GATEWAY_NAME}\` and verify \`openshell status\` is healthy before reconnecting.`, ); console.error( " If the gateway never becomes healthy, rebuild the gateway and then recreate the affected sandbox.", @@ -490,7 +490,7 @@ export async function ensureLiveSandboxOrExit( console.error(lookup.output); } console.error( - " Start the gateway again with `openshell gateway start --name nemoclaw` before retrying.", + ` Start the gateway again with \`openshell gateway start --name ${DEFAULT_GATEWAY_NAME}\` before retrying.`, ); console.error( " If the gateway had to be rebuilt from scratch, recreate the affected sandbox afterward.", From 1978b74f5cbab62df5316d11a4f2b39433469d6e Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Mon, 1 Jun 2026 14:45:51 +0000 Subject: [PATCH 16/27] fix(state): inject active gateway name into reuse migration instead of deriving from dashboardPort Signed-off-by: Tinson Lai --- src/lib/onboard.ts | 2 +- .../onboard/sandbox-registry-metadata.test.ts | 22 ++++++++++++++++++- src/lib/onboard/sandbox-registry-metadata.ts | 11 ++++++++-- 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index d8908bce345..0f316d4074f 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -2755,9 +2755,9 @@ const { getSandboxRuntimeRegistryFields, hasSandboxGpuDrift, updateReusedSandbox isLinuxDockerDriverGatewayEnabled, getInstalledOpenshellVersion, runCaptureOpenshell, + getActiveGatewayName: () => GATEWAY_NAME, }); - // ── Step 5: Sandbox ────────────────────────────────────────────── async function createSandbox( diff --git a/src/lib/onboard/sandbox-registry-metadata.test.ts b/src/lib/onboard/sandbox-registry-metadata.test.ts index 46137fa486a..b7a48de3281 100644 --- a/src/lib/onboard/sandbox-registry-metadata.test.ts +++ b/src/lib/onboard/sandbox-registry-metadata.test.ts @@ -37,11 +37,12 @@ function restorePlatform(): void { } } -function makeHelpers(opts: { dockerDriverEnabled: boolean }) { +function makeHelpers(opts: { dockerDriverEnabled: boolean; activeGatewayName?: string }) { return createSandboxRegistryMetadataHelpers({ isLinuxDockerDriverGatewayEnabled: () => opts.dockerDriverEnabled, getInstalledOpenshellVersion: () => "0.0.42", runCaptureOpenshell: () => null, + getActiveGatewayName: () => opts.activeGatewayName ?? "nemoclaw", }); } @@ -141,4 +142,23 @@ describe("updateReusedSandboxMetadata gatewayName migration", () => { expect(registry.getSandbox("alpha")?.gatewayName).toBe("nemoclaw-8081"); }); + + it("backfills using the active gateway name even when dashboardPort and gateway port differ", () => { + // Regression guard: the helper used to call `getGatewayName(dashboardPort)` + // which would derive a wrong binding once the resolver flips to per-port + // names — `dashboardPort` is the chat-UI forward, not the gateway port. + // The deps-injected `getActiveGatewayName()` must win. + registry.registerSandbox({ name: "legacy", model: "m", provider: "p" }); + + const helpers = makeHelpers({ + dockerDriverEnabled: true, + activeGatewayName: "nemoclaw-8081", + }); + // Pass a dashboardPort that is obviously not the gateway port (e.g. a UI + // forward port like 9081). The migration must record the injected + // gateway name, not anything derived from this port. + helpers.updateReusedSandboxMetadata("legacy", null, "m", "p", 9081); + + expect(registry.getSandbox("legacy")?.gatewayName).toBe("nemoclaw-8081"); + }); }); diff --git a/src/lib/onboard/sandbox-registry-metadata.ts b/src/lib/onboard/sandbox-registry-metadata.ts index 5fe657ab5c7..fe63f072615 100644 --- a/src/lib/onboard/sandbox-registry-metadata.ts +++ b/src/lib/onboard/sandbox-registry-metadata.ts @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 import type { AgentDefinition } from "../agent/defs"; -import { getGatewayName } from "../state/gateway-name"; import type { SandboxEntry } from "../state/registry"; import * as registry from "../state/registry"; import { getSandboxAgentRegistryFields } from "./sandbox-agent"; @@ -12,6 +11,14 @@ export interface SandboxRegistryMetadataDeps { isLinuxDockerDriverGatewayEnabled(): boolean; getInstalledOpenshellVersion(versionOutput?: string | null): string | null; runCaptureOpenshell(args: string[], opts?: Record): string | null; + /** + * Resolve the active OpenShell gateway name for this process. Injected so + * the legacy-reuse backfill records the active gateway binding without + * reaching for `getGatewayName(dashboardPort)` — `dashboardPort` is the + * chat-UI forward, not the gateway port, and would produce a wrong + * binding once the resolver flips to per-port names. + */ + getActiveGatewayName(): string; } export interface SandboxRegistryMetadataHelpers { @@ -109,7 +116,7 @@ export function createSandboxRegistryMetadataHelpers( // cleanup. const gatewayMigration = existingEntry && existingEntry.gatewayName === undefined - ? { gatewayName: getGatewayName(dashboardPort) } + ? { gatewayName: deps.getActiveGatewayName() } : {}; registry.updateSandbox(sandboxName, { ...selectionUpdates, From 196725f1a45158c2c3f0ebc0f1495c4035807864 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Mon, 1 Jun 2026 15:16:17 +0000 Subject: [PATCH 17/27] fix(state): drop unused DEFAULT_GATEWAY_NAME import from registry Signed-off-by: Tinson Lai --- src/lib/state/registry.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/lib/state/registry.ts b/src/lib/state/registry.ts index 62a1c4985be..41bfcb57d86 100644 --- a/src/lib/state/registry.ts +++ b/src/lib/state/registry.ts @@ -6,7 +6,7 @@ import path from "node:path"; import { ensureConfigDir, readConfigFile, writeConfigFile } from "./config-io"; import { isErrnoException } from "../core/errno"; -import { DEFAULT_GATEWAY_NAME, validateGatewayName } from "./gateway-name"; +import { validateGatewayName } from "./gateway-name"; import type { MessagingChannelConfig } from "../messaging-channel-config"; export { getSandboxGatewayName } from "./gateway-name"; @@ -50,9 +50,10 @@ export interface SandboxEntry { /** * OpenShell gateway name this sandbox is bound to. Optional for backward * compatibility — legacy entries created before per-sandbox gateway tracking - * resolve to {@link DEFAULT_GATEWAY_NAME} via {@link getSandboxGatewayName}. - * Currently every sandbox uses the singleton name; the field exists so - * follow-up work can record per-port gateway names without a schema change. + * resolve to the singleton default via `getSandboxGatewayName` (exported + * from `./gateway-name`). Currently every sandbox uses the singleton name; + * the field exists so follow-up work can record per-port gateway names + * without a schema change. */ gatewayName?: string; } From 40042b86b6cc5c3b248ba033157fda3b1551dd15 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Fri, 5 Jun 2026 03:24:56 +0000 Subject: [PATCH 18/27] test(e2e): add concurrent-gateway-ports E2E and expand advisor catalog Signed-off-by: Tinson Lai --- .github/workflows/nightly-e2e.yaml | 57 ++++- test/e2e/test-concurrent-gateway-ports.sh | 245 ++++++++++++++++++++++ 2 files changed, 301 insertions(+), 1 deletion(-) create mode 100755 test/e2e/test-concurrent-gateway-ports.sh diff --git a/.github/workflows/nightly-e2e.yaml b/.github/workflows/nightly-e2e.yaml index e39855b3578..0d76cdd604e 100644 --- a/.github/workflows/nightly-e2e.yaml +++ b/.github/workflows/nightly-e2e.yaml @@ -62,6 +62,14 @@ # openclaw-inference-switch-e2e # Switches a running OpenClaw sandbox with `nemoclaw inference set` # and verifies route, openclaw.json, hashes, and live requests. +# openclaw-skill-cli-e2e Validates workspace-installed OpenClaw skills survive sandbox +# lifecycle through OPENCLAW_HOME/STATE_DIR/WORKSPACE_DIR pinning +# (#4766 / #4709). Seven-phase deterministic skill-CLI exercise +# inside a real onboarded sandbox (install, list, info, check). +# channels-add-remove-e2e Telegram/Discord/Slack channel add/remove lifecycle plus +# gateway-credential reuse on rebuild (#4745 / #3895). Exercises +# the path where the host env credential is empty but the +# gateway already holds the provider credential. # issue-4434-tui-unreachable-inference-e2e # Recreates #4434's NVIDIA endpoint firewall block and verifies # OpenClaw TUI shows a visible error and stops the active spinner. @@ -71,6 +79,10 @@ # launchable-smoke-e2e Community install path (brev-launchable-ci-cpu.sh) on ubuntu-latest. # gpu-e2e Local Ollama inference on an NVKS ephemeral GPU runner. # gpu-double-onboard-e2e Ollama proxy token consistency after re-onboard (#2553). +# concurrent-gateway-ports-e2e +# Two sandboxes coexisting on the same host with distinct +# NEMOCLAW_GATEWAY_PORT values; covers the multi-instance +# scenarios referenced in #3053, #4422, and #4520. # notify-on-failure Auto-creates a GitHub issue when any E2E job fails. # # Runs directly on the runner (not inside Docker) because OpenShell bootstraps @@ -117,7 +129,8 @@ on: rebuild-hermes-stale-base-e2e, double-onboard-e2e, onboard-repair-e2e, onboard-resume-e2e, onboard-negative-paths-e2e, runtime-overrides-e2e, credential-sanitization-e2e, telegram-injection-e2e, overlayfs-autofix-e2e, - device-auth-health-e2e, launchable-smoke-e2e, gpu-e2e, gpu-double-onboard-e2e + device-auth-health-e2e, launchable-smoke-e2e, gpu-e2e, gpu-double-onboard-e2e, + concurrent-gateway-ports-e2e required: false type: string default: "" @@ -2009,6 +2022,45 @@ jobs: path: /tmp/nemoclaw-gpu-double-onboard-test.log if-no-files-found: ignore + concurrent-gateway-ports-e2e: + if: >- + github.repository == 'NVIDIA/NemoClaw' && + (github.event_name != 'workflow_dispatch' || + inputs.jobs == '' || + contains(format(',{0},', inputs.jobs), ',concurrent-gateway-ports-e2e,')) + runs-on: ubuntu-latest + timeout-minutes: 60 + env: + NEMOCLAW_NON_INTERACTIVE: "1" + NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE: "1" + NEMOCLAW_E2E_PHASE_TIMEOUT: "1200" + steps: + - *target-ref-checkout + - *dockerhub-auth-step + - name: Run concurrent gateway ports E2E test + run: bash test/e2e/test-concurrent-gateway-ports.sh + - name: Upload sandbox A onboard log on failure + if: failure() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: concurrent-gateway-ports-sandbox-a-onboard-log + path: /tmp/e2e-cgp-a-onboard.log + if-no-files-found: ignore + - name: Upload sandbox B onboard log on failure + if: failure() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: concurrent-gateway-ports-sandbox-b-onboard-log + path: /tmp/e2e-cgp-b-onboard.log + if-no-files-found: ignore + - name: Upload sandbox B destroy log on failure + if: failure() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: concurrent-gateway-ports-sandbox-b-destroy-log + path: /tmp/e2e-cgp-b-destroy.log + if-no-files-found: ignore + notify-on-failure: runs-on: ubuntu-latest needs: @@ -2074,6 +2126,7 @@ jobs: launchable-smoke-e2e, gpu-e2e, gpu-double-onboard-e2e, + concurrent-gateway-ports-e2e, ] if: ${{ always() && github.event_name == 'schedule' && (contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled')) }} permissions: @@ -2185,6 +2238,7 @@ jobs: launchable-smoke-e2e, gpu-e2e, gpu-double-onboard-e2e, + concurrent-gateway-ports-e2e, ] if: ${{ always() && github.event_name == 'workflow_dispatch' }} permissions: @@ -2353,6 +2407,7 @@ jobs: launchable-smoke-e2e, gpu-e2e, gpu-double-onboard-e2e, + concurrent-gateway-ports-e2e, ] if: ${{ always() && (github.event_name == 'schedule' || github.event_name == 'workflow_dispatch') }} permissions: diff --git a/test/e2e/test-concurrent-gateway-ports.sh b/test/e2e/test-concurrent-gateway-ports.sh new file mode 100755 index 00000000000..863cdc2df51 --- /dev/null +++ b/test/e2e/test-concurrent-gateway-ports.sh @@ -0,0 +1,245 @@ +#!/bin/bash +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +# Concurrent gateway ports — covers the multi-instance scenarios across three +# linked issues that share the same underlying use case but were patched +# piecemeal in earlier PRs: +# +# #3053 — parent ask: multiple NemoClaw-managed instances on a single host +# with full segregation of state, registry, and gateways. +# #4422 — NEMOCLAW_GATEWAY_PORT=N onboard recreates the global gateway and +# destroys the previous sandbox; concurrent instances unsupported. +# QA flagged that the per-port fix in #4645 still collides on the +# dashboard port even though the gateway port is bound per instance. +# #4520 — containerised-compat gateway mode (host glibc < gateway requirement) +# judges a healthy compat gateway stale on the second onboard, causing +# a port 8080 recreate-collision. +# +# Scenario shape: +# 1. Onboard sandbox A on the default gateway port (8080) + default dashboard +# port (18789). +# 2. Onboard sandbox B with NEMOCLAW_GATEWAY_PORT set to a non-default port +# that drives the per-port binding path. The dashboard port should +# auto-allocate from the 18789-18799 range without colliding with A. +# 3. Verify both sandboxes coexist: distinct gateways, distinct dashboards, +# distinct sandbox containers, no SIGKILL of A during B's onboard, and +# nemoclaw list reports two entries with two distinct dashboard URLs. +# 4. Destroy B and verify A remains healthy. +# +# This script intentionally uses a local fake OpenAI-compatible endpoint so it +# does not depend on real NVIDIA endpoints, matching the pattern in +# test-double-onboard.sh. + +# ShellCheck cannot see EXIT trap invocations of cleanup helpers in this E2E script. +# shellcheck disable=SC2317 +set -uo pipefail + +export NEMOCLAW_E2E_DEFAULT_TIMEOUT=4800 +SCRIPT_DIR_TIMEOUT="$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")" && pwd)" +# shellcheck source=test/e2e/e2e-timeout.sh +source "${SCRIPT_DIR_TIMEOUT}/e2e-timeout.sh" + +PASS=0 +FAIL=0 +TOTAL=0 + +pass() { + ((PASS++)) + ((TOTAL++)) + printf '\033[32m PASS: %s\033[0m\n' "$1" +} +fail() { + ((FAIL++)) + ((TOTAL++)) + printf '\033[31m FAIL: %s\033[0m\n' "$1" +} +section() { + echo "" + printf '\033[1;36m=== %s ===\033[0m\n' "$1" +} +info() { printf '\033[1;34m [info]\033[0m %s\n' "$1"; } + +PHASE_TIMEOUT="${NEMOCLAW_E2E_PHASE_TIMEOUT:-1200}" + +SANDBOX_A="e2e-cgp-a" +SANDBOX_B="e2e-cgp-b" +GATEWAY_PORT_A=8080 +GATEWAY_PORT_B="${NEMOCLAW_E2E_GATEWAY_PORT_B:-18080}" +DASHBOARD_PORT_A=18789 + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")" && pwd)" +REPO_ROOT="$(cd "${SCRIPT_DIR}/../.." && pwd)" +FAKE_HOST="127.0.0.1" +FAKE_PORT="${NEMOCLAW_E2E_FAKE_PORT:-18180}" +FAKE_BASE_URL="http://${FAKE_HOST}:${FAKE_PORT}/v1" +FAKE_LOG="$(mktemp)" +FAKE_PID="" + +if command -v node >/dev/null 2>&1 && [ -f "$REPO_ROOT/bin/nemoclaw.js" ]; then + NEMOCLAW_CMD=(node "$REPO_ROOT/bin/nemoclaw.js") +else + NEMOCLAW_CMD=(nemoclaw) +fi + +# shellcheck disable=SC2329 +cleanup() { + if [ -n "$FAKE_PID" ] && kill -0 "$FAKE_PID" 2>/dev/null; then + kill "$FAKE_PID" 2>/dev/null || true + wait "$FAKE_PID" 2>/dev/null || true + fi + rm -f "$FAKE_LOG" +} +trap cleanup EXIT + +start_fake_openai() { + python3 - "$FAKE_HOST" "$FAKE_PORT" >"$FAKE_LOG" 2>&1 & + FAKE_PID=$! + sleep 1 + if ! kill -0 "$FAKE_PID" 2>/dev/null; then + fail "Fake OpenAI server did not start; see ${FAKE_LOG}" + cat "$FAKE_LOG" + exit 1 + fi + info "Fake OpenAI server up on ${FAKE_BASE_URL} (pid ${FAKE_PID})" +} + +dashboard_port_from_list() { + local sandbox="$1" + "${NEMOCLAW_CMD[@]}" list 2>/dev/null \ + | grep -E "^[[:space:]]*${sandbox}[[:space:]]" \ + | grep -oE 'http://127\.0\.0\.1:[0-9]+' \ + | head -1 \ + | grep -oE '[0-9]+$' || true +} + +dump_diagnostics() { + local label="${1:-unknown}" + info "=== Diagnostics for ${label} ===" + info "nemoclaw list:" + "${NEMOCLAW_CMD[@]}" list 2>&1 | sed 's/^/ /' || true + info "openshell sandbox list:" + openshell sandbox list 2>&1 | sed 's/^/ /' || true + info "openshell forward list:" + openshell forward list 2>&1 | sed 's/^/ /' || true + info "docker ps -a:" + docker ps -a --format 'table {{.Names}}\t{{.Status}}' 2>&1 | sed 's/^/ /' || true + info "ss -ltn (gateway/dashboard ports):" + ss -ltn 2>&1 | grep -E ":(${GATEWAY_PORT_A}|${GATEWAY_PORT_B}|1878[0-9]|1879[0-9])" | sed 's/^/ /' || true +} + +onboard_sandbox() { + local name="$1" + local gateway_port="$2" + local label="onboard-${name}" + local start_time + start_time="$(date +%s)" + info "Starting onboard of '${name}' with NEMOCLAW_GATEWAY_PORT=${gateway_port}" + if NEMOCLAW_NON_INTERACTIVE=1 \ + NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1 \ + NEMOCLAW_PROVIDER=openai-compatible \ + NEMOCLAW_OPENAI_API_KEY=test-key \ + NEMOCLAW_OPENAI_BASE_URL="${FAKE_BASE_URL}" \ + NEMOCLAW_MODEL=test-model \ + NEMOCLAW_GATEWAY_PORT="${gateway_port}" \ + NEMOCLAW_SANDBOX_NAME="${name}" \ + timeout "${PHASE_TIMEOUT}" "${NEMOCLAW_CMD[@]}" onboard --fresh --name "${name}" \ + >"/tmp/${name}-onboard.log" 2>&1; then + local elapsed + elapsed=$(( $(date +%s) - start_time )) + pass "${label} completed in ${elapsed}s" + return 0 + fi + fail "${label} did not complete within ${PHASE_TIMEOUT}s" + dump_diagnostics "${label}" + tail -200 "/tmp/${name}-onboard.log" | sed 's/^/ /' + return 1 +} + +verify_sandbox_alive() { + local name="$1" + local label="${2:-${name} alive}" + local status + status="$("${NEMOCLAW_CMD[@]}" "${name}" status 2>&1 || true)" + if echo "${status}" | grep -qE 'Phase:[[:space:]]+(Ready|Running)'; then + pass "${label}" + return 0 + fi + fail "${label} (status: ${status})" + return 1 +} + +# === Scenario === + +section "Stage 0: prepare fake inference endpoint" +start_fake_openai + +section "Stage 1: onboard sandbox A on default gateway port (${GATEWAY_PORT_A})" +onboard_sandbox "${SANDBOX_A}" "${GATEWAY_PORT_A}" || exit 1 +verify_sandbox_alive "${SANDBOX_A}" "Sandbox A reaches Ready/Running on default port" + +DASHBOARD_A="$(dashboard_port_from_list "${SANDBOX_A}")" +if [ -n "${DASHBOARD_A}" ] && [ "${DASHBOARD_A}" = "${DASHBOARD_PORT_A}" ]; then + pass "Sandbox A holds default dashboard port ${DASHBOARD_PORT_A}" +else + fail "Sandbox A dashboard port is '${DASHBOARD_A:-missing}', expected ${DASHBOARD_PORT_A}" +fi + +section "Stage 2: onboard sandbox B with NEMOCLAW_GATEWAY_PORT=${GATEWAY_PORT_B} (#4422 / #3053)" +onboard_sandbox "${SANDBOX_B}" "${GATEWAY_PORT_B}" || { + info "B onboard failed; capturing pre-fail state of A for #4422 diagnostics" + dump_diagnostics "stage-2-onboard-B" + exit 1 +} + +section "Stage 3: assert both sandboxes coexist" +verify_sandbox_alive "${SANDBOX_A}" "Sandbox A still alive after B's onboard (#4422 SIGKILL regression)" +verify_sandbox_alive "${SANDBOX_B}" "Sandbox B reaches Ready/Running on per-port gateway" + +DASHBOARD_B="$(dashboard_port_from_list "${SANDBOX_B}")" +if [ -n "${DASHBOARD_B}" ] && [ "${DASHBOARD_B}" != "${DASHBOARD_A:-${DASHBOARD_PORT_A}}" ]; then + pass "Sandbox B got a distinct dashboard port (A=${DASHBOARD_A:-missing} B=${DASHBOARD_B}) (#4422 dashboard-port QA gap)" +else + fail "Sandbox B dashboard port collides with A: A=${DASHBOARD_A:-missing} B=${DASHBOARD_B:-missing}" + dump_diagnostics "dashboard-port-collision" +fi + +if ss -ltn 2>/dev/null | grep -qE ":${GATEWAY_PORT_A}\\b"; then + pass "Sandbox A gateway port ${GATEWAY_PORT_A} still listening (#4520 drift detection regression)" +else + fail "Sandbox A gateway port ${GATEWAY_PORT_A} no longer listening — recreate destroyed first gateway" + dump_diagnostics "gateway-port-A-missing" +fi + +if ss -ltn 2>/dev/null | grep -qE ":${GATEWAY_PORT_B}\\b"; then + pass "Sandbox B gateway port ${GATEWAY_PORT_B} listening (#4422 per-port binding)" +else + fail "Sandbox B gateway port ${GATEWAY_PORT_B} not listening" + dump_diagnostics "gateway-port-B-missing" +fi + +LIST_OUTPUT="$("${NEMOCLAW_CMD[@]}" list 2>&1 || true)" +if echo "${LIST_OUTPUT}" | grep -qE "^[[:space:]]*${SANDBOX_A}[[:space:]]" \ + && echo "${LIST_OUTPUT}" | grep -qE "^[[:space:]]*${SANDBOX_B}[[:space:]]"; then + pass "nemoclaw list shows both sandbox A and B" +else + fail "nemoclaw list missing one of A/B" + # shellcheck disable=SC2001 + echo "${LIST_OUTPUT}" | sed 's/^/ /' +fi + +section "Stage 4: destroy sandbox B; assert sandbox A still healthy" +if NEMOCLAW_NON_INTERACTIVE=1 timeout 300 "${NEMOCLAW_CMD[@]}" "${SANDBOX_B}" destroy --yes \ + >"/tmp/${SANDBOX_B}-destroy.log" 2>&1; then + pass "Sandbox B destroyed" +else + fail "Sandbox B destroy timed out or failed" + tail -100 "/tmp/${SANDBOX_B}-destroy.log" | sed 's/^/ /' +fi +verify_sandbox_alive "${SANDBOX_A}" "Sandbox A still alive after B's destroy" + +section "Summary: PASS=${PASS} FAIL=${FAIL} TOTAL=${TOTAL}" +if [ "${FAIL}" -gt 0 ]; then + exit 1 +fi +exit 0 From 404de0c3f0764770e794d036ddacd40bd616d571 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Fri, 5 Jun 2026 04:15:18 +0000 Subject: [PATCH 19/27] fix(e2e): add missing fake OpenAI server heredoc + readiness poll Signed-off-by: Tinson Lai --- test/e2e/test-concurrent-gateway-ports.sh | 100 +++++++++++++++++----- 1 file changed, 80 insertions(+), 20 deletions(-) diff --git a/test/e2e/test-concurrent-gateway-ports.sh b/test/e2e/test-concurrent-gateway-ports.sh index 863cdc2df51..a9d32bd2a1b 100755 --- a/test/e2e/test-concurrent-gateway-ports.sh +++ b/test/e2e/test-concurrent-gateway-ports.sh @@ -93,15 +93,75 @@ cleanup() { trap cleanup EXIT start_fake_openai() { - python3 - "$FAKE_HOST" "$FAKE_PORT" >"$FAKE_LOG" 2>&1 & + python3 - "$FAKE_HOST" "$FAKE_PORT" >"$FAKE_LOG" 2>&1 <<'PY' & +import json +import sys +from http.server import BaseHTTPRequestHandler, HTTPServer + +HOST = sys.argv[1] +PORT = int(sys.argv[2]) + + +class Handler(BaseHTTPRequestHandler): + def _send(self, status, payload): + body = json.dumps(payload).encode("utf-8") + self.send_response(status) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def log_message(self, format, *args): + return + + def do_GET(self): + if self.path in ("/v1/models", "/models"): + self._send(200, {"data": [{"id": "test-model", "object": "model"}]}) + return + self._send(404, {"error": {"message": "not found"}}) + + def do_POST(self): + length = int(self.headers.get("Content-Length", "0")) + if length: + self.rfile.read(length) + if self.path in ("/v1/chat/completions", "/chat/completions"): + self._send( + 200, + { + "id": "chatcmpl-test", + "object": "chat.completion", + "choices": [{"index": 0, "message": {"role": "assistant", "content": "ok"}, "finish_reason": "stop"}], + }, + ) + return + if self.path in ("/v1/responses", "/responses"): + self._send( + 200, + { + "id": "resp-test", + "object": "response", + "output": [{"type": "message", "role": "assistant", "content": [{"type": "output_text", "text": "ok"}]}], + }, + ) + return + self._send(404, {"error": {"message": "not found"}}) + + +HTTPServer((HOST, PORT), Handler).serve_forever() +PY FAKE_PID=$! - sleep 1 - if ! kill -0 "$FAKE_PID" 2>/dev/null; then - fail "Fake OpenAI server did not start; see ${FAKE_LOG}" - cat "$FAKE_LOG" - exit 1 - fi - info "Fake OpenAI server up on ${FAKE_BASE_URL} (pid ${FAKE_PID})" + + for _ in $(seq 1 20); do + if curl -sf "${FAKE_BASE_URL}/models" >/dev/null 2>&1; then + info "Fake OpenAI server up on ${FAKE_BASE_URL} (pid ${FAKE_PID})" + return 0 + fi + sleep 1 + done + + fail "Fake OpenAI server did not become ready on ${FAKE_BASE_URL}; see ${FAKE_LOG}" + cat "$FAKE_LOG" + exit 1 } dashboard_port_from_list() { @@ -136,17 +196,17 @@ onboard_sandbox() { start_time="$(date +%s)" info "Starting onboard of '${name}' with NEMOCLAW_GATEWAY_PORT=${gateway_port}" if NEMOCLAW_NON_INTERACTIVE=1 \ - NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1 \ - NEMOCLAW_PROVIDER=openai-compatible \ - NEMOCLAW_OPENAI_API_KEY=test-key \ - NEMOCLAW_OPENAI_BASE_URL="${FAKE_BASE_URL}" \ - NEMOCLAW_MODEL=test-model \ - NEMOCLAW_GATEWAY_PORT="${gateway_port}" \ - NEMOCLAW_SANDBOX_NAME="${name}" \ - timeout "${PHASE_TIMEOUT}" "${NEMOCLAW_CMD[@]}" onboard --fresh --name "${name}" \ - >"/tmp/${name}-onboard.log" 2>&1; then + NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1 \ + NEMOCLAW_PROVIDER=openai-compatible \ + NEMOCLAW_OPENAI_API_KEY=test-key \ + NEMOCLAW_OPENAI_BASE_URL="${FAKE_BASE_URL}" \ + NEMOCLAW_MODEL=test-model \ + NEMOCLAW_GATEWAY_PORT="${gateway_port}" \ + NEMOCLAW_SANDBOX_NAME="${name}" \ + timeout "${PHASE_TIMEOUT}" "${NEMOCLAW_CMD[@]}" onboard --fresh --name "${name}" \ + >"/tmp/${name}-onboard.log" 2>&1; then local elapsed - elapsed=$(( $(date +%s) - start_time )) + elapsed=$(($(date +%s) - start_time)) pass "${label} completed in ${elapsed}s" return 0 fi @@ -220,7 +280,7 @@ fi LIST_OUTPUT="$("${NEMOCLAW_CMD[@]}" list 2>&1 || true)" if echo "${LIST_OUTPUT}" | grep -qE "^[[:space:]]*${SANDBOX_A}[[:space:]]" \ - && echo "${LIST_OUTPUT}" | grep -qE "^[[:space:]]*${SANDBOX_B}[[:space:]]"; then + && echo "${LIST_OUTPUT}" | grep -qE "^[[:space:]]*${SANDBOX_B}[[:space:]]"; then pass "nemoclaw list shows both sandbox A and B" else fail "nemoclaw list missing one of A/B" @@ -230,7 +290,7 @@ fi section "Stage 4: destroy sandbox B; assert sandbox A still healthy" if NEMOCLAW_NON_INTERACTIVE=1 timeout 300 "${NEMOCLAW_CMD[@]}" "${SANDBOX_B}" destroy --yes \ - >"/tmp/${SANDBOX_B}-destroy.log" 2>&1; then + >"/tmp/${SANDBOX_B}-destroy.log" 2>&1; then pass "Sandbox B destroyed" else fail "Sandbox B destroy timed out or failed" From ac1c1a959204416fa899a794619fb1db9209f186 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Fri, 5 Jun 2026 04:22:13 +0000 Subject: [PATCH 20/27] fix(ci): install NemoClaw before concurrent-gateway-ports E2E Signed-off-by: Tinson Lai --- .github/workflows/nightly-e2e.yaml | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/.github/workflows/nightly-e2e.yaml b/.github/workflows/nightly-e2e.yaml index 0d76cdd604e..02eee7c30ba 100644 --- a/.github/workflows/nightly-e2e.yaml +++ b/.github/workflows/nightly-e2e.yaml @@ -2037,8 +2037,21 @@ jobs: steps: - *target-ref-checkout - *dockerhub-auth-step + - name: Install NemoClaw + env: + NVIDIA_API_KEY: ${{ secrets.NVIDIA_API_KEY }} + NEMOCLAW_NON_INTERACTIVE: "1" + NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE: "1" + run: bash install.sh --non-interactive --yes-i-accept-third-party-software - name: Run concurrent gateway ports E2E test - run: bash test/e2e/test-concurrent-gateway-ports.sh + env: + NVIDIA_API_KEY: ${{ secrets.NVIDIA_API_KEY }} + run: | + [ -f "$HOME/.bashrc" ] && source "$HOME/.bashrc" 2>/dev/null || true + export NVM_DIR="${NVM_DIR:-$HOME/.nvm}" + [ -s "$NVM_DIR/nvm.sh" ] && . "$NVM_DIR/nvm.sh" + [ -d "$HOME/.local/bin" ] && [[ ":$PATH:" != *":$HOME/.local/bin:"* ]] && export PATH="$HOME/.local/bin:$PATH" + bash test/e2e/test-concurrent-gateway-ports.sh - name: Upload sandbox A onboard log on failure if: failure() uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 From 00a243ef6fd9c4f7fb464e52cb19d646b8cde016 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Fri, 5 Jun 2026 04:28:42 +0000 Subject: [PATCH 21/27] fix(e2e): use custom provider + clear default sandbox before stages Signed-off-by: Tinson Lai --- test/e2e/test-concurrent-gateway-ports.sh | 41 ++++++++++++++++++++--- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/test/e2e/test-concurrent-gateway-ports.sh b/test/e2e/test-concurrent-gateway-ports.sh index a9d32bd2a1b..9eef9f8c56d 100755 --- a/test/e2e/test-concurrent-gateway-ports.sh +++ b/test/e2e/test-concurrent-gateway-ports.sh @@ -195,15 +195,18 @@ onboard_sandbox() { local start_time start_time="$(date +%s)" info "Starting onboard of '${name}' with NEMOCLAW_GATEWAY_PORT=${gateway_port}" - if NEMOCLAW_NON_INTERACTIVE=1 \ + if COMPATIBLE_API_KEY=dummy \ + NEMOCLAW_NON_INTERACTIVE=1 \ NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1 \ - NEMOCLAW_PROVIDER=openai-compatible \ - NEMOCLAW_OPENAI_API_KEY=test-key \ - NEMOCLAW_OPENAI_BASE_URL="${FAKE_BASE_URL}" \ + NEMOCLAW_PROVIDER=custom \ + NEMOCLAW_ENDPOINT_URL="${FAKE_BASE_URL}" \ NEMOCLAW_MODEL=test-model \ + NEMOCLAW_POLICY_MODE=skip \ + NEMOCLAW_DASHBOARD_PORT='' \ + CHAT_UI_URL='' \ NEMOCLAW_GATEWAY_PORT="${gateway_port}" \ NEMOCLAW_SANDBOX_NAME="${name}" \ - timeout "${PHASE_TIMEOUT}" "${NEMOCLAW_CMD[@]}" onboard --fresh --name "${name}" \ + timeout "${PHASE_TIMEOUT}" "${NEMOCLAW_CMD[@]}" onboard --non-interactive \ >"/tmp/${name}-onboard.log" 2>&1; then local elapsed elapsed=$(($(date +%s) - start_time)) @@ -216,6 +219,31 @@ onboard_sandbox() { return 1 } +destroy_default_install_sandbox() { + local default_name + default_name="$("${NEMOCLAW_CMD[@]}" list 2>/dev/null \ + | grep -E '^[[:space:]]+[a-zA-Z0-9_-]+ \*' \ + | awk '{print $1}' \ + | head -1 || true)" + if [ -z "${default_name}" ]; then + info "no pre-existing default sandbox to destroy" + return 0 + fi + if [ "${default_name}" = "${SANDBOX_A}" ] || [ "${default_name}" = "${SANDBOX_B}" ]; then + info "default sandbox is one under test (${default_name}); skipping pre-destroy" + return 0 + fi + info "destroying pre-existing default sandbox '${default_name}' (created by install.sh)" + if NEMOCLAW_NON_INTERACTIVE=1 timeout 300 "${NEMOCLAW_CMD[@]}" "${default_name}" destroy --yes \ + >"/tmp/${default_name}-predestroy.log" 2>&1; then + pass "pre-existing default sandbox '${default_name}' destroyed" + else + fail "could not destroy pre-existing default sandbox '${default_name}'" + tail -100 "/tmp/${default_name}-predestroy.log" | sed 's/^/ /' + return 1 + fi +} + verify_sandbox_alive() { local name="$1" local label="${2:-${name} alive}" @@ -234,6 +262,9 @@ verify_sandbox_alive() { section "Stage 0: prepare fake inference endpoint" start_fake_openai +section "Stage 0.5: destroy default sandbox created by install.sh (if any)" +destroy_default_install_sandbox || exit 1 + section "Stage 1: onboard sandbox A on default gateway port (${GATEWAY_PORT_A})" onboard_sandbox "${SANDBOX_A}" "${GATEWAY_PORT_A}" || exit 1 verify_sandbox_alive "${SANDBOX_A}" "Sandbox A reaches Ready/Running on default port" From 88a83856d053ab881ee88218dca88b3d447ed8a2 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Fri, 5 Jun 2026 04:48:47 +0000 Subject: [PATCH 22/27] fix(e2e): parse dashboard URL across lines + read phase from openshell list Signed-off-by: Tinson Lai --- test/e2e/test-concurrent-gateway-ports.sh | 33 ++++++++++++++++------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/test/e2e/test-concurrent-gateway-ports.sh b/test/e2e/test-concurrent-gateway-ports.sh index 9eef9f8c56d..32783f38e7f 100755 --- a/test/e2e/test-concurrent-gateway-ports.sh +++ b/test/e2e/test-concurrent-gateway-ports.sh @@ -167,10 +167,18 @@ PY dashboard_port_from_list() { local sandbox="$1" "${NEMOCLAW_CMD[@]}" list 2>/dev/null \ - | grep -E "^[[:space:]]*${sandbox}[[:space:]]" \ - | grep -oE 'http://127\.0\.0\.1:[0-9]+' \ - | head -1 \ - | grep -oE '[0-9]+$' || true + | awk -v want="${sandbox}" ' + /^[[:space:]]+[A-Za-z0-9_-]+( \*)?[[:space:]]*$/ { + name=$1 + inblock=(name == want) ? 1 : 0 + next + } + inblock && /dashboard:[[:space:]]*http:\/\/[0-9.]+:[0-9]+/ { + match($0, /:[0-9]+/) + print substr($0, RSTART+1, RLENGTH-1) + exit + } + ' } dump_diagnostics() { @@ -244,16 +252,23 @@ destroy_default_install_sandbox() { fi } +sandbox_phase() { + local name="$1" + openshell sandbox list 2>/dev/null \ + | sed 's/\x1b\[[0-9;]*m//g' \ + | awk -v want="${name}" '$1 == want { print $NF; exit }' +} + verify_sandbox_alive() { local name="$1" local label="${2:-${name} alive}" - local status - status="$("${NEMOCLAW_CMD[@]}" "${name}" status 2>&1 || true)" - if echo "${status}" | grep -qE 'Phase:[[:space:]]+(Ready|Running)'; then - pass "${label}" + local phase + phase="$(sandbox_phase "${name}")" + if [ "${phase}" = "Ready" ] || [ "${phase}" = "Running" ]; then + pass "${label} (phase=${phase})" return 0 fi - fail "${label} (status: ${status})" + fail "${label} (phase='${phase:-missing}')" return 1 } From f502f984e85bff9179eb2ffe7d763bc3d7ce9a2f Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Fri, 5 Jun 2026 05:18:16 +0000 Subject: [PATCH 23/27] fix(onboard): skip gateway retire when foreign-active per-port gateway exists Signed-off-by: Tinson Lai --- .../onboard/machine/handlers/gateway.test.ts | 18 ++++++++++++++++++ src/lib/onboard/machine/handlers/gateway.ts | 8 +++++++- test/e2e/test-concurrent-gateway-ports.sh | 4 ++-- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/lib/onboard/machine/handlers/gateway.test.ts b/src/lib/onboard/machine/handlers/gateway.test.ts index b184fdb8263..304745c529e 100644 --- a/src/lib/onboard/machine/handlers/gateway.test.ts +++ b/src/lib/onboard/machine/handlers/gateway.test.ts @@ -314,4 +314,22 @@ describe("handleGatewayState", () => { " Replacing legacy OpenShell gateway metadata with Docker-driver gateway.", ); }); + + it("does not retire a foreign-active Docker-driver gateway (#4422 concurrent instances)", async () => { + const { deps, calls } = createDeps({ + isLinuxDockerDriverGatewayEnabled: vi.fn(() => true), + reconcileGatewayGpuReuseForGpuIntent: vi.fn( + () => "foreign-active" as GatewayReuseState, + ), + }); + + const result = await handleGatewayState(baseOptions(deps, "foreign-active")); + + expect(calls.retireLegacy).not.toHaveBeenCalled(); + expect(calls.note).not.toHaveBeenCalledWith( + " Replacing legacy OpenShell gateway metadata with Docker-driver gateway.", + ); + expect(calls.startGateway).toHaveBeenCalledOnce(); + expect(result.gatewayReuseState).toBe("missing"); + }); }); diff --git a/src/lib/onboard/machine/handlers/gateway.ts b/src/lib/onboard/machine/handlers/gateway.ts index 6589db29cda..54ec2e73d57 100644 --- a/src/lib/onboard/machine/handlers/gateway.ts +++ b/src/lib/onboard/machine/handlers/gateway.ts @@ -204,10 +204,16 @@ export async function handleGatewayState({ } } await deps.startRecordedStep("gateway"); - if (deps.isLinuxDockerDriverGatewayEnabled() && gatewayReuseState !== "missing") { + if ( + deps.isLinuxDockerDriverGatewayEnabled() && + gatewayReuseState !== "missing" && + gatewayReuseState !== "foreign-active" + ) { deps.note(" Replacing legacy OpenShell gateway metadata with Docker-driver gateway."); deps.retireLegacyGatewayForDockerDriverUpgrade(); gatewayReuseState = "missing"; + } else if (gatewayReuseState === "foreign-active") { + gatewayReuseState = "missing"; } await withGatewayTrace(gatewayReuseState, gpuPassthrough, () => deps.startGateway(gpu, { gpuPassthrough }), diff --git a/test/e2e/test-concurrent-gateway-ports.sh b/test/e2e/test-concurrent-gateway-ports.sh index 32783f38e7f..6e42d24c256 100755 --- a/test/e2e/test-concurrent-gateway-ports.sh +++ b/test/e2e/test-concurrent-gateway-ports.sh @@ -325,8 +325,8 @@ else fi LIST_OUTPUT="$("${NEMOCLAW_CMD[@]}" list 2>&1 || true)" -if echo "${LIST_OUTPUT}" | grep -qE "^[[:space:]]*${SANDBOX_A}[[:space:]]" \ - && echo "${LIST_OUTPUT}" | grep -qE "^[[:space:]]*${SANDBOX_B}[[:space:]]"; then +if echo "${LIST_OUTPUT}" | grep -qE "^[[:space:]]+${SANDBOX_A}( \*)?[[:space:]]*$" \ + && echo "${LIST_OUTPUT}" | grep -qE "^[[:space:]]+${SANDBOX_B}( \*)?[[:space:]]*$"; then pass "nemoclaw list shows both sandbox A and B" else fail "nemoclaw list missing one of A/B" From 9de4b99eb994c7f3d094eae8c19ea3018be583e2 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Fri, 5 Jun 2026 05:43:58 +0000 Subject: [PATCH 24/27] fix(onboard): preserve foreign sandbox's dashboard forward during preflight Signed-off-by: Tinson Lai --- src/lib/onboard.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 15a14dbd650..6b0252d9f53 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -437,6 +437,7 @@ const tiers: typeof import("./policy/tiers") = require("./policy/tiers"); const { ensureUsageNoticeConsent } = require("./onboard/usage-notice"); const { findAvailableDashboardPort, + getOccupiedPorts, preflightDashboardPortRangeAvailability, } = require("./onboard/dashboard-port") as typeof import("./onboard/dashboard-port"); const { destroyGatewayForReuse } = require("./onboard/gateway-cleanup") as typeof import("./onboard/gateway-cleanup"); @@ -2082,6 +2083,21 @@ async function preflight( // Use `ps` to get the command line — works on Linux, macOS, and WSL. const cmdline = captureProcessArgs(portCheck.pid); if (cmdline.includes("openshell")) { + // #4422: if another live sandbox owns this dashboard forward, do NOT + // kill it. The runtime allocator (findAvailableDashboardPort) will + // pick a different port for this sandbox at create time. + const forwardListOutput = runCaptureOpenshell(["forward", "list"], { + ignoreError: true, + suppressOutput: true, + timeout: 10_000, + }); + const owner = getOccupiedPorts(forwardListOutput).get(String(port)) ?? null; + if (owner) { + console.log( + ` Port ${port} held by live sandbox '${owner}'; leaving its forward intact (this sandbox will auto-allocate a different dashboard port).`, + ); + continue; + } console.log( ` Cleaning up orphaned SSH port-forward on port ${port} (PID ${portCheck.pid})...`, ); From 3ce01f04d0d51fc7e72ea8f37c9ca1d4dc525c22 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Fri, 5 Jun 2026 06:00:47 +0000 Subject: [PATCH 25/27] fix(e2e): retry verify_sandbox_alive on Provisioning until Ready or terminal Signed-off-by: Tinson Lai --- test/e2e/test-concurrent-gateway-ports.sh | 24 ++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/test/e2e/test-concurrent-gateway-ports.sh b/test/e2e/test-concurrent-gateway-ports.sh index 6e42d24c256..71980e44d76 100755 --- a/test/e2e/test-concurrent-gateway-ports.sh +++ b/test/e2e/test-concurrent-gateway-ports.sh @@ -262,13 +262,23 @@ sandbox_phase() { verify_sandbox_alive() { local name="$1" local label="${2:-${name} alive}" - local phase - phase="$(sandbox_phase "${name}")" - if [ "${phase}" = "Ready" ] || [ "${phase}" = "Running" ]; then - pass "${label} (phase=${phase})" - return 0 - fi - fail "${label} (phase='${phase:-missing}')" + local retries="${3:-12}" + local phase="" + for _ in $(seq 1 "${retries}"); do + phase="$(sandbox_phase "${name}")" + case "${phase}" in + Ready | Running) + pass "${label} (phase=${phase})" + return 0 + ;; + Error | Failed | CrashLoopBackOff) + fail "${label} terminal (phase='${phase}')" + return 1 + ;; + esac + sleep 5 + done + fail "${label} did not reach Ready/Running within ${retries} polls (last phase='${phase:-missing}')" return 1 } From 939a18277073bc1555d6fc408919f5e31e94349e Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Fri, 5 Jun 2026 06:23:14 +0000 Subject: [PATCH 26/27] fix(e2e): query each sandbox via its own gateway in verify_sandbox_alive Signed-off-by: Tinson Lai --- test/e2e/test-concurrent-gateway-ports.sh | 31 ++++++++++++++++++----- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/test/e2e/test-concurrent-gateway-ports.sh b/test/e2e/test-concurrent-gateway-ports.sh index 71980e44d76..21e1dff8b85 100755 --- a/test/e2e/test-concurrent-gateway-ports.sh +++ b/test/e2e/test-concurrent-gateway-ports.sh @@ -252,9 +252,23 @@ destroy_default_install_sandbox() { fi } +gateway_name_for_port() { + local port="$1" + if [ "${port}" = "8080" ]; then + echo "nemoclaw" + else + echo "nemoclaw-${port}" + fi +} + sandbox_phase() { local name="$1" - openshell sandbox list 2>/dev/null \ + local gateway="${2:-}" + local args=("sandbox" "list") + if [ -n "${gateway}" ]; then + args+=("-g" "${gateway}") + fi + openshell "${args[@]}" 2>/dev/null \ | sed 's/\x1b\[[0-9;]*m//g' \ | awk -v want="${name}" '$1 == want { print $NF; exit }' } @@ -262,10 +276,11 @@ sandbox_phase() { verify_sandbox_alive() { local name="$1" local label="${2:-${name} alive}" - local retries="${3:-12}" + local gateway="${3:-}" + local retries="${4:-12}" local phase="" for _ in $(seq 1 "${retries}"); do - phase="$(sandbox_phase "${name}")" + phase="$(sandbox_phase "${name}" "${gateway}")" case "${phase}" in Ready | Running) pass "${label} (phase=${phase})" @@ -291,8 +306,10 @@ section "Stage 0.5: destroy default sandbox created by install.sh (if any)" destroy_default_install_sandbox || exit 1 section "Stage 1: onboard sandbox A on default gateway port (${GATEWAY_PORT_A})" +GATEWAY_A_NAME="$(gateway_name_for_port "${GATEWAY_PORT_A}")" +GATEWAY_B_NAME="$(gateway_name_for_port "${GATEWAY_PORT_B}")" onboard_sandbox "${SANDBOX_A}" "${GATEWAY_PORT_A}" || exit 1 -verify_sandbox_alive "${SANDBOX_A}" "Sandbox A reaches Ready/Running on default port" +verify_sandbox_alive "${SANDBOX_A}" "Sandbox A reaches Ready/Running on default port" "${GATEWAY_A_NAME}" DASHBOARD_A="$(dashboard_port_from_list "${SANDBOX_A}")" if [ -n "${DASHBOARD_A}" ] && [ "${DASHBOARD_A}" = "${DASHBOARD_PORT_A}" ]; then @@ -309,8 +326,8 @@ onboard_sandbox "${SANDBOX_B}" "${GATEWAY_PORT_B}" || { } section "Stage 3: assert both sandboxes coexist" -verify_sandbox_alive "${SANDBOX_A}" "Sandbox A still alive after B's onboard (#4422 SIGKILL regression)" -verify_sandbox_alive "${SANDBOX_B}" "Sandbox B reaches Ready/Running on per-port gateway" +verify_sandbox_alive "${SANDBOX_A}" "Sandbox A still alive after B's onboard (#4422 SIGKILL regression)" "${GATEWAY_A_NAME}" +verify_sandbox_alive "${SANDBOX_B}" "Sandbox B reaches Ready/Running on per-port gateway" "${GATEWAY_B_NAME}" DASHBOARD_B="$(dashboard_port_from_list "${SANDBOX_B}")" if [ -n "${DASHBOARD_B}" ] && [ "${DASHBOARD_B}" != "${DASHBOARD_A:-${DASHBOARD_PORT_A}}" ]; then @@ -352,7 +369,7 @@ else fail "Sandbox B destroy timed out or failed" tail -100 "/tmp/${SANDBOX_B}-destroy.log" | sed 's/^/ /' fi -verify_sandbox_alive "${SANDBOX_A}" "Sandbox A still alive after B's destroy" +verify_sandbox_alive "${SANDBOX_A}" "Sandbox A still alive after B's destroy" "${GATEWAY_A_NAME}" section "Summary: PASS=${PASS} FAIL=${FAIL} TOTAL=${TOTAL}" if [ "${FAIL}" -gt 0 ]; then From 69dcdefee627aadbd9268eeaaa4902645ef36b71 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Fri, 5 Jun 2026 07:11:46 +0000 Subject: [PATCH 27/27] refactor(onboard): extract orphaned dashboard forward cleanup helper Signed-off-by: Tinson Lai --- .github/workflows/nightly-e2e.yaml | 4 +- src/lib/onboard.ts | 38 ++---- .../onboard/machine/handlers/gateway.test.ts | 2 +- .../orphaned-dashboard-forward.test.ts | 124 ++++++++++++++++++ src/lib/onboard/orphaned-dashboard-forward.ts | 109 +++++++++++++++ test/e2e/test-concurrent-gateway-ports.sh | 30 ++--- 6 files changed, 255 insertions(+), 52 deletions(-) create mode 100644 src/lib/onboard/orphaned-dashboard-forward.test.ts create mode 100644 src/lib/onboard/orphaned-dashboard-forward.ts diff --git a/.github/workflows/nightly-e2e.yaml b/.github/workflows/nightly-e2e.yaml index 02eee7c30ba..68d34400ea7 100644 --- a/.github/workflows/nightly-e2e.yaml +++ b/.github/workflows/nightly-e2e.yaml @@ -81,8 +81,8 @@ # gpu-double-onboard-e2e Ollama proxy token consistency after re-onboard (#2553). # concurrent-gateway-ports-e2e # Two sandboxes coexisting on the same host with distinct -# NEMOCLAW_GATEWAY_PORT values; covers the multi-instance -# scenarios referenced in #3053, #4422, and #4520. +# NEMOCLAW_GATEWAY_PORT values; verifies per-instance +# gateway and dashboard segregation. # notify-on-failure Auto-creates a GitHub issue when any E2E job fails. # # Runs directly on the runner (not inside Docker) because OpenShell bootstraps diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 6b0252d9f53..e4950393439 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -437,9 +437,10 @@ const tiers: typeof import("./policy/tiers") = require("./policy/tiers"); const { ensureUsageNoticeConsent } = require("./onboard/usage-notice"); const { findAvailableDashboardPort, - getOccupiedPorts, preflightDashboardPortRangeAvailability, } = require("./onboard/dashboard-port") as typeof import("./onboard/dashboard-port"); +const { tryCleanupOrphanedDashboardForward } = + require("./onboard/orphaned-dashboard-forward") as typeof import("./onboard/orphaned-dashboard-forward"); const { destroyGatewayForReuse } = require("./onboard/gateway-cleanup") as typeof import("./onboard/gateway-cleanup"); const { applyPreflightGatewayCleanup } = require("./onboard/preflight-gateway-cleanup-decision") as typeof import("./onboard/preflight-gateway-cleanup-decision"); @@ -2080,35 +2081,12 @@ async function preflight( // if its command line contains "openshell" to avoid killing unrelated SSH // tunnels the user may have set up on the same port. (#1950) if (port === DASHBOARD_PORT && portCheck.process === "ssh" && portCheck.pid) { - // Use `ps` to get the command line — works on Linux, macOS, and WSL. - const cmdline = captureProcessArgs(portCheck.pid); - if (cmdline.includes("openshell")) { - // #4422: if another live sandbox owns this dashboard forward, do NOT - // kill it. The runtime allocator (findAvailableDashboardPort) will - // pick a different port for this sandbox at create time. - const forwardListOutput = runCaptureOpenshell(["forward", "list"], { - ignoreError: true, - suppressOutput: true, - timeout: 10_000, - }); - const owner = getOccupiedPorts(forwardListOutput).get(String(port)) ?? null; - if (owner) { - console.log( - ` Port ${port} held by live sandbox '${owner}'; leaving its forward intact (this sandbox will auto-allocate a different dashboard port).`, - ); - continue; - } - console.log( - ` Cleaning up orphaned SSH port-forward on port ${port} (PID ${portCheck.pid})...`, - ); - run(["kill", String(portCheck.pid)], { ignoreError: true }); - sleepSeconds(1); - portCheck = await checkPortAvailable(port, portCheckOptions); - if (portCheck.ok) { - console.log(` ✓ Port ${port} available after orphaned forward cleanup (${label})`); - continue; - } - } + const outcome = await tryCleanupOrphanedDashboardForward({ + port, pid: portCheck.pid, label, portCheckOptions, + captureProcessArgs, runCaptureOpenshell, run, sleepSeconds, checkPortAvailable, + }); + if (outcome.kind === "killed-still-blocked") portCheck = outcome.portCheck; + else if (outcome.kind !== "not-openshell") continue; } console.error(""); console.error(` !! Port ${port} is not available.`); diff --git a/src/lib/onboard/machine/handlers/gateway.test.ts b/src/lib/onboard/machine/handlers/gateway.test.ts index 304745c529e..ca69d2dbc3f 100644 --- a/src/lib/onboard/machine/handlers/gateway.test.ts +++ b/src/lib/onboard/machine/handlers/gateway.test.ts @@ -315,7 +315,7 @@ describe("handleGatewayState", () => { ); }); - it("does not retire a foreign-active Docker-driver gateway (#4422 concurrent instances)", async () => { + it("does not retire a foreign-active Docker-driver gateway (concurrent instances)", async () => { const { deps, calls } = createDeps({ isLinuxDockerDriverGatewayEnabled: vi.fn(() => true), reconcileGatewayGpuReuseForGpuIntent: vi.fn( diff --git a/src/lib/onboard/orphaned-dashboard-forward.test.ts b/src/lib/onboard/orphaned-dashboard-forward.test.ts new file mode 100644 index 00000000000..b99b4a494dc --- /dev/null +++ b/src/lib/onboard/orphaned-dashboard-forward.test.ts @@ -0,0 +1,124 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it, vi } from "vitest"; + +import { + tryCleanupOrphanedDashboardForward, + type OrphanedDashboardForwardDeps, +} from "../../../dist/lib/onboard/orphaned-dashboard-forward"; + +function forwardListWith( + entries: Array<{ sandbox: string; port: number; status?: string }>, +): string { + const header = "SANDBOX BIND PORT PID STATUS"; + const rows = entries.map( + (e) => `${e.sandbox} 127.0.0.1 ${e.port} 1234 ${e.status ?? "running"}`, + ); + return [header, ...rows].join("\n"); +} + +interface MakeDepsOverrides { + cmdline?: string; + listFn?: () => string; + portCheckResult?: { ok: boolean; process?: string; pid?: number | null; reason?: string }; +} + +function makeDeps(overrides: MakeDepsOverrides = {}) { + const calls = { + captureProcessArgs: vi.fn((_pid: number) => overrides.cmdline ?? "ssh -L openshell-forward 18789:..."), + runCaptureOpenshell: vi.fn( + overrides.listFn ?? (() => forwardListWith([])), + ) as OrphanedDashboardForwardDeps["runCaptureOpenshell"], + run: vi.fn() as unknown as OrphanedDashboardForwardDeps["run"], + sleepSeconds: vi.fn() as OrphanedDashboardForwardDeps["sleepSeconds"], + checkPortAvailable: vi.fn(async () => overrides.portCheckResult ?? { ok: true }) as unknown as OrphanedDashboardForwardDeps["checkPortAvailable"], + log: vi.fn(), + }; + const deps: OrphanedDashboardForwardDeps = { + port: 18789, + pid: 4321, + label: "Test dashboard", + captureProcessArgs: calls.captureProcessArgs, + runCaptureOpenshell: calls.runCaptureOpenshell, + run: calls.run, + sleepSeconds: calls.sleepSeconds, + checkPortAvailable: calls.checkPortAvailable, + log: calls.log, + }; + return { deps, calls }; +} + +describe("tryCleanupOrphanedDashboardForward", () => { + it("returns not-openshell when the listener is unrelated SSH", async () => { + const { deps, calls } = makeDeps({ cmdline: "ssh -L 18789:remote-host:80 user@bastion" }); + const outcome = await tryCleanupOrphanedDashboardForward(deps); + expect(outcome).toEqual({ kind: "not-openshell" }); + expect(calls.runCaptureOpenshell).not.toHaveBeenCalled(); + expect(calls.run).not.toHaveBeenCalled(); + }); + + it("returns list-failed and skips the kill when forward list throws", async () => { + const { deps, calls } = makeDeps({ + listFn: () => { + throw new Error("gateway probe timed out"); + }, + }); + const outcome = await tryCleanupOrphanedDashboardForward(deps); + expect(outcome).toEqual({ kind: "list-failed" }); + expect(calls.run).not.toHaveBeenCalled(); + expect(calls.checkPortAvailable).not.toHaveBeenCalled(); + expect(calls.log).toHaveBeenCalledWith( + expect.stringContaining("Could not enumerate OpenShell forwards"), + ); + }); + + it("does not pass ignoreError to runCaptureOpenshell (failures must throw to be classified list-failed)", async () => { + const { deps, calls } = makeDeps(); + await tryCleanupOrphanedDashboardForward(deps); + expect(calls.runCaptureOpenshell).toHaveBeenCalledWith( + ["forward", "list"], + expect.objectContaining({ timeout: 10_000, suppressOutput: true }), + ); + expect(calls.runCaptureOpenshell).not.toHaveBeenCalledWith( + ["forward", "list"], + expect.objectContaining({ ignoreError: true }), + ); + }); + + it("returns owned-by-live when another live sandbox owns the port", async () => { + const { deps, calls } = makeDeps({ + listFn: () => forwardListWith([{ sandbox: "other-sandbox", port: 18789 }]), + }); + const outcome = await tryCleanupOrphanedDashboardForward(deps); + expect(outcome).toEqual({ kind: "owned-by-live", owner: "other-sandbox" }); + expect(calls.run).not.toHaveBeenCalled(); + expect(calls.checkPortAvailable).not.toHaveBeenCalled(); + }); + + it("returns killed-cleared when the kill frees the port", async () => { + const { deps, calls } = makeDeps({ portCheckResult: { ok: true } }); + const outcome = await tryCleanupOrphanedDashboardForward(deps); + expect(outcome).toEqual({ kind: "killed-cleared" }); + expect(calls.run).toHaveBeenCalledWith(["kill", "4321"], { ignoreError: true }); + expect(calls.sleepSeconds).toHaveBeenCalledWith(1); + expect(calls.checkPortAvailable).toHaveBeenCalledWith(18789, undefined); + }); + + it("returns killed-still-blocked when the kill ran but the port stayed blocked", async () => { + const refreshedCheck = { ok: false, process: "ssh", pid: 4321, reason: "still busy" }; + const { deps, calls } = makeDeps({ portCheckResult: refreshedCheck }); + const outcome = await tryCleanupOrphanedDashboardForward(deps); + expect(outcome).toEqual({ kind: "killed-still-blocked", portCheck: refreshedCheck }); + expect(calls.run).toHaveBeenCalledTimes(1); + }); + + it("ignores non-live forward statuses when deciding ownership", async () => { + const { deps, calls } = makeDeps({ + listFn: () => forwardListWith([{ sandbox: "other-sandbox", port: 18789, status: "stopped" }]), + }); + const outcome = await tryCleanupOrphanedDashboardForward(deps); + expect(outcome.kind).toBe("killed-cleared"); + expect(calls.run).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/lib/onboard/orphaned-dashboard-forward.ts b/src/lib/onboard/orphaned-dashboard-forward.ts new file mode 100644 index 00000000000..b7343a75205 --- /dev/null +++ b/src/lib/onboard/orphaned-dashboard-forward.ts @@ -0,0 +1,109 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import type { CheckPortOpts, PortProbeResult } from "./preflight"; + +import { getOccupiedPorts } from "./dashboard-port"; + +export type ListForwardsRunner = ( + args: string[], + opts: { timeout?: number; suppressOutput?: boolean }, +) => string; + +export type KillRunner = (args: string[], opts: { ignoreError?: boolean }) => unknown; + +export type CheckPortAvailableFn = ( + port: number, + opts?: CheckPortOpts, +) => Promise; + +export type SleepFn = (seconds: number) => void; + +export interface OrphanedDashboardForwardDeps { + port: number; + pid: number; + label: string; + portCheckOptions?: CheckPortOpts; + captureProcessArgs(pid: number): string; + runCaptureOpenshell: ListForwardsRunner; + run: KillRunner; + sleepSeconds: SleepFn; + checkPortAvailable: CheckPortAvailableFn; + log?: (message: string) => void; +} + +export type OrphanedDashboardForwardOutcome = + | { kind: "not-openshell" } + | { kind: "list-failed" } + | { kind: "owned-by-live"; owner: string } + | { kind: "killed-cleared" } + | { kind: "killed-still-blocked"; portCheck: PortProbeResult }; + +/** + * Decide whether an orphaned SSH port-forward sitting on the dashboard port + * can be killed to free the port. The caller has already detected that the + * port is blocked by an `ssh` listener (typical signature of a stale + * `openshell forward start` left behind after a previous session). + * + * Cross-instance safety is enforced by consulting `openshell forward list` + * for the live owner: + * - `not-openshell` — listener is unrelated SSH; caller should fall + * through to the generic port-blocked error path. + * - `list-failed` — could not enumerate forwards; the kill is + * SKIPPED. With no ownership data, a kill could + * collateral-damage a concurrent live sandbox's + * dashboard forward. Caller continues — the runtime + * allocator will pick a different dashboard port. + * - `owned-by-live` — another live sandbox holds the forward; kill is + * skipped, caller continues with auto-allocation. + * - `killed-cleared` — kill succeeded and the port is now free. + * - `killed-still-blocked` — kill ran but the port stayed blocked; the + * refreshed `portCheck` is returned so the caller + * can fall through to the generic port-blocked + * error path with up-to-date diagnostics. + * + * The `forward list` call is intentionally allowed to throw — `ignoreError` + * would swallow the failure into an empty string, which `getOccupiedPorts` + * parses as an empty map, and the "no entry → kill" branch would still run + * with no ownership data. + */ +export async function tryCleanupOrphanedDashboardForward( + deps: OrphanedDashboardForwardDeps, +): Promise { + const log = deps.log ?? ((message: string) => console.log(message)); + const cmdline = deps.captureProcessArgs(deps.pid); + if (!cmdline.includes("openshell")) { + return { kind: "not-openshell" }; + } + + let listOutput: string; + try { + listOutput = deps.runCaptureOpenshell(["forward", "list"], { + suppressOutput: true, + timeout: 10_000, + }); + } catch { + log( + ` Could not enumerate OpenShell forwards while checking port ${deps.port}; leaving its forward intact to avoid killing a live sandbox.`, + ); + return { kind: "list-failed" }; + } + + const owner = getOccupiedPorts(listOutput).get(String(deps.port)) ?? null; + if (owner) { + log( + ` Port ${deps.port} held by live sandbox '${owner}'; leaving its forward intact (this sandbox will auto-allocate a different dashboard port).`, + ); + return { kind: "owned-by-live", owner }; + } + + log(` Cleaning up orphaned SSH port-forward on port ${deps.port} (PID ${deps.pid})...`); + deps.run(["kill", String(deps.pid)], { ignoreError: true }); + deps.sleepSeconds(1); + const portCheck = await deps.checkPortAvailable(deps.port, deps.portCheckOptions); + if (portCheck.ok) { + log(` ✓ Port ${deps.port} available after orphaned forward cleanup (${deps.label})`); + return { kind: "killed-cleared" }; + } + return { kind: "killed-still-blocked", portCheck }; +} diff --git a/test/e2e/test-concurrent-gateway-ports.sh b/test/e2e/test-concurrent-gateway-ports.sh index 21e1dff8b85..af3db85d7cd 100755 --- a/test/e2e/test-concurrent-gateway-ports.sh +++ b/test/e2e/test-concurrent-gateway-ports.sh @@ -2,19 +2,11 @@ # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 -# Concurrent gateway ports — covers the multi-instance scenarios across three -# linked issues that share the same underlying use case but were patched -# piecemeal in earlier PRs: -# -# #3053 — parent ask: multiple NemoClaw-managed instances on a single host -# with full segregation of state, registry, and gateways. -# #4422 — NEMOCLAW_GATEWAY_PORT=N onboard recreates the global gateway and -# destroys the previous sandbox; concurrent instances unsupported. -# QA flagged that the per-port fix in #4645 still collides on the -# dashboard port even though the gateway port is bound per instance. -# #4520 — containerised-compat gateway mode (host glibc < gateway requirement) -# judges a healthy compat gateway stale on the second onboard, causing -# a port 8080 recreate-collision. +# Concurrent gateway ports — exercises multiple NemoClaw-managed sandboxes on a +# single host with fully segregated gateways, dashboards, and registries. A +# second onboard with NEMOCLAW_GATEWAY_PORT set to a non-default port must not +# touch the first sandbox's gateway process, dashboard SSH forward, or sandbox +# container. # # Scenario shape: # 1. Onboard sandbox A on the default gateway port (8080) + default dashboard @@ -318,34 +310,34 @@ else fail "Sandbox A dashboard port is '${DASHBOARD_A:-missing}', expected ${DASHBOARD_PORT_A}" fi -section "Stage 2: onboard sandbox B with NEMOCLAW_GATEWAY_PORT=${GATEWAY_PORT_B} (#4422 / #3053)" +section "Stage 2: onboard sandbox B with NEMOCLAW_GATEWAY_PORT=${GATEWAY_PORT_B}" onboard_sandbox "${SANDBOX_B}" "${GATEWAY_PORT_B}" || { - info "B onboard failed; capturing pre-fail state of A for #4422 diagnostics" + info "B onboard failed; capturing pre-fail state of A for diagnostics" dump_diagnostics "stage-2-onboard-B" exit 1 } section "Stage 3: assert both sandboxes coexist" -verify_sandbox_alive "${SANDBOX_A}" "Sandbox A still alive after B's onboard (#4422 SIGKILL regression)" "${GATEWAY_A_NAME}" +verify_sandbox_alive "${SANDBOX_A}" "Sandbox A still alive after B's onboard" "${GATEWAY_A_NAME}" verify_sandbox_alive "${SANDBOX_B}" "Sandbox B reaches Ready/Running on per-port gateway" "${GATEWAY_B_NAME}" DASHBOARD_B="$(dashboard_port_from_list "${SANDBOX_B}")" if [ -n "${DASHBOARD_B}" ] && [ "${DASHBOARD_B}" != "${DASHBOARD_A:-${DASHBOARD_PORT_A}}" ]; then - pass "Sandbox B got a distinct dashboard port (A=${DASHBOARD_A:-missing} B=${DASHBOARD_B}) (#4422 dashboard-port QA gap)" + pass "Sandbox B got a distinct dashboard port (A=${DASHBOARD_A:-missing} B=${DASHBOARD_B})" else fail "Sandbox B dashboard port collides with A: A=${DASHBOARD_A:-missing} B=${DASHBOARD_B:-missing}" dump_diagnostics "dashboard-port-collision" fi if ss -ltn 2>/dev/null | grep -qE ":${GATEWAY_PORT_A}\\b"; then - pass "Sandbox A gateway port ${GATEWAY_PORT_A} still listening (#4520 drift detection regression)" + pass "Sandbox A gateway port ${GATEWAY_PORT_A} still listening" else fail "Sandbox A gateway port ${GATEWAY_PORT_A} no longer listening — recreate destroyed first gateway" dump_diagnostics "gateway-port-A-missing" fi if ss -ltn 2>/dev/null | grep -qE ":${GATEWAY_PORT_B}\\b"; then - pass "Sandbox B gateway port ${GATEWAY_PORT_B} listening (#4422 per-port binding)" + pass "Sandbox B gateway port ${GATEWAY_PORT_B} listening" else fail "Sandbox B gateway port ${GATEWAY_PORT_B} not listening" dump_diagnostics "gateway-port-B-missing"