diff --git a/src/lib/actions/sandbox/snapshot-auto-create-failure.test.ts b/src/lib/actions/sandbox/snapshot-auto-create-failure.test.ts index b9d6692b7b1..b6959ee9be9 100644 --- a/src/lib/actions/sandbox/snapshot-auto-create-failure.test.ts +++ b/src/lib/actions/sandbox/snapshot-auto-create-failure.test.ts @@ -89,6 +89,7 @@ vi.mock("../../state/gateway", () => ({ ), })); vi.mock("../../state/mcp-lifecycle-lock", () => ({ + withMcpLifecycleLock: vi.fn((_key, fn) => fn()), withSandboxMutationLock: vi.fn((_sandbox, fn) => fn()), })); vi.mock("../../state/registry", () => ({ diff --git a/src/lib/actions/sandbox/snapshot-restore-clone-ports.test.ts b/src/lib/actions/sandbox/snapshot-restore-clone-ports.test.ts new file mode 100644 index 00000000000..77564061353 --- /dev/null +++ b/src/lib/actions/sandbox/snapshot-restore-clone-ports.test.ts @@ -0,0 +1,223 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { + HERMES_DASHBOARD_ENABLE_ENV, + HERMES_DASHBOARD_INTERNAL_PORT_ENV, + HERMES_DASHBOARD_PORT_ENV, + HERMES_DASHBOARD_TUI_ENV, +} from "../../hermes-dashboard"; +import { resolveRebuildHermesDashboardEnv } from "./rebuild-durable-config"; +import * as f from "./snapshot-restore-test-fixture"; + +const dashboardPortMocks = vi.hoisted(() => ({ + findAvailableDashboardPort: vi.fn(() => 18901), + getRegistryOccupiedDashboardPorts: vi.fn(() => new Map()), + withDashboardPortReservationLock: vi.fn(async (operation: () => unknown) => await operation()), +})); + +vi.mock("../../onboard/dashboard-port", () => ({ + findAvailableDashboardPort: dashboardPortMocks.findAvailableDashboardPort, + getRegistryOccupiedDashboardPorts: dashboardPortMocks.getRegistryOccupiedDashboardPorts, + withDashboardPortReservationLock: dashboardPortMocks.withDashboardPortReservationLock, +})); + +beforeEach(f.resetSnapshotRestoreMocks); +afterEach(f.cleanupSnapshotRestoreMocks); +describe("runSandboxSnapshot restore: clone dashboard port identity", () => { + it("allocates the auto-created clone its own dashboard port instead of inheriting the source's (#6746)", async () => { + let registeredClone: f.SandboxRecord | null = null; + f.registerSandboxMock.mockImplementation( + (entry) => (registeredClone = entry as f.SandboxRecord), + ); + f.getSandboxMock.mockImplementation((name) => + name === "alpha" + ? { + name: "alpha", + agent: "openclaw", + imageTag: "nemoclaw-alpha:test", + openshellDriver: "docker", + provider: "nvidia-nim", + model: "nvidia/model-a", + dashboardPort: 18790, + } + : registeredClone, + ); + f.captureOpenshellMock.mockImplementation((args) => + f.openshellResponses(args, { + "sandbox exec": { status: 0, output: f.dcodeProbeOutput("idle") }, + "sandbox list": { status: 0, output: "alpha Ready\nbeta Ready\n" }, + }), + ); + f.parseLiveSandboxNamesMock.mockReturnValue(new Set(["alpha"])); + f.getLatestBackupMock.mockReturnValue({ ...f.latestBackupFixture }); + const { runSandboxSnapshot } = await import("./snapshot"); + await runSandboxSnapshot("alpha", { kind: "restore", to: "beta" }); + expect(dashboardPortMocks.findAvailableDashboardPort).toHaveBeenCalledWith( + "beta", + 18790, + expect.any(String), + undefined, + expect.any(Map), + ); + expect(dashboardPortMocks.withDashboardPortReservationLock).toHaveBeenCalledOnce(); + const createArgs = f.streamSandboxCreateMock.mock.calls[0]?.[1] ?? []; + expect(createArgs.slice(createArgs.lastIndexOf("--") + 1)).toEqual([ + "env", + "NEMOCLAW_OBSERVABILITY=0", + "CHAT_UI_URL=http://127.0.0.1:18901", + "NEMOCLAW_DASHBOARD_PORT=18901", + "nemoclaw-start", + ]); + expect(f.registerSandboxMock).toHaveBeenCalledWith( + expect.objectContaining({ + name: "beta", + dashboardPort: 18901, + }), + ); + }); + + it("keeps a Hermes clone rebuildable with its new public port and inherited internal port (#6746)", async () => { + dashboardPortMocks.findAvailableDashboardPort.mockReturnValueOnce(18902); + let registeredClone: f.SandboxRecord | null = null; + f.registerSandboxMock.mockImplementation( + (entry) => (registeredClone = entry as f.SandboxRecord), + ); + f.getSandboxMock.mockImplementation((name) => + name === "alpha" + ? { + name: "alpha", + agent: "hermes", + imageTag: "nemoclaw-alpha:test", + openshellDriver: "docker", + provider: "nvidia-nim", + model: "nvidia/model-a", + dashboardPort: 18790, + hermesDashboardEnabled: true, + hermesDashboardPort: 18790, + hermesDashboardInternalPort: 18901, + hermesDashboardTui: true, + } + : registeredClone, + ); + f.captureOpenshellMock.mockImplementation((args) => + f.openshellResponses(args, { + "sandbox exec": { status: 0, output: f.dcodeProbeOutput("no-runtime") }, + "sandbox list": { status: 0, output: "alpha Ready\nbeta Ready\n" }, + }), + ); + f.parseLiveSandboxNamesMock.mockReturnValue(new Set(["alpha"])); + f.getLatestBackupMock.mockReturnValue({ ...f.latestBackupFixture }); + const { runSandboxSnapshot } = await import("./snapshot"); + + await runSandboxSnapshot("alpha", { kind: "restore", to: "beta" }); + + expect(dashboardPortMocks.findAvailableDashboardPort).toHaveBeenCalledWith( + "beta", + 18790, + expect.any(String), + undefined, + new Map([["18901", "alpha (Hermes dashboard internal)"]]), + ); + expect(f.registerSandboxMock).toHaveBeenCalledWith( + expect.objectContaining({ + name: "beta", + dashboardPort: 18902, + hermesDashboardPort: 18902, + hermesDashboardInternalPort: 18901, + hermesDashboardTui: true, + }), + ); + const createArgs = f.streamSandboxCreateMock.mock.calls[0]?.[1] ?? []; + expect(createArgs.slice(createArgs.lastIndexOf("--") + 1)).toEqual([ + "env", + "NEMOCLAW_OBSERVABILITY=0", + "CHAT_UI_URL=http://127.0.0.1:18902", + "NEMOCLAW_DASHBOARD_PORT=18902", + `${HERMES_DASHBOARD_ENABLE_ENV}=1`, + `${HERMES_DASHBOARD_PORT_ENV}=18902`, + `${HERMES_DASHBOARD_INTERNAL_PORT_ENV}=18901`, + `${HERMES_DASHBOARD_TUI_ENV}=1`, + "nemoclaw-start", + ]); + expect(resolveRebuildHermesDashboardEnv("hermes", registeredClone as never, 18902)).toEqual({ + ok: true, + env: { + [HERMES_DASHBOARD_ENABLE_ENV]: "1", + [HERMES_DASHBOARD_PORT_ENV]: "18902", + [HERMES_DASHBOARD_INTERNAL_PORT_ENV]: "18901", + [HERMES_DASHBOARD_TUI_ENV]: "1", + }, + }); + }); + + it("aborts before deleting a --force destination when no dashboard port is free (#6746)", async () => { + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); + dashboardPortMocks.findAvailableDashboardPort.mockImplementationOnce(() => { + throw new Error("All dashboard ports in range 18789-18799 are occupied:"); + }); + f.getSandboxMock.mockImplementation((name) => ({ + name: name ?? "alpha", + agent: "openclaw", + imageTag: `nemoclaw-${name}:test`, + openshellDriver: "docker", + provider: "nvidia-nim", + model: "nvidia/model-a", + dashboardPort: 18790, + })); + f.parseLiveSandboxNamesMock.mockReturnValue(new Set(["alpha", "beta"])); + f.captureOpenshellMock.mockImplementation((args) => + f.openshellResponses(args, { + "sandbox exec": { status: 0, output: f.dcodeProbeOutput("no-runtime") }, + "sandbox list": { status: 0, output: "alpha Ready\nbeta Ready\n" }, + }), + ); + f.getLatestBackupMock.mockReturnValue({ ...f.latestBackupFixture }); + const { runSandboxSnapshot } = await import("./snapshot"); + + await expect( + runSandboxSnapshot("alpha", { kind: "restore", to: "beta", force: true, yes: true }), + ).rejects.toMatchObject({ exitCode: 1 }); + + expect(dashboardPortMocks.findAvailableDashboardPort).toHaveBeenCalled(); + expect(consoleError.mock.calls.flat().join("\n")).toContain("are occupied"); + expect(f.lifecycleMock.events).not.toContain("delete"); + expect(f.streamSandboxCreateMock).not.toHaveBeenCalled(); + expect(f.registerSandboxMock).not.toHaveBeenCalled(); + }); + + it("registers a clone of a source without a dashboard port with the field unset (#6746)", async () => { + let registeredClone: f.SandboxRecord | null = null; + f.registerSandboxMock.mockImplementation( + (entry) => (registeredClone = entry as f.SandboxRecord), + ); + f.getSandboxMock.mockImplementation((name) => + name === "alpha" + ? { + name: "alpha", + agent: "openclaw", + imageTag: "nemoclaw-alpha:test", + openshellDriver: "docker", + provider: "nvidia-nim", + model: "nvidia/model-a", + } + : registeredClone, + ); + f.captureOpenshellMock.mockImplementation((args) => + f.openshellResponses(args, { + "sandbox exec": { status: 0, output: f.dcodeProbeOutput("idle") }, + "sandbox list": { status: 0, output: "alpha Ready\nbeta Ready\n" }, + }), + ); + f.parseLiveSandboxNamesMock.mockReturnValue(new Set(["alpha"])); + f.getLatestBackupMock.mockReturnValue({ ...f.latestBackupFixture }); + const { runSandboxSnapshot } = await import("./snapshot"); + await runSandboxSnapshot("alpha", { kind: "restore", to: "beta" }); + expect(dashboardPortMocks.findAvailableDashboardPort).not.toHaveBeenCalled(); + expect(f.registerSandboxMock).toHaveBeenCalledWith( + expect.objectContaining({ name: "beta", dashboardPort: null }), + ); + }); +}); diff --git a/src/lib/actions/sandbox/snapshot-restore-test-fixture.ts b/src/lib/actions/sandbox/snapshot-restore-test-fixture.ts index cfa7be5d49a..684ff3fa05c 100644 --- a/src/lib/actions/sandbox/snapshot-restore-test-fixture.ts +++ b/src/lib/actions/sandbox/snapshot-restore-test-fixture.ts @@ -23,6 +23,11 @@ export type SandboxRecord = { observabilityEnabled?: boolean; provider?: string | null; model?: string | null; + dashboardPort?: number | null; + hermesDashboardEnabled?: boolean; + hermesDashboardPort?: number | null; + hermesDashboardInternalPort?: number | null; + hermesDashboardTui?: boolean; }; export type DcodeProbeState = "active" | "idle" | "unverifiable" | "no-runtime"; diff --git a/src/lib/actions/sandbox/snapshot.ts b/src/lib/actions/sandbox/snapshot.ts index a2a48301591..13f4d003979 100644 --- a/src/lib/actions/sandbox/snapshot.ts +++ b/src/lib/actions/sandbox/snapshot.ts @@ -14,6 +14,12 @@ import { CLI_NAME } from "../../cli/branding"; import { prompt as askPrompt } from "../../credentials/store"; import { formatFailedBackupItems } from "../../domain/backup-failure"; import { getSandboxDeleteOutcome } from "../../domain/sandbox/destroy"; +import { + HERMES_DASHBOARD_ENABLE_ENV, + HERMES_DASHBOARD_INTERNAL_PORT_ENV, + HERMES_DASHBOARD_PORT_ENV, + HERMES_DASHBOARD_TUI_ENV, +} from "../../hermes-dashboard"; import { checkGatewayRouteCompatibility, formatGatewayRouteConflict, @@ -21,7 +27,14 @@ import { import { withGatewayRouteMutationLock } from "../../inference/gateway-route-mutation-lock"; import * as nim from "../../inference/nim"; import { listMessagingProviderSuffixes } from "../../messaging/channels"; +import { + findAvailableDashboardPort, + getRegistryOccupiedDashboardPorts, + withDashboardPortReservationLock, +} from "../../onboard/dashboard-port"; +import { isValidForwardPort } from "../../onboard/dashboard-runtime"; import { resolveSandboxGatewayName } from "../../onboard/gateway-binding"; +import { resolveHermesDashboardOnboardState } from "../../onboard/hermes-dashboard"; import { isDcodeAgent, OBSERVABILITY_OTLP_LOCAL_POLICY_PRESET, @@ -41,13 +54,13 @@ import type { SandboxEntry } from "../../state/registry"; import * as registry from "../../state/registry"; import { getSandboxEntryInference } from "../../state/registry-entry-view"; import * as sandboxState from "../../state/sandbox"; -import { cleanupShieldsDestroyArtifacts, removeSandboxRegistryEntry } from "./destroy"; import { DCODE_AGENT_NAME, DCODE_BUSY_PROBE_SCRIPT, DCODE_PROBE_STATE, parseDcodeProbeState, } from "./dcode-activity-probe"; +import { cleanupShieldsDestroyArtifacts, removeSandboxRegistryEntry } from "./destroy"; import { buildSandboxExecMarkedCommand, createSandboxExecMarker, @@ -177,7 +190,88 @@ function resolveSrcPodImage( } } -// Auto-create a sandbox that clones the image of an existing one. +// Allocate the clone's own dashboard port. Dashboard ports are per-sandbox +// host resources: the host forward for src's port is owned by src, so a clone +// that inherits the port gets a dashboard URL that points at src's dashboard +// and a rebuild preflight that rejects the clone forever (#6746). Allocate +// dst's own port instead, from the same per-gateway forward list + +// cross-gateway registry occupancy view as onboard's `ensureDashboardForward`. +// Sources without a dashboard port (non-dashboard-managed agents) return null +// so the clone's field stays unset. Callers must invoke this before any +// destructive step (e.g. deleting a `--force` destination) so port-range +// exhaustion aborts before, not after, the mutation. +function allocateCloneDashboardPort( + dstName: string, + srcEntry: { + name?: string; + dashboardPort?: number | null; + hermesDashboardEnabled?: boolean; + hermesDashboardInternalPort?: number | null; + }, +): number | null { + const srcPort = srcEntry.dashboardPort; + if (typeof srcPort !== "number" || !Number.isInteger(srcPort) || srcPort <= 0) return null; + const forwards = captureOpenshell(["forward", "list"], { ignoreError: true }); + const occupied = getRegistryOccupiedDashboardPorts(dstName); + const hermesInternalPort = srcEntry.hermesDashboardInternalPort; + if (srcEntry.hermesDashboardEnabled === true && isValidForwardPort(hermesInternalPort)) { + occupied.set( + String(hermesInternalPort), + `${srcEntry.name ?? "source"} (Hermes dashboard internal)`, + ); + } + try { + return findAvailableDashboardPort(dstName, srcPort, forwards.output || "", undefined, occupied); + } catch (err) { + console.error(` ${err instanceof Error ? err.message : String(err)}`); + snapshotExit(1); + } +} + +function resolveCloneDashboardEnvArgs( + srcEntry: SandboxEntry | { name: string }, + dstDashboardPort: number | null, +): string[] { + const envArgs: string[] = []; + if (dstDashboardPort !== null) { + envArgs.push(`CHAT_UI_URL=http://127.0.0.1:${dstDashboardPort}`); + envArgs.push(`NEMOCLAW_DASHBOARD_PORT=${dstDashboardPort}`); + } + + const source = srcEntry as SandboxEntry; + if (source.agent !== "hermes") return envArgs; + if (source.hermesDashboardEnabled !== true) { + envArgs.push(`${HERMES_DASHBOARD_ENABLE_ENV}=0`); + return envArgs; + } + if (dstDashboardPort === null) { + console.error(" Cannot clone enabled Hermes dashboard settings without a dashboard port."); + snapshotExit(1); + } + const hermesEnv: NodeJS.ProcessEnv = { + [HERMES_DASHBOARD_ENABLE_ENV]: "1", + [HERMES_DASHBOARD_PORT_ENV]: String(dstDashboardPort), + [HERMES_DASHBOARD_INTERNAL_PORT_ENV]: String(source.hermesDashboardInternalPort), + [HERMES_DASHBOARD_TUI_ENV]: source.hermesDashboardTui === true ? "1" : "0", + }; + try { + resolveHermesDashboardOnboardState({ + agentName: source.agent, + effectivePort: dstDashboardPort, + env: hermesEnv, + }); + } catch (error) { + console.error( + ` Cannot clone Hermes dashboard settings: ${error instanceof Error ? error.message : String(error)}.`, + ); + snapshotExit(1); + } + for (const [name, value] of Object.entries(hermesEnv)) { + envArgs.push(`${name}=${value}`); + } + return envArgs; +} + // Used by `snapshot restore --to ` when dst does not exist yet: reuses // the source's baked image so the user does not have to re-run onboarding. // Returns true on success; on failure, logs and throws SnapshotCommandError. @@ -186,6 +280,8 @@ async function autoCreateSandboxFromSource( dstName: string, srcEntry: SandboxEntry | { name: string }, fromImage: string, + dstDashboardPort: number | null, + dashboardEnvArgs: readonly string[], ): Promise { const basePolicy = path.join(ROOT, "nemoclaw-blueprint", "policies", "openclaw-sandbox.yaml"); const openshellBin = getOpenshellBinary(); @@ -194,6 +290,7 @@ async function autoCreateSandboxFromSource( const startupCommand = [ "env", `NEMOCLAW_OBSERVABILITY=${sourceObservabilityEnabled ? "1" : "0"}`, + ...dashboardEnvArgs, "nemoclaw-start", ]; const createEnv = { ...process.env }; @@ -269,6 +366,14 @@ async function autoCreateSandboxFromSource( // so clear src's proof rather than inheriting it — otherwise dst could show // `Sandbox GPU: enabled (CUDA verified)` based on another sandbox's run (#4231). sandboxGpuProof: null, + dashboardPort: dstDashboardPort, + // The shared image keeps Hermes' image-baked internal listener port, but + // the public WebUI port is a per-sandbox host resource and must follow the + // clone's newly allocated dashboard port so rebuild validation converges. + hermesDashboardPort: + (srcEntry as SandboxEntry).hermesDashboardEnabled === true + ? dstDashboardPort + : (srcEntry as SandboxEntry).hermesDashboardPort, }); console.log(` ${G}\u2713${R} Sandbox '${dstName}' created`); @@ -873,7 +978,7 @@ async function runSnapshotRestoreUnlocked( } } const sourceGatewayName = resolveSandboxGatewayName(srcEntry); - await withGatewayRouteMutationLock(sourceGatewayName, async () => { + const createAndRegisterClone = async (): Promise => { if (!targetExists && registry.getSandbox(targetSandbox)) { console.error( ` Destination sandbox '${targetSandbox}' was registered while this restore was waiting. Retry with --force only after reviewing that sandbox.`, @@ -917,6 +1022,12 @@ async function runSnapshotRestoreUnlocked( console.error(` Error: ${formatGatewayRouteConflict(compatibility)}`); snapshotExit(1); } + // Allocate the clone's dashboard port before any destructive action, so + // dashboard-port-range exhaustion aborts before `deleteSandboxForRestore` + // removes the existing `--force` destination — matching the pre-delete + // validation the image and gateway-route checks above already do (#3756). + const dstDashboardPort = allocateCloneDashboardPort(targetSandbox, lockedSourceEntry); + const dashboardEnvArgs = resolveCloneDashboardEnvArgs(lockedSourceEntry, dstDashboardPort); if (targetExists) { if (targetEntry) { verifyRestoreDestinationOnOwnGateway(targetSandbox); @@ -932,8 +1043,16 @@ async function runSnapshotRestoreUnlocked( targetSandbox, lockedSourceEntry, lockedFromImage, + dstDashboardPort, + dashboardEnvArgs, ); - }); + }; + // Lock order matches onboard: sandbox (outer caller), host dashboard, + // gateway route. The host-wide lease stays held from port selection until + // the clone is durably registered, including across different gateways. + await withDashboardPortReservationLock(() => + withGatewayRouteMutationLock(sourceGatewayName, createAndRegisterClone), + ); } withTimerBoundShieldsMutationLock(targetSandbox, "restore sandbox snapshot", () => { // Serialize filesystem restore, mutable-permission repair, and policy diff --git a/src/lib/onboard/dashboard-port.test.ts b/src/lib/onboard/dashboard-port.test.ts index fbbfaf6ed36..5b9ba556694 100644 --- a/src/lib/onboard/dashboard-port.test.ts +++ b/src/lib/onboard/dashboard-port.test.ts @@ -2,15 +2,20 @@ // SPDX-License-Identifier: Apache-2.0 import assert from "node:assert/strict"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; import { describe, it } from "vitest"; +import { withGatewayRouteMutationLock } from "../inference/gateway-route-mutation-lock"; import { findAvailableDashboardPort, findDashboardForwardOwner, getRegistryOccupiedDashboardPorts, preflightDashboardPortRangeAvailability, resolveCreateSandboxDashboardPort, + withDashboardPortReservationLock, } from "./dashboard-port"; describe("findDashboardForwardOwner", () => { @@ -322,6 +327,83 @@ describe("getRegistryOccupiedDashboardPorts", () => { }); }); +describe("dashboard port reservation lock", () => { + it("serializes onboard and restore ownership across different gateways", async () => { + const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "nemoclaw-dashboard-port-lock-")); + let releaseOnboard!: () => void; + const onboardReleased = new Promise((resolve) => { + releaseOnboard = resolve; + }); + let reportOnboardEntered!: () => void; + const onboardEntered = new Promise((resolve) => { + reportOnboardEntered = resolve; + }); + const events: string[] = []; + const options = { stateDir, pollIntervalMs: 1, timeoutMs: 5_000 }; + try { + const onboard = withDashboardPortReservationLock( + () => + withGatewayRouteMutationLock( + "gateway-a", + async () => { + events.push("onboard-gateway-a-select"); + reportOnboardEntered(); + await onboardReleased; + events.push("onboard-gateway-a-register"); + }, + options, + ), + options, + ); + await onboardEntered; + const restore = withDashboardPortReservationLock( + () => + withGatewayRouteMutationLock( + "gateway-b", + () => { + events.push("restore-gateway-b-select"); + events.push("restore-gateway-b-register"); + }, + options, + ), + options, + ); + await new Promise((resolve) => setTimeout(resolve, 20)); + assert.deepEqual(events, ["onboard-gateway-a-select"]); + releaseOnboard(); + await Promise.all([onboard, restore]); + assert.deepEqual(events, [ + "onboard-gateway-a-select", + "onboard-gateway-a-register", + "restore-gateway-b-select", + "restore-gateway-b-register", + ]); + } finally { + releaseOnboard(); + await fs.rm(stateDir, { recursive: true, force: true }); + } + }); + + it("releases a failed reservation so the next allocator can proceed", async () => { + const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "nemoclaw-dashboard-port-lock-")); + const options = { stateDir, pollIntervalMs: 1, timeoutMs: 5_000 }; + try { + await assert.rejects( + withDashboardPortReservationLock(() => { + throw new Error("onboard allocation failed"); + }, options), + /onboard allocation failed/, + ); + assert.equal( + await withDashboardPortReservationLock(() => "restore acquired", options), + "restore acquired", + ); + } finally { + await fs.rm(stateDir, { recursive: true, force: true }); + } + }); +}); + describe("preflightDashboardPortRangeAvailability (#3953)", () => { const allBound = (_p: number) => true; const noneBound = (_p: number) => false; diff --git a/src/lib/onboard/dashboard-port.ts b/src/lib/onboard/dashboard-port.ts index 1e420cbfd71..56bc5c26358 100644 --- a/src/lib/onboard/dashboard-port.ts +++ b/src/lib/onboard/dashboard-port.ts @@ -20,6 +20,7 @@ import { DASHBOARD_PORT_RANGE_END, DASHBOARD_PORT_RANGE_START, } from "../core/ports"; +import { type McpLifecycleLockOptions, withMcpLifecycleLock } from "../state/mcp-lifecycle-lock"; // runner.ts is still CommonJS — use require so module shape matches. const { runCapture } = require("../runner"); @@ -32,6 +33,25 @@ type SandboxRegistryEntry = { export type ListSandboxesFn = () => { sandboxes: SandboxRegistryEntry[] }; +const DASHBOARD_PORT_RESERVATION_LOCK = "dashboard-port-reservation:host"; + +/** + * Serialize host-wide dashboard-port selection through durable ownership. + * + * OpenShell forward listings are gateway-scoped while dashboard ports and the + * NemoClaw registry are host-scoped. Holding one cross-process lease across + * allocation and registration prevents onboard or snapshot restores on + * different gateways from selecting the same currently-free port. + * Callers that also need lifecycle locks must use the shared order: + * sandbox mutation → this host reservation → gateway route mutation. + */ +export function withDashboardPortReservationLock( + operation: () => Promise | T, + options: McpLifecycleLockOptions = {}, +): Promise { + return withMcpLifecycleLock(DASHBOARD_PORT_RESERVATION_LOCK, operation, options); +} + // Match the broader pattern used by onboard.ts (covers CSI, OSC, and Fe escapes) // so colorised `openshell forward list` output parses correctly. const ANSI_RE = /\x1B(?:\[[0-?]*[ -/]*[@-~]|\][^\x07]*(?:\x07|\x1B\\)|[@-_])/g; diff --git a/src/lib/onboard/machine/handlers/sandbox-route-mutation-lock.test.ts b/src/lib/onboard/machine/handlers/sandbox-route-mutation-lock.test.ts index 941803dcbdc..6308d906346 100644 --- a/src/lib/onboard/machine/handlers/sandbox-route-mutation-lock.test.ts +++ b/src/lib/onboard/machine/handlers/sandbox-route-mutation-lock.test.ts @@ -49,7 +49,7 @@ describe("sandbox registration route transaction", () => { expect(calls.error).toHaveBeenCalledWith(expect.stringContaining("peer")); }); - it("holds sandbox then gateway locks through sandbox creation and route registration", async () => { + it("holds sandbox, host dashboard, then gateway locks through creation and registration", async () => { const events: string[] = []; const { deps } = createDeps({ checkGatewayRouteCompatibility: () => { @@ -60,6 +60,10 @@ describe("sandbox registration route transaction", () => { events.push("sandbox-lock"); return await operation(); }, + withDashboardPortReservationLock: async (operation) => { + events.push("dashboard-lock"); + return await operation(); + }, withGatewayRouteMutationLock: async (_gatewayName, operation) => { events.push("gateway-lock"); return await operation(); @@ -76,7 +80,14 @@ describe("sandbox registration route transaction", () => { await expect(handleSandboxState(baseOptions(deps))).resolves.toMatchObject({ sandboxName: "my-assistant", }); - expect(events).toEqual(["sandbox-lock", "gateway-lock", "guard", "create", "registry"]); + expect(events).toEqual([ + "sandbox-lock", + "dashboard-lock", + "gateway-lock", + "guard", + "create", + "registry", + ]); }); it("fails when a competing same-name registration changed routes", async () => { diff --git a/src/lib/onboard/machine/handlers/sandbox-test-fixtures.ts b/src/lib/onboard/machine/handlers/sandbox-test-fixtures.ts index a75cd3378d1..2497879fc05 100644 --- a/src/lib/onboard/machine/handlers/sandbox-test-fixtures.ts +++ b/src/lib/onboard/machine/handlers/sandbox-test-fixtures.ts @@ -163,7 +163,14 @@ export function createDeps( throw new Error(`exit ${code}`); }), withGatewayRouteMutationLock: vi.fn(), + withDashboardPortReservationLock: vi.fn(), }; + const runWithDashboardPortReservationLock = + overrides.withDashboardPortReservationLock ?? + (async (operation: () => Promise | T): Promise => { + calls.withDashboardPortReservationLock(operation); + return await operation(); + }); const runWithGatewayRouteMutationLock = async ( gatewayName: string, operation: () => Promise | T, @@ -234,6 +241,7 @@ export function createDeps( ...overrides, checkGatewayRouteCompatibility: overrides.checkGatewayRouteCompatibility ?? calls.checkGatewayRouteCompatibility, + withDashboardPortReservationLock: runWithDashboardPortReservationLock, withGatewayRouteMutationLock: runWithGatewayRouteMutationLock, }, getSession: () => session, diff --git a/src/lib/onboard/machine/handlers/sandbox.ts b/src/lib/onboard/machine/handlers/sandbox.ts index 13dfba4c51c..9d465f8f445 100644 --- a/src/lib/onboard/machine/handlers/sandbox.ts +++ b/src/lib/onboard/machine/handlers/sandbox.ts @@ -18,6 +18,8 @@ import type { HermesAuthMethod, Session, SessionUpdates } from "../../../state/o import type { SandboxEntry } from "../../../state/registry"; import { getSandboxEntryInference } from "../../../state/registry-entry-view"; import { toolDisclosureOrDefault } from "../../../tool-disclosure"; +import { withDashboardPortReservationLock as withHostDashboardPortReservationLock } from "../../dashboard-port"; +import { type DashboardRuntimeAgent, shouldManageDashboardForAgent } from "../../dashboard-runtime"; import { type DcodeAutoApprovalMode, DEFAULT_DCODE_AUTO_APPROVAL_MODE, @@ -92,6 +94,7 @@ export interface SandboxStateOptions< gatewayName: string, operation: () => Promise | T, ): Promise; + withDashboardPortReservationLock?(operation: () => Promise | T): Promise; resolvePath(value: string): string; agentSupportsWebSearch( agent: Agent, @@ -768,9 +771,15 @@ class SandboxStateFlow< }; const withGatewayLock = () => this.deps.withGatewayRouteMutationLock(this.options.gatewayName, createAndRecord); + const withDashboardPortLock = + this.deps.withDashboardPortReservationLock ?? withHostDashboardPortReservationLock; + const withDashboardAndGatewayLocks = () => + shouldManageDashboardForAgent(this.options.agent as DashboardRuntimeAgent) + ? withDashboardPortLock(withGatewayLock) + : withGatewayLock(); return this.deps.withSandboxMutationLock - ? this.deps.withSandboxMutationLock(requestedSandboxName, withGatewayLock) - : withGatewayLock(); + ? this.deps.withSandboxMutationLock(requestedSandboxName, withDashboardAndGatewayLocks) + : withDashboardAndGatewayLocks(); } private async recreateSandbox( diff --git a/test/e2e/live/upgrade-stale-sandbox-helpers.ts b/test/e2e/live/upgrade-stale-sandbox-helpers.ts index 7b2da7fa60d..e6981ec7aaf 100644 --- a/test/e2e/live/upgrade-stale-sandbox-helpers.ts +++ b/test/e2e/live/upgrade-stale-sandbox-helpers.ts @@ -20,16 +20,10 @@ import { import { REPO_ROOT } from "../fixtures/paths.ts"; import type { ShellProbeResult } from "../fixtures/shell-probe.ts"; import { isTransientProviderValidationFailure } from "./network-policy-transient-provider.ts"; +import { createOldBaseBuildContext } from "./rebuild-openclaw-old-base-context.ts"; export { REPO_ROOT }; -const BLUEPRINT_RELPATH = path.join("nemoclaw-blueprint", "blueprint.yaml"); -const BLUEPRINT = path.join(REPO_ROOT, BLUEPRINT_RELPATH); -const BASE_CONTEXT_SCRIPT_RELPATH = path.join("scripts", "lib", "sandbox-rlimits.sh"); -const MCPORTER_RUNTIME_RELPATHS = [ - path.join("agents", "openclaw", "mcporter-runtime", "package.json"), - path.join("agents", "openclaw", "mcporter-runtime", "package-lock.json"), -]; const TEST_SANDBOX_PREFIX = "e2e-upgrade-stale"; export const SANDBOX_NAME = process.env.NEMOCLAW_SANDBOX_NAME ?? @@ -73,35 +67,6 @@ async function bestEffortPreclean(run: () => Promise): Promise { } } -function createOldBaseBuildContext(): string { - const buildContext = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-upgrade-stale-base-")); - fs.mkdirSync(path.join(buildContext, path.dirname(BLUEPRINT_RELPATH)), { recursive: true }); - fs.mkdirSync(path.join(buildContext, path.dirname(BASE_CONTEXT_SCRIPT_RELPATH)), { - recursive: true, - }); - const original = fs.readFileSync(BLUEPRINT, "utf8"); - const minOpenClawVersion = /^(\s*min_openclaw_version:\s*).*/m; - expect( - minOpenClawVersion.test(original), - "blueprint min_openclaw_version line was not found", - ).toBe(true); - fs.writeFileSync( - path.join(buildContext, BLUEPRINT_RELPATH), - original.replace(minOpenClawVersion, `$1"${OLD_OPENCLAW_VERSION}"`), - "utf8", - ); - fs.copyFileSync( - path.join(REPO_ROOT, BASE_CONTEXT_SCRIPT_RELPATH), - path.join(buildContext, BASE_CONTEXT_SCRIPT_RELPATH), - ); - for (const relativePath of MCPORTER_RUNTIME_RELPATHS) { - const target = path.join(buildContext, relativePath); - fs.mkdirSync(path.dirname(target), { recursive: true }); - fs.copyFileSync(path.join(REPO_ROOT, relativePath), target); - } - return buildContext; -} - export function writeStaleRegistryEntry(): void { const session = readJsonFileOrFallback>(SESSION_FILE, {}); const envProvider =