diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 17c180a611..67b359821c 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -2974,7 +2974,7 @@ async function createSandbox( } } const preferredPort = - controlUiPort ?? envPort ?? persistedPort ?? (agent ? agent.forwardPort : CONTROL_UI_PORT); + controlUiPort ?? envPort ?? persistedPort ?? (agent ? agent.forwardPort : DASHBOARD_PORT); const earlyForwards = runCaptureOpenshell(["forward", "list"], { ignoreError: true }); const effectivePort = findAvailableDashboardPort(sandboxName, preferredPort, earlyForwards); if (effectivePort !== preferredPort) { @@ -6694,8 +6694,6 @@ async function setupPoliciesWithSelection( // ── Dashboard ──────────────────────────────────────────────────── -const CONTROL_UI_PORT = DASHBOARD_PORT; - const { buildChain, buildControlUiUrls, @@ -6710,6 +6708,7 @@ const { } = onboardDashboard.createOnboardDashboardHelpers({ runOpenshell, runCaptureOpenshell, + openshellArgv, runCapture, cliName, agentProductName, diff --git a/src/lib/onboard/dashboard.ts b/src/lib/onboard/dashboard.ts index bfd3d35e56..2840bf7b40 100644 --- a/src/lib/onboard/dashboard.ts +++ b/src/lib/onboard/dashboard.ts @@ -16,11 +16,14 @@ import { getOccupiedPorts, isLiveForwardStatus, } from "./dashboard-port"; +import { OPENSHELL_PROBE_TIMEOUT_MS } from "../adapters/openshell/timeouts"; import { + buildDetachedForwardStartSpawn, + buildForwardStartProgressLogger, looksLikeForwardPortConflict, - runBackgroundForwardStartWithPortReleaseRetries, + runDetachedForwardStartWithPortReleaseRetries, } from "./forward-start"; -import { bestEffortForwardStop } from "./forward-cleanup"; +import { bestEffortForwardStop, bestEffortForwardStopForSandbox } from "./forward-cleanup"; const ANSI_RE = /\x1B(?:\[[0-?]*[ -/]*[@-~]|\][^\x07]*(?:\x07|\x1B\\)|[@-_])/g; export const CONTROL_UI_PORT = DASHBOARD_PORT; @@ -30,6 +33,7 @@ type CommandResult = { status: number | null }; export interface OnboardDashboardDeps { runOpenshell(args: string[], opts?: Record): CommandResult; runCaptureOpenshell(args: string[], opts?: Record): string | null; + openshellArgv(args: string[]): string[]; runCapture?: typeof defaultRunCapture; cliName(): string; agentProductName(): string; @@ -215,13 +219,20 @@ export function createOnboardDashboardHelpers(deps: OnboardDashboardDeps): Onboa ): number { const { rollbackSandboxOnFailure = false } = options; const preferredPort = Number(getDashboardForwardPort(chatUiUrl)); + const stopForwardForSandbox = (port: string | number) => + bestEffortForwardStopForSandbox( + deps.runOpenshell, + (args, opts) => (deps.runCaptureOpenshell(args, opts) ?? "") as string, + port, + sandboxName, + ); let existingForwards = deps.runCaptureOpenshell(["forward", "list"], { ignoreError: true }); const preferredEntry = findForwardEntry(existingForwards, String(preferredPort)); if ( preferredEntry && (preferredEntry.sandboxName === sandboxName || !isLiveForwardStatus(preferredEntry.status)) ) { - bestEffortForwardStop(deps.runOpenshell, preferredPort); + stopForwardForSandbox(preferredPort); existingForwards = deps.runCaptureOpenshell(["forward", "list"], { ignoreError: true }); } let actualPort: number; @@ -248,26 +259,28 @@ export function createOnboardDashboardHelpers(deps: OnboardDashboardDeps): Onboa const occupied = getOccupiedPorts(existingForwards); for (const [port, owner] of occupied.entries()) { if (owner === sandboxName && Number(port) !== actualPort) { - bestEffortForwardStop(deps.runOpenshell, port); + stopForwardForSandbox(port); } } const parsedUrl = new URL(chatUiUrl.includes("://") ? chatUiUrl : `http://${chatUiUrl}`); parsedUrl.port = String(actualPort); const actualTarget = getDashboardForwardTarget(parsedUrl.toString()); - bestEffortForwardStop(deps.runOpenshell, actualPort); - const { result: fwdResult, diagnostic: fwdDiagnostic } = runBackgroundForwardStartWithPortReleaseRetries( - (stdio, timeout) => - deps.runOpenshell( - ["forward", "start", "--background", actualTarget, sandboxName], - { ignoreError: true, suppressOutput: true, stdio, timeout }, - ), + stopForwardForSandbox(actualPort); + const { ok: fwdOk, diagnostic: fwdDiagnostic } = runDetachedForwardStartWithPortReleaseRetries( + buildDetachedForwardStartSpawn( + deps.openshellArgv(["forward", "start", "--background", actualTarget, sandboxName]), + ), + () => + (deps.runCaptureOpenshell(["forward", "list"], { timeout: OPENSHELL_PROBE_TIMEOUT_MS }) ?? "") as string, + { port: actualPort, sandboxName }, () => { deps.sleep(1); - bestEffortForwardStop(deps.runOpenshell, actualPort); + stopForwardForSandbox(actualPort); }, + { onProgress: buildForwardStartProgressLogger(actualPort) }, ); - if (fwdResult && fwdResult.status !== 0) { + if (!fwdOk) { const looksLikePortConflict = looksLikeForwardPortConflict(fwdDiagnostic); if (rollbackSandboxOnFailure) { const err = new Error( diff --git a/src/lib/onboard/forward-cleanup.test.ts b/src/lib/onboard/forward-cleanup.test.ts new file mode 100644 index 0000000000..f43c2bfbd4 --- /dev/null +++ b/src/lib/onboard/forward-cleanup.test.ts @@ -0,0 +1,115 @@ +// 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 { + bestEffortForwardStop, + bestEffortForwardStopForSandbox, +} from "../../../dist/lib/onboard/forward-cleanup"; + +function forwardListWith(entries: Array<{ sandbox: string; port: number; status?: string }>): string { + const header = "SANDBOX BIND PORT PID STATUS"; + const rows = entries.map( + (e) => `${e.sandbox} 127.0.0.1 ${e.port} 1234 ${e.status ?? "running"}`, + ); + return [header, ...rows].join("\n"); +} + +describe("bestEffortForwardStop", () => { + it("invokes `forward stop` with the port and silently ignores errors", () => { + const run = vi.fn(); + bestEffortForwardStop(run, 18789); + expect(run).toHaveBeenCalledWith( + ["forward", "stop", "18789"], + { ignoreError: true, suppressOutput: true }, + ); + }); +}); + +describe("bestEffortForwardStopForSandbox", () => { + it("returns owned-other and skips stop when the port belongs to a different sandbox", () => { + const run = vi.fn(); + const fetch = vi + .fn() + .mockReturnValue(forwardListWith([{ sandbox: "other-sandbox", port: 18789 }])); + + const outcome = bestEffortForwardStopForSandbox(run, fetch, 18789, "my-sandbox"); + + expect(outcome).toBe("owned-other"); + expect(run).not.toHaveBeenCalled(); + expect(fetch).toHaveBeenCalledWith( + ["forward", "list"], + expect.objectContaining({ timeout: 15_000 }), + ); + // Caller must NOT pass ignoreError; failures should throw so the catch + // branch returns "list-failed" instead of running a stop with no owner data. + expect(fetch).not.toHaveBeenCalledWith( + ["forward", "list"], + expect.objectContaining({ ignoreError: true }), + ); + }); + + it("returns stopped and uses the sandbox-scoped forward stop form when ownership matches", () => { + const run = vi.fn(); + const fetch = vi + .fn() + .mockReturnValue(forwardListWith([{ sandbox: "my-sandbox", port: 18789 }])); + + const outcome = bestEffortForwardStopForSandbox(run, fetch, 18789, "my-sandbox"); + + expect(outcome).toBe("stopped"); + // Sandbox-scoped stop closes the TOCTOU window between list and stop: + // even if another sandbox bound the port in the meantime, openshell + // forward stop with both args will refuse to kill it. + expect(run).toHaveBeenCalledWith( + ["forward", "stop", "18789", "my-sandbox"], + { ignoreError: true, suppressOutput: true }, + ); + }); + + it("returns no-entry and runs a sandbox-scoped stop when no live forward is on that port", () => { + const run = vi.fn(); + const fetch = vi.fn().mockReturnValue(forwardListWith([])); + + const outcome = bestEffortForwardStopForSandbox(run, fetch, 18789, "my-sandbox"); + + expect(outcome).toBe("no-entry"); + expect(run).toHaveBeenCalledWith( + ["forward", "stop", "18789", "my-sandbox"], + { ignoreError: true, suppressOutput: true }, + ); + }); + + it("skips the stop entirely when `forward list` itself throws (owner unknown)", () => { + const run = vi.fn(); + const fetch = vi.fn().mockImplementation(() => { + throw new Error("gateway timed out"); + }); + + const outcome = bestEffortForwardStopForSandbox(run, fetch, 18789, "my-sandbox"); + + expect(outcome).toBe("list-failed"); + // Without ownership data, a port-only stop could kill another + // sandbox's forward — better to leave the port alone and let the + // helper's retry / next poll observe the real state. + expect(run).not.toHaveBeenCalled(); + }); + + it("ignores forwards with non-live status when deciding ownership", () => { + // `getOccupiedPorts` filters by `isLiveForwardStatus`, so a "stopped" + // entry on the requested port should be treated as no-entry (not as a + // foreign owner). + const run = vi.fn(); + const fetch = vi + .fn() + .mockReturnValue( + forwardListWith([{ sandbox: "other-sandbox", port: 18789, status: "stopped" }]), + ); + + const outcome = bestEffortForwardStopForSandbox(run, fetch, 18789, "my-sandbox"); + + expect(outcome).toBe("no-entry"); + expect(run).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/lib/onboard/forward-cleanup.ts b/src/lib/onboard/forward-cleanup.ts index 6f1eed8094..5ccffe71f5 100644 --- a/src/lib/onboard/forward-cleanup.ts +++ b/src/lib/onboard/forward-cleanup.ts @@ -1,11 +1,29 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import { OPENSHELL_PROBE_TIMEOUT_MS } from "../adapters/openshell/timeouts"; + +import { getOccupiedPorts } from "./dashboard-port"; + export type ForwardStopRunner = ( args: string[], opts: { ignoreError?: boolean; suppressOutput?: boolean }, ) => unknown; +export type ForwardListRunner = ( + args: string[], + opts: { ignoreError?: boolean; timeout?: number }, +) => string; + +/** + * `openshell forward stop ` — port-scoped, kills whatever forward is + * currently bound to that port. Use only when the caller has no sandbox + * context (or is intentionally targeting any owner — e.g. wholesale + * dashboard-port reclaim during recovery). Sandbox-aware call sites should + * prefer `bestEffortForwardStopForSandbox`, which uses the sandbox-scoped + * `forward stop ` form so the kill cannot collateral + * another sandbox's forward in a TOCTOU window. + */ export function bestEffortForwardStop( runOpenshell: ForwardStopRunner, port: string | number, @@ -15,3 +33,57 @@ export function bestEffortForwardStop( suppressOutput: true, }); } + +/** + * Stop the forward on `port` only when `openshell forward list` reports it + * is owned by `sandboxName` (or unowned). When the list query fails the + * stop is skipped — without ownership data we cannot tell whether port + * belongs to a concurrent onboard / connect on a different sandbox, so the + * safe choice is to leave the port alone and let the helper's retry path + * (or the next forward list poll once the gateway is responsive) sort + * itself out. + * + * The stop itself uses the sandbox-scoped `forward stop ` + * form so a TOCTOU window between list and stop cannot accidentally kill + * another sandbox's forward that bound to the same port in the meantime. + * + * Returns: + * - "stopped" — entry matched sandboxName and the stop ran. + * - "owned-other" — entry exists for a different sandbox; stop skipped. + * - "no-entry" — no live entry for that port; stop ran defensively + * (sandbox-scoped, so a concurrent racer's forward + * that may have bound the port between list and stop + * is left alone). + * - "list-failed" — could not enumerate forwards; stop SKIPPED. The + * owner is unknown and the port-only `forward stop` + * could kill an unrelated sandbox's forward. + */ +export function bestEffortForwardStopForSandbox( + runOpenshell: ForwardStopRunner, + runCaptureOpenshell: ForwardListRunner, + port: string | number, + sandboxName: string, +): "stopped" | "owned-other" | "no-entry" | "list-failed" { + // Let runCaptureOpenshell throw on failure/timeout so the catch branch + // returns "list-failed". With ignoreError: true the runner would swallow + // the error and return "", which getOccupiedPorts parses as an empty map + // and the "no-entry" branch below would still run the stop — exactly the + // collateral-damage case this helper exists to avoid. + let listOutput = ""; + try { + listOutput = runCaptureOpenshell(["forward", "list"], { + timeout: OPENSHELL_PROBE_TIMEOUT_MS, + }); + } catch { + return "list-failed"; + } + const owner = getOccupiedPorts(listOutput).get(String(port)) ?? null; + if (owner && owner !== sandboxName) { + return "owned-other"; + } + runOpenshell(["forward", "stop", String(port), sandboxName], { + ignoreError: true, + suppressOutput: true, + }); + return owner === sandboxName ? "stopped" : "no-entry"; +} diff --git a/src/lib/onboard/forward-start.test.ts b/src/lib/onboard/forward-start.test.ts new file mode 100644 index 0000000000..7a4cfefc35 --- /dev/null +++ b/src/lib/onboard/forward-start.test.ts @@ -0,0 +1,385 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import fs from "node:fs"; + +import { describe, expect, it, vi } from "vitest"; + +import { + buildDetachedForwardStartSpawn, + looksLikeForwardPortConflict, + runDetachedForwardStartWithDiagnostics, + runDetachedForwardStartWithPortReleaseRetries, +} from "../../../dist/lib/onboard/forward-start"; + +// Build an `openshell forward list`-shaped output for the given live entries. +// Mirrors the column layout (SANDBOX BIND PORT PID STATUS) that +// `getOccupiedPorts` parses, so the helper recognises the forward as live. +function forwardListWith(entries: Array<{ sandbox: string; port: number; status?: string }>): string { + const header = "SANDBOX BIND PORT PID STATUS"; + const rows = entries.map( + (e) => `${e.sandbox} 127.0.0.1 ${e.port} 1234 ${e.status ?? "running"}`, + ); + return [header, ...rows].join("\n"); +} + +describe("runDetachedForwardStartWithDiagnostics", () => { + it("returns ok as soon as the forward appears in the list", () => { + const fetchList = vi + .fn() + .mockReturnValueOnce(forwardListWith([])) // first poll: nothing yet + .mockReturnValue(forwardListWith([{ sandbox: "my-sandbox", port: 18789 }])); + const spawn = vi.fn().mockReturnValue({ pid: 42 }); + const sleep = vi.fn(); + + const result = runDetachedForwardStartWithDiagnostics( + spawn, + fetchList, + { port: 18789, sandboxName: "my-sandbox" }, + { overallTimeoutMs: 10_000, pollIntervalMs: 10, sleepMs: sleep }, + ); + + expect(result.ok).toBe(true); + expect(result.reason).toBe("ok"); + expect(result.pid).toBe(42); + expect(spawn).toHaveBeenCalledTimes(1); + // First poll missed → one sleep before the second poll observed the entry. + expect(sleep).toHaveBeenCalledTimes(1); + }); + + it("ignores entries that belong to a different sandbox", () => { + const fetchList = vi + .fn() + .mockReturnValue(forwardListWith([{ sandbox: "other-sandbox", port: 18789 }])); + const spawn = vi.fn().mockReturnValue({ pid: 42 }); + const sleep = vi.fn(); + + const result = runDetachedForwardStartWithDiagnostics( + spawn, + fetchList, + { port: 18789, sandboxName: "my-sandbox" }, + { overallTimeoutMs: 50, pollIntervalMs: 10, sleepMs: sleep }, + ); + + expect(result.ok).toBe(false); + expect(result.reason).toBe("timeout"); + }); + + it("reports timeout when the forward never appears", () => { + const fetchList = vi.fn().mockReturnValue(forwardListWith([])); + const spawn = vi.fn().mockReturnValue({ pid: 42 }); + const sleep = vi.fn(); + + const result = runDetachedForwardStartWithDiagnostics( + spawn, + fetchList, + { port: 18789, sandboxName: "my-sandbox" }, + { overallTimeoutMs: 30, pollIntervalMs: 10, sleepMs: sleep }, + ); + + expect(result.ok).toBe(false); + expect(result.reason).toBe("timeout"); + expect(result.diagnostic).toMatch(/forward did not appear in list within 30ms/); + }); + + it("surfaces spawn errors immediately without polling", () => { + const fetchList = vi.fn(); + const spawn = vi.fn().mockReturnValue({ error: new Error("ENOENT: openshell not found") }); + const sleep = vi.fn(); + + const result = runDetachedForwardStartWithDiagnostics( + spawn, + fetchList, + { port: 18789, sandboxName: "my-sandbox" }, + { overallTimeoutMs: 10_000, pollIntervalMs: 10, sleepMs: sleep }, + ); + + expect(result.ok).toBe(false); + expect(result.reason).toBe("spawn-error"); + expect(result.diagnostic).toMatch(/ENOENT/); + expect(fetchList).not.toHaveBeenCalled(); + expect(sleep).not.toHaveBeenCalled(); + }); + + it("preflights argv[0] and short-circuits on a missing openshell binary", () => { + const fetchList = vi.fn().mockReturnValue(""); + const sleep = vi.fn(); + // Real `buildDetachedForwardStartSpawn` checks `fs.accessSync(argv[0], + // X_OK)` before spawning, so a missing binary surfaces as a synchronous + // spawn-error instead of relying on Node's async `error` event (which + // cannot fire while the helper is sleeping inside spawnSync). + const spawn = buildDetachedForwardStartSpawn(["/nonexistent/openshell-binary-for-test"]); + + const result = runDetachedForwardStartWithDiagnostics( + spawn, + fetchList, + { port: 18789, sandboxName: "my-sandbox" }, + { overallTimeoutMs: 5_000, pollIntervalMs: 5, sleepMs: sleep }, + ); + + expect(result.ok).toBe(false); + expect(result.reason).toBe("spawn-error"); + expect(result.diagnostic).toMatch(/ENOENT|EACCES|no such file|permission denied/i); + // No polling should have happened; the helper returned at the spawn + // preflight step. + expect(fetchList).not.toHaveBeenCalled(); + expect(sleep).not.toHaveBeenCalled(); + }); + + it("invokes onProgress while waiting for the forward to appear", () => { + let now = 0; + const realNow = Date.now; + Date.now = () => now; + try { + const fetchList = vi.fn().mockReturnValue(""); + const spawn = vi.fn().mockReturnValue({ pid: 42 }); + const sleep = vi.fn().mockImplementation((ms) => { + now += ms; + }); + const onProgress = vi.fn(); + + const result = runDetachedForwardStartWithDiagnostics( + spawn, + fetchList, + { port: 18789, sandboxName: "my-sandbox" }, + { + overallTimeoutMs: 120_000, + pollIntervalMs: 1_000, + sleepMs: sleep, + onProgress, + progressIntervalMs: 30_000, + }, + ); + + expect(result.ok).toBe(false); + expect(onProgress).toHaveBeenCalled(); + const calls = onProgress.mock.calls; + expect(calls.length).toBeGreaterThanOrEqual(3); + expect(calls[0][0].elapsedMs).toBeGreaterThanOrEqual(30_000); + expect(result.diagnostic).toMatch(/forward did not appear in list within 120000ms/); + expect(result.diagnostic).toMatch(/last forward list: /); + } finally { + Date.now = realNow; + } + }); + + it("surfaces persistent fetchForwardList failures in the timeout diagnostic", () => { + const fetchList = vi.fn().mockImplementation(() => { + throw new Error("gateway transport: connection refused"); + }); + const spawn = vi.fn().mockReturnValue({ pid: 42 }); + const sleep = vi.fn(); + + const result = runDetachedForwardStartWithDiagnostics( + spawn, + fetchList, + { port: 18789, sandboxName: "my-sandbox" }, + { overallTimeoutMs: 30, pollIntervalMs: 10, sleepMs: sleep }, + ); + + expect(result.ok).toBe(false); + expect(result.reason).toBe("timeout"); + expect(result.diagnostic).toMatch(/openshell forward list failed/); + expect(result.diagnostic).toMatch(/connection refused/); + }); + + it("treats fetchForwardList exceptions as transient and keeps polling", () => { + const fetchList = vi + .fn() + .mockImplementationOnce(() => { + throw new Error("gateway not reachable yet"); + }) + .mockReturnValue(forwardListWith([{ sandbox: "my-sandbox", port: 18789 }])); + const spawn = vi.fn().mockReturnValue({ pid: 42 }); + const sleep = vi.fn(); + + const result = runDetachedForwardStartWithDiagnostics( + spawn, + fetchList, + { port: 18789, sandboxName: "my-sandbox" }, + { overallTimeoutMs: 10_000, pollIntervalMs: 10, sleepMs: sleep }, + ); + + expect(result.ok).toBe(true); + expect(fetchList).toHaveBeenCalledTimes(2); + }); + + it("clears a transient fetch error from the diagnostic when a later poll succeeds", () => { + const fetchList = vi + .fn() + .mockImplementationOnce(() => { + throw new Error("transient gateway: connection refused"); + }) + .mockReturnValue(""); + const spawn = vi.fn().mockReturnValue({ pid: 42 }); + const sleep = vi.fn(); + + const result = runDetachedForwardStartWithDiagnostics( + spawn, + fetchList, + { port: 18789, sandboxName: "my-sandbox" }, + { overallTimeoutMs: 30, pollIntervalMs: 10, sleepMs: sleep }, + ); + + expect(result.ok).toBe(false); + expect(result.reason).toBe("timeout"); + expect(result.diagnostic).not.toMatch(/openshell forward list failed/); + }); + + it("SIGTERMs the detached child on timeout", () => { + const fetchList = vi.fn().mockReturnValue(""); + const spawn = vi.fn().mockReturnValue({ pid: 4242 }); + const sleep = vi.fn(); + const realKill = process.kill; + const killSpy = vi.fn(); + // Replace process.kill so the test does not actually try to signal pid 4242. + (process as { kill: typeof process.kill }).kill = killSpy as unknown as typeof process.kill; + try { + const result = runDetachedForwardStartWithDiagnostics( + spawn, + fetchList, + { port: 18789, sandboxName: "my-sandbox" }, + { overallTimeoutMs: 20, pollIntervalMs: 10, sleepMs: sleep }, + ); + expect(result.ok).toBe(false); + expect(result.reason).toBe("timeout"); + expect(killSpy).toHaveBeenCalledWith(4242, "SIGTERM"); + } finally { + (process as { kill: typeof process.kill }).kill = realKill; + } + }); + + it("SIGTERMs the detached child on a port-conflict diagnostic", () => { + // Spawn writes an EADDRINUSE line to the stderr file descriptor so the + // first poll iteration reads it back and trips the conflict branch. + const fetchList = vi.fn().mockReturnValue(""); + const spawn = vi.fn().mockImplementation(({ stderr }: { stderr: number }) => { + fs.writeSync(stderr, "listen tcp 0.0.0.0:18789: bind: address already in use\n"); + return { pid: 8888 }; + }); + const sleep = vi.fn(); + const realKill = process.kill; + const killSpy = vi.fn(); + (process as { kill: typeof process.kill }).kill = killSpy as unknown as typeof process.kill; + try { + const result = runDetachedForwardStartWithDiagnostics( + spawn, + fetchList, + { port: 18789, sandboxName: "my-sandbox" }, + { overallTimeoutMs: 1_000, pollIntervalMs: 10, sleepMs: sleep }, + ); + expect(result.ok).toBe(false); + expect(result.reason).toBe("spawn-conflict"); + expect(killSpy).toHaveBeenCalledWith(8888, "SIGTERM"); + } finally { + (process as { kill: typeof process.kill }).kill = realKill; + } + }); + + it("does not SIGTERM when spawn never produced a pid", () => { + const fetchList = vi.fn(); + const spawn = vi.fn().mockReturnValue({ error: new Error("ENOENT") }); + const sleep = vi.fn(); + const realKill = process.kill; + const killSpy = vi.fn(); + (process as { kill: typeof process.kill }).kill = killSpy as unknown as typeof process.kill; + try { + const result = runDetachedForwardStartWithDiagnostics( + spawn, + fetchList, + { port: 18789, sandboxName: "my-sandbox" }, + { overallTimeoutMs: 1_000, pollIntervalMs: 10, sleepMs: sleep }, + ); + expect(result.reason).toBe("spawn-error"); + expect(killSpy).not.toHaveBeenCalled(); + } finally { + (process as { kill: typeof process.kill }).kill = realKill; + } + }); +}); + +describe("runDetachedForwardStartWithPortReleaseRetries", () => { + it("retries after a port-conflict diagnostic, then succeeds", () => { + const fetchList = vi + .fn() + .mockReturnValueOnce(forwardListWith([])) // first attempt: never appears + .mockReturnValueOnce(forwardListWith([])) // (timeout settles) + .mockReturnValue(forwardListWith([{ sandbox: "my-sandbox", port: 18789 }])); + const beforeRetry = vi.fn(); + // First spawn surfaces a port-conflict in its diagnostic synthesised via + // an Error message; the second spawn succeeds and the forward appears. + const spawn = vi + .fn() + .mockReturnValueOnce({ error: new Error("EADDRINUSE: address already in use") }) + .mockReturnValueOnce({ pid: 99 }); + const sleep = vi.fn(); + + const result = runDetachedForwardStartWithPortReleaseRetries( + spawn, + fetchList, + { port: 18789, sandboxName: "my-sandbox" }, + beforeRetry, + { overallTimeoutMs: 30, pollIntervalMs: 10, sleepMs: sleep, maxRetries: 3 }, + ); + + expect(result.ok).toBe(true); + expect(beforeRetry).toHaveBeenCalledTimes(1); + expect(spawn).toHaveBeenCalledTimes(2); + }); + + it("does not retry when the failure does not look like a port conflict", () => { + const fetchList = vi.fn().mockReturnValue(forwardListWith([])); + const beforeRetry = vi.fn(); + const spawn = vi.fn().mockReturnValue({ pid: 42 }); + const sleep = vi.fn(); + + const result = runDetachedForwardStartWithPortReleaseRetries( + spawn, + fetchList, + { port: 18789, sandboxName: "my-sandbox" }, + beforeRetry, + { overallTimeoutMs: 20, pollIntervalMs: 10, sleepMs: sleep, maxRetries: 3 }, + ); + + expect(result.ok).toBe(false); + expect(result.reason).toBe("timeout"); + expect(beforeRetry).not.toHaveBeenCalled(); + expect(spawn).toHaveBeenCalledTimes(1); + }); + + it("stops retrying after maxRetries even if conflict diagnostics persist", () => { + const fetchList = vi.fn().mockReturnValue(forwardListWith([])); + const beforeRetry = vi.fn(); + const spawn = vi + .fn() + .mockReturnValue({ error: new Error("EADDRINUSE: address already in use") }); + const sleep = vi.fn(); + + const result = runDetachedForwardStartWithPortReleaseRetries( + spawn, + fetchList, + { port: 18789, sandboxName: "my-sandbox" }, + beforeRetry, + { overallTimeoutMs: 20, pollIntervalMs: 10, sleepMs: sleep, maxRetries: 2 }, + ); + + expect(result.ok).toBe(false); + expect(beforeRetry).toHaveBeenCalledTimes(2); + expect(spawn).toHaveBeenCalledTimes(3); // initial + 2 retries + }); +}); + +describe("looksLikeForwardPortConflict", () => { + it("matches the common port-in-use signals", () => { + expect(looksLikeForwardPortConflict("listen tcp 0.0.0.0:18789: bind: address already in use")).toBe( + true, + ); + expect(looksLikeForwardPortConflict("EADDRINUSE")).toBe(true); + expect(looksLikeForwardPortConflict("port 18789 in use")).toBe(true); + }); + + it("returns false for unrelated errors", () => { + expect(looksLikeForwardPortConflict("transport: connection refused")).toBe(false); + expect(looksLikeForwardPortConflict("")).toBe(false); + }); +}); diff --git a/src/lib/onboard/forward-start.ts b/src/lib/onboard/forward-start.ts index 05e45e958a..a9274f1716 100644 --- a/src/lib/onboard/forward-start.ts +++ b/src/lib/onboard/forward-start.ts @@ -1,25 +1,54 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import { spawn as spawnChild } from "node:child_process"; 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 { getOccupiedPorts } from "./dashboard-port"; import { cleanupTempDir, secureTempFile } from "./temp-files"; -export type BackgroundForwardStartResult = { - status: number | null; - stdout?: string | Buffer | null; - stderr?: string | Buffer | null; - error?: Error; -}; +// `openshell forward start --background` daemonises the actual forward +// process, but the parent CLI's stdio is inherited by the daemon child on +// some platforms (notably the Docker compatibility gateway used when the +// host glibc is older than the openshell-gateway requirement). spawnSync +// then waits on those fds until the daemon exits — minutes later — and +// reports ETIMEDOUT even though the forward is established. +// +// The detached path below spawns the CLI with `detached: true`, hands it +// independent diagnostic file descriptors, and confirms success by polling +// `openshell forward list` for an entry matching `(port, sandboxName)`. +// The CLI's exit code is no longer the success signal — the appearance of +// the live forward in the list is. -export type BackgroundForwardStartRunner = ( - stdio: StdioOptions, - timeoutMs: number, -) => BackgroundForwardStartResult; +export type ForwardListFetcher = () => string; + +export type DetachedForwardSpawnRunner = ( + stdio: { stdout: number; stderr: number }, +) => { pid?: number; error?: Error }; + +export interface DetachedForwardStartOutcome { + ok: boolean; + diagnostic: string; + pid?: number; + reason: "ok" | "spawn-error" | "timeout" | "spawn-conflict"; +} + +export interface DetachedForwardStartOptions { + overallTimeoutMs?: number; + pollIntervalMs?: number; + sleepMs?: (ms: number) => void; + // Called once per `progressIntervalMs` while the helper is still waiting + // for the forward to appear in `openshell forward list`. The default is a + // no-op so the helper stays terminal-quiet in non-interactive contexts. + onProgress?: (info: { elapsedMs: number; listSnapshot: string }) => void; + progressIntervalMs?: number; + // Number of EADDRINUSE-style retries after the initial attempt. Honoured + // only by `runDetachedForwardStartWithPortReleaseRetries`. Defaults to 3. + maxRetries?: number; +} function readDiagnosticFile(filePath: string): string { try { @@ -41,26 +70,153 @@ export function looksLikeForwardPortConflict(diagnostic: string): boolean { return /eaddrinuse|address already in use|port .* in use|bind: .*in use/i.test(diagnostic); } -export function runBackgroundForwardStartWithDiagnostics( - runForwardStart: BackgroundForwardStartRunner, - timeoutMs = 30_000, -): { result: BackgroundForwardStartResult; diagnostic: string } { +function blockingSleepMs(ms: number): void { + if (ms <= 0) return; + // Synchronous sleep — onboard's forward-start sits in a sync code path, + // so we cannot await. spawnSync of `node -e setTimeout` is the same + // primitive `sleepMs` in core/wait uses, but we keep the call site + // injectable so tests can stub it without spawning subprocesses. We + // intentionally do NOT `.unref()` the timer in the child: an unref'd + // timer lets the child's event loop drain immediately, so spawnSync + // returns instantly and the caller spins through the poll loop without + // actually waiting. + const { spawnSync } = require("node:child_process"); + spawnSync(process.execPath, ["-e", `setTimeout(() => {}, ${ms});`], { + stdio: "ignore", + timeout: ms + 5_000, + }); +} + +/** + * Build a `DetachedForwardSpawnRunner` that spawns the given argv as a + * detached child, writing stdio to the file descriptors supplied by + * `runDetachedForwardStartWithDiagnostics`. Kept in this module so the + * onboard call site stays a thin wire-up and the spawn-on-Node detail + * (`detached: true` + `unref()`) lives next to the consumer that relies + * on it. + */ +export function buildDetachedForwardStartSpawn( + argv: readonly string[], +): DetachedForwardSpawnRunner { + return ({ stdout, stderr }) => { + // Preflight: the helper polls synchronously, so a Node `error` event + // dispatched after `spawn` returns cannot reach the poll loop while it + // is sleeping on a `spawnSync` child. Catch the obvious ENOENT/EACCES + // cases up front via `fs.accessSync` so the helper returns the real + // failure immediately instead of timing out 180s later. + try { + fs.accessSync(argv[0], fs.constants.X_OK); + } catch (e) { + return { error: e instanceof Error ? e : new Error(String(e)) }; + } + try { + const child = spawnChild(argv[0], argv.slice(1), { + stdio: ["ignore", stdout, stderr], + detached: true, + }); + // Swallow any belated `error` event so a race between accessSync and + // execve does not crash the process via an unhandled emitter. + child.on("error", () => {}); + // A null/undefined pid means execve failed even though the preflight + // succeeded (race against permission changes, ulimit, etc.). The async + // `error` event would otherwise be swallowed by the listener above and + // the caller would wait the full deadline for a child that never ran. + if (child.pid == null) { + return { error: new Error(`spawn ${argv[0]} returned no pid`) }; + } + child.unref(); + return { pid: child.pid }; + } catch (e) { + return { error: e instanceof Error ? e : new Error(String(e)) }; + } + }; +} + +function isForwardConfirmed( + forwardListOutput: string, + expect: { port: number; sandboxName: string }, +): boolean { + return getOccupiedPorts(forwardListOutput).get(String(expect.port)) === expect.sandboxName; +} + +/** + * Best-effort SIGTERM of the detached `openshell forward start --background` + * process when the helper gives up. Without this, a slow gateway handshake + * can still register a forward minutes after onboard already rolled the + * sandbox back, causing the next onboard attempt on the same port to race + * an orphan CLI for the dashboard. `kill` swallows ESRCH so a child that + * already exited is a no-op. + */ +function terminateDetachedForwardChild(pid: number | undefined): void { + if (!pid || pid <= 0) return; + try { + process.kill(pid, "SIGTERM"); + } catch { + /* already exited or out of our reach */ + } +} + +/** + * Default progress logger for the detached forward-start helper. Emits a + * single line to stdout every `progressIntervalMs` while the helper is + * still polling. Kept here so the onboard call site does not need to + * recreate the same closure inline. + */ +export function buildForwardStartProgressLogger(port: number): (info: { elapsedMs: number }) => void { + return ({ elapsedMs }) => { + console.log( + ` Still waiting for forward on port ${port} to register (${Math.round(elapsedMs / 1000)}s elapsed)...`, + ); + }; +} + +/** + * Spawn `openshell forward start --background` as a detached child and wait + * for the resulting forward to appear in `openshell forward list`. Returns + * `ok: true` as soon as the live entry is observed, regardless of whether + * the original spawn process has exited yet. Returns `ok: false` with a + * captured diagnostic when: + * - the spawn itself failed (ENOENT, permission denied, …); + * - the parent process wrote an EADDRINUSE-style error to stderr before + * the deadline (port conflict — retry path); + * - the deadline expired without the forward appearing. + * + * The diagnostic file pair is removed before return, so the temp dir does + * not leak across retries. + */ +export function runDetachedForwardStartWithDiagnostics( + runDetachedSpawn: DetachedForwardSpawnRunner, + fetchForwardList: ForwardListFetcher, + expect: { port: number; sandboxName: string }, + options: DetachedForwardStartOptions = {}, +): DetachedForwardStartOutcome { + // 180s deadline accommodates Docker compatibility gateways (host glibc + // older than openshell-gateway's requirement runs the gateway in an extra + // Docker container, adding per-call gRPC latency that can push the + // forward-registration handshake past a tighter timeout). + const overallTimeoutMs = options.overallTimeoutMs ?? 180_000; + const pollIntervalMs = options.pollIntervalMs ?? 500; + const sleepImpl = options.sleepMs ?? blockingSleepMs; + const onProgress = options.onProgress; + const progressIntervalMs = options.progressIntervalMs ?? 30_000; + let nextProgressAt = Date.now() + progressIntervalMs; + 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; + // `fs.openSync` with `"w"` truncates / creates the diagnostic files; the + // child inherits the fds via posix_spawn semantics. We close the host's + // copies immediately so only the child's reference keeps them alive, + // which lets the kernel reclaim them when the (detached) child exits. const outFd = fs.openSync(forwardDiagPath, "w", 0o600); const errFd = fs.openSync(forwardErrPath, "w", 0o600); + let pid: number | undefined; + let spawnError: Error | undefined; try { - try { - result = runForwardStart(["ignore", outFd, errFd], timeoutMs); - } catch (error) { - result = { - status: null, - error: error instanceof Error ? error : new Error(String(error)), - }; - } + const spawnResult = runDetachedSpawn({ stdout: outFd, stderr: errFd }); + pid = spawnResult.pid; + spawnError = spawnResult.error; } finally { try { fs.closeSync(outFd); @@ -74,32 +230,102 @@ export function runBackgroundForwardStartWithDiagnostics( } } - try { + let lastFetchError: string | null = null; + const readDiag = (): string => { const stderr = readDiagnosticFile(forwardErrPath); const stdout = readDiagnosticFile(forwardDiagPath); - const message = result?.error instanceof Error ? result.error.message : ""; + const message = spawnError instanceof Error ? spawnError.message : ""; + const fetchSuffix = lastFetchError ? ` openshell forward list failed: ${lastFetchError}` : ""; + return compactText(redact(`${stderr} ${stdout} ${message}${fetchSuffix}`)); + }; + + try { + if (spawnError) { + return { ok: false, diagnostic: readDiag(), pid, reason: "spawn-error" }; + } + + const start = Date.now(); + const deadline = start + overallTimeoutMs; + let lastListSnapshot = ""; + while (Date.now() < deadline) { + let list = ""; + try { + list = fetchForwardList() || ""; + // Clear the cached transient error so a recovered gateway does not + // leave a stale "openshell forward list failed: …" suffix on the + // eventual timeout diagnostic. + lastFetchError = null; + } catch (err) { + lastFetchError = err instanceof Error ? err.message : String(err); + } + lastListSnapshot = list; + if (isForwardConfirmed(list, expect)) { + return { ok: true, diagnostic: readDiag(), pid, reason: "ok" }; + } + const diagSoFar = readDiag(); + if (looksLikeForwardPortConflict(diagSoFar)) { + terminateDetachedForwardChild(pid); + return { ok: false, diagnostic: diagSoFar, pid, reason: "spawn-conflict" }; + } + if (onProgress && Date.now() >= nextProgressAt) { + onProgress({ elapsedMs: Date.now() - start, listSnapshot: list }); + nextProgressAt = Date.now() + progressIntervalMs; + } + sleepImpl(pollIntervalMs); + } + const finalDiag = readDiag(); + const listTail = lastListSnapshot + ? ` last forward list: ${compactText(redact(lastListSnapshot)).slice(0, 240)}` + : " last forward list: "; + const timeoutSummary = `forward did not appear in list within ${overallTimeoutMs}ms;${listTail}`; + // The detached `openshell forward start --background` process may still + // be running (e.g. blocked on a slow gateway handshake). If the caller + // is about to roll back the sandbox, leaving an orphan CLI that may yet + // succeed would race with the next onboard attempt for the same port. + terminateDetachedForwardChild(pid); return { - result: result ?? { status: null, error: new Error("forward start did not return a result") }, - diagnostic: compactText(redact(`${stderr} ${stdout} ${message}`)), + ok: false, + diagnostic: finalDiag ? `${timeoutSummary} ${finalDiag}` : timeoutSummary, + pid, + reason: "timeout", }; } finally { cleanupTempDir(forwardDiagPath, "nemoclaw-forward-start"); } } -export function runBackgroundForwardStartWithPortReleaseRetries( - runForwardStart: BackgroundForwardStartRunner, +/** + * Retry the detached forward-start when the diagnostic looks like an + * EADDRINUSE-style port conflict. `beforeRetry` runs between attempts so + * the caller can drop any stale forward bound to the same port before + * trying again. + */ +export function runDetachedForwardStartWithPortReleaseRetries( + runDetachedSpawn: DetachedForwardSpawnRunner, + fetchForwardList: ForwardListFetcher, + expect: { port: number; sandboxName: string }, beforeRetry: () => void, - maxRetries = 3, -): { result: BackgroundForwardStartResult; diagnostic: string } { - let attempt = runBackgroundForwardStartWithDiagnostics(runForwardStart); + options: DetachedForwardStartOptions = {}, +): DetachedForwardStartOutcome { + const maxRetries = options.maxRetries ?? 3; + let attempt = runDetachedForwardStartWithDiagnostics( + runDetachedSpawn, + fetchForwardList, + expect, + options, + ); for ( let retries = 0; - attempt.result.status !== 0 && looksLikeForwardPortConflict(attempt.diagnostic) && retries < maxRetries; + !attempt.ok && looksLikeForwardPortConflict(attempt.diagnostic) && retries < maxRetries; retries++ ) { beforeRetry(); - attempt = runBackgroundForwardStartWithDiagnostics(runForwardStart); + attempt = runDetachedForwardStartWithDiagnostics( + runDetachedSpawn, + fetchForwardList, + expect, + options, + ); } return attempt; } diff --git a/test/onboard-custom-dockerfile.test.ts b/test/onboard-custom-dockerfile.test.ts index cad7ac0b8d..af82a197da 100644 --- a/test/onboard-custom-dockerfile.test.ts +++ b/test/onboard-custom-dockerfile.test.ts @@ -119,7 +119,9 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - const cmd = _n(args[1][1]); + child.unref = () => {}; + child.pid = 4242; + const cmd = _n([args[0], ...(Array.isArray(args[1]) ? args[1] : [])]); commands.push({ command: cmd, env: args[2]?.env || null }); // Observe the staged build context state while the sandbox create is in // flight — onboard deletes it once streamSandboxCreate resolves. diff --git a/test/onboard-dashboard.test.ts b/test/onboard-dashboard.test.ts index ecb9aaaedb..c8408934f0 100644 --- a/test/onboard-dashboard.test.ts +++ b/test/onboard-dashboard.test.ts @@ -1,11 +1,16 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; const { getPortConflictServiceHints } = require("../dist/lib/onboard") as { getPortConflictServiceHints: (platform?: string) => string[]; }; +const { createOnboardDashboardHelpers } = require("../dist/lib/onboard/dashboard") as { + createOnboardDashboardHelpers: (deps: Record) => { + ensureDashboardForward: (sandboxName: string, chatUiUrl?: string) => number; + }; +}; describe("onboard dashboard helpers", () => { it("prints platform-appropriate service hints for port conflicts", () => { @@ -15,4 +20,45 @@ describe("onboard dashboard helpers", () => { /systemctl --user stop openclaw-gateway.service/, ); }); + + it("uses sandbox-scoped forward stops for same-sandbox dashboard cleanup", () => { + const forwardList = + "SANDBOX BIND PORT PID STATUS\n" + + "my-sandbox 127.0.0.1 18789 12345 running\n" + + "my-sandbox 127.0.0.1 19000 12346 running"; + const runOpenshell = vi.fn((_args: string[], _opts?: Record) => ({ + status: 0, + })); + const runCaptureOpenshell = vi.fn((args: string[], _opts?: Record) => + args.join(" ") === "forward list" ? forwardList : "", + ); + const helpers = createOnboardDashboardHelpers({ + runOpenshell, + runCaptureOpenshell, + openshellArgv: (args: string[]) => [process.execPath, "-e", "", ...args], + cliName: () => "nemoclaw", + agentProductName: () => "NemoClaw", + getProviderLabel: (provider: string) => provider, + note: vi.fn(), + isWsl: () => false, + redact: (value: unknown) => String(value), + sleep: vi.fn(), + printAgentDashboardUi: vi.fn(), + }); + + expect(helpers.ensureDashboardForward("my-sandbox", "http://127.0.0.1:18789")).toBe(18789); + + const stopArgs = runOpenshell.mock.calls.map(([args]) => args); + expect(stopArgs).toContainEqual(["forward", "stop", "18789", "my-sandbox"]); + expect(stopArgs).toContainEqual(["forward", "stop", "19000", "my-sandbox"]); + expect( + stopArgs.some( + (args) => + Array.isArray(args) && + args[0] === "forward" && + args[1] === "stop" && + args.length === 3, + ), + ).toBe(false); + }); }); diff --git a/test/onboard-messaging.test.ts b/test/onboard-messaging.test.ts index 36caddfc39..7056e9a75f 100644 --- a/test/onboard-messaging.test.ts +++ b/test/onboard-messaging.test.ts @@ -72,7 +72,7 @@ runner.runCapture = (command) => { if (_n(command).includes("sandbox get my-assistant")) return ""; if (_n(command).includes("sandbox list")) return "my-assistant Ready"; if (_n(command).includes("provider get")) return "Provider: discord-bridge"; - if (_n(command).includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running"; + if (_n(command).includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running\nmy-assistant 127.0.0.1 8642 12346 running"; { const sandboxExecCurl = require(${onboardScriptMocksPath}).mockSandboxExecCurl(command, { defaultCurlOutput: "ok", @@ -92,7 +92,9 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - const command = _n(args[1][1]); + child.unref = () => {}; + child.pid = 4242; + const command = _n([args[0], ...(Array.isArray(args[1]) ? args[1] : [])]); const entry = { command, env: args[2]?.env || null }; const policyMatch = command.match(/--policy ([^ ]+)/); if (policyMatch) { @@ -327,7 +329,7 @@ runner.run = (command, opts = {}) => { runner.runCapture = (command) => { if (_n(command).includes("sandbox get my-assistant")) return ""; if (_n(command).includes("sandbox list")) return "my-assistant Ready"; - if (_n(command).includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running"; + if (_n(command).includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running\nmy-assistant 127.0.0.1 8642 12346 running"; { const sandboxExecCurl = require(${onboardScriptMocksPath}).mockSandboxExecCurl(command, { defaultCurlOutput: "ok", @@ -350,7 +352,9 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - const command = _n(args[1][1]); + child.unref = () => {}; + child.pid = 4242; + const command = _n([args[0], ...(Array.isArray(args[1]) ? args[1] : [])]); const entry = { command, env: args[2]?.env || null }; const policyMatch = command.match(/--policy ([^ ]+)/); if (policyMatch) { @@ -516,7 +520,7 @@ runner.runCapture = (command) => { const sandboxExecCurl = require(${onboardScriptMocksPath}).mockSandboxExecCurl(command); if (sandboxExecCurl !== null) return sandboxExecCurl; } - if (_n(command).includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running"; + if (_n(command).includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running\nmy-assistant 127.0.0.1 8642 12346 running"; return ""; }; registry.registerSandbox = (entry) => { @@ -533,7 +537,9 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - const command = _n(args[1][1]); + child.unref = () => {}; + child.pid = 4242; + const command = _n([args[0], ...(Array.isArray(args[1]) ? args[1] : [])]); const entry = { command, env: args[2]?.env || null }; const dockerfileMatch = command.match(/--from ([^ ]+Dockerfile)/); if (dockerfileMatch) { @@ -681,7 +687,7 @@ runner.runCapture = (command) => { const sandboxExecCurl = require(${onboardScriptMocksPath}).mockSandboxExecCurl(command); if (sandboxExecCurl !== null) return sandboxExecCurl; } - if (_n(command).includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running"; + if (_n(command).includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running\nmy-assistant 127.0.0.1 8642 12346 running"; return ""; }; registry.registerSandbox = (entry) => { @@ -698,7 +704,9 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - const command = _n(args[1][1]); + child.unref = () => {}; + child.pid = 4242; + const command = _n([args[0], ...(Array.isArray(args[1]) ? args[1] : [])]); const entry = { command, env: args[2]?.env || null }; const dockerfileMatch = command.match(/--from ([^ ]+Dockerfile)/); if (dockerfileMatch) { @@ -840,7 +848,7 @@ runner.runCapture = (command) => { const sandboxExecCurl = require(${onboardScriptMocksPath}).mockSandboxExecCurl(command); if (sandboxExecCurl !== null) return sandboxExecCurl; } - if (_n(command).includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running"; + if (_n(command).includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running\nmy-assistant 127.0.0.1 8642 12346 running"; return ""; }; registry.registerSandbox = (entry) => { @@ -857,7 +865,9 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - const command = _n(args[1][1]); + child.unref = () => {}; + child.pid = 4242; + const command = _n([args[0], ...(Array.isArray(args[1]) ? args[1] : [])]); const entry = { command, env: args[2]?.env || null }; const dockerfileMatch = command.match(/--from ([^ ]+Dockerfile)/); if (dockerfileMatch) { @@ -1005,7 +1015,7 @@ runner.runCapture = (command) => { const sandboxExecCurl = require(${onboardScriptMocksPath}).mockSandboxExecCurl(command); if (sandboxExecCurl !== null) return sandboxExecCurl; } - if (_n(command).includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running"; + if (_n(command).includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running\nmy-assistant 127.0.0.1 8642 12346 running"; return ""; }; registry.registerSandbox = (entry) => { @@ -1022,7 +1032,9 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - const command = _n(args[1][1]); + child.unref = () => {}; + child.pid = 4242; + const command = _n([args[0], ...(Array.isArray(args[1]) ? args[1] : [])]); const entry = { command, env: args[2]?.env || null }; const dockerfileMatch = command.match(/--from ([^ ]+Dockerfile)/); if (dockerfileMatch) { @@ -1221,7 +1233,7 @@ runner.runCapture = (command) => { if (_n(command).includes("sandbox list")) return "my-assistant Ready"; // All messaging providers already exist in gateway if (_n(command).includes("provider get")) return "Provider: exists"; - if (_n(command).includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running"; + if (_n(command).includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running\nmy-assistant 127.0.0.1 8642 12346 running"; return ""; }; registry.getSandbox = () => ({ name: "my-assistant", gpuEnabled: false }); @@ -1335,7 +1347,7 @@ runner.runCapture = (command) => { const sandboxExecCurl = require(${onboardScriptMocksPath}).mockSandboxExecCurl(command); if (sandboxExecCurl !== null) return sandboxExecCurl; } - if (_n(command).includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running"; + if (_n(command).includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running\nmy-assistant 127.0.0.1 8642 12346 running"; return ""; }; registry.registerSandbox = () => true; @@ -1349,7 +1361,9 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - commands.push({ command: _n(args[1][1]), env: args[2]?.env || null }); + child.unref = () => {}; + child.pid = 4242; + commands.push({ command: _n([args[0], ...(Array.isArray(args[1]) ? args[1] : [])]), env: args[2]?.env || null }); process.nextTick(() => { child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); child.emit("close", 0); @@ -1470,7 +1484,7 @@ runner.runCapture = (command) => { const sandboxExecCurl = require(${onboardScriptMocksPath}).mockSandboxExecCurl(command); if (sandboxExecCurl !== null) return sandboxExecCurl; } - if (_n(command).includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running"; + if (_n(command).includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running\nmy-assistant 127.0.0.1 8642 12346 running"; return ""; }; registry.registerSandbox = () => true; @@ -1484,7 +1498,9 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - commands.push({ command: _n(args[1][1]), env: args[2]?.env || null }); + child.unref = () => {}; + child.pid = 4242; + commands.push({ command: _n([args[0], ...(Array.isArray(args[1]) ? args[1] : [])]), env: args[2]?.env || null }); process.nextTick(() => { child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); child.emit("close", 0); diff --git a/test/onboard.test.ts b/test/onboard.test.ts index 5be0543a87..4bce1ed84b 100644 --- a/test/onboard.test.ts +++ b/test/onboard.test.ts @@ -2061,7 +2061,9 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - commands.push({ command: _n(args[1][1]), env: args[2]?.env || null }); + child.unref = () => {}; + child.pid = 4242; + commands.push({ command: _n([args[0], ...(Array.isArray(args[1]) ? args[1] : [])]), env: args[2]?.env || null }); process.nextTick(() => { child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); child.emit("close", 0); @@ -2260,7 +2262,9 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - commands.push({ command: _n(args[1][1]), env: args[2]?.env || null }); + child.unref = () => {}; + child.pid = 4242; + commands.push({ command: _n([args[0], ...(Array.isArray(args[1]) ? args[1] : [])]), env: args[2]?.env || null }); process.nextTick(() => { child.stdout.emit("data", Buffer.from("Created sandbox: hermes-sandbox\\n")); child.emit("close", 0); @@ -2439,7 +2443,9 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - commands.push({ command: _n(args[1][1]), env: args[2]?.env || null }); + child.unref = () => {}; + child.pid = 4242; + commands.push({ command: _n([args[0], ...(Array.isArray(args[1]) ? args[1] : [])]), env: args[2]?.env || null }); process.nextTick(() => { child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\\n")); child.emit("close", 0); @@ -2539,7 +2545,9 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - commands.push({ command: _n(args[1][1]), env: args[2]?.env || null }); + child.unref = () => {}; + child.pid = 4242; + commands.push({ command: _n([args[0], ...(Array.isArray(args[1]) ? args[1] : [])]), env: args[2]?.env || null }); process.nextTick(() => { child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); child.emit("close", 0); @@ -2638,7 +2646,9 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - commands.push({ command: _n(args[1][1]), env: args[2]?.env || null }); + child.unref = () => {}; + child.pid = 4242; + commands.push({ command: _n([args[0], ...(Array.isArray(args[1]) ? args[1] : [])]), env: args[2]?.env || null }); process.nextTick(() => { child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); child.emit("close", 0); @@ -2825,7 +2835,7 @@ runner.run = (command, opts = {}) => { runner.runCapture = (command) => { if (_n(command).includes("sandbox get my-assistant")) return "my-assistant"; if (_n(command).includes("sandbox list")) return "my-assistant Ready"; - if (_n(command).includes("forward list")) return ""; + if (_n(command).includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running"; { const sandboxExecCurl = require(${onboardScriptMocksPath}).mockSandboxExecCurl(command, { defaultCurlOutput: "ok", @@ -2847,7 +2857,9 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - commands.push({ command: _n(args[1][1]), env: args[2]?.env || null }); + child.unref = () => {}; + child.pid = 4242; + commands.push({ command: _n([args[0], ...(Array.isArray(args[1]) ? args[1] : [])]), env: args[2]?.env || null }); process.nextTick(() => { child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); child.emit("close", 0); @@ -2937,7 +2949,7 @@ runner.run = (command, opts = {}) => { runner.runCapture = (command) => { if (_n(command).includes("sandbox get my-assistant")) return "my-assistant"; if (_n(command).includes("sandbox list")) return "my-assistant Ready"; - if (_n(command).includes("forward list")) return ""; + if (_n(command).includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running"; { const sandboxExecCurl = require(${onboardScriptMocksPath}).mockSandboxExecCurl(command, { defaultCurlOutput: "ok", @@ -2968,7 +2980,9 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - commands.push({ command: _n(args[1][1]), env: args[2]?.env || null }); + child.unref = () => {}; + child.pid = 4242; + commands.push({ command: _n([args[0], ...(Array.isArray(args[1]) ? args[1] : [])]), env: args[2]?.env || null }); process.nextTick(() => { child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); child.emit("close", 0); @@ -3088,7 +3102,9 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - commands.push({ command: args[1]?.[1] || String(args[0]), env: args[2]?.env || null }); + child.unref = () => {}; + child.pid = 4242; + commands.push({ command: _n([args[0], ...(Array.isArray(args[1]) ? args[1] : [])]), env: args[2]?.env || null }); process.nextTick(() => { child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); child.emit("close", 0); @@ -3207,7 +3223,7 @@ runner.runFile = (file, args = [], opts = {}) => { runner.runCapture = (command) => { if (_n(command).includes("sandbox get my-assistant")) return "my-assistant"; if (_n(command).includes("sandbox list")) return "my-assistant Ready"; - if (_n(command).includes("forward list")) return ""; + if (_n(command).includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running"; { const sandboxExecCurl = require(${onboardScriptMocksPath}).mockSandboxExecCurl(command, { defaultCurlOutput: "ok", @@ -3232,7 +3248,9 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - commands.push({ command: _n(args[1][1]), env: args[2]?.env || null }); + child.unref = () => {}; + child.pid = 4242; + commands.push({ command: _n([args[0], ...(Array.isArray(args[1]) ? args[1] : [])]), env: args[2]?.env || null }); process.nextTick(() => { child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); child.emit("close", 0); @@ -3337,7 +3355,7 @@ runner.runCapture = (command) => { if (_n(command).includes("sandbox list")) { return sandboxDeleted ? "my-assistant Ready" : "my-assistant NotReady"; } - if (_n(command).includes("forward list")) return ""; + if (_n(command).includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running"; { const sandboxExecCurl = require(${onboardScriptMocksPath}).mockSandboxExecCurl(command, { defaultCurlOutput: "ok", @@ -3362,7 +3380,9 @@ const fakeSpawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - commands.push({ command: _n(args[1][1]), env: args[2]?.env || null }); + child.unref = () => {}; + child.pid = 4242; + commands.push({ command: _n([args[0], ...(Array.isArray(args[1]) ? args[1] : [])]), env: args[2]?.env || null }); process.nextTick(() => { child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); child.emit("close", 0); @@ -3491,6 +3511,8 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); + child.unref = () => {}; + child.pid = 4242; child.killCalls = []; child.unrefCalls = 0; child.stdout.destroyCalls = 0; @@ -3509,7 +3531,7 @@ childProcess.spawn = (...args) => { process.nextTick(() => child.emit("close", signal === "SIGTERM" ? 0 : 1)); return true; }; - commands.push({ command: _n(args[1][1]), env: args[2]?.env || null, child }); + commands.push({ command: _n([args[0], ...(Array.isArray(args[1]) ? args[1] : [])]), env: args[2]?.env || null, child }); process.nextTick(() => { child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); }); @@ -3581,6 +3603,8 @@ const { createSandbox } = require(${onboardPath}); const runner = require(${runnerPath}); const _n = (c) => (Array.isArray(c) ? c.join(" ") : String(c)).replace(/'/g, ""); const registry = require(${registryPath}); +const childProcess = require("node:child_process"); +const { EventEmitter } = require("node:events"); const commands = []; runner.run = (command, opts = {}) => { @@ -3595,6 +3619,17 @@ runner.runCapture = (command) => { }; registry.getSandbox = () => ({ name: "my-assistant", gpuEnabled: false }); +childProcess.spawn = (...args) => { + const child = new EventEmitter(); + child.stdout = new EventEmitter(); + child.stderr = new EventEmitter(); + child.unref = () => {}; + child.pid = 4242; + commands.push({ command: _n([args[0], ...(Array.isArray(args[1]) ? args[1] : [])]), env: args[2]?.env || null }); + process.nextTick(() => child.emit("close", 0)); + return child; +}; + const { createSandbox } = require(${onboardPath}); (async () => { diff --git a/test/shellquote-sandbox.test.ts b/test/shellquote-sandbox.test.ts index de6eb34722..dcf948dd64 100644 --- a/test/shellquote-sandbox.test.ts +++ b/test/shellquote-sandbox.test.ts @@ -82,7 +82,7 @@ runner.runCapture = (command) => { const text = asText(command); if (text.includes("sandbox get my-assistant")) return ""; if (text.includes("sandbox list")) return "my-assistant Ready"; - if (text.includes("forward list")) return ""; + if (text.includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running"; if (text.includes("sandbox exec") && text.includes("http://localhost:") && text.includes("/health")) return "200"; if (text === "uname -r") return "6.8.0"; return "";