diff --git a/ci/e2e-assertion-budget.json b/ci/e2e-assertion-budget.json index b92a83b9023..6fc1b3b5bc7 100644 --- a/ci/e2e-assertion-budget.json +++ b/ci/e2e-assertion-budget.json @@ -15,26 +15,26 @@ "testFileCount": 86, "liveFileCount": 222, "direct": { - "expectCalls": 1881, - "matcherAssertions": 1850, + "expectCalls": 1878, + "matcherAssertions": 1847, "nodeAssertions": 100, "namedAssertionHelpers": 624, "failCalls": 8, "throwGuards": 87, "objectFieldAssertions": 245, - "assertionPoints": 2914, + "assertionPoints": 2911, "generatedProbeBlocks": 135, "generatedProbeConditions": 353 }, "unique": { - "expectCalls": 2373, - "matcherAssertions": 2337, + "expectCalls": 2370, + "matcherAssertions": 2334, "nodeAssertions": 119, "namedAssertionHelpers": 927, "failCalls": 38, "throwGuards": 633, "objectFieldAssertions": 348, - "assertionPoints": 4402, + "assertionPoints": 4399, "generatedProbeBlocks": 290, "generatedProbeConditions": 975 }, @@ -60,7 +60,7 @@ "test/e2e/live/cron-preflight-inference-local.test.ts": [8,8,8,8,1], "test/e2e/live/dashboard-remote-bind.test.ts": [17,15,17,17,3], "test/e2e/live/device-auth-health.test.ts": [13,18,13,21,0], - "test/e2e/live/double-onboard.test.ts": [88,97,88,97,0], + "test/e2e/live/double-onboard.test.ts": [85,94,85,94,0], "test/e2e/live/external-gateway-health.test.ts": [4,5,4,11,0], "test/e2e/live/full-e2e.test.ts": [30,34,39,72,7], "test/e2e/live/gateway-guard-recovery.test.ts": [48,54,48,57,3], diff --git a/docs/manage-sandboxes/run-sandboxes.mdx b/docs/manage-sandboxes/run-sandboxes.mdx index 280397eb4ac..3277d0b460c 100644 --- a/docs/manage-sandboxes/run-sandboxes.mdx +++ b/docs/manage-sandboxes/run-sandboxes.mdx @@ -65,6 +65,11 @@ Gateway and dashboard cleanup is scoped by sandbox name and port. A later onboarding run that uses a different `NEMOCLAW_GATEWAY_PORT` or `--control-ui-port` does not tear down the first sandbox's gateway or dashboard forward. + +If re-onboarding finds the registered dashboard port already bound, NemoClaw verifies that the listener is the exact OpenShell forward for that sandbox and reuses it without restarting the sandbox. +If ownership cannot be proved, onboarding fails closed and reports the listener conflict. + + If you intentionally run separate OpenShell gateways on the same host, set a different `NEMOCLAW_GATEWAY_PORT` before each onboarding run. NemoClaw isolates the gateway name and local state by port so one port-specific gateway does not replace another. diff --git a/src/lib/actions/sandbox/forward-recovery-declared-ports.test.ts b/src/lib/actions/sandbox/forward-recovery-declared-ports.test.ts index 112f2a384c0..0b3e551b6fe 100644 --- a/src/lib/actions/sandbox/forward-recovery-declared-ports.test.ts +++ b/src/lib/actions/sandbox/forward-recovery-declared-ports.test.ts @@ -10,11 +10,13 @@ const mocks = vi.hoisted(() => ({ getSandbox: vi.fn(), getHermesDashboardRecoveryConfig: vi.fn(() => null), isLocalForwardReachable: vi.fn(() => true), + isForwardServiceListenerOwner: vi.fn(() => true), launchForwardService: vi.fn(), })); vi.mock("../../adapters/openshell/forward-service", async (importOriginal) => ({ ...(await importOriginal()), + isForwardServiceListenerOwner: mocks.isForwardServiceListenerOwner, launchForwardService: mocks.launchForwardService, })); @@ -63,6 +65,7 @@ beforeEach(() => { vi.unstubAllEnvs(); mocks.runOpenshell.mockReturnValue({ status: 0 }); mocks.isLocalForwardReachable.mockReturnValue(true); + mocks.isForwardServiceListenerOwner.mockReturnValue(true); mocks.launchForwardService.mockImplementation(() => { mocks.isLocalForwardReachable.mockReturnValue(true); }); @@ -82,9 +85,33 @@ describe("ensureDeclaredAgentForwardPortsHealthy", { timeout: 30_000 }, () => { const { ensureSandboxPortForward } = await import("./forward-recovery"); expect(ensureSandboxPortForward("remote-box")).toBe(true); + expect(mocks.isForwardServiceListenerOwner).toHaveBeenCalledWith({ + executable: "/usr/local/bin/openshell", + gatewayName: "nemoclaw", + workspace: "default", + sandboxName: "remote-box", + localHost: "0.0.0.0", + localPort: 18_789, + targetHost: "127.0.0.1", + targetPort: 18_789, + }); expect(mocks.launchForwardService).not.toHaveBeenCalled(); }); + it("fails closed when reachable direct service ownership cannot be proved", async () => { + mocks.getSandbox.mockReturnValue({ agent: "openclaw", dashboardPort: 18_789 }); + mocks.captureOpenshell.mockReturnValue(forwardList([])); + mocks.isForwardServiceListenerOwner.mockReturnValue(false); + mocks.launchForwardService.mockImplementation(() => { + throw new Error("host port is occupied"); + }); + const { ensureSandboxPortForward } = await import("./forward-recovery"); + + expect(ensureSandboxPortForward("foreign-listener")).toBe(false); + expect(mocks.isForwardServiceListenerOwner).toHaveBeenCalledOnce(); + expect(mocks.launchForwardService).toHaveBeenCalledOnce(); + }); + it("does not demand the manifest dashboard port from a sandbox that owns a different dashboard port (#8543)", async () => { mocks.getSandbox.mockReturnValue({ agent: "hermes", diff --git a/src/lib/actions/sandbox/forward-recovery.ts b/src/lib/actions/sandbox/forward-recovery.ts index 72d078565b1..49260f03ec8 100644 --- a/src/lib/actions/sandbox/forward-recovery.ts +++ b/src/lib/actions/sandbox/forward-recovery.ts @@ -6,6 +6,8 @@ import { withSelectedOpenShellCommandOptions, } from "../../adapters/openshell/command-argv"; import { + createForwardServiceTarget, + isForwardServiceListenerOwner, launchForwardService, type ForwardServiceTarget, } from "../../adapters/openshell/forward-service"; @@ -178,16 +180,16 @@ function forwardServiceTarget( expectedBind = "127.0.0.1", workspace = "default", ): ForwardServiceTarget { - return { - executable, - gatewayName, - workspace, - sandboxName, - localHost: expectedBind === "0.0.0.0" ? ("0.0.0.0" as const) : ("127.0.0.1" as const), - localPort: port, - targetHost: "127.0.0.1", - targetPort: port, - }; + return createForwardServiceTarget( + { + executable, + gatewayName, + workspace, + sandboxName, + localHost: expectedBind === "0.0.0.0" ? "0.0.0.0" : "127.0.0.1", + }, + port, + ); } function isValidPort(value: unknown): value is number { @@ -350,7 +352,7 @@ export function isSandboxForwardHealthy( export function isSandboxPortForwardHealthy( sandboxName: string, port: number, - _expectedBind?: string, + expectedBind?: string, runtimeSelection?: OpenShellRuntimeSelection, ): SandboxForwardHealth { const sandbox = registry.getSandbox(sandboxName); @@ -376,7 +378,18 @@ export function isSandboxPortForwardHealthy( ) { return false; } - return true; + const executable = resolveOpenshell(); + if (!executable) return false; + return isForwardServiceListenerOwner( + forwardServiceTarget( + executable, + gatewayName, + sandboxName, + port, + expectedBind ?? "127.0.0.1", + runtimeSelection?.workspace ?? "default", + ), + ); } export function ensureSandboxPortForwardForPort( diff --git a/src/lib/actions/sandbox/process-recovery-managed-startup.test.ts b/src/lib/actions/sandbox/process-recovery-managed-startup.test.ts index 4bd01baf350..c3ecbd7dad2 100644 --- a/src/lib/actions/sandbox/process-recovery-managed-startup.test.ts +++ b/src/lib/actions/sandbox/process-recovery-managed-startup.test.ts @@ -3,6 +3,8 @@ import { afterEach, describe, expect, it, vi } from "vitest"; +import * as forwardService from "../../adapters/openshell/forward-service"; +import * as openshellResolve from "../../adapters/openshell/resolve"; import * as openshellRuntime from "../../adapters/openshell/runtime"; import * as agentRuntime from "../../agent/runtime"; import * as registry from "../../state/registry"; @@ -36,6 +38,8 @@ function mockOpenClawSandbox(sandboxName: string): void { function mockRecoveredForward(_sandboxName: string): void { vi.spyOn(forwardHealth, "isLocalForwardReachable").mockReturnValue(true); + vi.spyOn(forwardService, "isForwardServiceListenerOwner").mockReturnValue(true); + vi.spyOn(openshellResolve, "resolveOpenshell").mockReturnValue("/usr/bin/openshell"); vi.spyOn(openshellRuntime, "captureOpenshell").mockReturnValue({ status: 0, output: "SANDBOX BIND PORT PID STATUS", diff --git a/src/lib/adapters/openshell/forward-service.test.ts b/src/lib/adapters/openshell/forward-service.test.ts index 540c40f1aca..2fbae7cbde7 100644 --- a/src/lib/adapters/openshell/forward-service.test.ts +++ b/src/lib/adapters/openshell/forward-service.test.ts @@ -1,10 +1,15 @@ // 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 { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { afterEach, describe, expect, it, vi } from "vitest"; import { buildForwardServiceArgs, + isForwardServiceListenerOwner, launchForwardService, type ForwardServiceTarget, } from "./forward-service"; @@ -20,6 +25,51 @@ const target: ForwardServiceTarget = { targetPort: 18_789, }; +const ownerTarget: ForwardServiceTarget = { ...target, executable: process.execPath }; +const temporaryDirectories: string[] = []; + +function createLinuxOwnerFixture(actualExecutable?: string) { + const root = mkdtempSync(path.join(os.tmpdir(), "nemoclaw-forward-owner-")); + temporaryDirectories.push(root); + const procRoot = path.join(root, "proc"); + const binRoot = path.join(root, "bin"); + mkdirSync(path.join(procRoot, "net"), { recursive: true }); + mkdirSync(path.join(procRoot, "4321", "fd"), { recursive: true }); + mkdirSync(path.join(procRoot, "9876", "fd"), { recursive: true }); + mkdirSync(binRoot); + const executable = path.join(binRoot, "openshell"); + const runtime = actualExecutable ? path.join(binRoot, actualExecutable) : executable; + writeFileSync(executable, ""); + writeFileSync(runtime, ""); + writeFileSync( + path.join(procRoot, "net", "tcp"), + " 0: 0100007F:4965 00000000:0000 0A 00000000:00000000 00:00000000 00000000 998 0 12345 1\n", + ); + writeFileSync( + path.join(procRoot, "net", "tcp6"), + " 1: 00000000000000000000000001000000:4965 00000000000000000000000000000000:0000 0A 00000000:00000000 00:00000000 00000000 998 0 67890 1\n", + ); + symlinkSync("socket:[12345]", path.join(procRoot, "4321", "fd", "7")); + symlinkSync("socket:[67890]", path.join(procRoot, "9876", "fd", "8")); + symlinkSync(runtime, path.join(procRoot, "4321", "exe")); + return { procRoot, target: { ...target, executable } }; +} + +function darwinOwnerProbe(commandLine: string, finalListener = "4321\n") { + return vi + .fn() + .mockReturnValueOnce({ status: 0, stdout: "4321\n" }) + .mockReturnValueOnce({ status: 0, stdout: `p4321\nftxt\nn${process.execPath}\n` }) + .mockReturnValueOnce({ status: 0, stdout: commandLine }) + .mockReturnValueOnce({ status: 0, stdout: finalListener }); +} + +afterEach(() => { + for (const directory of temporaryDirectories.splice(0)) { + rmSync(directory, { recursive: true, force: true }); + } +}); + describe("OpenShell forward service", () => { it("builds the direct ForwardTcp command with explicit gateway authority", () => { expect(buildForwardServiceArgs(target)).toEqual([ @@ -45,6 +95,106 @@ describe("OpenShell forward service", () => { ); }); + it("proves the exact direct ForwardTcp listener before reuse", () => { + const expected = [ownerTarget.executable, ...buildForwardServiceArgs(ownerTarget)].join(" "); + const probe = darwinOwnerProbe(`${expected}\n`); + + expect(isForwardServiceListenerOwner(ownerTarget, { platform: "darwin", probe })).toBe(true); + expect(probe).toHaveBeenCalledTimes(4); + }); + + it("rejects a listener whose process does not match the direct ForwardTcp target", () => { + const probe = darwinOwnerProbe("/usr/bin/node foreign-listener.js\n"); + + expect(isForwardServiceListenerOwner(ownerTarget, { platform: "darwin", probe })).toBe(false); + }); + + it("rejects ambiguous or changing listener ownership", () => { + const expected = [ownerTarget.executable, ...buildForwardServiceArgs(ownerTarget)].join(" "); + const probe = darwinOwnerProbe(`${expected}\n`, "9876\n"); + + expect(isForwardServiceListenerOwner(ownerTarget, { platform: "darwin", probe })).toBe(false); + }); + + it("rejects ownership when a host probe times out", () => { + const lsofTimeout = vi.fn(() => ({ status: null, stdout: "" })); + expect( + isForwardServiceListenerOwner(ownerTarget, { platform: "darwin", probe: lsofTimeout }), + ).toBe(false); + + const psTimeout = vi + .fn() + .mockReturnValueOnce({ status: 0, stdout: "4321\n" }) + .mockReturnValueOnce({ status: 0, stdout: `p4321\nftxt\nn${process.execPath}\n` }) + .mockReturnValueOnce({ status: null, stdout: "" }); + expect( + isForwardServiceListenerOwner(ownerTarget, { platform: "darwin", probe: psTimeout }), + ).toBe(false); + }); + + it("proves Linux IPv4 ownership while ignoring an IPv6-only listener", () => { + const fixture = createLinuxOwnerFixture(); + const expected = [fixture.target.executable, ...buildForwardServiceArgs(fixture.target)].join( + " ", + ); + const responses = { + lsof: { status: null, stdout: "" }, + ps: { status: 0, stdout: `${expected}\n` }, + }; + const probe = vi.fn( + (executable: string) => responses[executable as keyof typeof responses] ?? responses.lsof, + ); + + expect( + isForwardServiceListenerOwner(fixture.target, { + platform: "linux", + probe, + procRoot: fixture.procRoot, + }), + ).toBe(true); + expect(probe).toHaveBeenCalledTimes(3); + expect(probe).toHaveBeenCalledWith("lsof", ["-ti4TCP:18789", "-sTCP:LISTEN"]); + expect(probe).toHaveBeenCalledWith("ps", ["-ww", "-p", "4321", "-o", "args="]); + }); + + it("rejects spoofed arguments when the Linux executable is different", () => { + const fixture = createLinuxOwnerFixture("python3"); + const expected = [fixture.target.executable, ...buildForwardServiceArgs(fixture.target)].join( + " ", + ); + const responses = { + lsof: { status: null, stdout: "" }, + ps: { status: 0, stdout: `${expected}\n` }, + }; + const probe = vi.fn( + (executable: string) => responses[executable as keyof typeof responses] ?? responses.lsof, + ); + + expect( + isForwardServiceListenerOwner(fixture.target, { + platform: "linux", + probe, + procRoot: fixture.procRoot, + }), + ).toBe(false); + expect(probe).toHaveBeenCalledOnce(); + }); + + it("denies Linux ownership when the /proc work limit is reached", () => { + const fixture = createLinuxOwnerFixture(); + const probe = vi.fn(() => ({ status: null, stdout: "" })); + + expect( + isForwardServiceListenerOwner(fixture.target, { + platform: "linux", + probe, + procRoot: fixture.procRoot, + procWorkLimit: 1, + }), + ).toBe(false); + expect(probe).toHaveBeenCalledOnce(); + }); + it("detaches the OpenShell child and waits for its local port", () => { const unref = vi.fn(); const spawnDetached = vi.fn(() => ({ unref })); diff --git a/src/lib/adapters/openshell/forward-service.ts b/src/lib/adapters/openshell/forward-service.ts index 0eb1998e576..897bf87f6e4 100644 --- a/src/lib/adapters/openshell/forward-service.ts +++ b/src/lib/adapters/openshell/forward-service.ts @@ -1,7 +1,8 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { spawn } from "node:child_process"; +import { spawn, spawnSync } from "node:child_process"; +import { readFileSync, readdirSync, readlinkSync, realpathSync } from "node:fs"; import path from "node:path"; import { isValidName } from "../../name-validation"; @@ -37,6 +38,21 @@ export interface ForwardServiceLaunchOptions { readonly timeoutMs?: number; } +type ForwardServiceOwnerProbe = ( + executable: string, + args: readonly string[], +) => { status: number | null; stdout: string }; + +export interface ForwardServiceOwnerOptions { + readonly platform?: NodeJS.Platform; + readonly probe?: ForwardServiceOwnerProbe; + readonly procRoot?: string; + readonly procWorkLimit?: number; +} + +const FORWARD_OWNER_PROBE_TIMEOUT_MS = 5_000; +const LINUX_PROC_WORK_LIMIT = 50_000; + function isPort(value: unknown): value is number { return Number.isSafeInteger(value) && Number(value) >= 1 && Number(value) <= 65_535; } @@ -74,6 +90,21 @@ export function validateForwardServiceTarget(target: ForwardServiceTarget): Forw return target; } +export function createForwardServiceTarget( + target: Pick< + ForwardServiceTarget, + "executable" | "gatewayName" | "workspace" | "sandboxName" | "localHost" + >, + port: number, +): ForwardServiceTarget { + return validateForwardServiceTarget({ + ...target, + localPort: port, + targetHost: "127.0.0.1", + targetPort: port, + }); +} + /** Build the direct ForwardTcp command introduced in OpenShell 0.0.106. */ export function buildForwardServiceArgs(target: ForwardServiceTarget): string[] { validateForwardServiceTarget(target); @@ -94,6 +125,135 @@ export function buildForwardServiceArgs(target: ForwardServiceTarget): string[] ]; } +function captureProcess(executable: string, args: readonly string[]) { + const result = spawnSync(executable, [...args], { + encoding: "utf8", + timeout: FORWARD_OWNER_PROBE_TIMEOUT_MS, + }); + return { status: result.status, stdout: result.stdout ?? "" }; +} + +function lsofListenerPids(port: number, probe: ForwardServiceOwnerProbe): string[] | null { + const result = probe("lsof", [`-ti4TCP:${String(port)}`, "-sTCP:LISTEN"]); + if (result.status === null) return null; + if (result.status !== 0) return []; + return [ + ...new Set( + result.stdout + .split(/\r?\n/u) + .map((line) => line.trim()) + .filter(Boolean), + ), + ]; +} + +function linuxListenerPids(port: number, procRoot: string, workLimit: number): string[] { + if (!Number.isSafeInteger(workLimit) || workLimit < 1) return []; + const portSuffix = `:${port.toString(16).padStart(4, "0").toUpperCase()}`; + const socketInodes = new Set(); + try { + for (const line of readFileSync(path.join(procRoot, "net", "tcp"), "utf8").split("\n")) { + const fields = line.trim().split(/\s+/u); + if ( + fields[3] === "0A" && + fields[1]?.toUpperCase().endsWith(portSuffix) && + /^\d+$/u.test(fields[9] ?? "") + ) { + socketInodes.add(fields[9]!); + } + } + } catch { + // A missing or unreadable IPv4 table cannot prove ownership. + } + if (socketInodes.size === 0) return []; + + const pids = new Set(); + let inspected = 0; + try { + for (const entry of readdirSync(procRoot, { withFileTypes: true })) { + if (!entry.isDirectory() || !/^[1-9]\d*$/u.test(entry.name)) continue; + if (++inspected > workLimit) return []; + try { + for (const descriptor of readdirSync(path.join(procRoot, entry.name, "fd"))) { + if (++inspected > workLimit) return []; + const link = readlinkSync(path.join(procRoot, entry.name, "fd", descriptor)); + const match = /^socket:\[(\d+)\]$/u.exec(link); + if (match && socketInodes.has(match[1]!)) { + pids.add(entry.name); + break; + } + } + } catch { + // Processes can exit or deny access while /proc is being inspected. + } + } + } catch { + return []; + } + return [...pids]; +} + +function listenerPids( + port: number, + platform: NodeJS.Platform, + procRoot: string, + procWorkLimit: number, + probe: ForwardServiceOwnerProbe, +): string[] { + const lsof = lsofListenerPids(port, probe); + if (lsof !== null || platform !== "linux") return lsof ?? []; + return linuxListenerPids(port, procRoot, procWorkLimit); +} + +function executableMatches(actualExecutable: string, expectedExecutable: string): boolean { + try { + return realpathSync(actualExecutable) === realpathSync(expectedExecutable); + } catch { + return false; + } +} + +function processExecutableMatches( + pid: string, + target: ForwardServiceTarget, + platform: NodeJS.Platform, + procRoot: string, + probe: ForwardServiceOwnerProbe, +): boolean { + if (platform === "linux") { + return executableMatches(path.join(procRoot, pid, "exe"), target.executable); + } + if (platform !== "darwin") return false; + const result = probe("lsof", ["-a", "-p", pid, "-d", "txt", "-Fn"]); + if (result.status !== 0) return false; + return result.stdout + .split(/\r?\n/u) + .filter((line) => line.startsWith("n/")) + .some((line) => executableMatches(line.slice(1), target.executable)); +} + +/** Prove that the current listener is the exact direct ForwardTcp command. */ +export function isForwardServiceListenerOwner( + target: ForwardServiceTarget, + options: ForwardServiceOwnerOptions = {}, +): boolean { + validateForwardServiceTarget(target); + const platform = options.platform ?? process.platform; + const probe = options.probe ?? captureProcess; + const procRoot = options.procRoot ?? "/proc"; + const procWorkLimit = options.procWorkLimit ?? LINUX_PROC_WORK_LIMIT; + const before = listenerPids(target.localPort, platform, procRoot, procWorkLimit, probe); + if (before.length !== 1 || !/^[1-9]\d*$/u.test(before[0]!)) return false; + const pid = before[0]!; + if (!processExecutableMatches(pid, target, platform, procRoot, probe)) return false; + const commandLine = probe("ps", ["-ww", "-p", pid, "-o", "args="]); + if (commandLine.status !== 0) return false; + const expected = [target.executable, ...buildForwardServiceArgs(target)].join(" "); + if (commandLine.stdout.trim() !== expected) return false; + const after = listenerPids(target.localPort, platform, procRoot, procWorkLimit, probe); + return after.length === 1 && after[0] === pid; +} + function forwardServiceEnvironment(source: NodeJS.ProcessEnv): NodeJS.ProcessEnv { const environment = buildOpenShellSubprocessEnv(source); const configHome = source.XDG_CONFIG_HOME?.trim(); diff --git a/src/lib/onboard/agent-dashboard-forward.ts b/src/lib/onboard/agent-dashboard-forward.ts index e43b787318e..9ef697e67df 100644 --- a/src/lib/onboard/agent-dashboard-forward.ts +++ b/src/lib/onboard/agent-dashboard-forward.ts @@ -21,6 +21,7 @@ export type EnsureDashboardForward = ( chatUiUrl?: string, options?: { allowPortReallocation?: boolean; + reuseExistingOpenClawForward?: boolean; revalidateSandboxIdentity?: (operation: string) => void; }, ) => number; @@ -39,6 +40,7 @@ export async function ensureAgentDashboardForward(options: { /** Host port allocated to this sandbox's OpenAI-compatible API, when it has one. */ hermesApiPort?: number | null; beforeForwardPort?: (port: number) => Promise | void; + reuseExistingOpenClawForward?: boolean; revalidateSandboxIdentity?: (operation: string) => void; warn?: (message: string) => void; }): Promise { @@ -50,6 +52,7 @@ export async function ensureAgentDashboardForward(options: { controlUiPort, hermesApiPort, beforeForwardPort, + reuseExistingOpenClawForward = false, revalidateSandboxIdentity, warn = (message: string) => console.warn(message), } = options; @@ -108,6 +111,7 @@ export async function ensureAgentDashboardForward(options: { await beforeForwardPort?.(agentDashboardPort); const actualAgentDashboardPort = ensureDashboardForward(sandboxName, requestedDashboardUrl, { allowPortReallocation: false, + ...(reuseExistingOpenClawForward ? { reuseExistingOpenClawForward: true } : {}), ...(revalidateIdentity ? { revalidateSandboxIdentity: revalidateIdentity } : {}), }); if (!usesFixedApiPort) { diff --git a/src/lib/onboard/dashboard-forward-control.ts b/src/lib/onboard/dashboard-forward-control.ts index 7b58fd24393..57ff2f33acb 100644 --- a/src/lib/onboard/dashboard-forward-control.ts +++ b/src/lib/onboard/dashboard-forward-control.ts @@ -5,15 +5,18 @@ export interface DashboardForwardOptions { rollbackSandboxOnFailure?: boolean; gatewayName?: string; allowPortReallocation?: boolean; + reuseExistingOpenClawForward?: boolean; revalidateSandboxIdentity?: (operation: string) => void; } export function normalizeDashboardForwardOptions(options: DashboardForwardOptions = {}): { rollbackSandboxOnFailure: boolean; allowPortReallocation: boolean; + reuseExistingOpenClawForward: boolean; } { return { rollbackSandboxOnFailure: options.rollbackSandboxOnFailure === true, allowPortReallocation: options.allowPortReallocation !== false, + reuseExistingOpenClawForward: options.reuseExistingOpenClawForward === true, }; } diff --git a/src/lib/onboard/dashboard.ts b/src/lib/onboard/dashboard.ts index 615e45e0ad7..4062356bb4b 100644 --- a/src/lib/onboard/dashboard.ts +++ b/src/lib/onboard/dashboard.ts @@ -5,6 +5,8 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { + createForwardServiceTarget, + isForwardServiceListenerOwner, launchForwardService, type ForwardServiceTarget, } from "../adapters/openshell/forward-service"; @@ -87,6 +89,7 @@ export interface OnboardDashboardDeps { /** Direct ForwardTcp launcher. */ forwardService?: { executable(): string; + owns?(target: ForwardServiceTarget): boolean; launch?(target: ForwardServiceTarget): void; retireLegacy?(sandboxName: string, gatewayName: string, ports: readonly number[]): number; resolveGatewayName( @@ -224,6 +227,7 @@ export function createOnboardDashboardHelpers(deps: OnboardDashboardDeps): Onboa if (!executable) throw new Error("OpenShell is unavailable"); return executable; }, + owns: isForwardServiceListenerOwner, resolveGatewayName: productionForwardService.resolveGatewayName, retireLegacy: (sandboxName: string, gatewayName: string, ports: readonly number[]) => productionForwardService.retireLegacy(sandboxName, gatewayName, ports, { @@ -244,7 +248,6 @@ export function createOnboardDashboardHelpers(deps: OnboardDashboardDeps): Onboa }), } : undefined); - function resolveForwardServiceGateway( sandboxName: string, options: DashboardForwardOptions = {}, @@ -261,16 +264,29 @@ export function createOnboardDashboardHelpers(deps: OnboardDashboardDeps): Onboa port: number, target: string, ): ForwardServiceTarget { - return { - executable: forwardService!.executable(), - gatewayName, - workspace: "default", - sandboxName, - localHost: target.startsWith("0.0.0.0:") ? ("0.0.0.0" as const) : ("127.0.0.1" as const), - localPort: port, - targetHost: "127.0.0.1", - targetPort: port, - }; + return createForwardServiceTarget( + { + executable: forwardService!.executable(), + gatewayName, + workspace: "default", + sandboxName, + localHost: target.startsWith("0.0.0.0:") ? "0.0.0.0" : "127.0.0.1", + }, + port, + ); + } + + function ownsDashboardForward( + sandboxName: string, + gatewayName: string, + port: number, + chatUiUrl: string, + ): boolean { + return ( + forwardService?.owns?.( + forwardTarget(sandboxName, gatewayName, port, getDashboardForwardTarget(chatUiUrl)), + ) === true + ); } function getDashboardForwardPort( @@ -379,7 +395,7 @@ export function createOnboardDashboardHelpers(deps: OnboardDashboardDeps): Onboa options: DashboardForwardOptions = {}, ): number { chatUiUrl ||= `http://127.0.0.1:${CONTROL_UI_PORT}`; - const { rollbackSandboxOnFailure, allowPortReallocation } = + const { rollbackSandboxOnFailure, allowPortReallocation, reuseExistingOpenClawForward } = normalizeDashboardForwardOptions(options); const { revalidateSandboxIdentity } = options; const preferredPort = Number(getDashboardForwardPort(chatUiUrl)); @@ -394,7 +410,18 @@ export function createOnboardDashboardHelpers(deps: OnboardDashboardDeps): Onboa ); const isPortBound = deps.isPortBoundOnHost ?? isPortBoundOnHost; const persistedPort = getPersistedDashboardPort(sandboxName, listSandboxes); + const registryOccupiedPorts = getRegistryOccupiedDashboardPorts(sandboxName, listSandboxes); if (persistedPort === preferredPort && isPortBound(preferredPort)) { + if ( + reuseExistingOpenClawForward && + !registryOccupiedPorts.has(String(preferredPort)) && + ownsDashboardForward(sandboxName, forwardGateway, preferredPort, chatUiUrl) + ) { + revalidateSandboxIdentity?.( + `retain dashboard forward ${String(preferredPort)} for sandbox '${sandboxName}'`, + ); + return preferredPort; + } throw new Error( `Registered dashboard port ${String(preferredPort)} is already occupied; it cannot be reallocated or adopted.`, ); @@ -406,7 +433,7 @@ export function createOnboardDashboardHelpers(deps: OnboardDashboardDeps): Onboa preferredPort, existingForwards, isPortBound, - getRegistryOccupiedDashboardPorts(sandboxName, listSandboxes), + registryOccupiedPorts, ); } catch (err) { if (!rollbackSandboxOnFailure) throw err; @@ -498,10 +525,10 @@ export function createOnboardDashboardHelpers(deps: OnboardDashboardDeps): Onboa /** * Reconcile the dashboard forward for the agent-less OpenClaw finalization - * branch. The resume path skips sandbox creation, so `CHAT_UI_URL` does not - * carry the port the in-sandbox gateway listens on; the registry entry - * persisted by onboarding is the only record of that port. The forward and - * the in-sandbox gateway must share one port number (`openshell forward` + * branch. A resumed or repeated onboarding can skip sandbox creation, so + * `CHAT_UI_URL` may not carry the port the in-sandbox gateway listens on; + * the registry entry persisted by onboarding is the only record of that + * port. The forward and the in-sandbox gateway must share one port number (`openshell forward` * binds the same port on both sides), so when the persisted port cannot be * forwarded this throws instead of reallocating: the resumed gateway only * listens on the persisted port, and a forward on any other port serves @@ -519,6 +546,7 @@ export function createOnboardDashboardHelpers(deps: OnboardDashboardDeps): Onboa envUrl || (persistedPort === null ? undefined : `http://127.0.0.1:${String(persistedPort)}`); const actualPort = ensureDashboardForward(sandboxName, requestedUrl, { allowPortReallocation: false, + reuseExistingOpenClawForward: true, ...(revalidateSandboxIdentity ? { revalidateSandboxIdentity } : {}), }); revalidateSandboxIdentity?.(`publish the dashboard URL for sandbox '${sandboxName}'`); @@ -534,6 +562,7 @@ export function createOnboardDashboardHelpers(deps: OnboardDashboardDeps): Onboa agent: { forwardPort?: number | null; forward_ports?: number[] | null }, options: { beforeForwardPort?: (port: number) => Promise | void; + reuseExistingOpenClawForward?: boolean; revalidateSandboxIdentity?: (operation: string) => void; } = {}, ): Promise { @@ -546,6 +575,7 @@ export function createOnboardDashboardHelpers(deps: OnboardDashboardDeps): Onboa controlUiPort: chatUiUrl ? Number(getDashboardForwardPort(chatUiUrl)) : undefined, hermesApiPort: getSandbox?.(sandboxName)?.hermesApiPort, beforeForwardPort: options.beforeForwardPort, + reuseExistingOpenClawForward: options.reuseExistingOpenClawForward, revalidateSandboxIdentity: options.revalidateSandboxIdentity, }); } @@ -558,14 +588,23 @@ export function createOnboardDashboardHelpers(deps: OnboardDashboardDeps): Onboa releaseBeforeForward(agentName: string, port: number): Promise | void; }, ): Promise | number { - return agent - ? ensureAgentDashboardForward(sandboxName, agent, { - revalidateSandboxIdentity, - beforeForwardPort: portReservation - ? (port) => portReservation.releaseBeforeForward(agent.name, port) - : undefined, - }) - : ensureFinalizationDashboardForward(sandboxName, revalidateSandboxIdentity); + if (!agent) { + return ensureFinalizationDashboardForward(sandboxName, revalidateSandboxIdentity); + } + const mayReuseOpenClawForward = agent.name === "openclaw"; + if (mayReuseOpenClawForward) { + const registeredPort = getPersistedDashboardPort(sandboxName, listSandboxes); + if (!process.env.CHAT_UI_URL && registeredPort !== null) { + process.env.CHAT_UI_URL = `http://127.0.0.1:${String(registeredPort)}`; + } + } + return ensureAgentDashboardForward(sandboxName, agent, { + revalidateSandboxIdentity, + ...(mayReuseOpenClawForward ? { reuseExistingOpenClawForward: true } : {}), + beforeForwardPort: portReservation + ? (port) => portReservation.releaseBeforeForward(agent.name, port) + : undefined, + }); } function ensureAgentFixedForward( diff --git a/src/lib/onboard/sandbox-reuse.test.ts b/src/lib/onboard/sandbox-reuse.test.ts index c59aab6bc39..1b320781d82 100644 --- a/src/lib/onboard/sandbox-reuse.test.ts +++ b/src/lib/onboard/sandbox-reuse.test.ts @@ -33,6 +33,7 @@ describe("applyReusedSandboxDashboardState", () => { it("clears Hermes dashboard registry fields when the reused sandbox has it disabled", () => { const updateSandbox = vi.fn(); + const ensureDashboardForward = vi.fn(() => 18789); const sandboxGpuConfig: SandboxGpuConfig = { hostGpuDetected: false, hostGpuPlatform: null, @@ -53,7 +54,7 @@ describe("applyReusedSandboxDashboardState", () => { sandboxGpuConfig, gatewayName: "nemoclaw", gatewayPort: 8080, - ensureDashboardForward: vi.fn(() => 18789), + ensureDashboardForward, hermesDashboardForwarding: { resolveStateForPort: vi.fn(() => hermesDashboardState), ensureForState: vi.fn(), @@ -71,6 +72,9 @@ describe("applyReusedSandboxDashboardState", () => { gatewayPort: 8080, }); expect(result.hermesDashboardState).toBe(hermesDashboardState); + expect(ensureDashboardForward).toHaveBeenCalledWith("reuse-me", "http://127.0.0.1:18789", { + reuseExistingOpenClawForward: true, + }); }); it("skips dashboard forwarding while preserving reuse metadata for terminal agents", () => { @@ -181,6 +185,7 @@ describe("applyReusedSandboxDashboardState", () => { }, gatewayName: "nemoclaw", gatewayPort: 8080, + getSandbox: () => null, releaseDashboardPort: vi.fn(async () => undefined), ensureDashboardForward, hermesDashboardForwarding: { @@ -200,6 +205,46 @@ describe("applyReusedSandboxDashboardState", () => { expect(updateSandbox).not.toHaveBeenCalled(); }); + it("launches the registered OpenClaw port when reuse finds no listener", async () => { + const releaseDashboardPort = vi.fn(async () => undefined); + const ensureDashboardForward = vi.fn(() => 18_789); + + const result = await restoreReusedSandboxDashboardState({ + sandboxName: "reuse-me", + chatUiUrl: "http://127.0.0.1:18790", + env: {}, + agent: null, + model: "test-model", + provider: "openai-compatible", + selectionVerified: true, + sandboxGpuConfig: { + hostGpuDetected: false, + hostGpuPlatform: null, + sandboxGpuEnabled: false, + mode: "auto", + sandboxGpuDevice: null, + errors: [], + }, + gatewayName: "nemoclaw", + gatewayPort: 8080, + getSandbox: () => ({ dashboardPort: 18_789 }) as never, + releaseDashboardPort, + ensureDashboardForward, + hermesDashboardForwarding: { + resolveStateForPort: vi.fn(() => ({ enabled: false, config: null })), + ensureForState: vi.fn(), + }, + updateSandbox: vi.fn(), + updateReusedSandboxMetadata: vi.fn(), + }); + + expect(releaseDashboardPort).toHaveBeenCalledOnce(); + expect(ensureDashboardForward).toHaveBeenCalledWith("reuse-me", "http://127.0.0.1:18789", { + reuseExistingOpenClawForward: true, + }); + expect(result.dashboardPort).toBe(18_789); + }); + it("rechecks after Hermes forwarding before reuse metadata (#9833)", () => { const revalidateSandboxIdentity = vi .fn<(operation: string) => void>() @@ -210,6 +255,7 @@ describe("applyReusedSandboxDashboardState", () => { throw new Error("Sandbox identity changed before the dashboard entry"); }); const ensureForState = vi.fn(); + const ensureDashboardForward = vi.fn(() => 18790); const updateReusedSandboxMetadata = vi.fn(); const updateSandbox = vi.fn(); @@ -218,7 +264,7 @@ describe("applyReusedSandboxDashboardState", () => { sandboxName: "reuse-me", chatUiUrl: "http://127.0.0.1:18789", env: {}, - agent: null, + agent: { name: "hermes" } as any, model: "test-model", provider: "openai-compatible", selectionVerified: true, @@ -232,7 +278,7 @@ describe("applyReusedSandboxDashboardState", () => { }, gatewayName: "nemoclaw", gatewayPort: 8080, - ensureDashboardForward: vi.fn(() => 18790), + ensureDashboardForward, hermesDashboardForwarding: { resolveStateForPort: vi.fn(() => ({ enabled: false, config: null })), ensureForState, @@ -244,6 +290,9 @@ describe("applyReusedSandboxDashboardState", () => { ).toThrow(/Sandbox identity changed before/u); expect(ensureForState).toHaveBeenCalledOnce(); + expect(ensureDashboardForward).toHaveBeenCalledWith("reuse-me", "http://127.0.0.1:18789", { + revalidateSandboxIdentity, + }); expect(updateReusedSandboxMetadata).not.toHaveBeenCalled(); expect(updateSandbox).not.toHaveBeenCalled(); }); diff --git a/src/lib/onboard/sandbox-reuse.ts b/src/lib/onboard/sandbox-reuse.ts index 4cfebedd42a..d1a3e44af1f 100644 --- a/src/lib/onboard/sandbox-reuse.ts +++ b/src/lib/onboard/sandbox-reuse.ts @@ -104,7 +104,10 @@ export interface ReusedSandboxDashboardStateInput { ensureDashboardForward( sandboxName: string, chatUiUrl: string, - options?: { revalidateSandboxIdentity?: (operation: string) => void }, + options?: { + reuseExistingOpenClawForward?: boolean; + revalidateSandboxIdentity?: (operation: string) => void; + }, ): number; hermesDashboardForwarding: ReusedSandboxDashboardForwarding; updateSandbox?(sandboxName: string, updates: Partial): unknown; @@ -144,12 +147,14 @@ export function applyReusedSandboxDashboardState( input.revalidateSandboxIdentity?.( `restore dashboard state for sandbox '${input.sandboxName}'`, ); + const reuseExistingOpenClawForward = input.agent == null || input.agent.name === "openclaw"; const dashboardPort = manageDashboard - ? input.revalidateSandboxIdentity - ? input.ensureDashboardForward(input.sandboxName, input.chatUiUrl, { - revalidateSandboxIdentity: input.revalidateSandboxIdentity, - }) - : input.ensureDashboardForward(input.sandboxName, input.chatUiUrl) + ? input.ensureDashboardForward(input.sandboxName, input.chatUiUrl, { + ...(reuseExistingOpenClawForward ? { reuseExistingOpenClawForward: true } : {}), + ...(input.revalidateSandboxIdentity + ? { revalidateSandboxIdentity: input.revalidateSandboxIdentity } + : {}), + }) : 0; const chatUiUrl = manageDashboard ? `http://127.0.0.1:${dashboardPort}` : input.chatUiUrl; if (manageDashboard) { @@ -196,7 +201,25 @@ export async function restoreReusedSandboxDashboardState( input: ReusedSandboxDashboardStateInput & { releaseDashboardPort(): Promise }, ): Promise { await input.releaseDashboardPort(); - return applyReusedSandboxDashboardState(input); + const reusesOpenClaw = input.agent == null || input.agent.name === "openclaw"; + const registeredPort = (input.getSandbox ?? registry.getSandbox)( + input.sandboxName, + )?.dashboardPort; + const registeredOpenClawDashboardPort = + reusesOpenClaw && + typeof registeredPort === "number" && + Number.isInteger(registeredPort) && + registeredPort > 0 && + registeredPort <= 65_535 + ? registeredPort + : undefined; + const chatUiUrl = registeredOpenClawDashboardPort + ? `http://127.0.0.1:${String(registeredOpenClawDashboardPort)}` + : input.chatUiUrl; + return applyReusedSandboxDashboardState({ + ...input, + chatUiUrl, + }); } export function createSandboxReuseHelpers(deps: SandboxReuseDeps): SandboxReuseHelpers { diff --git a/test/cli/connect-recovery.test.ts b/test/cli/connect-recovery.test.ts index 47512a420d3..0a5e70db46d 100644 --- a/test/cli/connect-recovery.test.ts +++ b/test/cli/connect-recovery.test.ts @@ -12,7 +12,7 @@ import { LAUNCH_READINESS_PAIRING_QUALIFICATION_OUTPUT, launchReadinessRegistryFixture, } from "../helpers/launch-readiness-fixture"; -import { nonWslPlatformNodeOptions } from "../helpers/platform-override-node-options"; +import { syntheticForwardNodeOptions } from "../helpers/platform-override-node-options"; import { runWithEnv, testTimeoutOptions, @@ -278,7 +278,7 @@ describe("CLI connect recovery process contracts", () => { try { const result = runWithEnv("alpha connect --probe-only", { HOME: home, - NODE_OPTIONS: nonWslPlatformNodeOptions(home), + NODE_OPTIONS: syntheticForwardNodeOptions(home), PATH: `${localBin}:${process.env.PATH || ""}`, }); @@ -357,7 +357,7 @@ describe("CLI connect recovery process contracts", () => { try { const result = runWithEnv("alpha connect --probe-only", { HOME: home, - NODE_OPTIONS: nonWslPlatformNodeOptions(home), + NODE_OPTIONS: syntheticForwardNodeOptions(home), PATH: `${localBin}:${process.env.PATH || ""}`, }); @@ -479,7 +479,7 @@ describe("CLI connect recovery process contracts", () => { const result = runWithEnv("alpha connect", { HOME: home, - NODE_OPTIONS: nonWslPlatformNodeOptions(home), + NODE_OPTIONS: syntheticForwardNodeOptions(home), PATH: `${localBin}:${process.env.PATH || ""}`, }); diff --git a/test/e2e/fixtures/clients/host.ts b/test/e2e/fixtures/clients/host.ts index 4991582e2c3..903e88ead18 100644 --- a/test/e2e/fixtures/clients/host.ts +++ b/test/e2e/fixtures/clients/host.ts @@ -24,6 +24,12 @@ export interface HostClientOptions { openshellPath?: string; } +export interface ForwardListenerEvidence { + valid: boolean; + identity: string; + output: string; +} + const GATEWAY_ALREADY_ABSENT = /gateway[^\n]*(?:does not exist|not found)|No (?:active )?gateway|No gateway metadata found/i; const GATEWAY_REMOVE_UNSUPPORTED = @@ -177,6 +183,69 @@ export class HostCliClient { return result; } + async inspectOpenShellForwardListener( + port: string, + sandboxName: string, + options: ShellProbeRunOptions = {}, + ): Promise { + const artifactName = options.artifactName ?? `forward-listener-${port}`; + const probeOptions = { ...options, timeoutMs: options.timeoutMs ?? 15_000 }; + const [before, command] = await Promise.all([ + this.command("lsof", ["-ti", `:${port}`, "-sTCP:LISTEN"], { + ...probeOptions, + artifactName: `${artifactName}-listener-before`, + }), + this.command("which", [this.openshellPath], { + ...probeOptions, + artifactName: `${artifactName}-command`, + }), + ]); + const pids = [ + ...new Set(before.stdout.split(/\r?\n/u).map((line) => line.trim()).filter(Boolean)), + ]; + const pid = pids.length === 1 && /^[1-9]\d*$/u.test(pids[0]!) ? pids[0]! : ""; + const commandPath = command.stdout.trim(); + if (!pid || !commandPath) { + return { valid: false, identity: "", output: `${resultText(before)}\n${resultText(command)}` }; + } + + const [actualExecutable, expectedExecutable, commandLine, after] = await Promise.all([ + this.command("readlink", ["-f", `/proc/${pid}/exe`], { + ...probeOptions, + artifactName: `${artifactName}-actual-executable`, + }), + this.command("readlink", ["-f", commandPath], { + ...probeOptions, + artifactName: `${artifactName}-expected-executable`, + }), + this.command("ps", ["-ww", "-p", pid, "-o", "args="], { + ...probeOptions, + artifactName: `${artifactName}-command-line`, + }), + this.command("lsof", ["-ti", `:${port}`, "-sTCP:LISTEN"], { + ...probeOptions, + artifactName: `${artifactName}-listener-after`, + }), + ]); + const expectedCommandLine = `${commandPath} --gateway nemoclaw --workspace default forward service ${sandboxName} --target-port ${port} --target-host 127.0.0.1 --local 127.0.0.1:${port}`; + const afterPids = [ + ...new Set(after.stdout.split(/\r?\n/u).map((line) => line.trim()).filter(Boolean)), + ]; + const probes = [before, command, actualExecutable, expectedExecutable, commandLine, after]; + const identity = `${pid}\t${actualExecutable.stdout.trim()}\t${commandLine.stdout.trim()}`; + const valid = + probes.every((probe) => probe.exitCode === 0 && !probe.timedOut) && + actualExecutable.stdout.trim() === expectedExecutable.stdout.trim() && + commandLine.stdout.trim() === expectedCommandLine && + afterPids.length === 1 && + afterPids[0] === pid; + return { + valid, + identity, + output: probes.map(resultText).filter(Boolean).join("\n"), + }; + } + async destroySandbox( sandboxName: string, options: ShellProbeRunOptions = {}, diff --git a/test/e2e/live/double-onboard.test.ts b/test/e2e/live/double-onboard.test.ts index e90538c514d..56eb36eb2dc 100644 --- a/test/e2e/live/double-onboard.test.ts +++ b/test/e2e/live/double-onboard.test.ts @@ -32,7 +32,6 @@ const PHASE_TIMEOUT_MS = Number(process.env.NEMOCLAW_E2E_PHASE_TIMEOUT_MS ?? 1_2 const ONBOARD_TIMEOUT_MS = execTimeout(PHASE_TIMEOUT_MS); const PROBE_ATTEMPTS = Number(process.env.NEMOCLAW_E2E_PROBE_ATTEMPTS ?? 3); const PROBE_DELAY_MS = Number(process.env.NEMOCLAW_E2E_PROBE_DELAY_SECONDS ?? 3) * 1_000; -const PROBE_TIMEOUT_MS = Number(process.env.NEMOCLAW_E2E_PROBE_TIMEOUT_SECONDS ?? 180) * 1_000; const RECOVERY_PROBE_TIMEOUT_MS = Number(process.env.NEMOCLAW_E2E_RECOVERY_PROBE_TIMEOUT_SECONDS ?? 180) * 1_000; const TEST_TIMEOUT_MS = testTimeout(90 * 60_000); @@ -114,34 +113,50 @@ async function runOnboard( }); } -async function runProbeOnlyConnect( +async function waitForDashboardReachability( host: HostCliClient, - sandboxName: string, + port: string, + expectedReachable: boolean, + artifactPrefix: string, +): Promise<{ reachable: boolean; output: string }> { + let reachable = false; + let output = ""; + for (let attempt = 1; attempt <= PROBE_ATTEMPTS; attempt += 1) { + const result = await host.command( + "curl", + [ + "--silent", + "--show-error", + "--fail", + "--output", + "/dev/null", + "--max-time", + "5", + `http://127.0.0.1:${port}/`, + ], + { + artifactName: `${artifactPrefix}-attempt-${attempt}`, + env: commandEnv(), + timeoutMs: 15_000, + }, + ); + output = resultText(result); + reachable = result.exitCode === 0 && !result.timedOut; + if (reachable === expectedReachable) break; + if (attempt < PROBE_ATTEMPTS) await sleep(PROBE_DELAY_MS); + } + return { reachable, output }; +} + +async function inspectNoListener( + host: HostCliClient, + port: string, artifactName: string, ): Promise { return await host.command( - "bash", - [ - "-lc", - [ - "set +e", - 'log="$(mktemp)"', - '"$1" "$2" "$3" connect --probe-only >"$log" 2>&1', - "rc=$?", - 'cat "$log"', - 'rm -f "$log"', - 'exit "$rc"', - ].join("\n"), - "nemoclaw-probe-connect", - process.execPath, - CLI_ENTRYPOINT, - sandboxName, - ], - { - artifactName, - env: commandEnv(), - timeoutMs: PROBE_TIMEOUT_MS, - }, + "lsof", + ["-ti", `:${port}`, "-sTCP:LISTEN"], + { artifactName, env: commandEnv(), timeoutMs: 15_000 }, ); } @@ -234,44 +249,6 @@ function dashboardPortFromList(output: string, sandboxName: string): string | un return undefined; } -function forwardOwnerForPort(output: string, port: string): string | undefined { - for (const line of stripAnsi(output).split("\n")) { - const parts = line.trim().split(/\s+/); - if (parts.length < 5 || parts[0]?.toLowerCase() === "sandbox") continue; - const status = parts.slice(4).join(" ").toLowerCase(); - if (parts[2] === port && status.includes("running")) return parts[0]; - } - return undefined; -} - -async function waitForForwardOwner( - sandbox: SandboxClient, - port: string, - owner: string | undefined, - artifactPrefix: string, -): Promise<{ - owner: string | undefined; - output: string; - querySucceeded: boolean; -}> { - let observedOwner: string | undefined; - let lastOutput = ""; - let querySucceeded = false; - for (let attempt = 1; attempt <= PROBE_ATTEMPTS; attempt += 1) { - const result = await sandbox.openshell(["forward", "list"], { - artifactName: `${artifactPrefix}-attempt-${attempt}`, - env: commandEnv(), - timeoutMs: 30_000, - }); - lastOutput = resultText(result); - querySucceeded = result.exitCode === 0 && !result.timedOut; - observedOwner = querySucceeded ? forwardOwnerForPort(lastOutput, port) : undefined; - if (querySucceeded && observedOwner === owner) break; - if (attempt < PROBE_ATTEMPTS) await sleep(PROBE_DELAY_MS); - } - return { owner: observedOwner, output: lastOutput, querySucceeded }; -} - function hasOwn(object: object, key: string): boolean { return Object.prototype.hasOwnProperty.call(object, key); } @@ -554,6 +531,21 @@ test( expect(sandboxAIdAfterFirst, resultText(sandboxAAfterFirst)).not.toBeNull(); expect(registryHas(SANDBOX_A), `${REGISTRY_FILE} missing ${SANDBOX_A}`).toBe(true); assertRegistryInferenceMetadata(SANDBOX_A, fake.baseUrl); + const listAfterFirst = await command(host, ["list"], { + artifactName: "phase-2-nemoclaw-list", + env: commandEnv(), + timeoutMs: 60_000, + }); + const portAfterFirst = + dashboardPortFromList(listAfterFirst.stdout, SANDBOX_A) ?? ""; + const listenerBeforeSecond = await host.inspectOpenShellForwardListener( + portAfterFirst, + SANDBOX_A, + { + artifactName: "phase-2-dashboard-listener-before-second-onboard", + env: commandEnv(), + }, + ); progress.phase("re-onboard same sandbox on existing gateway"); // Phase 3: second onboard with the same name must reuse the healthy gateway. @@ -588,6 +580,26 @@ test( }); expect(listAfterSecond.exitCode, resultText(listAfterSecond)).toBe(0); expect(stripAnsi(listAfterSecond.stdout)).toContain(SANDBOX_A); + const portAfterSecond = dashboardPortFromList(listAfterSecond.stdout, SANDBOX_A); + expect(portAfterSecond, resultText(listAfterSecond)).toBe(portAfterFirst); + const dashboardAfterSecond = await waitForDashboardReachability( + host, + portAfterSecond ?? "", + true, + "phase-3-dashboard-after-second-onboard", + ); + const listenerAfterSecond = await host.inspectOpenShellForwardListener( + portAfterSecond ?? "", + SANDBOX_A, + { + artifactName: "phase-3-dashboard-listener-after-second-onboard", + env: commandEnv(), + }, + ); + expect( + `${dashboardAfterSecond.reachable}:${listenerBeforeSecond.valid}:${listenerAfterSecond.valid}:${listenerBeforeSecond.identity === listenerAfterSecond.identity}`, + `${dashboardAfterSecond.output}\n${listenerBeforeSecond.output}\n${listenerAfterSecond.output}`, + ).toBe("true:true:true:true"); progress.phase("recreate same sandbox on existing gateway"); const gatewayBeforeRecreate = await gatewayRuntimeId(gateway); @@ -668,39 +680,36 @@ test( expect(portB, `nemoclaw list did not show ${SANDBOX_B} dashboard: ${list.stdout}`).toBeTruthy(); expect(portB).not.toBe(portA); - await sandbox.openshell(["forward", "stop", portB ?? ""], { - artifactName: "phase-4-stop-sandbox-b-dashboard-forward", - env: commandEnv(), - timeoutMs: 30_000, - }); - let probe: ShellProbeResult | undefined; - for (let attempt = 1; attempt <= PROBE_ATTEMPTS; attempt += 1) { - probe = await runProbeOnlyConnect( - host, - SANDBOX_B, - `phase-4-probe-connect-sandbox-b-attempt-${attempt}`, - ); - if (probe.exitCode === 0 && !probe.timedOut) break; - if (attempt < PROBE_ATTEMPTS) await sleep(PROBE_DELAY_MS); - } - expect(probe?.exitCode, probe ? resultText(probe) : "probe did not run").toBe(0); - expect(probe?.timedOut, probe ? resultText(probe) : "probe did not run").toBe(false); - - const restoredForwardB = await waitForForwardOwner( - sandbox, - portB ?? "", - SANDBOX_B, - "phase-4-openshell-forward-list-b", + const dashboardABeforeStop = await waitForDashboardReachability( + host, + portA ?? "", + true, + "phase-4-dashboard-a-before-stop", ); - expect(restoredForwardB.owner, restoredForwardB.output).toBe(SANDBOX_B); - - const retainedForwardA = await waitForForwardOwner( - sandbox, + const listenerABeforeStop = await host.inspectOpenShellForwardListener( portA ?? "", SANDBOX_A, - "phase-4-openshell-forward-list-a", + { artifactName: "phase-4-dashboard-listener-a-before-stop", env: commandEnv() }, ); - expect(retainedForwardA.owner, retainedForwardA.output).toBe(SANDBOX_A); + expect( + `${dashboardABeforeStop.reachable}:${listenerABeforeStop.valid}`, + `${dashboardABeforeStop.output}\n${listenerABeforeStop.output}`, + ).toBe("true:true"); + const dashboardBBeforeStop = await waitForDashboardReachability( + host, + portB ?? "", + true, + "phase-4-dashboard-b-before-stop", + ); + const listenerBBeforeStop = await host.inspectOpenShellForwardListener( + portB ?? "", + SANDBOX_B, + { artifactName: "phase-4-dashboard-listener-b-before-stop", env: commandEnv() }, + ); + expect( + `${dashboardBBeforeStop.reachable}:${listenerBBeforeStop.valid}`, + `${dashboardBBeforeStop.output}\n${listenerBBeforeStop.output}`, + ).toBe("true:true"); progress.phase("stop sibling sandbox without disturbing the first forward"); const stopB = await command(host, [SANDBOX_B, "stop"], { @@ -710,14 +719,21 @@ test( }); expect(stopB.exitCode, resultText(stopB)).toBe(0); - const releasedForwardB = await waitForForwardOwner( - sandbox, + const releasedForwardB = await waitForDashboardReachability( + host, + portB ?? "", + false, + "phase-4-dashboard-b-after-stop", + ); + const listenerBAfterStop = await inspectNoListener( + host, portB ?? "", - undefined, - "phase-4-openshell-forward-list-b-after-stop", + "phase-4-dashboard-listener-b-after-stop", ); - expect(releasedForwardB.querySucceeded, releasedForwardB.output).toBe(true); - expect(releasedForwardB.owner, releasedForwardB.output).toBeUndefined(); + expect( + `${releasedForwardB.reachable}:${listenerBAfterStop.exitCode}:${listenerBAfterStop.timedOut}`, + `${releasedForwardB.output}\n${resultText(listenerBAfterStop)}`, + ).toBe("false:1:false"); const stoppedStatusB = await command(host, [SANDBOX_B, "status"], { artifactName: "phase-4-nemoclaw-status-sandbox-b-after-stop", @@ -729,27 +745,21 @@ test( expect(stoppedStatusTextB).toContain("sandbox_container_stopped"); expect(stoppedStatusTextB).not.toContain("sandbox_dashboard_port_conflict"); - const retainedForwardAAfterStop = await waitForForwardOwner( - sandbox, + const retainedForwardAAfterStop = await waitForDashboardReachability( + host, portA ?? "", - SANDBOX_A, - "phase-4-openshell-forward-list-a-after-b-stop", + true, + "phase-4-dashboard-a-after-b-stop", ); - expect(retainedForwardAAfterStop.owner, retainedForwardAAfterStop.output).toBe(SANDBOX_A); - - const startB = await command(host, [SANDBOX_B, "start"], { - artifactName: "phase-4-nemoclaw-start-sandbox-b", - env: commandEnv(), - timeoutMs: PHASE_TIMEOUT_MS, - }); - expect(startB.exitCode, resultText(startB)).toBe(0); - const restoredForwardBAfterStart = await waitForForwardOwner( - sandbox, - portB ?? "", - SANDBOX_B, - "phase-4-openshell-forward-list-b-after-start", + const listenerAAfterStop = await host.inspectOpenShellForwardListener( + portA ?? "", + SANDBOX_A, + { artifactName: "phase-4-dashboard-listener-a-after-b-stop", env: commandEnv() }, ); - expect(restoredForwardBAfterStart.owner, restoredForwardBAfterStart.output).toBe(SANDBOX_B); + expect( + `${retainedForwardAAfterStop.reachable}:${listenerAAfterStop.valid}:${listenerAAfterStop.identity === listenerABeforeStop.identity}`, + `${retainedForwardAAfterStop.output}\n${listenerABeforeStop.output}\n${listenerAAfterStop.output}`, + ).toBe("true:true:true"); progress.phase("replace sandbox after stale registry refusal"); // Phase 5: direct OpenShell deletion leaves a stale registry entry that @@ -888,19 +898,17 @@ test( gatewayStatusReportedServerEndpoint: Boolean(gatewayServerEndpoint), secondOnboardReusedGateway: gatewayAfterSecond === gatewayBeforeSecond && - secondText.includes("Reusing healthy NemoClaw gateway."), + secondText.includes("Reusing healthy NemoClaw gateway.") && + dashboardAfterSecond.reachable, thirdOnboardPreservedSibling: sandboxAAfterThird.exitCode === 0 && sandboxBAfterThird.exitCode === 0, distinctDashboardPorts: Boolean(portA && portB && portA !== portB), selectedStopReleasedOnlySelectedForward: stopB.exitCode === 0 && - releasedForwardB.querySucceeded && - releasedForwardB.owner === undefined && - retainedForwardAAfterStop.owner === SANDBOX_A && + !releasedForwardB.reachable && + retainedForwardAAfterStop.reachable && stoppedStatusTextB.includes("sandbox_container_stopped") && - !stoppedStatusTextB.includes("sandbox_dashboard_port_conflict") && - startB.exitCode === 0 && - restoredForwardBAfterStart.owner === SANDBOX_B, + !stoppedStatusTextB.includes("sandbox_dashboard_port_conflict"), staleRegistryRecovered: rebuild.exitCode === 0, gatewayStopGuidance: /Recovered NemoClaw gateway runtime|gateway is no longer configured after restart\/rebuild|gateway is still refusing connections after restart|gateway trust material rotated after restart/.test( diff --git a/test/e2e/live/onboard-resume.test.ts b/test/e2e/live/onboard-resume.test.ts index fe94c8b644e..c5ad98f7f10 100644 --- a/test/e2e/live/onboard-resume.test.ts +++ b/test/e2e/live/onboard-resume.test.ts @@ -10,6 +10,7 @@ import { ONBOARD_NO_RECREATE_COMMAND_TIMEOUT_MS, ONBOARD_RESUME_TEST_TIMEOUT_MS, } from "../../../tools/e2e/onboard-timeout-contract.mts"; +import { parseOpenShellSandboxId } from "../../../src/lib/adapters/openshell/sandbox-identity.ts"; import { parseSandboxPhase } from "../../../src/lib/state/gateway.ts"; import { execTimeout, testTimeout } from "../../helpers/timeouts.ts"; import { buildAvailabilityProbeEnv } from "../fixtures/availability-env.ts"; @@ -107,6 +108,14 @@ function markSessionInProgress(file: string): void { fs.writeFileSync(file, JSON.stringify(session, null, 2), "utf8"); } +function registeredDashboardPort(): string { + const registry = JSON.parse(fs.readFileSync(REGISTRY_FILE, "utf8")) as { + sandboxes?: Record; + }; + const port = registry.sandboxes?.[SANDBOX_NAME]?.dashboardPort; + return typeof port === "number" ? String(port) : ""; +} + function interruptedSessionSummary(session: SessionStateInterrupted): Record { return { status: session.status, @@ -192,6 +201,7 @@ test( "resume proves recreated sandbox provider attachments are selectively reconciled", "host trust-store anchor corporate CA source is baked and merged after resume", "an unreachable committed route pauses at final verification and completes after repair", + "non-recreate resume retains the Ready sandbox, dashboard port, and exact forward listener", "implicit resume is detected and --fresh suppresses that auto-resume", ], }); @@ -577,6 +587,23 @@ test( // re-probe and complete without recreating the sandbox. // ────────────────────────────────────────────────────────────────── progress.phase("retry final verification after route repair"); + const sandboxBeforeRouteFailure = await sandbox.openshell( + ["sandbox", "get", SANDBOX_NAME], + { + artifactName: "phase-3-5-sandbox-before-route-failure", + env: probeEnv, + timeoutMs: 30_000, + }, + ); + const sandboxIdBeforeRouteFailure = parseOpenShellSandboxId( + resultText(sandboxBeforeRouteFailure), + ); + const dashboardPortBeforeRouteFailure = registeredDashboardPort(); + const listenerBeforeRouteFailure = await host.inspectOpenShellForwardListener( + dashboardPortBeforeRouteFailure, + SANDBOX_NAME, + { artifactName: "phase-3-5-listener-before-route-failure", env: probeEnv }, + ); markSessionInProgress(SESSION_FILE); await fake.close(); @@ -591,7 +618,15 @@ test( }, ); const unavailableResumeText = `${unavailableResumeRun.stdout}\n${unavailableResumeRun.stderr}`; - expect(unavailableResumeRun.exitCode, unavailableResumeText).not.toBe(0); + const listenerAfterRouteFailure = await host.inspectOpenShellForwardListener( + dashboardPortBeforeRouteFailure, + SANDBOX_NAME, + { artifactName: "phase-3-5-listener-after-route-failure", env: probeEnv }, + ); + expect( + `${unavailableResumeRun.exitCode !== 0}:${listenerBeforeRouteFailure.valid}:${listenerAfterRouteFailure.valid}:${listenerBeforeRouteFailure.identity === listenerAfterRouteFailure.identity}`, + `${unavailableResumeText}\n${listenerBeforeRouteFailure.output}\n${listenerAfterRouteFailure.output}`, + ).toBe("true:true:true:true"); expect(unavailableResumeText).toContain("Compatible endpoint sandbox smoke check failed"); expect(unavailableResumeText).toContain("inference.local"); expect(unavailableResumeText).not.toContain( @@ -632,7 +667,39 @@ test( }, ); const repairedResumeText = `${repairedResumeRun.stdout}\n${repairedResumeRun.stderr}`; - expect(repairedResumeRun.exitCode, repairedResumeText).toBe(0); + const sandboxAfterRouteRepair = await sandbox.openshell(["sandbox", "get", SANDBOX_NAME], { + artifactName: "phase-3-5-sandbox-after-route-repair", + env: probeEnv, + timeoutMs: 30_000, + }); + const dashboardPortAfterRouteRepair = registeredDashboardPort(); + const listenerAfterRouteRepair = await host.inspectOpenShellForwardListener( + dashboardPortAfterRouteRepair, + SANDBOX_NAME, + { artifactName: "phase-3-5-listener-after-route-repair", env: probeEnv }, + ); + const dashboardAfterRouteRepair = await host.command( + "curl", + [ + "--silent", + "--show-error", + "--fail", + "--output", + "/dev/null", + "--max-time", + "5", + `http://127.0.0.1:${dashboardPortAfterRouteRepair}/`, + ], + { + artifactName: "phase-3-5-dashboard-after-route-repair", + env: probeEnv, + timeoutMs: 15_000, + }, + ); + expect( + `${repairedResumeRun.exitCode}:${parseOpenShellSandboxId(resultText(sandboxAfterRouteRepair)) === sandboxIdBeforeRouteFailure}:${dashboardPortAfterRouteRepair === dashboardPortBeforeRouteFailure}:${listenerAfterRouteRepair.valid}:${listenerAfterRouteRepair.identity === listenerBeforeRouteFailure.identity}:${dashboardAfterRouteRepair.exitCode}:${repairedResumeText.includes("cannot be reallocated or adopted")}`, + `${repairedResumeText}\n${resultText(sandboxBeforeRouteFailure)}\n${resultText(sandboxAfterRouteRepair)}\n${listenerBeforeRouteFailure.output}\n${listenerAfterRouteRepair.output}\n${resultText(dashboardAfterRouteRepair)}`, + ).toBe("0:true:true:true:true:0:false"); expect(repairedResumeText).toContain("is ready"); expect(repairedResumeText).not.toContain(`Deleting and recreating sandbox '${SANDBOX_NAME}'`); expect(repairedResumeText).not.toContain(`Sandbox '${SANDBOX_NAME}' created`); diff --git a/test/e2e/mock-parity.json b/test/e2e/mock-parity.json index a007f1d2e9e..b3db99e6ff0 100644 --- a/test/e2e/mock-parity.json +++ b/test/e2e/mock-parity.json @@ -328,6 +328,7 @@ "src/lib/onboard/sandbox-gpu-create-flow.test.ts", "src/lib/onboard/sandbox-readiness-tracing.test.ts", "test/onboarding/onboard-extra-provider-reconciliation.test.ts", + "test/onboarding/onboard-finalization-dashboard-forward.test.ts", "test/e2e/support/corporate-ca-workload-kind.test.ts", "test/runtime/gateway/gateway-state.test.ts", "test/e2e/support/e2e-cleanup-resources.test.ts", @@ -474,7 +475,8 @@ "src/lib/onboard/machine/handlers/sandbox-route-publication.test.ts", "src/lib/onboard/sandbox-registration.test.ts", "test/e2e/support/e2e-cleanup-resources.test.ts", - "test/e2e/support/e2e-clients.test.ts" + "test/e2e/support/e2e-clients.test.ts", + "test/onboarding/onboard-finalization-dashboard-forward.test.ts" ] }, { diff --git a/test/e2e/support/e2e-clients.test.ts b/test/e2e/support/e2e-clients.test.ts index 9bf04d2b2e0..4d500b25fef 100644 --- a/test/e2e/support/e2e-clients.test.ts +++ b/test/e2e/support/e2e-clients.test.ts @@ -199,6 +199,30 @@ describe("E2E fixture clients", () => { expect(host.openshellCommandPath).toBe("openshell"); }); + it.each([ + { actualExecutable: "/opt/openshell", expected: true }, + { actualExecutable: "/usr/bin/python3", expected: false }, + ])( + "host client verifies an exact ForwardTcp listener executable [expected=$expected]", + async ({ actualExecutable, expected }) => { + const runner = new FakeRunner(); + runner.enqueue({ stdout: "4321\n" }); + runner.enqueue({ stdout: "/usr/local/bin/openshell\n" }); + runner.enqueue({ stdout: `${actualExecutable}\n` }); + runner.enqueue({ stdout: "/opt/openshell\n" }); + runner.enqueue({ + stdout: + "/usr/local/bin/openshell --gateway nemoclaw --workspace default forward service alpha --target-port 18789 --target-host 127.0.0.1 --local 127.0.0.1:18789\n", + }); + runner.enqueue({ stdout: "4321\n" }); + const host = new HostCliClient(runner); + + await expect( + host.inspectOpenShellForwardListener("18789", "alpha"), + ).resolves.toMatchObject({ valid: expected }); + }, + ); + it("composes installation, OpenShell resolution, and launch in authority order", async () => { const runner = new FakeRunner(); runner.enqueue({ stdout: "installation complete\n" }); diff --git a/test/helpers/onboard-script-mocks.cjs b/test/helpers/onboard-script-mocks.cjs index e0eb13a68d4..0bfe731f98a 100644 --- a/test/helpers/onboard-script-mocks.cjs +++ b/test/helpers/onboard-script-mocks.cjs @@ -46,6 +46,12 @@ if (process.env.NEMOCLAW_TEST_FORWARD_SERVICE_FIXTURE === "1") { return ready; }; } + if ( + resolved.includes(`${path.sep}adapters${path.sep}openshell${path.sep}forward-service.`) && + typeof loaded?.isForwardServiceListenerOwner === "function" + ) { + loaded.isForwardServiceListenerOwner = () => true; + } return loaded; }; } diff --git a/test/helpers/platform-override-node-options.ts b/test/helpers/platform-override-node-options.ts index 7a3caa83997..374c048fd9a 100644 --- a/test/helpers/platform-override-node-options.ts +++ b/test/helpers/platform-override-node-options.ts @@ -4,17 +4,26 @@ import fs from "node:fs"; import path from "node:path"; -export function nonWslPlatformNodeOptions( +export function syntheticForwardNodeOptions( directory: string, inheritedNodeOptions = process.env.NODE_OPTIONS, ): string { - const preload = path.join(directory, "force-non-wsl-platform.cjs"); + const preload = path.join(directory, "synthetic-forward-platform.cjs"); fs.writeFileSync( preload, [ "delete process.env.WSL_DISTRO_NAME;", "delete process.env.WSL_INTEROP;", 'require("node:os").release = () => "6.8.0-linux";', + 'const Module = require("node:module");', + "const originalLoad = Module._load;", + "Module._load = function loadSyntheticForward(request, parent, isMain) {", + " const loaded = originalLoad.call(this, request, parent, isMain);", + ' if (String(request).endsWith("/adapters/openshell/forward-service")) {', + " loaded.isForwardServiceListenerOwner = () => true;", + " }", + " return loaded;", + "};", "", ].join("\n"), { mode: 0o600 }, diff --git a/test/onboarding/onboard-finalization-dashboard-forward.test.ts b/test/onboarding/onboard-finalization-dashboard-forward.test.ts index 6a8eefd0f5d..6530b47867e 100644 --- a/test/onboarding/onboard-finalization-dashboard-forward.test.ts +++ b/test/onboarding/onboard-finalization-dashboard-forward.test.ts @@ -3,14 +3,17 @@ import { describe, expect, it, vi } from "vitest"; +import type { ForwardServiceTarget } from "../../src/lib/adapters/openshell/forward-service"; import { createOnboardDashboardHelpers } from "../../src/lib/onboard/dashboard"; import type { ListSandboxesFn } from "../../src/lib/onboard/dashboard-port"; function harness(options: { listSandboxes: ListSandboxesFn; isPortBound?: (port: number) => boolean; + ownsForward?: (target: ForwardServiceTarget) => boolean; }) { const launch = vi.fn(); + const owns = vi.fn(options.ownsForward ?? (() => false)); const helpers = createOnboardDashboardHelpers({ runOpenshell: vi.fn(() => ({ status: 0 })), runCaptureOpenshell: vi.fn(() => ""), @@ -28,11 +31,12 @@ function harness(options: { forwardService: { executable: () => "/usr/local/bin/openshell", launch, + owns, resolveGatewayName: () => "nemoclaw", retireLegacy: vi.fn(() => 0), }, }); - return { helpers, launch }; + return { helpers, launch, owns }; } describe("finalization dashboard ForwardTcp launch", () => { @@ -56,7 +60,7 @@ describe("finalization dashboard ForwardTcp launch", () => { expect(process.env.CHAT_UI_URL).toBe("http://127.0.0.1:18790"); }); - it("fails closed when a foreign listener occupies the persisted port", () => { + it("fails closed when a foreign or ambiguous listener occupies the persisted port", () => { vi.stubEnv("CHAT_UI_URL", undefined); const { helpers, launch } = harness({ listSandboxes: () => ({ @@ -66,11 +70,92 @@ describe("finalization dashboard ForwardTcp launch", () => { }); expect(() => helpers.ensureFinalizationDashboardForward("reonboard-test")).toThrow( - /cannot be reallocated/u, + /cannot be reallocated or adopted/u, ); expect(launch).not.toHaveBeenCalled(); }); + it("reuses an exactly owned dashboard forward (#11074)", () => { + vi.stubEnv("CHAT_UI_URL", undefined); + const { helpers, launch, owns } = harness({ + listSandboxes: () => ({ + sandboxes: [{ name: "reonboard-test", dashboardPort: 18_790 }], + }), + isPortBound: (port) => port === 18_790, + ownsForward: () => true, + }); + + expect(helpers.ensureFinalizationDashboardForward("reonboard-test")).toBe(18_790); + expect(owns).toHaveBeenCalledOnce(); + expect(owns).toHaveBeenCalledWith({ + executable: "/usr/local/bin/openshell", + gatewayName: "nemoclaw", + workspace: "default", + sandboxName: "reonboard-test", + localHost: "127.0.0.1", + localPort: 18_790, + targetHost: "127.0.0.1", + targetPort: 18_790, + }); + expect(launch).not.toHaveBeenCalled(); + expect(process.env.CHAT_UI_URL).toBe("http://127.0.0.1:18790"); + }); + + it("does not reuse a forward when another sandbox registers the same port", () => { + vi.stubEnv("CHAT_UI_URL", undefined); + const { helpers, launch } = harness({ + listSandboxes: () => ({ + sandboxes: [ + { name: "reonboard-test", dashboardPort: 18_790 }, + { name: "other", dashboardPort: 18_790 }, + ], + }), + isPortBound: (port) => port === 18_790, + }); + + expect(() => helpers.ensureFinalizationDashboardForward("reonboard-test")).toThrow( + /cannot be reallocated or adopted/u, + ); + expect(launch).not.toHaveBeenCalled(); + }); + + it("enables owned-forward reuse only for OpenClaw agents", async () => { + vi.stubEnv("CHAT_UI_URL", undefined); + const openClaw = harness({ + listSandboxes: () => ({ + sandboxes: [{ name: "reonboard-test", dashboardPort: 18_790 }], + }), + isPortBound: (port) => port === 18_790, + ownsForward: () => true, + }); + + await expect( + openClaw.helpers.ensureFinalizationAgentDashboardForward( + "reonboard-test", + { name: "openclaw", forwardPort: 18_790 }, + undefined, + undefined, + ), + ).resolves.toBe(18_790); + + const hermes = harness({ + listSandboxes: () => ({ + sandboxes: [{ name: "reonboard-test", dashboardPort: 18_790 }], + }), + isPortBound: (port) => port === 18_790, + ownsForward: () => true, + }); + + await expect( + hermes.helpers.ensureFinalizationAgentDashboardForward( + "reonboard-test", + { name: "hermes", forwardPort: 18_790 }, + undefined, + undefined, + ), + ).rejects.toThrow(/cannot be reallocated/u); + }); + it("honors an explicit dashboard URL", () => { vi.stubEnv("CHAT_UI_URL", "http://127.0.0.1:19001"); const { helpers, launch } = harness({ diff --git a/test/onboarding/onboard-fresh-create-identity.test.ts b/test/onboarding/onboard-fresh-create-identity.test.ts index f9f3ab84f19..d9d7ef4bb2e 100644 --- a/test/onboarding/onboard-fresh-create-identity.test.ts +++ b/test/onboarding/onboard-fresh-create-identity.test.ts @@ -686,7 +686,7 @@ if (${JSON.stringify( console.error(error); process.exit(1); }); -`; +`.replaceAll("18080", String(gatewayPort)); fs.writeFileSync(scriptPath, script); const childEnv = { diff --git a/test/process-recovery/process-recovery-custom-agent.test.ts b/test/process-recovery/process-recovery-custom-agent.test.ts index 763471e4fa2..4dd675f8103 100644 --- a/test/process-recovery/process-recovery-custom-agent.test.ts +++ b/test/process-recovery/process-recovery-custom-agent.test.ts @@ -12,6 +12,9 @@ const requireSource = createRequire(import.meta.url); const { checkAndRecoverSandboxProcesses: checkAndRecoverSandboxProcessesImpl } = requireSource( "../../src/lib/actions/sandbox/process-recovery.ts", ) as typeof import("../../src/lib/actions/sandbox/process-recovery.js"); +const forwardService = requireSource( + "../../src/lib/adapters/openshell/forward-service.ts", +) as typeof import("../../src/lib/adapters/openshell/forward-service.js"); function checkAndRecoverSandboxProcesses( sandboxName: string, @@ -144,6 +147,7 @@ describe("checkAndRecoverSandboxProcesses custom agent recovery", () => { dashboardPort: 19000, }); vi.spyOn(forwardHealth, "isLocalForwardReachable").mockReturnValue(true); + vi.spyOn(forwardService, "isForwardServiceListenerOwner").mockReturnValue(true); vi.spyOn(openshellRuntime, "captureOpenshell").mockReturnValue({ status: 0, output: "SANDBOX BIND PORT PID STATUS", @@ -217,6 +221,7 @@ describe("checkAndRecoverSandboxProcesses custom agent recovery", () => { dashboardPort: 19000, }); vi.spyOn(forwardHealth, "isLocalForwardReachable").mockReturnValue(true); + vi.spyOn(forwardService, "isForwardServiceListenerOwner").mockReturnValue(true); vi.spyOn(openshellRuntime, "captureOpenshell").mockReturnValue({ status: 0, output: runningForward, diff --git a/test/process-recovery/process-recovery-managed-controller.test.ts b/test/process-recovery/process-recovery-managed-controller.test.ts index 67c4df954cc..ef4d2cb2215 100644 --- a/test/process-recovery/process-recovery-managed-controller.test.ts +++ b/test/process-recovery/process-recovery-managed-controller.test.ts @@ -13,6 +13,9 @@ const requireSource = createRequire(import.meta.url); const { checkAndRecoverSandboxProcesses: checkAndRecoverSandboxProcessesImpl } = requireSource( "../../src/lib/actions/sandbox/process-recovery.ts", ) as typeof import("../../src/lib/actions/sandbox/process-recovery.js"); +const forwardService = requireSource( + "../../src/lib/adapters/openshell/forward-service.ts", +) as typeof import("../../src/lib/adapters/openshell/forward-service.js"); function checkAndRecoverSandboxProcesses( sandboxName: string, @@ -411,6 +414,7 @@ describe("managed gateway recovery controller", () => { ); vi.spyOn(agentRuntime, "getSessionAgent").mockReturnValue(null); vi.spyOn(forwardHealth, "isLocalForwardReachable").mockReturnValue(true); + vi.spyOn(forwardService, "isForwardServiceListenerOwner").mockReturnValue(true); vi.spyOn(registry, "getSandbox").mockReturnValue({ name: "beta", agent: "openclaw", diff --git a/test/process-recovery/process-recovery.test.ts b/test/process-recovery/process-recovery.test.ts index cad24dd2916..ff644030e76 100644 --- a/test/process-recovery/process-recovery.test.ts +++ b/test/process-recovery/process-recovery.test.ts @@ -19,6 +19,9 @@ const { ensureSandboxPortForwardForPort } = requireSource( const { createProbeTimingRecorder } = requireSource( "../../src/lib/actions/sandbox/probe/timing.ts", ) as typeof import("../../src/lib/actions/sandbox/probe/timing.js"); +const forwardService = requireSource( + "../../src/lib/adapters/openshell/forward-service.ts", +) as typeof import("../../src/lib/adapters/openshell/forward-service.js"); function checkAndRecoverSandboxProcesses( sandboxName: string, @@ -172,6 +175,7 @@ hermes-box 127.0.0.1 18789 12345 running`; dashboardPort: 18789, }); vi.spyOn(forwardHealth, "isLocalForwardReachable").mockReturnValue(true); + vi.spyOn(forwardService, "isForwardServiceListenerOwner").mockReturnValue(true); vi.spyOn(openshellRuntime, "captureOpenshell").mockReturnValue({ status: 0, output: runningForward, @@ -334,6 +338,7 @@ hermes-box 127.0.0.1 18789 12345 running`; hermesDashboardInternalPort: 19119, }); vi.spyOn(forwardHealth, "isLocalForwardReachable").mockReturnValue(true); + vi.spyOn(forwardService, "isForwardServiceListenerOwner").mockReturnValue(true); vi.spyOn(openshellRuntime, "captureOpenshell").mockReturnValue({ status: 0, output: "SANDBOX BIND PORT PID STATUS", @@ -563,6 +568,7 @@ hermes-box 127.0.0.1 18789 12345 running`; dashboardPort: 18789, }); vi.spyOn(forwardHealth, "isLocalForwardReachable").mockReturnValue(true); + vi.spyOn(forwardService, "isForwardServiceListenerOwner").mockReturnValue(true); vi.spyOn(openshellRuntime, "captureOpenshell").mockReturnValue({ status: 0, output: "SANDBOX BIND PORT PID STATUS", @@ -622,6 +628,7 @@ hermes-box 127.0.0.1 18789 12345 running`; dashboardPort: 18789, }); vi.spyOn(forwardHealth, "isLocalForwardReachable").mockReturnValue(true); + vi.spyOn(forwardService, "isForwardServiceListenerOwner").mockReturnValue(true); vi.spyOn(openshellRuntime, "captureOpenshell").mockReturnValue({ status: 0, output: `SANDBOX BIND PORT PID STATUS\nhermes-box 127.0.0.1 18789 12345 running`, @@ -671,6 +678,7 @@ hermes-box 127.0.0.1 18789 12345 running`; dashboardPort: 18789, }); vi.spyOn(forwardHealth, "isLocalForwardReachable").mockReturnValue(true); + vi.spyOn(forwardService, "isForwardServiceListenerOwner").mockReturnValue(true); vi.spyOn(openshellRuntime, "captureOpenshell").mockReturnValue({ status: 0, output: `SANDBOX BIND PORT PID STATUS\nbeta 127.0.0.1 18789 12345 running`, diff --git a/test/runtime/gateway/recover-port-forward.test.ts b/test/runtime/gateway/recover-port-forward.test.ts index 27848ae8343..c026c2886ad 100644 --- a/test/runtime/gateway/recover-port-forward.test.ts +++ b/test/runtime/gateway/recover-port-forward.test.ts @@ -11,7 +11,7 @@ import { LAUNCH_READINESS_FIXTURE_POLICY, launchReadinessRegistryFixture, } from "../../helpers/launch-readiness-fixture"; -import { nonWslPlatformNodeOptions } from "../../helpers/platform-override-node-options"; +import { syntheticForwardNodeOptions } from "../../helpers/platform-override-node-options"; import { execTimeout, testTimeoutOptions } from "../../helpers/timeouts"; const tmpFixtures: string[] = []; @@ -332,7 +332,7 @@ function runRecover(fixture: Fixture) { env: { ...process.env, HOME: fixture.tmpDir, - NODE_OPTIONS: nonWslPlatformNodeOptions(fixture.tmpDir), + NODE_OPTIONS: syntheticForwardNodeOptions(fixture.tmpDir), PATH: "/usr/bin:/bin", NEMOCLAW_NO_CONNECT_HINT: "1", NEMOCLAW_FORWARD_RECOVERY_WAIT_MS: fixture.recoveryWaitMs, diff --git a/test/sandbox-connect-inference/helpers.ts b/test/sandbox-connect-inference/helpers.ts index e2fa8ee1e64..42f9c31c696 100644 --- a/test/sandbox-connect-inference/helpers.ts +++ b/test/sandbox-connect-inference/helpers.ts @@ -11,7 +11,7 @@ import { LAUNCH_READINESS_PAIRING_QUALIFICATION_OUTPUT, launchReadinessRegistryFixture, } from "../helpers/launch-readiness-fixture"; -import { nonWslPlatformNodeOptions } from "../helpers/platform-override-node-options"; +import { syntheticForwardNodeOptions } from "../helpers/platform-override-node-options"; import { execTimeout } from "../helpers/timeouts"; /** @@ -587,7 +587,7 @@ export function runConnect( encoding: "utf-8", env: { HOME: tmpDir, - NODE_OPTIONS: nonWslPlatformNodeOptions(tmpDir, ""), + NODE_OPTIONS: syntheticForwardNodeOptions(tmpDir, ""), PATH: `${path.join(tmpDir, ".local", "bin")}:/usr/bin:/bin`, NEMOCLAW_DISABLE_GATEWAY_DRIFT_PREFLIGHT: "1", NEMOCLAW_NO_CONNECT_HINT: "1", diff --git a/test/state/snapshot-gateway-guard.test.ts b/test/state/snapshot-gateway-guard.test.ts index 39debb670e2..8c64c0cb4e5 100644 --- a/test/state/snapshot-gateway-guard.test.ts +++ b/test/state/snapshot-gateway-guard.test.ts @@ -12,6 +12,7 @@ import os from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; +import { syntheticForwardNodeOptions } from "../helpers/platform-override-node-options"; import { execTimeout } from "../helpers/timeouts"; const CLI = path.join(import.meta.dirname, "../..", "bin", "nemoclaw.js"); @@ -334,6 +335,7 @@ function makeVmRestoreToEnv( return { HOME: home, + NODE_OPTIONS: syntheticForwardNodeOptions(home), NEMOCLAW_OPENSHELL_BIN: path.join(localBin, "openshell"), NEMOCLAW_GATEWAY_RECOVERY_SETTLE_SECONDS: "0", NEMOCLAW_TEST_SNAPSHOT_RESTORE_MARKER: snapshotRestoreMarker,