From bfb41f7e32b84fc558ed4aaeec887c85a5b867a6 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Sat, 13 Jun 2026 13:28:02 +0000 Subject: [PATCH 1/3] fix(onboard): announce and recover declared agent forward_ports Signed-off-by: Tinson Lai --- src/lib/actions/sandbox/process-recovery.ts | 79 ++++++++++++++++++- src/lib/agent/onboard.test.ts | 30 +++++++ src/lib/agent/onboard.ts | 50 ++++++++++++ test/process-recovery.test.ts | 87 +++++++++++++++++++++ 4 files changed, 243 insertions(+), 3 deletions(-) diff --git a/src/lib/actions/sandbox/process-recovery.ts b/src/lib/actions/sandbox/process-recovery.ts index ffdfa4728f..941e93afd4 100644 --- a/src/lib/actions/sandbox/process-recovery.ts +++ b/src/lib/actions/sandbox/process-recovery.ts @@ -431,6 +431,49 @@ function ensureHermesDashboardPortForwardIfEnabled(sandboxName: string): boolean }); } +/** + * Re-establish every declared `forward_ports` entry on the active agent + * manifest that is not the primary dashboard port. The primary dashboard + * port is owned by `ensureSandboxPortForward`; the optional Hermes web + * dashboard is owned by `ensureHermesDashboardPortForwardIfEnabled`. Any + * remaining manifest-declared port (e.g. Hermes' OpenAI-compatible API on + * 8642) would otherwise be silently dropped after a gateway restart and + * never re-established by the recovery flow. + * + * Returns true when every non-primary declared port is healthy (probed or + * re-established), false when at least one declared port could not be + * re-established, and `null` when there is no active agent or no declared + * non-primary ports to manage. + */ +function ensureDeclaredAgentForwardPortsHealthy( + sandboxName: string, + primaryPort: number, +): boolean | null { + const agent = agentRuntime.getSessionAgent(sandboxName); + if (!agent) return null; + const declared = (agent as { forward_ports?: unknown }).forward_ports; + if (!Array.isArray(declared) || declared.length === 0) return null; + let sawNonPrimary = false; + let allHealthy = true; + for (const candidate of declared) { + if (typeof candidate !== "number") continue; + if (!Number.isInteger(candidate) || candidate < 1 || candidate > 65535) continue; + if (candidate === primaryPort) continue; + sawNonPrimary = true; + const health = isSandboxPortForwardHealthy(sandboxName, candidate); + if (health === true) continue; + if (health === "occupied") { + allHealthy = false; + continue; + } + if (!ensureSandboxPortForwardForPort(sandboxName, candidate)) { + allHealthy = false; + } + } + if (!sawNonPrimary) return null; + return allHealthy; +} + function recoverHermesDashboardProcessIfEnabled(sandboxName: string): boolean | null { return recoverHermesDashboardProcess(sandboxName, { executeCommand: executeSandboxCommand }); } @@ -466,6 +509,10 @@ export function checkAndRecoverSandboxProcesses( } const forwardRecovered = ensureSandboxPortForward(sandboxName); const dashboardForwardRecovered = ensureHermesDashboardPortForwardIfEnabled(sandboxName); + const declaredForwardsRecovered = ensureDeclaredAgentForwardPortsHealthy( + sandboxName, + recoveryPort, + ); if (!quiet) { if (forwardRecovered) { console.log(` ${G}✓${R} Dashboard port forward re-established.`); @@ -475,6 +522,11 @@ export function checkAndRecoverSandboxProcesses( ` Run \`openshell forward start --background ${sandboxName}\` manually.`, ); } + if (declaredForwardsRecovered === false) { + console.error( + " One or more agent-declared port forwards could not be re-established.", + ); + } } return { checked: true, @@ -483,7 +535,8 @@ export function checkAndRecoverSandboxProcesses( forwardRecovered: forwardRecovered || dashboardForwardRecovered === true || - dashboardProcessRecovered === true, + dashboardProcessRecovered === true || + declaredForwardsRecovered === true, }; } if (forwardHealthy === "occupied") { @@ -495,11 +548,21 @@ export function checkAndRecoverSandboxProcesses( return { checked: true, wasRunning: true, recovered: false, forwardRecovered: false }; } const dashboardForwardRecovered = ensureHermesDashboardPortForwardIfEnabled(sandboxName); + const declaredForwardsRecovered = ensureDeclaredAgentForwardPortsHealthy( + sandboxName, + recoveryPort, + ); + if (!quiet && declaredForwardsRecovered === false) { + console.error(" One or more agent-declared port forwards could not be re-established."); + } return { checked: true, wasRunning: true, recovered: false, - forwardRecovered: dashboardForwardRecovered === true || dashboardProcessRecovered === true, + forwardRecovered: + dashboardForwardRecovered === true || + dashboardProcessRecovered === true || + declaredForwardsRecovered === true, }; } @@ -530,6 +593,10 @@ export function checkAndRecoverSandboxProcesses( } const forwardRecovered = ensureSandboxPortForward(sandboxName); const dashboardForwardRecovered = ensureHermesDashboardPortForwardIfEnabled(sandboxName); + const declaredForwardsRecovered = ensureDeclaredAgentForwardPortsHealthy( + sandboxName, + recoveryPort, + ); if (!quiet) { console.log( ` ${G}✓${R} ${agentRuntime.getAgentDisplayName(recoveryAgent)} gateway restarted inside sandbox.`, @@ -542,12 +609,18 @@ export function checkAndRecoverSandboxProcesses( ` Run \`openshell forward start --background ${sandboxName}\` manually.`, ); } + if (declaredForwardsRecovered === false) { + console.error(" One or more agent-declared port forwards could not be re-established."); + } } return { checked: true, wasRunning: false, recovered, - forwardRecovered: forwardRecovered || dashboardForwardRecovered === true, + forwardRecovered: + forwardRecovered || + dashboardForwardRecovered === true || + declaredForwardsRecovered === true, }; } if (!quiet) { diff --git a/src/lib/agent/onboard.test.ts b/src/lib/agent/onboard.test.ts index 05de082d10..8727cf281c 100644 --- a/src/lib/agent/onboard.test.ts +++ b/src/lib/agent/onboard.test.ts @@ -188,6 +188,36 @@ describe("printDashboardUi — regression for #2078 (port 8642 is not a chat UI) expect(noteSpy).not.toHaveBeenCalled(); }); + it("announces manifest-declared secondary forward_ports alongside the primary dashboard", () => { + const hermesShipped = makeAgent({ + name: "hermes", + displayName: "Hermes Agent", + forwardPort: 18789, + forward_ports: [18789, 8642], + healthProbe: { url: "http://localhost:8642/health", port: 8642, timeout_seconds: 90 }, + dashboard: { + kind: "ui", + label: "Dashboard", + path: "/", + healthPath: "/api/status", + auth: "session", + }, + }); + + printDashboardUi("hermes-box", null, hermesShipped, { + note: noteSpy, + buildControlUiUrls: buildUrlsLoopback, + }); + + const output = logSpy.mock.calls.map((args) => String(args[0])).join("\n"); + expect(output).toContain("Hermes Agent Dashboard"); + expect(output).toContain("Port 18789 must be forwarded before opening this URL."); + expect(output).toContain("http://127.0.0.1:18789/"); + expect(output).toContain("Hermes Agent OpenAI-compatible API"); + expect(output).toContain("Port 8642 must be forwarded before connecting."); + expect(output).toContain("http://127.0.0.1:8642/v1"); + }); + it("redacts tokenized URLs for UI-kind agents and shows the token retrieval command", () => { const token = "a".repeat(64); printDashboardUi("sandbox-y", token, uiAgent, { diff --git a/src/lib/agent/onboard.ts b/src/lib/agent/onboard.ts index 2fa3bca55d..6b935aebfe 100644 --- a/src/lib/agent/onboard.ts +++ b/src/lib/agent/onboard.ts @@ -569,6 +569,7 @@ export function printDashboardUi( console.log(` ${dashboardUrlForDisplay(url)}`); } printOptionalDashboardUi(agent, { ...deps, redactUrl: dashboardUrlForDisplay }); + printAdditionalForwardPorts(agent, info.port, deps.buildControlUiUrls); return; } @@ -578,6 +579,8 @@ export function printDashboardUi( for (const url of deps.buildControlUiUrls(null, info.port)) { console.log(` ${dashboardUrlForDisplay(url)}`); } + printOptionalDashboardUi(agent, { ...deps, redactUrl: dashboardUrlForDisplay }); + printAdditionalForwardPorts(agent, info.port, deps.buildControlUiUrls); return; } @@ -598,4 +601,51 @@ export function printDashboardUi( } } printOptionalDashboardUi(agent, { ...deps, redactUrl: dashboardUrlForDisplay }); + printAdditionalForwardPorts(agent, info.port, deps.buildControlUiUrls); +} + +/** + * Print one block per manifest-declared `forward_ports` entry that is not + * the primary dashboard port. Each block announces the port and renders a + * loopback URL using the same `buildControlUiUrls` chain as the primary + * dashboard so WSL host-address fallbacks remain consistent. + * + * The label is sourced from the agent's `health_probe.port` match — that + * is the only manifest signal today that a declared secondary port is the + * OpenAI-compatible API surface (Hermes manifest sets + * `health_probe.port: 8642` alongside `forward_ports: [18789, 8642]`). + * Any other declared port gets a neutral "additional port" label. + */ +function printAdditionalForwardPorts( + agent: AgentDefinition, + primaryPort: number, + buildControlUiUrls: (token: string | null, port: number) => string[], +): void { + const declared = Array.isArray(agent.forward_ports) ? agent.forward_ports : []; + if (declared.length === 0) return; + const apiPort = agent.healthProbe.port; + for (const port of declared) { + if (!Number.isInteger(port) || port < 1 || port > 65535) continue; + if (port === primaryPort) continue; + const isApi = port === apiPort; + const sectionLabel = isApi ? "OpenAI-compatible API" : "additional port"; + console.log(""); + console.log(` ${agent.displayName} ${sectionLabel}`); + console.log(` Port ${port} must be forwarded before connecting.`); + const seen = new Set(); + for (const baseUrl of buildControlUiUrls(null, port)) { + const withoutHash = baseUrl.split("#")[0].replace(/\/$/, ""); + let urlPort = ""; + try { + urlPort = new URL(withoutHash).port; + } catch { + urlPort = ""; + } + if (urlPort !== String(port)) continue; + const url = isApi ? `${withoutHash}/v1` : `${withoutHash}/`; + if (seen.has(url)) continue; + seen.add(url); + console.log(` ${dashboardUrlForDisplay(url)}`); + } + } } diff --git a/test/process-recovery.test.ts b/test/process-recovery.test.ts index 08349f6043..b5acb49b4c 100644 --- a/test/process-recovery.test.ts +++ b/test/process-recovery.test.ts @@ -304,4 +304,91 @@ beta 127.0.0.1 18789 12345 running`; } } }); + + it("re-establishes manifest-declared non-primary forward ports when only the primary is healthy", () => { + const openshellRuntime = requireDist("../dist/lib/adapters/openshell/runtime.js"); + const agentRuntime = requireDist("../dist/lib/agent/runtime.js"); + const registry = requireDist("../dist/lib/state/registry.js"); + const forwardHealth = requireDist("../dist/lib/actions/sandbox/forward-health.js"); + const childProcess = requireDist("node:child_process"); + const onlyPrimaryForward = `SANDBOX BIND PORT PID STATUS +hermes-box 127.0.0.1 18789 12345 running`; + const bothForwards = `SANDBOX BIND PORT PID STATUS +hermes-box 127.0.0.1 18789 12345 running +hermes-box 127.0.0.1 8642 12346 running`; + let secondaryStarted = false; + + vi.spyOn(childProcess, "spawnSync").mockReturnValue({ + status: 0, + stdout: "__NEMOCLAW_SANDBOX_EXEC_STARTED__\nRUNNING\n", + stderr: "", + } as never); + vi.spyOn(agentRuntime, "getSessionAgent").mockReturnValue({ + name: "hermes", + forwardPort: 18789, + forward_ports: [18789, 8642], + }); + vi.spyOn(registry, "getSandbox").mockReturnValue({ + name: "hermes-box", + agent: "hermes", + dashboardPort: 18789, + }); + vi.spyOn(forwardHealth, "isLocalForwardReachable").mockImplementation((port: unknown) => { + if (Number(port) === 18789) return true; + if (Number(port) === 8642) return secondaryStarted; + return false; + }); + vi.spyOn(openshellRuntime, "captureOpenshell").mockImplementation(() => ({ + status: 0, + output: secondaryStarted ? bothForwards : onlyPrimaryForward, + })); + const runOpenshell = vi + .spyOn(openshellRuntime, "runOpenshell") + .mockImplementation((rawArgs: unknown) => { + const args = Array.isArray(rawArgs) ? rawArgs.map(String) : []; + if (args[0] === "forward" && args[1] === "start" && args.includes("8642")) { + secondaryStarted = true; + } + return { status: 0 } as never; + }); + + expect( + withFakeOpenshellBinary(() => + checkAndRecoverSandboxProcesses("hermes-box", { quiet: true }), + ), + ).toEqual({ + checked: true, + wasRunning: true, + recovered: false, + forwardRecovered: true, + }); + + const startedNonPrimary = runOpenshell.mock.calls.some( + ([rawArgs]) => { + const args = Array.isArray(rawArgs) ? rawArgs.map(String) : []; + return ( + args[0] === "forward" && + args[1] === "start" && + args.includes("--background") && + args.includes("8642") && + args.includes("hermes-box") + ); + }, + ); + expect(startedNonPrimary).toBe(true); + + const startedPrimary = runOpenshell.mock.calls.some( + ([rawArgs]) => { + const args = Array.isArray(rawArgs) ? rawArgs.map(String) : []; + return ( + args[0] === "forward" && + args[1] === "start" && + args.includes("--background") && + args.includes("18789") && + args.includes("hermes-box") + ); + }, + ); + expect(startedPrimary).toBe(false); + }); }); From 4883743c816d54855887d33c751c7d705a3cd867 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Sat, 13 Jun 2026 14:10:32 +0000 Subject: [PATCH 2/3] fix(onboard): dedup Hermes dashboard port and refresh docs Signed-off-by: Tinson Lai --- docs/get-started/quickstart-hermes.mdx | 8 +++- src/lib/actions/sandbox/process-recovery.ts | 41 +++++++++++------- test/process-recovery.test.ts | 48 +++++++++------------ 3 files changed, 52 insertions(+), 45 deletions(-) diff --git a/docs/get-started/quickstart-hermes.mdx b/docs/get-started/quickstart-hermes.mdx index 3ddcd39fa3..a9d3098b95 100644 --- a/docs/get-started/quickstart-hermes.mdx +++ b/docs/get-started/quickstart-hermes.mdx @@ -101,9 +101,9 @@ Use the provider variables from [Inference Options](../inference/inference-optio ## Connect to Hermes -When onboarding completes, NemoClaw prints the sandbox name, model, lifecycle commands, and Hermes dashboard URL. +When onboarding completes, NemoClaw prints the sandbox name, model, lifecycle commands, the Hermes dashboard URL, and the OpenAI-compatible API URL. Hermes exposes its built-in browser dashboard on port `18789`. -NemoClaw also forwards the OpenAI-compatible API on port `8642` for local clients. +NemoClaw also forwards the OpenAI-compatible API on port `8642` for local clients, and the summary now announces both URLs. NemoClaw builds the Hermes dashboard assets into the sandbox image, so the dashboard starts without running `npm` as the sandbox user under `/opt/hermes`. Dashboard chat uses the prebuilt `/opt/hermes/ui-tui` bundle. If you need to recover the Hermes dashboard manually, use `hermes dashboard --tui --skip-build` so recovery does not try to rebuild assets under root-owned install paths. @@ -122,6 +122,10 @@ Access Port 18789 must be forwarded before opening this URL. http://127.0.0.1:18789/ + Hermes Agent OpenAI-compatible API + Port 8642 must be forwarded before connecting. + http://127.0.0.1:8642/v1 + Terminal: nemohermes my-hermes connect diff --git a/src/lib/actions/sandbox/process-recovery.ts b/src/lib/actions/sandbox/process-recovery.ts index 941e93afd4..907cce37c0 100644 --- a/src/lib/actions/sandbox/process-recovery.ts +++ b/src/lib/actions/sandbox/process-recovery.ts @@ -433,17 +433,23 @@ function ensureHermesDashboardPortForwardIfEnabled(sandboxName: string): boolean /** * Re-establish every declared `forward_ports` entry on the active agent - * manifest that is not the primary dashboard port. The primary dashboard - * port is owned by `ensureSandboxPortForward`; the optional Hermes web - * dashboard is owned by `ensureHermesDashboardPortForwardIfEnabled`. Any - * remaining manifest-declared port (e.g. Hermes' OpenAI-compatible API on - * 8642) would otherwise be silently dropped after a gateway restart and - * never re-established by the recovery flow. + * manifest that is not already owned by another recovery helper. The + * primary dashboard port is owned by `ensureSandboxPortForward`; the + * optional Hermes web dashboard port (a registry-recorded per-sandbox + * override that the manifest cannot statically declare) is owned by + * `ensureHermesDashboardPortForwardIfEnabled`. Skipping both here keeps + * the helpers orthogonal and avoids issuing duplicate `forward start` + * calls when an operator pins the Hermes dashboard to one of the + * manifest-declared ports. * - * Returns true when every non-primary declared port is healthy (probed or + * Without this helper, any remaining manifest-declared port (e.g. + * Hermes' OpenAI-compatible API on 8642) would be silently dropped after + * a gateway restart and never re-established by the recovery flow. + * + * Returns true when every covered declared port is healthy (probed or * re-established), false when at least one declared port could not be - * re-established, and `null` when there is no active agent or no declared - * non-primary ports to manage. + * re-established, and `null` when there is no active agent or no + * declared port left to manage after the skip set is applied. */ function ensureDeclaredAgentForwardPortsHealthy( sandboxName: string, @@ -453,13 +459,18 @@ function ensureDeclaredAgentForwardPortsHealthy( if (!agent) return null; const declared = (agent as { forward_ports?: unknown }).forward_ports; if (!Array.isArray(declared) || declared.length === 0) return null; - let sawNonPrimary = false; + const hermesDashboard = getHermesDashboardRecoveryConfig(sandboxName); + const skipSet = new Set([primaryPort]); + if (hermesDashboard && Number.isInteger(hermesDashboard.publicPort)) { + skipSet.add(hermesDashboard.publicPort); + } + let sawCovered = false; let allHealthy = true; for (const candidate of declared) { if (typeof candidate !== "number") continue; if (!Number.isInteger(candidate) || candidate < 1 || candidate > 65535) continue; - if (candidate === primaryPort) continue; - sawNonPrimary = true; + if (skipSet.has(candidate)) continue; + sawCovered = true; const health = isSandboxPortForwardHealthy(sandboxName, candidate); if (health === true) continue; if (health === "occupied") { @@ -470,7 +481,7 @@ function ensureDeclaredAgentForwardPortsHealthy( allHealthy = false; } } - if (!sawNonPrimary) return null; + if (!sawCovered) return null; return allHealthy; } @@ -523,9 +534,7 @@ export function checkAndRecoverSandboxProcesses( ); } if (declaredForwardsRecovered === false) { - console.error( - " One or more agent-declared port forwards could not be re-established.", - ); + console.error(" One or more agent-declared port forwards could not be re-established."); } } return { diff --git a/test/process-recovery.test.ts b/test/process-recovery.test.ts index b5acb49b4c..a0174140e4 100644 --- a/test/process-recovery.test.ts +++ b/test/process-recovery.test.ts @@ -353,9 +353,7 @@ hermes-box 127.0.0.1 8642 12346 running`; }); expect( - withFakeOpenshellBinary(() => - checkAndRecoverSandboxProcesses("hermes-box", { quiet: true }), - ), + withFakeOpenshellBinary(() => checkAndRecoverSandboxProcesses("hermes-box", { quiet: true })), ).toEqual({ checked: true, wasRunning: true, @@ -363,32 +361,28 @@ hermes-box 127.0.0.1 8642 12346 running`; forwardRecovered: true, }); - const startedNonPrimary = runOpenshell.mock.calls.some( - ([rawArgs]) => { - const args = Array.isArray(rawArgs) ? rawArgs.map(String) : []; - return ( - args[0] === "forward" && - args[1] === "start" && - args.includes("--background") && - args.includes("8642") && - args.includes("hermes-box") - ); - }, - ); + const startedNonPrimary = runOpenshell.mock.calls.some(([rawArgs]) => { + const args = Array.isArray(rawArgs) ? rawArgs.map(String) : []; + return ( + args[0] === "forward" && + args[1] === "start" && + args.includes("--background") && + args.includes("8642") && + args.includes("hermes-box") + ); + }); expect(startedNonPrimary).toBe(true); - const startedPrimary = runOpenshell.mock.calls.some( - ([rawArgs]) => { - const args = Array.isArray(rawArgs) ? rawArgs.map(String) : []; - return ( - args[0] === "forward" && - args[1] === "start" && - args.includes("--background") && - args.includes("18789") && - args.includes("hermes-box") - ); - }, - ); + const startedPrimary = runOpenshell.mock.calls.some(([rawArgs]) => { + const args = Array.isArray(rawArgs) ? rawArgs.map(String) : []; + return ( + args[0] === "forward" && + args[1] === "start" && + args.includes("--background") && + args.includes("18789") && + args.includes("hermes-box") + ); + }); expect(startedPrimary).toBe(false); }); }); From 6ea01a9b2c0ce7a83887c21169910404612f614b Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Sat, 13 Jun 2026 14:38:18 +0000 Subject: [PATCH 3/3] fix(onboard): handle scheme-default ports and add negative recovery coverage Signed-off-by: Tinson Lai --- docs/reference/troubleshooting.mdx | 14 +++ src/lib/agent/onboard.test.ts | 64 ++++++++++++ src/lib/agent/onboard.ts | 53 ++++++++-- test/process-recovery.test.ts | 150 +++++++++++++++++++++++++++++ 4 files changed, 274 insertions(+), 7 deletions(-) diff --git a/docs/reference/troubleshooting.mdx b/docs/reference/troubleshooting.mdx index 6c5f900453..27367e32a7 100644 --- a/docs/reference/troubleshooting.mdx +++ b/docs/reference/troubleshooting.mdx @@ -1624,6 +1624,20 @@ Expected output: Point an OpenAI-compatible client at `http://127.0.0.1:8642/v1` for chat completions. For terminal use, run `nemohermes connect` and then `hermes` inside the sandbox. +### `docker port` shows no mapping for 8642 even though forwarding works + +OpenShell port forwards are host-side relays managed by the OpenShell gateway process, not Docker `-p` publish mappings on the sandbox container. +`docker port openshell-hermes-` reflects only Docker-published ports, so it returns nothing for OpenShell-managed forwards even when the host bind is live. + +Use OpenShell's own view as the supported acceptance signal: + +```bash +openshell forward list # shows the host bind for each forwarded port +curl -sf http://127.0.0.1:8642/health # confirms the relayed endpoint answers +``` + +If `openshell forward list` does not show port `8642`, run `nemohermes connect --probe-only` (or `nemohermes recover`) to ask the recovery path to re-establish every manifest-declared agent forward port that has gone missing. + ### `nemohermes` reports `Sandbox 'X' already exists as OpenClaw` Each sandbox name maps to exactly one agent type. diff --git a/src/lib/agent/onboard.test.ts b/src/lib/agent/onboard.test.ts index 8727cf281c..9185b74746 100644 --- a/src/lib/agent/onboard.test.ts +++ b/src/lib/agent/onboard.test.ts @@ -218,6 +218,70 @@ describe("printDashboardUi — regression for #2078 (port 8642 is not a chat UI) expect(output).toContain("http://127.0.0.1:8642/v1"); }); + it("labels a non-health-probe secondary forward port as 'additional port' rooted at /", () => { + const dualAgent = makeAgent({ + name: "experimental", + displayName: "Experimental", + forwardPort: 18789, + forward_ports: [18789, 9100], + healthProbe: { url: "http://localhost:18789/health", port: 18789, timeout_seconds: 30 }, + dashboard: { + kind: "ui", + label: "Dashboard", + path: "/", + healthPath: "/health", + auth: "session", + }, + }); + + printDashboardUi("agent-box", null, dualAgent, { + note: noteSpy, + buildControlUiUrls: buildUrlsLoopback, + }); + + const output = logSpy.mock.calls.map((args) => String(args[0])).join("\n"); + expect(output).toContain("Experimental additional port"); + expect(output).toContain("Port 9100 must be forwarded before connecting."); + expect(output).toContain("http://127.0.0.1:9100/"); + expect(output).not.toContain("OpenAI-compatible API"); + expect(output).not.toContain("http://127.0.0.1:9100/v1"); + }); + + it("emits a URL for a secondary forward port that resolves to the scheme default", () => { + // Regression: `new URL("http://h:80").port === ""`. A strict equality + // filter against String(port) silently drops the URL line. The helper + // must normalise scheme-default ports before filtering. + const buildUrlsWithDefaultPort = (_token: string | null, port: number): string[] => { + if (port === 80) return ["http://127.0.0.1:80/"]; + return [`http://127.0.0.1:${port}/`]; + }; + + const httpAgent = makeAgent({ + name: "experimental", + displayName: "Experimental", + forwardPort: 18789, + forward_ports: [18789, 80], + healthProbe: { url: "http://localhost:18789/health", port: 18789, timeout_seconds: 30 }, + dashboard: { + kind: "ui", + label: "Dashboard", + path: "/", + healthPath: "/health", + auth: "session", + }, + }); + + printDashboardUi("agent-box", null, httpAgent, { + note: noteSpy, + buildControlUiUrls: buildUrlsWithDefaultPort, + }); + + const output = logSpy.mock.calls.map((args) => String(args[0])).join("\n"); + expect(output).toContain("Experimental additional port"); + expect(output).toContain("Port 80 must be forwarded before connecting."); + expect(output).toMatch(/http:\/\/127\.0\.0\.1(:80)?\//); + }); + it("redacts tokenized URLs for UI-kind agents and shows the token retrieval command", () => { const token = "a".repeat(64); printDashboardUi("sandbox-y", token, uiAgent, { diff --git a/src/lib/agent/onboard.ts b/src/lib/agent/onboard.ts index 6b935aebfe..b5b39f59b5 100644 --- a/src/lib/agent/onboard.ts +++ b/src/lib/agent/onboard.ts @@ -615,6 +615,14 @@ export function printDashboardUi( * OpenAI-compatible API surface (Hermes manifest sets * `health_probe.port: 8642` alongside `forward_ports: [18789, 8642]`). * Any other declared port gets a neutral "additional port" label. + * + * The URL filter normalises empty `URL.port` results to the scheme + * default. `new URL("http://h:80").port` returns `""` because WHATWG + * URL elides the default scheme port; a strict `urlPort === String(port)` + * comparison would silently drop URLs for ports 80 and 443 even though + * the underlying `forward_ports` validation accepts them. The + * normalisation keeps the filter sound while still excluding any URL + * whose port truly does not match the declared entry. */ function printAdditionalForwardPorts( agent: AgentDefinition, @@ -635,13 +643,8 @@ function printAdditionalForwardPorts( const seen = new Set(); for (const baseUrl of buildControlUiUrls(null, port)) { const withoutHash = baseUrl.split("#")[0].replace(/\/$/, ""); - let urlPort = ""; - try { - urlPort = new URL(withoutHash).port; - } catch { - urlPort = ""; - } - if (urlPort !== String(port)) continue; + const resolvedUrlPort = resolveUrlPort(withoutHash); + if (resolvedUrlPort !== port) continue; const url = isApi ? `${withoutHash}/v1` : `${withoutHash}/`; if (seen.has(url)) continue; seen.add(url); @@ -649,3 +652,39 @@ function printAdditionalForwardPorts( } } } + +/** + * Resolve the effective port of `candidate`, normalising the WHATWG + * URL behaviour that returns an empty string for the scheme-default + * port (`http://h:80` → `""`, `https://h:443` → `""`). Returns the + * integer port, or `null` when the input is unparseable or carries no + * recoverable port. The mapping is intentionally limited to `http` / + * `https` / `ws` / `wss` — the four schemes the dashboard URL builder + * emits — so an unknown scheme falls through to `null` instead of + * silently mapping to 80 or 443. + */ +function resolveUrlPort(candidate: string): number | null { + let parsed: URL; + try { + parsed = new URL(candidate); + } catch { + return null; + } + if (parsed.port !== "") { + const numeric = Number(parsed.port); + return Number.isInteger(numeric) ? numeric : null; + } + const protocol = parsed.protocol.replace(/:$/, "").toLowerCase(); + switch (protocol) { + case "http": + return 80; + case "https": + return 443; + case "ws": + return 80; + case "wss": + return 443; + default: + return null; + } +} diff --git a/test/process-recovery.test.ts b/test/process-recovery.test.ts index a0174140e4..ce784cc33c 100644 --- a/test/process-recovery.test.ts +++ b/test/process-recovery.test.ts @@ -385,4 +385,154 @@ hermes-box 127.0.0.1 8642 12346 running`; }); expect(startedPrimary).toBe(false); }); + + it("leaves a non-primary forward owned by another sandbox alone instead of taking it over", () => { + const openshellRuntime = requireDist("../dist/lib/adapters/openshell/runtime.js"); + const agentRuntime = requireDist("../dist/lib/agent/runtime.js"); + const registry = requireDist("../dist/lib/state/registry.js"); + const forwardHealth = requireDist("../dist/lib/actions/sandbox/forward-health.js"); + const childProcess = requireDist("node:child_process"); + const occupiedForwardList = `SANDBOX BIND PORT PID STATUS +hermes-box 127.0.0.1 18789 12345 running +sibling-box 127.0.0.1 8642 99999 running`; + + vi.spyOn(childProcess, "spawnSync").mockReturnValue({ + status: 0, + stdout: "__NEMOCLAW_SANDBOX_EXEC_STARTED__\nRUNNING\n", + stderr: "", + } as never); + vi.spyOn(agentRuntime, "getSessionAgent").mockReturnValue({ + name: "hermes", + forwardPort: 18789, + forward_ports: [18789, 8642], + }); + vi.spyOn(registry, "getSandbox").mockReturnValue({ + name: "hermes-box", + agent: "hermes", + dashboardPort: 18789, + }); + vi.spyOn(forwardHealth, "isLocalForwardReachable").mockImplementation( + (port: unknown) => Number(port) === 18789, + ); + vi.spyOn(openshellRuntime, "captureOpenshell").mockReturnValue({ + status: 0, + output: occupiedForwardList, + }); + const runOpenshell = vi + .spyOn(openshellRuntime, "runOpenshell") + .mockReturnValue({ status: 0 } as never); + + const result = withFakeOpenshellBinary(() => + checkAndRecoverSandboxProcesses("hermes-box", { quiet: true }), + ); + expect(result.checked).toBe(true); + expect(result.wasRunning).toBe(true); + expect(result.forwardRecovered).toBe(false); + + const touchedSecondary = runOpenshell.mock.calls.some(([rawArgs]) => { + const args = Array.isArray(rawArgs) ? rawArgs.map(String) : []; + return args[0] === "forward" && args.includes("8642"); + }); + expect(touchedSecondary).toBe(false); + }); + + it("ignores invalid forward_ports entries and never invokes openshell forward start for them", () => { + const openshellRuntime = requireDist("../dist/lib/adapters/openshell/runtime.js"); + const agentRuntime = requireDist("../dist/lib/agent/runtime.js"); + const registry = requireDist("../dist/lib/state/registry.js"); + const forwardHealth = requireDist("../dist/lib/actions/sandbox/forward-health.js"); + const childProcess = requireDist("node:child_process"); + const primaryOnlyForward = `SANDBOX BIND PORT PID STATUS +hermes-box 127.0.0.1 18789 12345 running`; + + vi.spyOn(childProcess, "spawnSync").mockReturnValue({ + status: 0, + stdout: "__NEMOCLAW_SANDBOX_EXEC_STARTED__\nRUNNING\n", + stderr: "", + } as never); + vi.spyOn(agentRuntime, "getSessionAgent").mockReturnValue({ + name: "hermes", + forwardPort: 18789, + // Mixed invalid entries the helper must skip: zero, negative, fractional, + // > 65535, non-numeric, and the primary entry. + forward_ports: [18789, 0, -1, 1.5, 70000, "8642" as unknown as number], + }); + vi.spyOn(registry, "getSandbox").mockReturnValue({ + name: "hermes-box", + agent: "hermes", + dashboardPort: 18789, + }); + vi.spyOn(forwardHealth, "isLocalForwardReachable").mockReturnValue(true); + vi.spyOn(openshellRuntime, "captureOpenshell").mockReturnValue({ + status: 0, + output: primaryOnlyForward, + }); + const runOpenshell = vi + .spyOn(openshellRuntime, "runOpenshell") + .mockReturnValue({ status: 0 } as never); + + withFakeOpenshellBinary(() => checkAndRecoverSandboxProcesses("hermes-box", { quiet: true })); + + const issuedForwardStart = runOpenshell.mock.calls.some(([rawArgs]) => { + const args = Array.isArray(rawArgs) ? rawArgs.map(String) : []; + return args[0] === "forward" && args[1] === "start"; + }); + expect(issuedForwardStart).toBe(false); + }); + + it("reports forwardRecovered=false when one declared secondary recovers and another fails", () => { + const openshellRuntime = requireDist("../dist/lib/adapters/openshell/runtime.js"); + const agentRuntime = requireDist("../dist/lib/agent/runtime.js"); + const registry = requireDist("../dist/lib/state/registry.js"); + const forwardHealth = requireDist("../dist/lib/actions/sandbox/forward-health.js"); + const childProcess = requireDist("node:child_process"); + const partialForward = `SANDBOX BIND PORT PID STATUS +hermes-box 127.0.0.1 18789 12345 running +hermes-box 127.0.0.1 8642 12346 running`; + + vi.spyOn(childProcess, "spawnSync").mockReturnValue({ + status: 0, + stdout: "__NEMOCLAW_SANDBOX_EXEC_STARTED__\nRUNNING\n", + stderr: "", + } as never); + vi.spyOn(agentRuntime, "getSessionAgent").mockReturnValue({ + name: "hermes", + forwardPort: 18789, + forward_ports: [18789, 8642, 9100], + }); + vi.spyOn(registry, "getSandbox").mockReturnValue({ + name: "hermes-box", + agent: "hermes", + dashboardPort: 18789, + }); + let port9100Started = false; + vi.spyOn(forwardHealth, "isLocalForwardReachable").mockImplementation((port: unknown) => { + if (Number(port) === 18789) return true; + if (Number(port) === 8642) return true; + if (Number(port) === 9100) return port9100Started; + return false; + }); + vi.spyOn(openshellRuntime, "captureOpenshell").mockReturnValue({ + status: 0, + output: partialForward, + }); + vi.spyOn(openshellRuntime, "runOpenshell").mockImplementation((rawArgs: unknown) => { + const args = Array.isArray(rawArgs) ? rawArgs.map(String) : []; + if (args[0] === "forward" && args[1] === "start" && args.includes("9100")) { + // Forward start succeeds at the OpenShell level but the post-start + // probe stays unhealthy — simulates a port that openshell launches + // and that immediately drops on the sandbox side. + return { status: 0 } as never; + } + return { status: 0 } as never; + }); + + const result = withFakeOpenshellBinary(() => + checkAndRecoverSandboxProcesses("hermes-box", { quiet: true }), + ); + void port9100Started; + expect(result.checked).toBe(true); + expect(result.wasRunning).toBe(true); + expect(result.forwardRecovered).toBe(false); + }); });