diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index f18e0554d5f..e61aac7045b 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -18,6 +18,9 @@ const { }: typeof import("./onboard/branding") = require("./onboard/branding"); const { cleanupTempDir }: typeof import("./onboard/temp-files") = require("./onboard/temp-files"); const { stopStaleDashboardListenersForSandbox } = require("./onboard/stale-gateway-cleanup"); +const { + runBackgroundForwardStartWithDiagnostics, +}: typeof import("./onboard/forward-start") = require("./onboard/forward-start"); const { ensureOllamaLoopbackSystemdOverride, }: typeof import("./onboard/ollama-systemd") = require("./onboard/ollama-systemd"); @@ -53,9 +56,6 @@ const { const { verifyWebSearchInsideSandbox: verifyWebSearchInsideSandboxWithDeps, }: typeof import("./onboard/web-search-verify") = require("./onboard/web-search-verify"); -const { - verifyWebSearchInsideSandbox: verifyWebSearchInsideSandboxWithDeps, -}: typeof import("./onboard/web-search-verify") = require("./onboard/web-search-verify"); const { buildDirectGpuPolicyYaml, buildDirectSandboxGpuProofCommands, @@ -260,6 +260,11 @@ const policies: typeof import("./policy") = require("./policy"); const shields = require("./shields"); const tiers: typeof import("./policy/tiers") = require("./policy/tiers"); const { ensureUsageNoticeConsent } = require("./onboard/usage-notice"); +const { + findAvailableDashboardPort, + getOccupiedPorts, + isLiveForwardStatus, +} = require("./onboard/dashboard-port") as typeof import("./onboard/dashboard-port"); const { destroyGatewayForReuse, warnIfGatewayDestroyFails, @@ -9402,10 +9407,6 @@ function findForwardEntry( return null; } -function isLiveForwardStatus(status: string): boolean { - return status === "running" || status === "active"; -} - function getRunningForwardPorts(forwardListOutput: string | null | undefined): string[] { const ports = new Set(); if (!forwardListOutput) return []; @@ -9429,85 +9430,6 @@ function stopAllDashboardForwards(): void { } } -/** - * Parse `openshell forward list` output into a Map. - * Only includes running forwards — stopped/stale entries are ignored so - * they don't block port allocation or cause false "range exhausted" errors. - * - * Output format (columns separated by whitespace): - * SANDBOX BIND PORT PID STATUS - */ -function getOccupiedPorts(forwardListOutput: string | null): Map { - const occupied = new Map(); - if (!forwardListOutput) return occupied; - for (const rawLine of forwardListOutput.split("\n")) { - const line = rawLine.replace(ANSI_RE, ""); - if (/^\s*SANDBOX\s/i.test(line)) continue; - const parts = line.trim().split(/\s+/); - // parts: [sandbox, bind, port, pid, status...] - if (parts.length < 3 || !/^\d+$/.test(parts[2])) continue; - const status = (parts[4] || "").toLowerCase(); - if (!isLiveForwardStatus(status)) continue; - occupied.set(parts[2], parts[0]); - } - return occupied; -} - -/** - * Quick synchronous check whether a TCP port has an active listener on the host. - * Uses lsof when available; returns false (optimistic) if lsof is missing. - */ -function isPortBoundOnHost(port: number): boolean { - try { - const out = runCapture(["lsof", "-i", `:${port}`, "-sTCP:LISTEN", "-P", "-n"], { - ignoreError: true, - }); - return !!out && out.trim().length > 0; - } catch { - return false; - } -} - -/** - * Find the next available dashboard port for the given sandbox. - * Returns the preferred port if free or already owned by this sandbox, - * otherwise scans DASHBOARD_PORT_RANGE_START..END for a free port. - * Validates host-port availability (via lsof) so ports bound by - * non-OpenShell processes are skipped. - * Throws if the entire range is exhausted. - */ -function findAvailableDashboardPort( - sandboxName: string, - preferredPort: number, - forwardListOutput: string | null, -): number { - const occupied = getOccupiedPorts(forwardListOutput); - const preferredStr = String(preferredPort); - const owner = occupied.get(preferredStr) ?? null; - // If this sandbox already owns the forward, keep it. - if (owner === sandboxName) return preferredPort; - // If no forward claims the port, also check the host so we don't collide - // with non-OpenShell processes. - if (owner === null && !isPortBoundOnHost(preferredPort)) return preferredPort; - - for (let p = DASHBOARD_PORT_RANGE_START; p <= DASHBOARD_PORT_RANGE_END; p++) { - const pStr = String(p); - const pOwner = occupied.get(pStr) ?? null; - if (pOwner === sandboxName) return p; - if (pOwner === null && !isPortBoundOnHost(p)) return p; - } - - const owners = [...occupied.entries()] - .filter( - ([p]) => Number(p) >= DASHBOARD_PORT_RANGE_START && Number(p) <= DASHBOARD_PORT_RANGE_END, - ) - .map(([p, s]) => ` ${p} → ${s}`) - .join("\n"); - throw new Error( - `All dashboard ports in range ${DASHBOARD_PORT_RANGE_START}-${DASHBOARD_PORT_RANGE_END} are occupied:\n${owners}\n` + - `Free a sandbox or use --control-ui-port with a port outside this range.`, - ); -} /** * Build the actionable error lines printed when the just-created openshell @@ -9579,6 +9501,34 @@ function ensureDashboardForward( } if (actualPort !== preferredPort) { + if (rollbackSandboxOnFailure) { + // Create path: the sandbox was just built with CHAT_UI_URL and + // NEMOCLAW_DASHBOARD_PORT baked from `preferredPort` (see the + // `formatEnvAssignment("CHAT_UI_URL", …)` call in createSandbox). If + // the port was bound during the build window (TOCTOU), picking a new + // host port would leave the sandbox serving the dashboard on + // `preferredPort` internally while the forward listens on `actualPort` + // — reproducing the original "onboard exits but dashboard is + // unreachable" failure on the newly selected port. Reallocation is + // only safe on reuse paths where the sandbox image is fixed; on the + // create path we must roll back so the next onboard re-bakes with a + // clean port. (#3260) + const err = new Error( + `Dashboard port ${preferredPort} became host-bound during sandbox build; ` + + `cannot reallocate to ${actualPort} after the sandbox has been created with ` + + `CHAT_UI_URL=${preferredPort}. Free the port and re-run \`${cliName()} onboard\`, ` + + `or pass \`--control-ui-port \` to pick a different dashboard port.`, + ); + const delResult = runOpenshell(["sandbox", "delete", sandboxName], { ignoreError: true }); + for (const line of buildOrphanedSandboxRollbackMessage( + sandboxName, + err, + delResult.status === 0, + )) { + console.error(line); + } + process.exit(1); + } console.warn(` ! Port ${preferredPort} is taken. Using port ${actualPort} instead.`); } @@ -9596,18 +9546,58 @@ function ensureDashboardForward( parsedUrl.port = String(actualPort); const actualTarget = getDashboardForwardTarget(parsedUrl.toString()); runOpenshell(["forward", "stop", String(actualPort)], { ignoreError: true }); - const fwdResult = runOpenshell(["forward", "start", "--background", actualTarget, sandboxName], { - ignoreError: true, - stdio: ["ignore", "ignore", "ignore"], - }); - if (fwdResult && fwdResult.status !== 0) { - console.warn( - `! Port ${actualPort} forward did not start — port may be in use by another process.`, - ); - console.warn( - ` Check: docker ps --format 'table {{.Names}}\\t{{.Ports}}' | grep ${actualPort}`, + const { result: fwdResult, diagnostic: fwdDiagnostic } = + runBackgroundForwardStartWithDiagnostics((stdio, timeout) => + runOpenshell( + ["forward", "start", "--background", actualTarget, sandboxName], + { ignoreError: true, suppressOutput: true, stdio, timeout }, + ), ); - console.warn(` Free the port, then reconnect: ${cliName()} ${sandboxName} connect`); + if (fwdResult && fwdResult.status !== 0) { + const looksLikePortConflict = + fwdDiagnostic === "" || + /eaddrinuse|address already in use|port .* in use|bind: .*in use/i.test(fwdDiagnostic); + if (rollbackSandboxOnFailure) { + // The sandbox was just created, committed to actualPort via its + // baked-in CHAT_UI_URL and NEMOCLAW_DASHBOARD_PORT env. Silently + // returning here leaves the user with a dashboard URL that points + // at a port held by another process — a TOCTOU race where the + // proactive probe in findAvailableDashboardPort missed the + // conflict (e.g., another listener bound during the multi-minute + // image build). Roll back so the next `onboard` retry's allocator + // observes the bound port and picks a different one. Only the + // EADDRINUSE-style failure gets the port-conflict wording; other + // errors (gateway / transport) propagate the real diagnostic so + // users aren't pointed at the wrong fix (#3260). + const err = new Error( + looksLikePortConflict + ? `Failed to start dashboard forward on port ${actualPort} — the host port ` + + `is held by another process. Free it and run \`${cliName()} onboard\` again, ` + + `or pass \`--control-ui-port \` to pick a different dashboard port.` + : `Failed to start dashboard forward on port ${actualPort}: ${fwdDiagnostic.slice(0, 240)}`, + ); + const delResult = runOpenshell(["sandbox", "delete", sandboxName], { ignoreError: true }); + for (const line of buildOrphanedSandboxRollbackMessage( + sandboxName, + err, + delResult.status === 0, + )) { + console.error(line); + } + process.exit(1); + } + if (looksLikePortConflict) { + console.warn( + `! Port ${actualPort} forward did not start — port may be in use by another process.`, + ); + console.warn( + ` Check: docker ps --format 'table {{.Names}}\\t{{.Ports}}' | grep ${actualPort}`, + ); + console.warn(` Free the port, then reconnect: ${cliName()} ${sandboxName} connect`); + } else { + console.warn(`! Port ${actualPort} forward did not start: ${fwdDiagnostic.slice(0, 240)}`); + console.warn(` Reconnect after resolving the issue: ${cliName()} ${sandboxName} connect`); + } } return actualPort; } @@ -9722,26 +9712,6 @@ function getWslHostAddress( return dashboardAccess.getWslHostAddress({ ...options, runCapture: options.runCapture || runCapture }); } -function getDashboardAccessInfo( - sandboxName: string, - options: Parameters[1] = {}, -) { - return dashboardAccess.getDashboardAccessInfo(sandboxName, { - ...options, - runCapture: options.runCapture || runCapture, - fetchGatewayAuthToken: fetchGatewayAuthTokenFromSandbox, - }); -} - -function getDashboardGuidanceLines( - access: Parameters[0] = [], - options: Parameters[1] = {}, -): string[] { - return dashboardAccess.getDashboardGuidanceLines(access, { - ...options, - runCapture: options.runCapture || runCapture, - }); -} /** Print the post-onboard dashboard with sandbox status and reconfiguration hints. */ function printDashboard( sandboxName: string, @@ -11072,6 +11042,7 @@ module.exports = { buildControlUiUrls, startGateway, + findAvailableDashboardPort, findDashboardForwardOwner, startGatewayForRecovery, openshellArgv, diff --git a/src/lib/onboard/dashboard-port.ts b/src/lib/onboard/dashboard-port.ts new file mode 100644 index 00000000000..7015f2023c5 --- /dev/null +++ b/src/lib/onboard/dashboard-port.ts @@ -0,0 +1,184 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +/** + * Dashboard-port allocation for the OpenShell sandbox. + * + * The allocator runs at sandbox-create time and bakes the chosen port into + * the sandbox's Dockerfile ARG + NEMOCLAW_DASHBOARD_PORT env. If the port + * later turns out to be unavailable, the sandbox has to be torn down and + * re-created, so the allocator needs to be aggressive about detecting + * already-bound host ports — `lsof` alone misses root-owned listeners on + * macOS (docker-proxy) and TOCTOU windows where another listener binds + * mid-build. See #3260 and #2174. + */ + +import { spawnSync } from "node:child_process"; + +import { DASHBOARD_PORT_RANGE_END, DASHBOARD_PORT_RANGE_START } from "../core/ports"; + +// runner.ts is still CommonJS — use require so module shape matches. +const { runCapture } = require("../runner"); +type RunCaptureFn = typeof import("../runner").runCapture; + +// 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; + +/** OpenShell forward statuses that hold a port (and therefore block reuse). */ +export function isLiveForwardStatus(status: string): boolean { + return status === "running" || status === "active"; +} + +/** + * Parse `openshell forward list` output into a Map. + * Only includes running forwards — stopped/stale entries are ignored so + * they don't block port allocation or cause false "range exhausted" errors. + * + * ANSI escape codes (the openshell CLI colourises status columns when + * stdout is a TTY) are stripped per-line before tokenising so port numbers + * and status words are matched cleanly. + * + * Output format (columns separated by whitespace): + * SANDBOX BIND PORT PID STATUS + */ +export function getOccupiedPorts(forwardListOutput: string | null): Map { + const occupied = new Map(); + if (!forwardListOutput) return occupied; + for (const rawLine of forwardListOutput.split("\n")) { + const line = rawLine.replace(ANSI_RE, ""); + if (/^\s*SANDBOX\s/i.test(line)) continue; + const parts = line.trim().split(/\s+/); + // parts: [sandbox, bind, port, pid, status...] + if (parts.length < 3 || !/^\d+$/.test(parts[2])) continue; + const status = (parts[4] || "").toLowerCase(); + if (!isLiveForwardStatus(status)) continue; + occupied.set(parts[2], parts[0]); + } + return occupied; +} + +/** + * Synchronous Node `net` bind probe — tries to listen on the port and + * reports whether the bind would have failed with EADDRINUSE. Spawned via + * spawnSync of `node -e` because `findAvailableDashboardPort` runs deep in + * a sync allocation flow and `net.createServer().listen()` is async. + * + * Exit codes: 0 = bind succeeded (port free); 1 = EADDRINUSE; anything + * else = inconclusive (treated as free for safety — the forward-start + * check is authoritative). + */ +export function probePortBoundSync(port: number): boolean { + try { + const script = + "const net = require('node:net');" + + "const srv = net.createServer();" + + "let done = false;" + + "const exit = (code) => { if (!done) { done = true; process.exit(code); } };" + + "srv.once('error', (e) => exit(e && e.code === 'EADDRINUSE' ? 1 : 2));" + + `srv.listen(${port}, '127.0.0.1', () => srv.close(() => exit(0)));`; + const result = spawnSync(process.execPath, ["-e", script], { + stdio: "ignore", + timeout: 2_000, + }); + return result.status === 1; + } catch { + return false; + } +} + +/** + * Synchronous check whether a TCP port has an active listener on the host. + * + * Detection chain — any positive signal short-circuits: + * 1. `lsof` — finds listeners owned by the current user. + * 2. `sudo -n lsof` — catches root-owned listeners (e.g., docker-proxy on + * macOS) that the unprivileged lsof can't see. Silently no-ops when + * the user can't escalate non-interactively. + * 3. Node `net` bind probe — authoritative fallback when both lsof + * invocations come up empty, mirroring what `openshell forward start` + * will actually attempt. + * + * Returns false (optimistic) when every probe is inconclusive — the + * downstream forward-start check is the final authority (#3260). + */ +export function isPortBoundOnHost(port: number): boolean { + try { + const out: ReturnType = runCapture( + ["lsof", "-i", `:${port}`, "-sTCP:LISTEN", "-P", "-n"], + { ignoreError: true }, + ); + if (out && out.trim().length > 0) return true; + } catch { + /* fall through to the next probe */ + } + + try { + const sudoOut: ReturnType = runCapture( + ["sudo", "-n", "lsof", "-i", `:${port}`, "-sTCP:LISTEN", "-P", "-n"], + { ignoreError: true }, + ); + if (sudoOut && sudoOut.trim().length > 0) return true; + } catch { + /* fall through to the bind probe */ + } + + return probePortBoundSync(port); +} + +/** + * Find the next available dashboard port for the given sandbox. + * Returns the preferred port if free or already owned by this sandbox, + * otherwise scans DASHBOARD_PORT_RANGE_START..END for a free port. + * Validates host-port availability (via the proactive probe chain in + * isPortBoundOnHost) so ports bound by non-OpenShell processes are + * skipped (#3260). + * Throws if the entire range is exhausted. + * + * `isPortBoundCheck` is an injectable seam for tests so they don't have + * to spawn real lsof / Node probes; production callers leave it at the + * default. + */ +export function findAvailableDashboardPort( + sandboxName: string, + preferredPort: number, + forwardListOutput: string | null, + isPortBoundCheck: (port: number) => boolean = isPortBoundOnHost, +): number { + const occupied = getOccupiedPorts(forwardListOutput); + const hostBoundPorts: number[] = []; + // Try the preferred port first (it may be outside the dashboard range when + // a caller passes --control-ui-port), then the rest of the range. Each port + // is probed at most once so we don't pay for `lsof` + `sudo lsof` + Node + // bind multiple times per port. + const portsToScan = [ + preferredPort, + ...Array.from( + { length: DASHBOARD_PORT_RANGE_END - DASHBOARD_PORT_RANGE_START + 1 }, + (_, i) => DASHBOARD_PORT_RANGE_START + i, + ).filter((p) => p !== preferredPort), + ]; + for (const p of portsToScan) { + const pStr = String(p); + const pOwner = occupied.get(pStr) ?? null; + if (pOwner === sandboxName) return p; + if (pOwner === null) { + if (!isPortBoundCheck(p)) return p; + hostBoundPorts.push(p); + } + } + + const ownerLines = [...occupied.entries()] + .filter( + ([p]) => Number(p) >= DASHBOARD_PORT_RANGE_START && Number(p) <= DASHBOARD_PORT_RANGE_END, + ) + .map(([p, s]) => ` ${p} → ${s}`); + const hostLines = hostBoundPorts + .filter((p) => p >= DASHBOARD_PORT_RANGE_START && p <= DASHBOARD_PORT_RANGE_END) + .map((p) => ` ${p} → non-OpenShell host listener`); + const lines = [...ownerLines, ...hostLines].join("\n"); + throw new Error( + `All dashboard ports in range ${DASHBOARD_PORT_RANGE_START}-${DASHBOARD_PORT_RANGE_END} are occupied:\n${lines}\n` + + `Free a sandbox or use --control-ui-port with a port outside this range.`, + ); +} diff --git a/src/lib/onboard/forward-start.ts b/src/lib/onboard/forward-start.ts new file mode 100644 index 00000000000..2146203b6c0 --- /dev/null +++ b/src/lib/onboard/forward-start.ts @@ -0,0 +1,84 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import fs from "node:fs"; +import path from "node:path"; +import type { StdioOptions } from "node:child_process"; + +import { compactText } from "../core/url-utils"; +import { redact } from "../security/redact"; +import { cleanupTempDir, secureTempFile } from "./temp-files"; + +export type BackgroundForwardStartResult = { + status: number | null; + stdout?: string | Buffer | null; + stderr?: string | Buffer | null; + error?: Error; +}; + +export type BackgroundForwardStartRunner = ( + stdio: StdioOptions, + timeoutMs: number, +) => BackgroundForwardStartResult; + +function readDiagnosticFile(filePath: string): string { + try { + return fs.readFileSync(filePath, "utf-8"); + } catch (error) { + if ( + typeof error === "object" && + error !== null && + "code" in error && + error.code === "ENOENT" + ) { + return ""; + } + throw error; + } +} + +export function runBackgroundForwardStartWithDiagnostics( + runForwardStart: BackgroundForwardStartRunner, + timeoutMs = 30_000, +): { result: BackgroundForwardStartResult; diagnostic: string } { + const forwardDiagPath = secureTempFile("nemoclaw-forward-start", ".out"); + const forwardDiagDir = path.dirname(forwardDiagPath); + const forwardErrPath = path.join(forwardDiagDir, "nemoclaw-forward-start.err"); + let result: BackgroundForwardStartResult | null = null; + const outFd = fs.openSync(forwardDiagPath, "w", 0o600); + const errFd = fs.openSync(forwardErrPath, "w", 0o600); + + try { + try { + result = runForwardStart(["ignore", outFd, errFd], timeoutMs); + } catch (error) { + result = { + status: null, + error: error instanceof Error ? error : new Error(String(error)), + }; + } + } finally { + try { + fs.closeSync(outFd); + } catch { + /* best effort */ + } + try { + fs.closeSync(errFd); + } catch { + /* best effort */ + } + } + + try { + const stderr = readDiagnosticFile(forwardErrPath); + const stdout = readDiagnosticFile(forwardDiagPath); + const message = result?.error instanceof Error ? result.error.message : ""; + return { + result: result ?? { status: null, error: new Error("forward start did not return a result") }, + diagnostic: compactText(redact(`${stderr} ${stdout} ${message}`)), + }; + } finally { + cleanupTempDir(forwardDiagPath, "nemoclaw-forward-start"); + } +} diff --git a/test/onboard.test.ts b/test/onboard.test.ts index c72fc523218..83ad9b1bbee 100644 --- a/test/onboard.test.ts +++ b/test/onboard.test.ts @@ -65,6 +65,12 @@ type OnboardTestInternals = { options?: { webSearchSupported?: boolean | null }, ) => T[]; formatEnvAssignment: (name: string, value: string) => string; + findAvailableDashboardPort: ( + sandboxName: string, + preferredPort: number, + forwardListOutput: string | null, + isPortBoundCheck?: (port: number) => boolean, + ) => number; findDashboardForwardOwner: ( forwardListOutput: string | null | undefined, portToStop: string, @@ -225,6 +231,7 @@ function isOnboardTestInternals( typeof value.buildDirectGpuPolicyYaml === "function" && typeof value.buildDirectSandboxGpuProofCommands === "function" && typeof value.classifySandboxCreateFailure === "function" && + typeof value.findAvailableDashboardPort === "function" && typeof value.getDockerDriverGatewayEnv === "function" && typeof value.getGatewayStartEnv === "function" && typeof value.shouldRequireDockerDriverEnv === "function" && @@ -329,6 +336,7 @@ const { shouldIncludeBuildContextPath, shouldRunCompatibleEndpointSandboxSmoke, writeSandboxConfigSyncFile, + findAvailableDashboardPort, findDashboardForwardOwner, formatOnboardConfigSummary, formatSandboxBuildEstimateNote, @@ -9687,7 +9695,8 @@ const { createSandbox } = require(${onboardPath}); ); assert.match(source, /const preferredEntry = findForwardEntry/); - assert.match(source, /function isLiveForwardStatus/); + // isLiveForwardStatus lives in ./onboard/dashboard-port and is imported above; + // the call site itself is the meaningful assertion. assert.match(source, /!isLiveForwardStatus\(preferredEntry\.status\)/); assert.match( source, @@ -9699,6 +9708,181 @@ const { createSandbox } = require(${onboardPath}); ); }); + describe("findAvailableDashboardPort port-conflict detection (#3260)", () => { + const stubBound = (...bound: number[]) => { + const set = new Set(bound); + return (port: number) => set.has(port); + }; + + it("returns the preferred port when no forward owns it and the host says it is free", () => { + assert.equal( + findAvailableDashboardPort("cursor", 18789, "", stubBound()), + 18789, + ); + }); + + it("skips the preferred port when host reports it bound and falls through to the range scan", () => { + // The proactive probe in isPortBoundOnHost can now see root-owned + // listeners (sudo lsof) and Node-bind-failure listeners that the + // bare lsof missed; the allocator must skip those ports just as it + // skips ports owned by other forwards. + assert.equal( + findAvailableDashboardPort("cursor", 18789, "", stubBound(18789)), + 18790, + ); + }); + + it("skips ports owned by other sandboxes and host-bound ports together", () => { + const forwardList = [ + "SANDBOX BIND PORT PID STATUS", + "alpha 127.0.0.1 18789 111 running", + ].join("\n"); + assert.equal( + findAvailableDashboardPort("cursor", 18789, forwardList, stubBound(18790)), + 18791, + ); + }); + + it("returns the preferred port when this sandbox already owns it", () => { + const forwardList = [ + "SANDBOX BIND PORT PID STATUS", + "cursor 127.0.0.1 18789 111 running", + ].join("\n"); + assert.equal( + findAvailableDashboardPort("cursor", 18789, forwardList, stubBound(18789)), + 18789, + ); + }); + + it("throws when every port in the range is occupied by other sandboxes", () => { + const lines = ["SANDBOX BIND PORT PID STATUS"]; + for (let p = 18789; p <= 18799; p++) { + lines.push(`other${p} 127.0.0.1 ${p} ${p} running`); + } + assert.throws( + () => findAvailableDashboardPort("cursor", 18789, lines.join("\n"), stubBound()), + /All dashboard ports in range 18789-18799 are occupied/, + ); + }); + + it("includes host-bound ports in the exhaustion error so users know what's blocking them", () => { + // When every candidate is skipped by isPortBoundCheck rather than by + // an OpenShell forward, the error must still surface which ports are + // bound — otherwise users see "all ports are occupied" with an empty + // owner list and no remediation hint (CodeRabbit catch on #3260). + const allBound = new Set(); + for (let p = 18789; p <= 18799; p++) allBound.add(p); + assert.throws( + () => findAvailableDashboardPort("cursor", 18789, "", (p) => allBound.has(p)), + /18789 → non-OpenShell host listener[\s\S]*18799 → non-OpenShell host listener/, + ); + }); + + it("probes each port at most once even when the preferred port is in the range", () => { + // Avoid re-probing the same port via the proactive lsof + sudo lsof + + // Node bind chain — those are subprocess-spawning probes and the call + // count matters. + const calls: number[] = []; + const stub = (p: number) => { + calls.push(p); + return false; + }; + findAvailableDashboardPort("cursor", 18789, "", stub); + assert.equal(calls.length, 1, `expected 1 probe call, got ${calls.length}`); + assert.equal(calls[0], 18789); + }); + }); + + it("isPortBoundOnHost has a layered probe chain — lsof, sudo lsof, Node bind (#3260)", () => { + // Source-shape guard for the strengthened detection chain. Behavioural + // testing of the real probes spawns subprocesses and is covered by + // higher-level tests; this assertion just keeps the chain in place + // when the function is refactored. + const source = fs.readFileSync( + path.join(import.meta.dirname, "..", "src", "lib", "onboard", "dashboard-port.ts"), + "utf-8", + ); + assert.match(source, /export function isPortBoundOnHost/); + assert.match(source, /\["lsof", "-i", `:\$\{port\}`, "-sTCP:LISTEN", "-P", "-n"\]/); + assert.match( + source, + /\["sudo", "-n", "lsof", "-i", `:\$\{port\}`, "-sTCP:LISTEN", "-P", "-n"\]/, + ); + assert.match(source, /export function probePortBoundSync/); + assert.match(source, /EADDRINUSE/); + }); + + it("ensureDashboardForward rolls back when forward-start fails on the create path (#3260)", () => { + // The sandbox is committed to its dashboard port at create time + // (Dockerfile ARG + NEMOCLAW_DASHBOARD_PORT env). If `openshell forward + // start` fails after the build (TOCTOU race), we must not silently + // return a broken port — roll back the sandbox so the next onboard + // can pick a different port. The rollback must classify the failure + // (port conflict vs other) so users aren't pointed at the wrong fix. + const source = fs.readFileSync( + path.join(import.meta.dirname, "..", "src", "lib", "onboard.ts"), + "utf-8", + ); + assert.match(source, /if \(fwdResult && fwdResult\.status !== 0\)/); + assert.match(source, /if \(rollbackSandboxOnFailure\)/); + assert.match(source, /const looksLikePortConflict =/); + assert.match(source, /eaddrinuse\|address already in use/i); + assert.match(source, /suppressOutput: true/); + assert.match(source, /runBackgroundForwardStartWithDiagnostics/); + assert.doesNotMatch( + source, + /forward", "start", "--background"[\s\S]{0,260}stdio: \["ignore", "pipe", "pipe"\]/, + "background forward start must not capture pipe stdio; daemonized children can keep pipes open and hang install.sh", + ); + const helperSource = fs.readFileSync( + path.join(import.meta.dirname, "..", "src", "lib", "onboard", "forward-start.ts"), + "utf-8", + ); + assert.match(helperSource, /secureTempFile\("nemoclaw-forward-start", "\.out"\)/); + assert.match(helperSource, /runForwardStart\(\["ignore", outFd, errFd\], timeoutMs\)/); + assert.match( + source, + /runOpenshell\(\["sandbox", "delete", sandboxName\], \{ ignoreError: true \}\)/, + ); + assert.match(source, /buildOrphanedSandboxRollbackMessage/); + }); + + it("ensureDashboardForward rolls back when the create path reallocates to a different port (#3260)", () => { + // The sandbox bakes CHAT_UI_URL and NEMOCLAW_DASHBOARD_PORT from + // `preselectedPort` at build time. If that port becomes host-bound + // during the multi-minute image build (TOCTOU), findAvailableDashboardPort + // returns a different port — but the sandbox is already configured to + // serve on the original one. Starting the forward on the new port + // would reproduce "onboard exits successfully but dashboard is + // unreachable" on the new port. The fix: on the create path, treat + // actualPort !== preferredPort as unrecoverable, roll back the sandbox, + // and let the next onboard re-bake with a clean port. Reuse paths still + // warn-and-continue because the sandbox image is fixed. + const source = fs.readFileSync( + path.join(import.meta.dirname, "..", "src", "lib", "onboard.ts"), + "utf-8", + ); + // Locate the actualPort != preferredPort branch and verify it carries + // both the create-path rollback (gated on rollbackSandboxOnFailure) and + // the reuse-path warn fallback. + const mismatchBranch = source.match( + /if \(actualPort !== preferredPort\) \{[\s\S]*?\n \}/, + ); + assert.ok(mismatchBranch, "Expected actualPort !== preferredPort branch in ensureDashboardForward"); + const branchBody = mismatchBranch[0]; + assert.match(branchBody, /if \(rollbackSandboxOnFailure\)/); + assert.match(branchBody, /became host-bound during sandbox build/); + assert.match( + branchBody, + /runOpenshell\(\["sandbox", "delete", sandboxName\], \{ ignoreError: true \}\)/, + ); + assert.match(branchBody, /buildOrphanedSandboxRollbackMessage/); + assert.match(branchBody, /process\.exit\(1\)/); + // Reuse-path fallback (the warn) must still be present so non-create + // callers keep the existing warn-and-continue semantics. + assert.match(branchBody, /is taken\. Using port .* instead/); + }); + it("formatOnboardConfigSummary renders all collected fields (#2165)", () => { const summary = formatOnboardConfigSummary({ provider: "gemini-api",