From f44c32aeb8c00a99ca2cb990d9e143e3ebe6b943 Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Mon, 13 Jul 2026 16:51:07 -0700 Subject: [PATCH 1/6] refactor(openshell): remove gateway-stop kubectl path Signed-off-by: Aaron Erickson --- scripts/checks/legacy-sandbox-transports.ts | 1 - src/lib/tunnel/sandbox-gateway-stop.test.ts | 100 ++------------------ src/lib/tunnel/sandbox-gateway-stop.ts | 73 +------------- 3 files changed, 13 insertions(+), 161 deletions(-) diff --git a/scripts/checks/legacy-sandbox-transports.ts b/scripts/checks/legacy-sandbox-transports.ts index edfb74d4a6a..00400152494 100644 --- a/scripts/checks/legacy-sandbox-transports.ts +++ b/scripts/checks/legacy-sandbox-transports.ts @@ -43,7 +43,6 @@ const REVIEWED_SITE_TUPLES = [ ["src/lib/share-command.ts", "sshfs-command", 1], ["src/lib/shields/index.ts", "privileged-sandbox-exec", 4], ["src/lib/shields/mutable-config-repair.ts", "privileged-sandbox-exec", 2], - ["src/lib/tunnel/sandbox-gateway-stop.ts", "docker-exec-command", 2], ] as const satisfies readonly ReviewedSiteTuple[]; export const REVIEWED_LEGACY_SANDBOX_TRANSPORT_SITES: readonly LegacySandboxTransportSite[] = diff --git a/src/lib/tunnel/sandbox-gateway-stop.test.ts b/src/lib/tunnel/sandbox-gateway-stop.test.ts index 8e1dda492d7..7216a33e3d0 100644 --- a/src/lib/tunnel/sandbox-gateway-stop.test.ts +++ b/src/lib/tunnel/sandbox-gateway-stop.test.ts @@ -37,7 +37,6 @@ function harness() { const resolveOpenshell = vi.fn>( () => "/usr/local/bin/openshell", ); - const runDocker = vi.fn>(() => spawnResult(1)); const runProcess = vi.fn>(() => spawnResult(0)); const info = vi.fn<(message: string) => void>(); const warn = vi.fn<(message: string) => void>(); @@ -45,7 +44,6 @@ function harness() { getSandbox, getRegisteredAgent, resolveOpenshell, - runDocker, runProcess, info, warn, @@ -56,49 +54,13 @@ function harness() { getSandbox, info, resolveOpenshell, - runDocker, runProcess, warn, }; } describe("stopSandboxChannels", () => { - it("uses kubectl via the OpenShell gateway container for privileged shutdown", () => { - const h = harness(); - h.runDocker - .mockReturnValueOnce(spawnResult(0, "pod/my-sandbox-0\n")) - .mockReturnValueOnce(spawnResult(0)); - - stopSandboxChannels("my-sandbox", h.deps); - - expect(h.runDocker).toHaveBeenNthCalledWith( - 1, - [ - "exec", - "openshell-cluster-nemoclaw", - "kubectl", - "get", - "pods", - "-n", - "openshell", - "-o", - "name", - ], - expect.objectContaining({ timeout: 10000 }), - ); - const args = h.runDocker.mock.calls[1][0]; - expect(args).toEqual( - expect.arrayContaining(["kubectl", "exec", "-n", "openshell", "-c", "agent"]), - ); - const script = String(args.at(-1)); - expect(script).toContain("ps -eo user=,pid=,args="); - expect(script).toContain("openclaw-gateway"); - expect(script).toContain("kill -TERM $pids"); - expect(script).toContain("kill -KILL $remaining"); - expect(h.info).toHaveBeenCalledWith("OpenClaw gateway stopped inside sandbox."); - }); - - it("falls back to gateway-scoped openshell sandbox exec through stdin", () => { + it("uses gateway-scoped openshell sandbox exec through stdin", () => { const h = harness(); stopSandboxChannels("my-sandbox", h.deps); @@ -113,33 +75,9 @@ describe("stopSandboxChannels", () => { ); }); - it("selects the exact generated sandbox pod and excludes overlapping names", () => { - const h = harness(); - h.runDocker - .mockReturnValueOnce(spawnResult(0, "pod/prod-app-abc\npod/app-abc\n")) - .mockReturnValueOnce(spawnResult(0)); - - stopSandboxChannels("app", h.deps); - - const args = h.runDocker.mock.calls[1][0]; - expect(args).toContain("pod/app-abc"); - expect(args).not.toContain("pod/prod-app-abc"); - }); - - it("falls back when no exact generated sandbox pod name is available", () => { - const h = harness(); - h.runDocker.mockReturnValueOnce(spawnResult(0, "pod/prod-app-abc\npod/app-copy-abc\n")); - - stopSandboxChannels("app", h.deps); - - expect(h.runProcess).toHaveBeenCalledTimes(1); - }); - it("treats stop script exit 1 as already stopped", () => { const h = harness(); - h.runDocker - .mockReturnValueOnce(spawnResult(0, "pod/my-sandbox-0\n")) - .mockReturnValueOnce(spawnResult(1)); + h.runProcess.mockReturnValueOnce(spawnResult(1)); stopSandboxChannels("my-sandbox", h.deps); @@ -159,7 +97,6 @@ describe("stopSandboxChannels", () => { stopSandboxChannels("my-sandbox", h.deps); - expect(h.runDocker).not.toHaveBeenCalled(); expect(h.runProcess).not.toHaveBeenCalled(); expect(h.info.mock.calls.map((call) => call[0]).join("\n")).toContain( "Hermes Agent gateway is managed by the sandbox", @@ -182,24 +119,20 @@ describe("stopSandboxChannels", () => { stopSandboxChannels("dcode-sandbox", h.deps); - expect(h.runDocker).not.toHaveBeenCalled(); expect(h.runProcess).not.toHaveBeenCalled(); expect(h.info).toHaveBeenCalledWith( "LangChain Deep Agents Code has no gateway runtime; skipping in-sandbox gateway stop.", ); }); - it("ignores an unrelated global session when the target registry row is missing", () => { + it("uses the default OpenClaw contract when the target registry row is missing", () => { const h = harness(); h.getSandbox.mockReturnValue(null); - h.runDocker - .mockReturnValueOnce(spawnResult(0, "pod/openclaw-target-abc\n")) - .mockReturnValueOnce(spawnResult(0)); stopSandboxChannels("openclaw-target", h.deps); expect(h.getRegisteredAgent).toHaveBeenCalledWith(null); - expect(h.runDocker).toHaveBeenCalledTimes(2); + expect(h.runProcess).toHaveBeenCalledTimes(1); expect(h.info.mock.calls.map((call) => call[0]).join("\n")).not.toContain("Hermes Agent"); }); @@ -212,7 +145,6 @@ describe("stopSandboxChannels", () => { expect(() => stopSandboxChannels("my-sandbox", h.deps)).not.toThrow(); expect(h.getRegisteredAgent).not.toHaveBeenCalled(); - expect(h.runDocker).not.toHaveBeenCalled(); expect(h.runProcess).not.toHaveBeenCalled(); expect(h.warn).toHaveBeenCalledWith( expect.stringContaining("Could not read the sandbox registry for 'my-sandbox'"), @@ -225,7 +157,6 @@ describe("stopSandboxChannels", () => { stopSandboxChannels("my-sandbox", h.deps); - expect(h.runDocker).not.toHaveBeenCalled(); expect(h.resolveOpenshell).not.toHaveBeenCalled(); expect(h.runProcess).not.toHaveBeenCalled(); expect(h.warn.mock.calls.map((call) => call[0]).join("\n")).toContain( @@ -239,18 +170,15 @@ describe("stopSandboxChannels", () => { stopSandboxChannels("my-sandbox", h.deps); - expect(h.runDocker).not.toHaveBeenCalled(); expect(h.runProcess).not.toHaveBeenCalled(); expect(h.warn).toHaveBeenCalledWith( expect.stringContaining("Could not resolve registered agent 'missing-agent'"), ); }); - it("warns when privileged shutdown reports the gateway may still be running", () => { + it("warns when sandbox exec reports the gateway may still be running", () => { const h = harness(); - h.runDocker - .mockReturnValueOnce(spawnResult(0, "pod/my-sandbox-0\n")) - .mockReturnValueOnce(spawnResult(2, "", "205")); + h.runProcess.mockReturnValueOnce(spawnResult(2, "", "205")); stopSandboxChannels("my-sandbox", h.deps); @@ -262,16 +190,14 @@ describe("stopSandboxChannels", () => { it("warns when spawn returns null status", () => { const h = harness(); - h.runDocker - .mockReturnValueOnce(spawnResult(0, "pod/my-sandbox-0\n")) - .mockReturnValueOnce(spawnResult(null)); + h.runProcess.mockReturnValueOnce(spawnResult(null)); stopSandboxChannels("my-sandbox", h.deps); expect(h.warn).toHaveBeenCalledWith(expect.stringContaining("exit unknown")); }); - it("warns when privileged shutdown is unavailable and openshell is not found", () => { + it("warns when openshell is not found", () => { const h = harness(); h.resolveOpenshell.mockReturnValue(null); @@ -289,10 +215,6 @@ describe("stopSandboxChannels", () => { stopSandboxChannels("my-sandbox", h.deps); - expect(h.runDocker).toHaveBeenCalledWith( - expect.arrayContaining(["exec", "openshell-cluster-nemoclaw-18080", "kubectl"]), - expect.any(Object), - ); expect(h.runProcess).toHaveBeenCalledWith( "/usr/local/bin/openshell", expect.arrayContaining(["--gateway", "nemoclaw-18080"]), @@ -302,13 +224,10 @@ describe("stopSandboxChannels", () => { it("targets launcher, re-exec, and identity-guarded bare gateway forms", () => { const h = harness(); - h.runDocker - .mockReturnValueOnce(spawnResult(0, "pod/my-sandbox-0\n")) - .mockReturnValueOnce(spawnResult(0)); stopSandboxChannels("my-sandbox", h.deps); - const script = String(h.runDocker.mock.calls[1][0].at(-1)); + const script = String(h.runProcess.mock.calls[0][2].input); expect(script).toContain("openclaw-gateway"); expect(script).toContain("openclaw[[:space:]]+gateway"); expect(script).toContain("openclaw[[:space:]]*$"); @@ -320,7 +239,6 @@ describe("stopSandboxChannels", () => { expect(() => stopSandboxChannels("../escape", h.deps)).toThrow("Invalid sandbox name"); expect(h.getSandbox).not.toHaveBeenCalled(); - expect(h.runDocker).not.toHaveBeenCalled(); expect(h.runProcess).not.toHaveBeenCalled(); }); }); diff --git a/src/lib/tunnel/sandbox-gateway-stop.ts b/src/lib/tunnel/sandbox-gateway-stop.ts index 7176783c7b1..4b663ed0a66 100644 --- a/src/lib/tunnel/sandbox-gateway-stop.ts +++ b/src/lib/tunnel/sandbox-gateway-stop.ts @@ -7,8 +7,6 @@ import { spawnSync, } from "node:child_process"; -import { dockerSpawnSync } from "../adapters/docker"; -import { getGatewayClusterContainerName } from "../adapters/openshell/gateway-drift"; import { resolveOpenshell } from "../adapters/openshell/resolve"; import * as agentRuntime from "../agent/runtime"; import { resolveSandboxGatewayName } from "../onboard/gateway-binding"; @@ -29,7 +27,6 @@ export type SandboxGatewayStopDeps = { getAgentDisplayName?: typeof agentRuntime.getAgentDisplayName; hasGatewayRuntime?: typeof agentRuntime.hasGatewayRuntime; resolveOpenshell?: typeof resolveOpenshell; - runDocker?: typeof dockerSpawnSync; runProcess?: ProcessRunner; info?: Reporter; warn?: Reporter; @@ -105,14 +102,6 @@ export function stopSandboxChannels(sandboxName: string, deps: SandboxGatewaySto const gatewayLabel = `${agentDisplayName} gateway`; info(`Stopping in-sandbox ${gatewayLabel} (sandbox: ${validatedSandboxName})...`); - const privilegedResult = stopSandboxChannelsViaKubectl( - validatedSandboxName, - gatewayName, - GATEWAY_STOP_SCRIPT, - deps.runDocker ?? dockerSpawnSync, - ); - if (reportStopResult(privilegedResult, gatewayLabel, info, warn)) return; - const openshell = (deps.resolveOpenshell ?? resolveOpenshell)(); if (!openshell) { warn(`openshell not found — cannot stop ${gatewayLabel} inside sandbox.`); @@ -132,72 +121,19 @@ export function stopSandboxChannels(sandboxName: string, deps: SandboxGatewaySto reportStopResult(fallbackResult, gatewayLabel, info, warn); } -function isSandboxPodName(line: string, sandboxName: string): boolean { - if (!line.startsWith("pod/")) return false; - const podName = line.slice("pod/".length); - if (podName === sandboxName) return true; - const prefix = `${sandboxName}-`; - if (!podName.startsWith(prefix)) return false; - const generatedSuffix = podName.slice(prefix.length); - return /^[a-z0-9]+$/.test(generatedSuffix); -} - -function stopSandboxChannelsViaKubectl( - sandboxName: string, - gatewayName: string, - gatewayStopScript: string, - runDocker: typeof dockerSpawnSync, -): StopAttemptResult | null { - const gatewayContainer = getGatewayClusterContainerName(gatewayName); - const podsResult = runDocker( - ["exec", gatewayContainer, "kubectl", "get", "pods", "-n", "openshell", "-o", "name"], - { encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"], timeout: 10000 }, - ); - if (podsResult.status !== 0 || !podsResult.stdout) return null; - - const podOutput = - typeof podsResult.stdout === "string" ? podsResult.stdout : podsResult.stdout.toString(); - const pod = podOutput - .split(/\r?\n/) - .map((line: string) => line.trim()) - .find((line: string) => isSandboxPodName(line, sandboxName)); - if (!pod) return null; - - return runDocker( - [ - "exec", - gatewayContainer, - "kubectl", - "exec", - "-n", - "openshell", - "-c", - "agent", - pod, - "--", - "sh", - "-lc", - gatewayStopScript, - ], - { encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"], timeout: 20000 }, - ); -} - function reportStopResult( - result: StopAttemptResult | null, + result: StopAttemptResult, gatewayLabel: string, info: Reporter, warn: Reporter, -): boolean { - if (!result) return false; - +): void { if (result.status === 0) { info(`${gatewayLabel} stopped inside sandbox.`); - return true; + return; } if (result.status === 1) { info(`${gatewayLabel} was not running inside sandbox.`); - return true; + return; } const details = [result.stderr, result.stdout] @@ -210,5 +146,4 @@ function reportStopResult( " The sandbox may be unreachable or the gateway may still be running." + (details ? ` Details: ${details}` : ""), ); - return true; } From 0714c18bc193af18155fb5ee9d238afeabb3989a Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Mon, 13 Jul 2026 16:56:43 -0700 Subject: [PATCH 2/6] refactor(openshell): remove bootstrap secret repair Signed-off-by: Aaron Erickson --- scripts/checks/legacy-sandbox-transports.ts | 3 - src/lib/adapters/docker/container.ts | 4 - src/lib/onboard.ts | 28 +--- src/lib/onboard/gateway-bootstrap.test.ts | 86 ---------- src/lib/onboard/gateway-bootstrap.ts | 168 -------------------- src/lib/onboard/gateway-health-wait.test.ts | 28 +--- src/lib/onboard/gateway-health-wait.ts | 11 +- test/gateway-start-wait.test.ts | 53 ------ test/legacy-sandbox-transports.test.ts | 2 - 9 files changed, 3 insertions(+), 380 deletions(-) delete mode 100644 src/lib/onboard/gateway-bootstrap.test.ts delete mode 100644 src/lib/onboard/gateway-bootstrap.ts diff --git a/scripts/checks/legacy-sandbox-transports.ts b/scripts/checks/legacy-sandbox-transports.ts index 00400152494..454213ccdae 100644 --- a/scripts/checks/legacy-sandbox-transports.ts +++ b/scripts/checks/legacy-sandbox-transports.ts @@ -35,8 +35,6 @@ const REVIEWED_SITE_TUPLES = [ ["src/lib/actions/dns/index.ts", "docker-exec-command", 4], ["src/lib/actions/sandbox/host-aliases.ts", "docker-exec-command", 1], ["src/lib/actions/sandbox/process-recovery.ts", "privileged-sandbox-exec", 2], - ["src/lib/adapters/docker/container.ts", "docker-exec-command", 1], - ["src/lib/onboard.ts", "docker-exec-builder", 1], ["src/lib/resources-cmd.ts", "docker-exec-command", 1], ["src/lib/sandbox/config.ts", "privileged-sandbox-exec", 2], ["src/lib/share-command-deps.ts", "openshell-ssh-config", 1], @@ -50,7 +48,6 @@ export const REVIEWED_LEGACY_SANDBOX_TRANSPORT_SITES: readonly LegacySandboxTran const HELPER_KINDS = new Map([ ["createTempSshConfig", "ssh-temp-config"], - ["dockerExecArgv", "docker-exec-builder"], ["privilegedSandboxExecArgv", "privileged-sandbox-exec"], ]); diff --git a/src/lib/adapters/docker/container.ts b/src/lib/adapters/docker/container.ts index 9dba35695ac..c66d5b9d35e 100644 --- a/src/lib/adapters/docker/container.ts +++ b/src/lib/adapters/docker/container.ts @@ -72,7 +72,3 @@ export function dockerPort( ): string { return dockerCapture(["port", containerName, String(containerPort)], opts); } - -export function dockerExecArgv(containerName: string, cmd: readonly string[]): string[] { - return ["docker", "exec", containerName, ...cmd]; -} diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index bc6ba7e13bf..78715bad448 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -98,11 +98,6 @@ const { }: typeof import("./onboard/web-search-support") = require("./onboard/web-search-support"); const onboardDashboard: typeof import("./onboard/dashboard") = require("./onboard/dashboard"); const dashboardRuntime: typeof import("./onboard/dashboard-runtime") = require("./onboard/dashboard-runtime"); -const { - buildGatewayBootstrapSecretsScript, - createGatewayBootstrapRepairHelpers, - getGatewayBootstrapRepairPlan, -}: typeof import("./onboard/gateway-bootstrap") = require("./onboard/gateway-bootstrap"); const { buildDirectGpuPolicyYaml, buildDirectSandboxGpuProofCommands, @@ -173,7 +168,6 @@ const { getNameValidationGuidance } = nameValidation; const docker: typeof import("./adapters/docker") = require("./adapters/docker"); const { dockerContainerInspectFormat, - dockerExecArgv, dockerInfoFormat, dockerInspect, dockerRemoveVolumesByPrefix, @@ -1380,10 +1374,6 @@ function getGatewayHealthWaitConfig(_startStatus = 0, containerState = "") { }; } -function buildGatewayClusterExecArgv(script: string): string[] { - return dockerExecArgv(getGatewayClusterContainerName(GATEWAY_NAME), ["sh", "-lc", script]); -} - function captureProcessArgs(pid: number): string { return runCapture(["ps", "-p", String(pid), "-o", "args="], { ignoreError: true, @@ -1398,13 +1388,6 @@ function getGatewayLocalEndpoint(): string { return dockerDriverGatewayEnv.getGatewayHttpsEndpoint(GATEWAY_PORT); } -const { gatewayClusterHealthcheckPassed, repairGatewayBootstrapSecrets } = - createGatewayBootstrapRepairHelpers({ - buildGatewayClusterExecArgv, - run, - runCapture, - }); - function registerDockerDriverGatewayEndpoint(): boolean { const selectExisting = runQuietOpenshell(["gateway", "select", GATEWAY_NAME]); if (selectExisting.status === 0) { @@ -1870,14 +1853,12 @@ async function startGatewayWithOptions( if ( await waitForGatewayHealth({ attachGatewayMetadataIfNeeded, - gatewayClusterHealthcheckPassed, gatewayName: GATEWAY_NAME, healthPollCount: healthWait.count, healthPollIntervalSeconds: healthWait.interval, isGatewayHealthy, isGatewayHttpReady: (signal) => isGatewayHttpReady(undefined, undefined, undefined, signal), - repairGatewayBootstrapSecrets, runCaptureOpenshell, sleepSeconds, }) @@ -2234,12 +2215,7 @@ async function recoverGatewayRuntime() { ? recoveryWait.interval : envInt("NEMOCLAW_HEALTH_POLL_INTERVAL", 2); for (let i = 0; i < recoveryPollCount; i++) { - const repairResult = repairGatewayBootstrapSecrets(); - if (repairResult.repaired) { - attachGatewayMetadataIfNeeded({ forceRefresh: true }); - } else if (gatewayClusterHealthcheckPassed()) { - attachGatewayMetadataIfNeeded(); - } + attachGatewayMetadataIfNeeded(); status = runCaptureOpenshell(["status"], { ignoreError: true }); if (status.includes("Connected") && isSelectedGateway(status) && (await isGatewayHttpReady())) { process.env.OPENSHELL_GATEWAY = GATEWAY_NAME; @@ -4697,7 +4673,6 @@ async function runOnboard(opts: OnboardOptions = {}): Promise { module.exports = { buildOrphanedSandboxRollbackMessage, buildProviderArgs, - buildGatewayBootstrapSecretsScript, buildCompatibleEndpointSandboxSmokeCommand, buildCompatibleEndpointSandboxSmokeScript, buildSandboxConfigSyncScript, @@ -4716,7 +4691,6 @@ module.exports = { areRequiredDockerDriverBinariesPresent, ensureOpenshellForOnboard, shouldRequireDockerDriverEnv, - getGatewayBootstrapRepairPlan, getGatewayLocalEndpoint, getGatewayStartEnv, getDockerDriverGatewayEnv, diff --git a/src/lib/onboard/gateway-bootstrap.test.ts b/src/lib/onboard/gateway-bootstrap.test.ts deleted file mode 100644 index bbf9505e154..00000000000 --- a/src/lib/onboard/gateway-bootstrap.test.ts +++ /dev/null @@ -1,86 +0,0 @@ -// 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 { - buildGatewayBootstrapSecretsScript, - createGatewayBootstrapRepairHelpers, - getGatewayBootstrapRepairPlan, -} from "./gateway-bootstrap"; - -describe("gateway bootstrap helpers", () => { - it("normalizes missing secrets into a repair plan", () => { - expect( - getGatewayBootstrapRepairPlan([ - "openshell-client-tls", - "noise", - " openshell-server-tls ", - "", - ]), - ).toEqual({ - missingSecrets: ["openshell-client-tls", "openshell-server-tls"], - needsRepair: true, - needsServerTls: true, - needsClientBundle: true, - needsHandshake: false, - }); - }); - - it("builds a no-op script when nothing is missing", () => { - expect(buildGatewayBootstrapSecretsScript([]).trim()).toBe("exit 0"); - }); - - it("builds a repair script only for requested secret classes", () => { - const script = buildGatewayBootstrapSecretsScript([ - "openshell-server-tls", - "openshell-ssh-handshake", - ]); - - expect(script).toContain("openshell-server-tls"); - expect(script).toContain("openshell-ssh-handshake"); - expect(script).toContain("if true; then"); - expect(script).toContain("if false; then"); - }); - - it("lists missing secrets through the gateway cluster executor", () => { - const buildGatewayClusterExecArgv = vi.fn((script: string) => ["docker", "exec", script]); - const runCapture = vi.fn(() => "openshell-client-tls\n\nopenshell-server-tls\n"); - const helpers = createGatewayBootstrapRepairHelpers({ - buildGatewayClusterExecArgv, - run: vi.fn(() => ({ status: 0 })), - runCapture, - }); - - expect(helpers.listMissingGatewayBootstrapSecrets()).toEqual([ - "openshell-client-tls", - "openshell-server-tls", - ]); - expect(runCapture).toHaveBeenCalledWith( - expect.arrayContaining([expect.stringContaining("kubectl -n openshell get secret")]), - { ignoreError: true }, - ); - }); - - it("repairs missing secrets and reports success when the second probe is clean", () => { - const log = vi.fn(); - const runCapture = vi - .fn() - .mockReturnValueOnce("openshell-client-tls\n") - .mockReturnValueOnce(""); - const run = vi.fn(() => ({ status: 0 })); - const helpers = createGatewayBootstrapRepairHelpers({ - buildGatewayClusterExecArgv: (script) => ["docker", "exec", script], - run, - runCapture, - log, - }); - - expect(helpers.repairGatewayBootstrapSecrets()).toEqual({ repaired: true, missingSecrets: [] }); - expect(run).toHaveBeenCalledWith(expect.any(Array), { - ignoreError: true, - suppressOutput: true, - }); - expect(log).toHaveBeenCalledWith(" ✓ OpenShell bootstrap secrets created"); - }); -}); diff --git a/src/lib/onboard/gateway-bootstrap.ts b/src/lib/onboard/gateway-bootstrap.ts deleted file mode 100644 index 311268ff8d7..00000000000 --- a/src/lib/onboard/gateway-bootstrap.ts +++ /dev/null @@ -1,168 +0,0 @@ -// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -import { shellQuote } from "../core/shell-quote"; - -export const GATEWAY_BOOTSTRAP_SECRET_NAMES = [ - "openshell-server-tls", - "openshell-server-client-ca", - "openshell-client-tls", - "openshell-ssh-handshake", -] as const; - -export type GatewayBootstrapRepairPlan = { - missingSecrets: string[]; - needsRepair: boolean; - needsServerTls: boolean; - needsClientBundle: boolean; - needsHandshake: boolean; -}; - -export type GatewayBootstrapRepairResult = { - repaired: boolean; - missingSecrets: string[]; -}; - -type RunnerOptions = { - ignoreError?: boolean; - suppressOutput?: boolean; - timeout?: number; -}; - -type RunResult = { - status: number | null; -}; - -export type GatewayBootstrapRepairDeps = { - buildGatewayClusterExecArgv: (script: string) => string[]; - run: (args: string[], opts?: RunnerOptions) => RunResult; - runCapture: (args: string[], opts?: RunnerOptions) => string; - log?: (message?: string) => void; -}; - -export function getGatewayBootstrapRepairPlan( - missingSecrets: string[] = [], -): GatewayBootstrapRepairPlan { - const allowed = new Set(GATEWAY_BOOTSTRAP_SECRET_NAMES); - const normalized = [ - ...new Set((missingSecrets || []).map((name) => String(name).trim()).filter(Boolean)), - ].filter((name) => allowed.has(name)); - const missing = new Set(normalized); - const needsClientBundle = - missing.has("openshell-server-client-ca") || missing.has("openshell-client-tls"); - - return { - missingSecrets: normalized, - needsRepair: normalized.length > 0, - needsServerTls: missing.has("openshell-server-tls"), - needsClientBundle, - needsHandshake: missing.has("openshell-ssh-handshake"), - }; -} - -export function buildGatewayBootstrapSecretsScript(missingSecrets: string[] = []): string { - const plan = getGatewayBootstrapRepairPlan(missingSecrets); - if (!plan.needsRepair) return "exit 0"; - - return ` -set -eu -export KUBECONFIG=/etc/rancher/k3s/k3s.yaml -kubectl get namespace openshell >/dev/null 2>&1 -kubectl -n openshell get statefulset/openshell >/dev/null 2>&1 -TMPDIR="$(mktemp -d)" -cleanup() { - rm -rf "$TMPDIR" -} -trap cleanup EXIT -if ${plan.needsServerTls ? "true" : "false"}; then - cat >"$TMPDIR/server-ext.cnf" <<'EOF' -subjectAltName=DNS:openshell,DNS:openshell.openshell,DNS:openshell.openshell.svc,DNS:openshell.openshell.svc.cluster.local,DNS:localhost,IP:127.0.0.1 -extendedKeyUsage=serverAuth -EOF - openssl req -nodes -newkey rsa:2048 -keyout "$TMPDIR/server.key" -out "$TMPDIR/server.csr" -subj "/CN=openshell.openshell.svc.cluster.local" >/dev/null 2>&1 - openssl x509 -req -in "$TMPDIR/server.csr" -signkey "$TMPDIR/server.key" -out "$TMPDIR/server.crt" -days 3650 -sha256 -extfile "$TMPDIR/server-ext.cnf" >/dev/null 2>&1 - kubectl create secret tls -n openshell openshell-server-tls --cert="$TMPDIR/server.crt" --key="$TMPDIR/server.key" --dry-run=client -o yaml | kubectl apply -f - -fi -if ${plan.needsClientBundle ? "true" : "false"}; then - cat >"$TMPDIR/client-ext.cnf" <<'EOF' -extendedKeyUsage=clientAuth -EOF - openssl req -x509 -nodes -newkey rsa:2048 -keyout "$TMPDIR/client-ca.key" -out "$TMPDIR/client-ca.crt" -subj "/CN=openshell-client-ca" -days 3650 >/dev/null 2>&1 - openssl req -nodes -newkey rsa:2048 -keyout "$TMPDIR/client.key" -out "$TMPDIR/client.csr" -subj "/CN=openshell-client" >/dev/null 2>&1 - openssl x509 -req -in "$TMPDIR/client.csr" -CA "$TMPDIR/client-ca.crt" -CAkey "$TMPDIR/client-ca.key" -CAcreateserial -out "$TMPDIR/client.crt" -days 3650 -sha256 -extfile "$TMPDIR/client-ext.cnf" >/dev/null 2>&1 - kubectl create secret generic -n openshell openshell-server-client-ca --from-file=ca.crt="$TMPDIR/client-ca.crt" --dry-run=client -o yaml | kubectl apply -f - - kubectl create secret generic -n openshell openshell-client-tls --from-file=tls.crt="$TMPDIR/client.crt" --from-file=tls.key="$TMPDIR/client.key" --from-file=ca.crt="$TMPDIR/client-ca.crt" --dry-run=client -o yaml | kubectl apply -f - -fi -if ${plan.needsHandshake ? "true" : "false"}; then - kubectl create secret generic -n openshell openshell-ssh-handshake --from-literal=secret="$(openssl rand -hex 32)" --dry-run=client -o yaml | kubectl apply -f - -fi -`; -} - -export function createGatewayBootstrapRepairHelpers(deps: GatewayBootstrapRepairDeps) { - const log = deps.log ?? console.log; - - function runGatewayClusterCapture(script: string, opts: RunnerOptions = {}): string { - return deps.runCapture(deps.buildGatewayClusterExecArgv(script), opts); - } - - function runGatewayCluster(script: string, opts: RunnerOptions = {}): RunResult { - return deps.run(deps.buildGatewayClusterExecArgv(script), opts); - } - - function listMissingGatewayBootstrapSecrets(): string[] { - const output = runGatewayClusterCapture( - ` -set -eu -export KUBECONFIG=/etc/rancher/k3s/k3s.yaml -kubectl get namespace openshell >/dev/null 2>&1 || exit 0 -kubectl -n openshell get statefulset/openshell >/dev/null 2>&1 || exit 0 -for name in ${GATEWAY_BOOTSTRAP_SECRET_NAMES.map((name) => shellQuote(name)).join(" ")}; do - kubectl -n openshell get secret "$name" >/dev/null 2>&1 || printf '%s\\n' "$name" -done -`, - { ignoreError: true }, - ); - return output - .split("\n") - .map((line) => line.trim()) - .filter(Boolean); - } - - function gatewayClusterHealthcheckPassed(): boolean { - const result = runGatewayCluster("/usr/local/bin/cluster-healthcheck.sh", { - ignoreError: true, - suppressOutput: true, - }); - return result.status === 0; - } - - function repairGatewayBootstrapSecrets(): GatewayBootstrapRepairResult { - const missingSecrets = listMissingGatewayBootstrapSecrets(); - const plan = getGatewayBootstrapRepairPlan(missingSecrets); - if (!plan.needsRepair) return { repaired: false, missingSecrets }; - - log(` OpenShell bootstrap secrets missing: ${plan.missingSecrets.join(", ")}. Repairing...`); - const repairResult = runGatewayCluster( - buildGatewayBootstrapSecretsScript(plan.missingSecrets), - { - ignoreError: true, - suppressOutput: true, - }, - ); - const remainingSecrets = listMissingGatewayBootstrapSecrets(); - if (repairResult.status === 0 && remainingSecrets.length === 0) { - log(" ✓ OpenShell bootstrap secrets created"); - return { repaired: true, missingSecrets: remainingSecrets }; - } - return { repaired: false, missingSecrets: remainingSecrets }; - } - - return { - runGatewayClusterCapture, - runGatewayCluster, - listMissingGatewayBootstrapSecrets, - gatewayClusterHealthcheckPassed, - repairGatewayBootstrapSecrets, - }; -} diff --git a/src/lib/onboard/gateway-health-wait.test.ts b/src/lib/onboard/gateway-health-wait.test.ts index 66ebb68316f..de1533dfd20 100644 --- a/src/lib/onboard/gateway-health-wait.test.ts +++ b/src/lib/onboard/gateway-health-wait.test.ts @@ -14,13 +14,11 @@ import { function buildOptions(overrides: Partial = {}): GatewayHealthWaitOptions { return { attachGatewayMetadataIfNeeded: vi.fn(), - gatewayClusterHealthcheckPassed: vi.fn(() => false), gatewayName: "nemoclaw", healthPollCount: 1, healthPollIntervalSeconds: 2, isGatewayHealthy: vi.fn(() => true), isGatewayHttpReady: vi.fn(async () => true), - repairGatewayBootstrapSecrets: vi.fn(() => ({ repaired: false })), runCaptureOpenshell: vi.fn((args: string[]) => { if (args[0] === "status") return "status"; if (args[0] === "gateway" && args[1] === "info" && args[2] === "-g") return "named"; @@ -50,6 +48,7 @@ describe("waitForGatewayHealth", () => { expect(isGatewayHealthy).toHaveBeenCalledTimes(2); expect(isGatewayHttpReady).toHaveBeenCalledTimes(2); + expect(options.attachGatewayMetadataIfNeeded).toHaveBeenCalledTimes(2); expect(options.sleepSeconds).toHaveBeenCalledTimes(1); expect(options.sleepSeconds).toHaveBeenCalledWith(2); }); @@ -70,30 +69,6 @@ describe("waitForGatewayHealth", () => { expect(options.sleepSeconds).toHaveBeenCalledTimes(2); }); - it("force-refreshes metadata after bootstrap secret repair", async () => { - const options = buildOptions({ - gatewayClusterHealthcheckPassed: vi.fn(() => true), - repairGatewayBootstrapSecrets: vi.fn(() => ({ repaired: true })), - }); - - await expect(waitForGatewayHealth(options)).resolves.toBe(true); - - expect(options.attachGatewayMetadataIfNeeded).toHaveBeenCalledOnce(); - expect(options.attachGatewayMetadataIfNeeded).toHaveBeenCalledWith({ forceRefresh: true }); - expect(options.gatewayClusterHealthcheckPassed).not.toHaveBeenCalled(); - }); - - it("attaches metadata without force when cluster healthcheck passes without repair", async () => { - const options = buildOptions({ - gatewayClusterHealthcheckPassed: vi.fn(() => true), - }); - - await expect(waitForGatewayHealth(options)).resolves.toBe(true); - - expect(options.attachGatewayMetadataIfNeeded).toHaveBeenCalledOnce(); - expect(options.attachGatewayMetadataIfNeeded).toHaveBeenCalledWith(); - }); - it("polls until the configured health deadline instead of stopping at the count cap (#3768)", async () => { const clock = createVirtualClock(); const isGatewayHealthy = vi.fn(() => { @@ -189,7 +164,6 @@ describe("waitForGatewayHealth", () => { await expect(waitForGatewayHealth(options)).resolves.toBe(false); - expect(options.repairGatewayBootstrapSecrets).not.toHaveBeenCalled(); expect(options.attachGatewayMetadataIfNeeded).not.toHaveBeenCalled(); expect(options.runCaptureOpenshell).not.toHaveBeenCalled(); expect(options.isGatewayHealthy).not.toHaveBeenCalled(); diff --git a/src/lib/onboard/gateway-health-wait.ts b/src/lib/onboard/gateway-health-wait.ts index 2193baff3a6..3e9f87c238d 100644 --- a/src/lib/onboard/gateway-health-wait.ts +++ b/src/lib/onboard/gateway-health-wait.ts @@ -7,13 +7,11 @@ type RunCaptureOpenshell = (args: string[], opts?: { ignoreError?: boolean }) => export interface GatewayHealthWaitOptions { attachGatewayMetadataIfNeeded: (options?: { forceRefresh?: boolean }) => void; - gatewayClusterHealthcheckPassed: () => boolean; gatewayName: string; healthPollCount: number; healthPollIntervalSeconds: number; isGatewayHealthy: (status: string, namedInfo: string, currentInfo: string) => boolean; isGatewayHttpReady: (signal?: AbortSignal) => Promise; - repairGatewayBootstrapSecrets: () => { repaired: boolean }; runCaptureOpenshell: RunCaptureOpenshell; sleepSeconds: (seconds: number) => void; now?: () => number; @@ -118,13 +116,11 @@ function startAbortableGatewayHttpProbe( export async function waitForGatewayHealth({ attachGatewayMetadataIfNeeded, - gatewayClusterHealthcheckPassed, gatewayName, healthPollCount, healthPollIntervalSeconds, isGatewayHealthy, isGatewayHttpReady, - repairGatewayBootstrapSecrets, runCaptureOpenshell, sleepSeconds, now = Date.now, @@ -138,12 +134,7 @@ export async function waitForGatewayHealth({ return ( waitOptions !== null && (await waitUntilAsync(async () => { - const repairResult = repairGatewayBootstrapSecrets(); - if (repairResult.repaired) { - attachGatewayMetadataIfNeeded({ forceRefresh: true }); - } else if (gatewayClusterHealthcheckPassed()) { - attachGatewayMetadataIfNeeded(); - } + attachGatewayMetadataIfNeeded(); const httpProbe = startAbortableGatewayHttpProbe(isGatewayHttpReady); runCaptureOpenshell(["gateway", "select", gatewayName], { ignoreError: true }); const status = runCaptureOpenshell(["status"], { ignoreError: true }); diff --git a/test/gateway-start-wait.test.ts b/test/gateway-start-wait.test.ts index 394b7c0dff7..4c1ea33d4e7 100644 --- a/test/gateway-start-wait.test.ts +++ b/test/gateway-start-wait.test.ts @@ -146,57 +146,4 @@ describe("gateway bootstrap secret repair", () => { OPENSHELL_SSH_GATEWAY_PORT: "9443", }); }); - - it("repairs the client CA and client TLS secrets together", () => { - const { getGatewayBootstrapRepairPlan } = loadOnboard(); - expect(getGatewayBootstrapRepairPlan(["openshell-client-tls"])).toEqual({ - missingSecrets: ["openshell-client-tls"], - needsRepair: true, - needsServerTls: false, - needsClientBundle: true, - needsHandshake: false, - }); - }); - - it("ignores unknown secret names when planning repairs", () => { - const { getGatewayBootstrapRepairPlan } = loadOnboard(); - - expect( - getGatewayBootstrapRepairPlan([ - "openshell-client-tls", - "noise", - " openshell-server-tls ", - "", - ]), - ).toEqual({ - missingSecrets: ["openshell-client-tls", "openshell-server-tls"], - needsRepair: true, - needsServerTls: true, - needsClientBundle: true, - needsHandshake: false, - }); - }); - - it("emits a script that creates all missing bootstrap secrets", () => { - const { buildGatewayBootstrapSecretsScript } = loadOnboard(); - const script = buildGatewayBootstrapSecretsScript([ - "openshell-server-tls", - "openshell-server-client-ca", - "openshell-client-tls", - "openshell-ssh-handshake", - ]); - - expect(script).toContain("openshell-server-tls"); - expect(script).toContain("openshell-server-client-ca"); - expect(script).toContain("openshell-client-tls"); - expect(script).toContain("openshell-ssh-handshake"); - expect(script).toContain("CN=openshell-client-ca"); - expect(script).toContain("CN=openshell-client"); - expect(script).toContain("subjectAltName=DNS:openshell"); - }); - - it("skips secret generation when nothing is missing", () => { - const { buildGatewayBootstrapSecretsScript } = loadOnboard(); - expect(buildGatewayBootstrapSecretsScript([]).trim()).toBe("exit 0"); - }); }); diff --git a/test/legacy-sandbox-transports.test.ts b/test/legacy-sandbox-transports.test.ts index 385d59785f1..0ff5bb0b03a 100644 --- a/test/legacy-sandbox-transports.test.ts +++ b/test/legacy-sandbox-transports.test.ts @@ -41,13 +41,11 @@ describe("legacy sandbox transport inventory", () => { 'captureOpenshell(["sandbox", "ssh-config", "alpha"]);', 'createTempSshConfig(config, "prefix-");', 'privilegedSandboxExecArgv("alpha", ["true"]);', - 'dockerExecArgv("openshell-cluster-nemoclaw", ["true"]);', 'dockerSpawnSync(["exec", "openshell-cluster-nemoclaw", "true"]);', ].join("\n"), }); expect(discoverLegacySandboxTransportSites(root)).toEqual([ - { relativePath: "src/transport.ts", kind: "docker-exec-builder", calls: 1 }, { relativePath: "src/transport.ts", kind: "docker-exec-command", calls: 1 }, { relativePath: "src/transport.ts", kind: "openshell-ssh-config", calls: 1 }, { relativePath: "src/transport.ts", kind: "privileged-sandbox-exec", calls: 1 }, From 701e7f1f00eae994e8e9efff8329c6f9c4a9925f Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Tue, 14 Jul 2026 23:24:58 -0700 Subject: [PATCH 3/6] refactor(openshell): route gateway stop through sandbox control Signed-off-by: Aaron Erickson --- scripts/checks/legacy-sandbox-transports.ts | 1 - src/commands/sandbox/stop.test.ts | 6 +- src/commands/sandbox/stop.ts | 2 +- src/commands/stop.ts | 2 +- src/commands/tunnel/stop.ts | 7 +- src/lib/actions/sandbox/destroy-flow.test.ts | 2 +- src/lib/actions/sandbox/destroy.ts | 18 +- src/lib/actions/sandbox/stop.test.ts | 88 ++--- src/lib/actions/sandbox/stop.ts | 6 +- src/lib/coverage-hotspots.test.ts | 7 +- src/lib/tunnel/gateway-stop-script.ts | 2 +- src/lib/tunnel/sandbox-gateway-stop.test.ts | 306 ++++++++---------- src/lib/tunnel/sandbox-gateway-stop.ts | 162 ++++------ src/lib/tunnel/service-command.test.ts | 12 +- src/lib/tunnel/service-command.ts | 6 +- .../tunnel/services-gateway-ownership.test.ts | 12 +- src/lib/tunnel/services-sandbox.test.ts | 63 ++-- src/lib/tunnel/services.test.ts | 18 +- src/lib/tunnel/services.ts | 20 +- test/destroy-cleanup-sandbox-services.test.ts | 61 +++- test/helpers/destroy-flow-test-assertions.ts | 15 +- test/helpers/destroy-flow-test-harness.ts | 2 +- 22 files changed, 403 insertions(+), 415 deletions(-) diff --git a/scripts/checks/legacy-sandbox-transports.ts b/scripts/checks/legacy-sandbox-transports.ts index eb33d6db101..a9b3160a2f2 100644 --- a/scripts/checks/legacy-sandbox-transports.ts +++ b/scripts/checks/legacy-sandbox-transports.ts @@ -53,7 +53,6 @@ const REVIEWED_SITE_TUPLES = [ ["src/lib/state/openclaw-plugin-restore.ts", "grpc-cli-read-only-fallback", 1], ["src/lib/state/sandbox.ts", "grpc-cli-read-only-fallback", 1], ["src/lib/state/user-managed-files-probe.ts", "grpc-cli-read-only-fallback", 1], - ["src/lib/tunnel/sandbox-gateway-stop.ts", "docker-exec-command", 2], ] as const satisfies readonly ReviewedSiteTuple[]; export const REVIEWED_LEGACY_SANDBOX_TRANSPORT_SITES: readonly LegacySandboxTransportSite[] = diff --git a/src/commands/sandbox/stop.test.ts b/src/commands/sandbox/stop.test.ts index b3243a113a9..f4502aff6dd 100644 --- a/src/commands/sandbox/stop.test.ts +++ b/src/commands/sandbox/stop.test.ts @@ -13,8 +13,8 @@ const rootDir = process.cwd(); describe("SandboxStopCommand", () => { beforeEach(() => { - stopSandbox.mockClear(); - stopSandbox.mockReturnValue({ exitCode: 0 }); + stopSandbox.mockReset(); + stopSandbox.mockResolvedValue({ exitCode: 0 }); }); afterEach(() => { @@ -30,7 +30,7 @@ describe("SandboxStopCommand", () => { it("propagates the action's failure exit code (#6026)", async () => { const error = vi.spyOn(console, "error").mockImplementation(() => {}); - stopSandbox.mockReturnValue({ exitCode: 1, message: " boom" }); + stopSandbox.mockResolvedValue({ exitCode: 1, message: " boom" }); await SandboxStopCommand.run(["alpha"], rootDir); diff --git a/src/commands/sandbox/stop.ts b/src/commands/sandbox/stop.ts index 46add54dc79..f33c1f5c2fd 100644 --- a/src/commands/sandbox/stop.ts +++ b/src/commands/sandbox/stop.ts @@ -21,6 +21,6 @@ export default class SandboxStopCommand extends NemoClawCommand { public async run(): Promise { const { args } = await this.parse(SandboxStopCommand); - this.applyExitResult(stopSandbox(args.sandboxName)); + this.applyExitResult(await stopSandbox(args.sandboxName)); } } diff --git a/src/commands/stop.ts b/src/commands/stop.ts index 29f4abff8df..4edff982e25 100644 --- a/src/commands/stop.ts +++ b/src/commands/stop.ts @@ -23,6 +23,6 @@ export default class DeprecatedStopCommand extends NemoClawCommand { public async run(): Promise { await this.parse(DeprecatedStopCommand); - runStopCommand({ ...serviceDeps(), stopAll, releaseGatewayPort: true }); + await runStopCommand({ ...serviceDeps(), stopAll, releaseGatewayPort: true }); } } diff --git a/src/commands/tunnel/stop.ts b/src/commands/tunnel/stop.ts index d1e5c700d8d..bb75cb21c77 100644 --- a/src/commands/tunnel/stop.ts +++ b/src/commands/tunnel/stop.ts @@ -2,10 +2,9 @@ // SPDX-License-Identifier: Apache-2.0 import { NemoClawCommand } from "../../lib/cli/nemoclaw-oclif-command"; - -import { stopAll } from "../../lib/tunnel/services"; -import { runStopCommand } from "../../lib/tunnel/service-command"; import { serviceDeps } from "../../lib/tunnel/command-support"; +import { runStopCommand } from "../../lib/tunnel/service-command"; +import { stopAll } from "../../lib/tunnel/services"; export default class TunnelStopCommand extends NemoClawCommand { static id = "tunnel:stop"; @@ -18,6 +17,6 @@ export default class TunnelStopCommand extends NemoClawCommand { public async run(): Promise { await this.parse(TunnelStopCommand); - runStopCommand({ ...serviceDeps(), stopAll }); + await runStopCommand({ ...serviceDeps(), stopAll }); } } diff --git a/src/lib/actions/sandbox/destroy-flow.test.ts b/src/lib/actions/sandbox/destroy-flow.test.ts index 7e3c1694d13..c2b857a3fd8 100644 --- a/src/lib/actions/sandbox/destroy-flow.test.ts +++ b/src/lib/actions/sandbox/destroy-flow.test.ts @@ -46,7 +46,7 @@ describe("destroySandbox flow", () => { }); it("selects the sandbox gateway, deletes live resources, cleans host state, and removes registry state", async () => { - const harness = createDestroyHarness(); + const harness = createDestroyHarness({ registeredSandboxCount: 1 }); await expect( harness.destroySandbox("alpha", { yes: true, cleanupGateway: true }), diff --git a/src/lib/actions/sandbox/destroy.ts b/src/lib/actions/sandbox/destroy.ts index 071b8605e87..ea5187dcd46 100644 --- a/src/lib/actions/sandbox/destroy.ts +++ b/src/lib/actions/sandbox/destroy.ts @@ -29,8 +29,8 @@ import { resolveNemoclawStateDir } from "../../state/paths"; import * as registry from "../../state/registry"; import { confirmSandboxDestroy } from "./destroy-confirmation"; import { executeSandboxDestroy } from "./destroy-execution"; -import { shouldCleanupGatewayAfterConfirmedFinalDestroy } from "./destroy-gateway-cleanup"; import { cleanupGatewayAfterLastSandbox } from "./destroy-gateway"; +import { shouldCleanupGatewayAfterConfirmedFinalDestroy } from "./destroy-gateway-cleanup"; import { prepareSandboxDestroy } from "./destroy-preflight"; import { type WipeSandboxStateDeps, wipeSandboxState } from "./wipe-state"; @@ -57,7 +57,7 @@ type RunOpenshell = (args: string[], opts?: Record) => { status export type CleanupSandboxServicesDeps = { getSandbox?: typeof registry.getSandbox; - stopAll?: (opts: { sandboxName: string }) => void; + stopAll?: (opts: { sandboxName: string; skipSandboxStop: true }) => Promise; unloadOllamaModels?: () => void; runOpenshell?: RunOpenshell; rmSync?: typeof fs.rmSync; @@ -99,11 +99,11 @@ async function resolveCleanupGatewayDecision(options: DestroySandboxOptions): Pr return trimmed === "y" || trimmed === "yes"; } -export function cleanupSandboxServices( +export async function cleanupSandboxServices( sandboxName: string, { stopHostServices = false }: { stopHostServices?: boolean } = {}, deps: CleanupSandboxServicesDeps = {}, -): void { +): Promise { // Source boundary: this exported helper can be called independently of CLI // dispatch, including from forced local recovery. Validate once before every // host and provider cleanup side effect, then derive the PID path from that @@ -114,11 +114,11 @@ export function cleanupSandboxServices( const getSandbox = deps.getSandbox ?? registry.getSandbox; const stopAll = deps.stopAll ?? - ((opts: { sandboxName: string }) => { + ((opts: { sandboxName: string; skipSandboxStop: true }) => { const services = require("../../tunnel/services") as { - stopAll: (opts: { sandboxName: string }) => void; + stopAll: (opts: { sandboxName: string; skipSandboxStop: true }) => Promise; }; - services.stopAll(opts); + return services.stopAll(opts); }); const unloadOllamaModels = deps.unloadOllamaModels ?? @@ -141,7 +141,7 @@ export function cleanupSandboxServices( if (stopHostServices) { // `stopAll()` already runs `unloadOllamaModels()` unconditionally — // see src/lib/tunnel/services.ts. Don't double-call here. - stopAll({ sandboxName: validatedSandboxName }); + await stopAll({ sandboxName: validatedSandboxName, skipSandboxStop: true }); } else { // No global stop, so `stopAll()` did not run; explicitly free Ollama // models for this sandbox if its provider used Ollama. Without this @@ -380,7 +380,7 @@ async function destroySandboxUnlocked( sandboxStillRegistered: !!registry.getSandbox(sandboxName), }); - cleanupSandboxServices(sandboxName, { + await cleanupSandboxServices(sandboxName, { stopHostServices: shouldStopHostServices, }); // The sandbox's gateway was captured before the registry entry is removed — diff --git a/src/lib/actions/sandbox/stop.test.ts b/src/lib/actions/sandbox/stop.test.ts index d2ebcb2a209..6ca79a46a09 100644 --- a/src/lib/actions/sandbox/stop.test.ts +++ b/src/lib/actions/sandbox/stop.test.ts @@ -24,7 +24,9 @@ function harness(overrides: Partial = {}) { const findLabeledSandboxContainers = vi.fn< NonNullable >(() => [container("openshell-my-sandbox", true)]); - const stopSandboxChannels = vi.fn>(); + const stopSandboxChannels = vi.fn>( + async () => {}, + ); const dockerStop = vi.fn>(() => ({ status: 0 })); const log = vi.fn<(message: string) => void>(); const warn = vi.fn<(message: string) => void>(); @@ -53,16 +55,28 @@ function harness(overrides: Partial = {}) { } describe("stopSandbox", () => { - it("gracefully stops in-sandbox channels before stopping the container (#6026)", () => { + it("waits for in-sandbox channels before stopping the container (#6026)", async () => { const h = harness(); + let resolveChannelStop!: () => void; + h.stopSandboxChannels.mockImplementation( + () => + new Promise((resolve) => { + resolveChannelStop = resolve; + }), + ); - const result = stopSandbox("my-sandbox", h.deps); + const pending = stopSandbox("my-sandbox", h.deps); - expect(result.exitCode).toBe(0); expect(h.stopSandboxChannels).toHaveBeenCalledWith( "my-sandbox", expect.objectContaining({ info: expect.any(Function), warn: expect.any(Function) }), ); + expect(h.dockerStop).not.toHaveBeenCalled(); + + resolveChannelStop(); + const result = await pending; + + expect(result.exitCode).toBe(0); expect(h.dockerStop).toHaveBeenCalledWith("openshell-my-sandbox", { ignoreError: true, timeout: 30_000, @@ -72,23 +86,23 @@ describe("stopSandbox", () => { ); }); - it("routes channel-stop reporter lines through the action's log and warn (#6026)", () => { + it("routes channel-stop reporter lines through the action's log and warn (#6026)", async () => { const h = harness(); - h.stopSandboxChannels.mockImplementation((_name, channelDeps) => { + h.stopSandboxChannels.mockImplementation(async (_name, channelDeps) => { channelDeps?.info?.("gateway stopped inside sandbox."); channelDeps?.warn?.("could not reach gateway."); }); - stopSandbox("my-sandbox", h.deps); + await stopSandbox("my-sandbox", h.deps); expect(h.log).toHaveBeenCalledWith(" gateway stopped inside sandbox."); expect(h.warn).toHaveBeenCalledWith(" could not reach gateway."); }); - it("preserves the registry entry and tells the user how to start again (#6026)", () => { + it("preserves the registry entry and tells the user how to start again (#6026)", async () => { const h = harness(); - const result = stopSandbox("my-sandbox", h.deps); + const result = await stopSandbox("my-sandbox", h.deps); expect(result.exitCode).toBe(0); const output = h.log.mock.calls.map(([line]) => line).join("\n"); @@ -96,11 +110,11 @@ describe("stopSandbox", () => { expect(output).toContain("nemoclaw my-sandbox start"); }); - it("succeeds idempotently when the container is already stopped (#6026)", () => { + it("succeeds idempotently when the container is already stopped (#6026)", async () => { const h = harness(); h.findLabeledSandboxContainers.mockReturnValue([container("openshell-my-sandbox", false)]); - const result = stopSandbox("my-sandbox", h.deps); + const result = await stopSandbox("my-sandbox", h.deps); expect(result.exitCode).toBe(0); expect(h.dockerStop).not.toHaveBeenCalled(); @@ -108,13 +122,13 @@ describe("stopSandbox", () => { expect(output).toContain("already stopped"); }); - it("stops a crash-looping container instead of calling it stopped (#6026)", () => { + it("stops a crash-looping container instead of calling it stopped (#6026)", async () => { const h = harness(); h.findLabeledSandboxContainers.mockReturnValue([ { name: "openshell-my-sandbox", status: "Restarting (137) 2 seconds ago", running: false }, ]); - const result = stopSandbox("my-sandbox", h.deps); + const result = await stopSandbox("my-sandbox", h.deps); expect(result.exitCode).toBe(0); expect(h.dockerStop).toHaveBeenCalledWith("openshell-my-sandbox", { @@ -123,38 +137,36 @@ describe("stopSandbox", () => { }); }); - it("stops a paused container (#6026)", () => { + it("stops a paused container (#6026)", async () => { const h = harness(); h.findLabeledSandboxContainers.mockReturnValue([ { name: "openshell-my-sandbox", status: "Up 5 minutes (Paused)", running: true }, ]); - const result = stopSandbox("my-sandbox", h.deps); + const result = await stopSandbox("my-sandbox", h.deps); expect(result.exitCode).toBe(0); expect(h.dockerStop).toHaveBeenCalledTimes(1); }); - it("stops every running labeled container, including backup siblings (#6026)", () => { + it("stops every running labeled container, including backup siblings (#6026)", async () => { const h = harness(); h.findLabeledSandboxContainers.mockReturnValue([ container("openshell-my-sandbox", true), container("openshell-my-sandbox-nemoclaw-gpu-backup-1700000000000", true), ]); - const result = stopSandbox("my-sandbox", h.deps); + const result = await stopSandbox("my-sandbox", h.deps); expect(result.exitCode).toBe(0); expect(h.dockerStop).toHaveBeenCalledTimes(2); }); - it("continues to docker stop when the graceful channel stop throws (#6026)", () => { + it("continues to docker stop when the graceful channel stop rejects (#6026)", async () => { const h = harness(); - h.stopSandboxChannels.mockImplementation(() => { - throw new Error("gateway unreachable"); - }); + h.stopSandboxChannels.mockRejectedValue(new Error("gateway unreachable")); - const result = stopSandbox("my-sandbox", h.deps); + const result = await stopSandbox("my-sandbox", h.deps); expect(result.exitCode).toBe(0); expect(h.dockerStop).toHaveBeenCalledTimes(1); @@ -162,11 +174,11 @@ describe("stopSandbox", () => { expect(warned).toContain("gateway unreachable"); }); - it("names the Docker daemon outage instead of claiming the container was removed (#6026)", () => { + it("names the Docker daemon outage instead of claiming the container was removed (#6026)", async () => { const h = harness(); h.isDockerRuntimeDown.mockReturnValue(true); - const result = stopSandbox("my-sandbox", h.deps); + const result = await stopSandbox("my-sandbox", h.deps); expect(result.exitCode).toBe(1); expect(result.message).toBeUndefined(); @@ -177,11 +189,11 @@ describe("stopSandbox", () => { expect(h.dockerStop).not.toHaveBeenCalled(); }); - it("fails with a rebuild hint when no labeled container exists (#6026)", () => { + it("fails with a rebuild hint when no labeled container exists (#6026)", async () => { const h = harness(); h.findLabeledSandboxContainers.mockReturnValue([]); - const result = stopSandbox("my-sandbox", h.deps); + const result = await stopSandbox("my-sandbox", h.deps); expect(result.exitCode).toBe(1); expect(result.message).toContain("No Docker container"); @@ -189,22 +201,22 @@ describe("stopSandbox", () => { expect(h.dockerStop).not.toHaveBeenCalled(); }); - it("refuses an unregistered sandbox (#6026)", () => { + it("refuses an unregistered sandbox (#6026)", async () => { const h = harness(); h.getSandbox.mockReturnValue(null); - const result = stopSandbox("ghost", h.deps); + const result = await stopSandbox("ghost", h.deps); expect(result.exitCode).toBe(1); expect(result.message).toContain("not registered"); expect(h.findLabeledSandboxContainers).not.toHaveBeenCalled(); }); - it("refuses non-direct drivers instead of guessing at container control (#6026)", () => { + it("refuses non-direct drivers instead of guessing at container control (#6026)", async () => { const h = harness(); h.getSandbox.mockReturnValue(sandbox({ openshellDriver: "kubernetes" })); - const result = stopSandbox("my-sandbox", h.deps); + const result = await stopSandbox("my-sandbox", h.deps); expect(result.exitCode).toBe(1); expect(result.message).toContain("kubernetes"); @@ -215,27 +227,27 @@ describe("stopSandbox", () => { ["null driver", sandbox({ openshellDriver: null })], ["docker driver", sandbox({ openshellDriver: "docker" })], ["vm driver", sandbox({ openshellDriver: "vm" })], - ])("allows the %s like privileged exec does (#6026)", (_label, entry) => { + ])("allows the %s like privileged exec does (#6026)", async (_label, entry) => { const h = harness(); h.getSandbox.mockReturnValue(entry); - const result = stopSandbox("my-sandbox", h.deps); + const result = await stopSandbox("my-sandbox", h.deps); expect(result.exitCode).toBe(0); }); - it("surfaces a docker stop failure with the container name (#6026)", () => { + it("surfaces a docker stop failure with the container name (#6026)", async () => { const h = harness(); h.dockerStop.mockReturnValue({ status: 125 }); - const result = stopSandbox("my-sandbox", h.deps); + const result = await stopSandbox("my-sandbox", h.deps); expect(result.exitCode).toBe(1); expect(result.message).toContain("openshell-my-sandbox"); expect(result.message).toContain("125"); }); - it("attempts every container and aggregates failures when one stop fails (#6026)", () => { + it("attempts every container and aggregates failures when one stop fails (#6026)", async () => { const h = harness(); h.findLabeledSandboxContainers.mockReturnValue([ container("openshell-my-sandbox", true), @@ -244,7 +256,7 @@ describe("stopSandbox", () => { // First container fails to stop; the sibling still must be attempted. h.dockerStop.mockReturnValueOnce({ status: 137 }).mockReturnValueOnce({ status: 0 }); - const result = stopSandbox("my-sandbox", h.deps); + const result = await stopSandbox("my-sandbox", h.deps); expect(result.exitCode).toBe(1); expect(h.dockerStop).toHaveBeenCalledTimes(2); @@ -261,10 +273,10 @@ describe("stopSandbox", () => { expect(result.message).not.toContain("gpu-backup"); }); - it("never removes containers or touches the registry entry (#6026)", () => { + it("never removes containers or touches the registry entry (#6026)", async () => { const h = harness(); - stopSandbox("my-sandbox", h.deps); + await stopSandbox("my-sandbox", h.deps); // The deps surface has no removal lever at all; assert the only docker // mutation issued is the stop of the labeled container. diff --git a/src/lib/actions/sandbox/stop.ts b/src/lib/actions/sandbox/stop.ts index 1fc5e20376d..2dd57d48504 100644 --- a/src/lib/actions/sandbox/stop.ts +++ b/src/lib/actions/sandbox/stop.ts @@ -106,10 +106,10 @@ export function gateDockerRuntimeUp( * The shared host gateway, tunnel, and any NIM inference container are * gateway-scoped and serve other sandboxes — deliberately untouched. */ -export function stopSandbox( +export async function stopSandbox( sandboxName: string, deps: SandboxStopDeps = {}, -): SandboxLifecycleResult { +): Promise { const log = deps.log ?? console.log; const warn = deps.warn ?? console.warn; @@ -148,7 +148,7 @@ export function stopSandbox( // Agent-managed gateways (e.g. Hermes) are supervised inside the sandbox // and shut down with the container's stop signal instead. try { - (deps.stopSandboxChannels ?? stopSandboxChannels)(sandboxName, { + await (deps.stopSandboxChannels ?? stopSandboxChannels)(sandboxName, { info: (message) => log(` ${message}`), warn: (message) => warn(` ${message}`), }); diff --git a/src/lib/coverage-hotspots.test.ts b/src/lib/coverage-hotspots.test.ts index 899ddf4a8a4..ba7956182db 100644 --- a/src/lib/coverage-hotspots.test.ts +++ b/src/lib/coverage-hotspots.test.ts @@ -5,7 +5,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { describe, expect, it } from "vitest"; - +import { getVersion } from "./core/version"; import { parseDuration } from "./domain/duration"; import { parseGatewayTokenArgs, runGatewayTokenCommand } from "./gateway-token-command"; import { @@ -13,7 +13,6 @@ import { runStartCommand, runStopCommand, } from "./tunnel/service-command"; -import { getVersion } from "./core/version"; // Narrow coverage guard for small helper modules that are otherwise only // exercised through subprocess CLI flows in this migration stack. @@ -70,9 +69,9 @@ describe("small CLI helper coverage", () => { expect(startCalls).toEqual([{ sandboxName: "alpha" }]); const stopCalls: Array<{ sandboxName?: string }> = []; - runStopCommand({ + await runStopCommand({ listSandboxes: () => ({ defaultSandbox: null }), - stopAll: (options) => { + stopAll: async (options) => { stopCalls.push(options); }, }); diff --git a/src/lib/tunnel/gateway-stop-script.ts b/src/lib/tunnel/gateway-stop-script.ts index 57cd5bf51da..bae5c6d4265 100644 --- a/src/lib/tunnel/gateway-stop-script.ts +++ b/src/lib/tunnel/gateway-stop-script.ts @@ -26,7 +26,7 @@ // IMPORTANT: keep example argv strings (e.g. "openclaw gateway") out of the awk // program text. awk's own argv is captured by the concurrent `ps` snapshot, so // any such literal inside the program makes awk match itself and the scan never -// drains. The shell `sh -lc` process is exempt (excluded as $self), comments in +// drains. The shell `sh -s` process is exempt (excluded as $self), comments in // the shell portion below are fine, but the awk body must stay token-clean. export const GATEWAY_STOP_SCRIPT = String.raw` set -eu diff --git a/src/lib/tunnel/sandbox-gateway-stop.test.ts b/src/lib/tunnel/sandbox-gateway-stop.test.ts index 8e1dda492d7..2eca9986055 100644 --- a/src/lib/tunnel/sandbox-gateway-stop.test.ts +++ b/src/lib/tunnel/sandbox-gateway-stop.test.ts @@ -1,22 +1,16 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import type { SpawnSyncReturns } from "node:child_process"; import { describe, expect, it, vi } from "vitest"; +import type { SandboxExecResult } from "../adapters/openshell/sandbox-control"; import type { AgentDefinition } from "../agent/defs"; import type { SandboxEntry } from "../state/registry"; +import { GATEWAY_STOP_SCRIPT } from "./gateway-stop-script"; import { type SandboxGatewayStopDeps, stopSandboxChannels } from "./sandbox-gateway-stop"; -function spawnResult(status: number | null, stdout = "", stderr = ""): SpawnSyncReturns { - return { - pid: 1, - output: [null, stdout, stderr], - stdout, - stderr, - status, - signal: null, - }; +function execResult(status: number | null, stdout = "", stderr = ""): SandboxExecResult { + return { status, stdout, stderr }; } function registeredAgent( @@ -34,119 +28,152 @@ function harness() { const getRegisteredAgent = vi.fn>( () => null, ); - const resolveOpenshell = vi.fn>( - () => "/usr/local/bin/openshell", - ); - const runDocker = vi.fn>(() => spawnResult(1)); - const runProcess = vi.fn>(() => spawnResult(0)); + const exec = vi.fn(async () => execResult(0)); + const close = vi.fn(); + const assertNoGatewayEndpointOverride = vi.fn(); + const selectControl = vi.fn>(() => ({ + control: { exec }, + transport: "grpc", + close, + })); const info = vi.fn<(message: string) => void>(); const warn = vi.fn<(message: string) => void>(); const deps: SandboxGatewayStopDeps = { + assertNoGatewayEndpointOverride, getSandbox, getRegisteredAgent, - resolveOpenshell, - runDocker, - runProcess, + selectControl, info, warn, }; return { + assertNoGatewayEndpointOverride, + close, deps, + exec, getRegisteredAgent, getSandbox, info, - resolveOpenshell, - runDocker, - runProcess, + selectControl, warn, }; } describe("stopSandboxChannels", () => { - it("uses kubectl via the OpenShell gateway container for privileged shutdown", () => { + it("selects the persisted gateway and sends one bounded stdin request", async () => { const h = harness(); - h.runDocker - .mockReturnValueOnce(spawnResult(0, "pod/my-sandbox-0\n")) - .mockReturnValueOnce(spawnResult(0)); - - stopSandboxChannels("my-sandbox", h.deps); - - expect(h.runDocker).toHaveBeenNthCalledWith( - 1, - [ - "exec", - "openshell-cluster-nemoclaw", - "kubectl", - "get", - "pods", - "-n", - "openshell", - "-o", - "name", - ], - expect.objectContaining({ timeout: 10000 }), + h.getSandbox.mockReturnValue(sandbox({ gatewayName: "nemoclaw-18080", gatewayPort: 18080 })); + + await stopSandboxChannels("my-sandbox", h.deps); + + expect(h.assertNoGatewayEndpointOverride).toHaveBeenCalledTimes(1); + expect(h.selectControl).toHaveBeenCalledOnce(); + expect(h.selectControl).toHaveBeenCalledWith("nemoclaw-18080"); + expect(h.exec).toHaveBeenCalledOnce(); + expect(h.exec).toHaveBeenCalledWith({ + sandboxName: "my-sandbox", + command: ["sh", "-s"], + stdin: GATEWAY_STOP_SCRIPT, + timeoutMs: 20_000, + maxOutputBytes: 64 * 1024, + }); + expect(h.close).toHaveBeenCalledOnce(); + expect(h.assertNoGatewayEndpointOverride.mock.invocationCallOrder[0]).toBeLessThan( + h.selectControl.mock.invocationCallOrder[0], ); - const args = h.runDocker.mock.calls[1][0]; - expect(args).toEqual( - expect.arrayContaining(["kubectl", "exec", "-n", "openshell", "-c", "agent"]), + expect(h.selectControl.mock.invocationCallOrder[0]).toBeLessThan( + h.exec.mock.invocationCallOrder[0], ); - const script = String(args.at(-1)); - expect(script).toContain("ps -eo user=,pid=,args="); - expect(script).toContain("openclaw-gateway"); - expect(script).toContain("kill -TERM $pids"); - expect(script).toContain("kill -KILL $remaining"); - expect(h.info).toHaveBeenCalledWith("OpenClaw gateway stopped inside sandbox."); + expect(h.exec.mock.invocationCallOrder[0]).toBeLessThan(h.close.mock.invocationCallOrder[0]); }); - it("falls back to gateway-scoped openshell sandbox exec through stdin", () => { + it.each([ + [0, "OpenClaw gateway stopped inside sandbox."], + [1, "OpenClaw gateway was not running inside sandbox."], + ])("reports stop script exit %i as success", async (status, message) => { const h = harness(); + h.exec.mockResolvedValueOnce(execResult(status)); - stopSandboxChannels("my-sandbox", h.deps); + await stopSandboxChannels("my-sandbox", h.deps); - expect(h.runProcess).toHaveBeenCalledWith( - "/usr/local/bin/openshell", - ["sandbox", "exec", "--name", "my-sandbox", "--gateway", "nemoclaw", "--", "sh", "-s"], - expect.objectContaining({ - input: expect.stringContaining("find_gateway_pids"), - timeout: 20000, - }), - ); + expect(h.info).toHaveBeenCalledWith(message); + expect(h.warn).not.toHaveBeenCalled(); }); - it("selects the exact generated sandbox pod and excludes overlapping names", () => { + it("closes the selected control after an exec failure without replaying", async () => { const h = harness(); - h.runDocker - .mockReturnValueOnce(spawnResult(0, "pod/prod-app-abc\npod/app-abc\n")) - .mockReturnValueOnce(spawnResult(0)); + h.exec.mockRejectedValueOnce(new Error("connection closed")); - stopSandboxChannels("app", h.deps); + await stopSandboxChannels("my-sandbox", h.deps); - const args = h.runDocker.mock.calls[1][0]; - expect(args).toContain("pod/app-abc"); - expect(args).not.toContain("pod/prod-app-abc"); + expect(h.selectControl).toHaveBeenCalledOnce(); + expect(h.exec).toHaveBeenCalledOnce(); + expect(h.close).toHaveBeenCalledOnce(); + expect(h.warn).toHaveBeenCalledWith(expect.stringContaining("connection closed")); }); - it("falls back when no exact generated sandbox pod name is available", () => { + it("reports bounded exec failures and closes the selected control", async () => { const h = harness(); - h.runDocker.mockReturnValueOnce(spawnResult(0, "pod/prod-app-abc\npod/app-copy-abc\n")); + h.exec.mockResolvedValueOnce({ + ...execResult(null, "stdout detail", "stderr detail"), + error: new Error("deadline exceeded"), + }); - stopSandboxChannels("app", h.deps); + await stopSandboxChannels("my-sandbox", h.deps); - expect(h.runProcess).toHaveBeenCalledTimes(1); + expect(h.exec).toHaveBeenCalledOnce(); + expect(h.close).toHaveBeenCalledOnce(); + expect(h.warn).toHaveBeenCalledWith(expect.stringContaining("exit unknown")); + const output = h.warn.mock.calls.map((call) => call[0]).join("\n"); + expect(output).toContain("deadline exceeded"); + expect(output).toContain("stderr detail"); + expect(output).toContain("stdout detail"); + expect(output).toContain("gateway may still be running"); }); - it("treats stop script exit 1 as already stopped", () => { + it.each([ + [0, { error: new Error("stream reset") }, "stream reset"], + [1, { signal: "SIGTERM" as NodeJS.Signals }, "signal SIGTERM"], + ])("does not treat exit %i with transport failure state as success", async (status, failure, detail) => { const h = harness(); - h.runDocker - .mockReturnValueOnce(spawnResult(0, "pod/my-sandbox-0\n")) - .mockReturnValueOnce(spawnResult(1)); + h.exec.mockResolvedValueOnce({ ...execResult(status), ...failure }); - stopSandboxChannels("my-sandbox", h.deps); + await stopSandboxChannels("my-sandbox", h.deps); - expect(h.info).toHaveBeenCalledWith("OpenClaw gateway was not running inside sandbox."); + expect(h.info).not.toHaveBeenCalledWith(expect.stringMatching(/stopped|not running/u)); + expect(h.warn).toHaveBeenCalledWith(expect.stringContaining(detail)); }); - it("does not transiently kill a supervisor-managed Hermes gateway (#6392)", () => { + it("does not select a control when the endpoint guard rejects overrides", async () => { + const h = harness(); + h.assertNoGatewayEndpointOverride.mockImplementationOnce(() => { + throw new Error("endpoint override is forbidden"); + }); + + await stopSandboxChannels("my-sandbox", h.deps); + + expect(h.selectControl).not.toHaveBeenCalled(); + expect(h.exec).not.toHaveBeenCalled(); + expect(h.close).not.toHaveBeenCalled(); + expect(h.warn).toHaveBeenCalledWith(expect.stringContaining("endpoint override is forbidden")); + }); + + it("fails closed before dispatch when mutation control selection fails", async () => { + const h = harness(); + h.selectControl.mockImplementationOnce(() => { + throw new Error("direct gRPC configuration unavailable"); + }); + + await stopSandboxChannels("my-sandbox", h.deps); + + expect(h.exec).not.toHaveBeenCalled(); + expect(h.close).not.toHaveBeenCalled(); + expect(h.warn).toHaveBeenCalledWith( + expect.stringContaining("direct gRPC configuration unavailable"), + ); + }); + + it("does not transiently kill a supervisor-managed Hermes gateway (#6392)", async () => { const h = harness(); const hermes = registeredAgent({ name: "hermes", @@ -157,16 +184,15 @@ describe("stopSandboxChannels", () => { h.getSandbox.mockReturnValue(sandbox({ agent: "hermes" })); h.getRegisteredAgent.mockReturnValue(hermes); - stopSandboxChannels("my-sandbox", h.deps); + await stopSandboxChannels("my-sandbox", h.deps); - expect(h.runDocker).not.toHaveBeenCalled(); - expect(h.runProcess).not.toHaveBeenCalled(); + expect(h.selectControl).not.toHaveBeenCalled(); expect(h.info.mock.calls.map((call) => call[0]).join("\n")).toContain( "Hermes Agent gateway is managed by the sandbox", ); }); - it("does not treat a terminal agent command as a gateway process", () => { + it("does not treat a terminal agent command as a gateway process", async () => { const h = harness(); const terminal = registeredAgent({ name: "langchain-deepagents-code", @@ -180,79 +206,71 @@ describe("stopSandboxChannels", () => { h.getSandbox.mockReturnValue(sandbox({ agent: "langchain-deepagents-code" })); h.getRegisteredAgent.mockReturnValue(terminal); - stopSandboxChannels("dcode-sandbox", h.deps); + await stopSandboxChannels("dcode-sandbox", h.deps); - expect(h.runDocker).not.toHaveBeenCalled(); - expect(h.runProcess).not.toHaveBeenCalled(); + expect(h.selectControl).not.toHaveBeenCalled(); expect(h.info).toHaveBeenCalledWith( "LangChain Deep Agents Code has no gateway runtime; skipping in-sandbox gateway stop.", ); }); - it("ignores an unrelated global session when the target registry row is missing", () => { + it("requires an exact persisted sandbox record", async () => { const h = harness(); h.getSandbox.mockReturnValue(null); - h.runDocker - .mockReturnValueOnce(spawnResult(0, "pod/openclaw-target-abc\n")) - .mockReturnValueOnce(spawnResult(0)); - stopSandboxChannels("openclaw-target", h.deps); + await stopSandboxChannels("openclaw-target", h.deps); - expect(h.getRegisteredAgent).toHaveBeenCalledWith(null); - expect(h.runDocker).toHaveBeenCalledTimes(2); - expect(h.info.mock.calls.map((call) => call[0]).join("\n")).not.toContain("Hermes Agent"); + expect(h.getRegisteredAgent).not.toHaveBeenCalled(); + expect(h.selectControl).not.toHaveBeenCalled(); + expect(h.warn).toHaveBeenCalledWith( + "Sandbox 'openclaw-target' is not registered; skipping in-sandbox gateway stop.", + ); }); - it("fails closed when the sandbox registry cannot be read", () => { + it("fails closed when the sandbox registry cannot be read", async () => { const h = harness(); h.getSandbox.mockImplementation(() => { throw new Error("invalid registry data"); }); - expect(() => stopSandboxChannels("my-sandbox", h.deps)).not.toThrow(); + await expect(stopSandboxChannels("my-sandbox", h.deps)).resolves.toBeUndefined(); expect(h.getRegisteredAgent).not.toHaveBeenCalled(); - expect(h.runDocker).not.toHaveBeenCalled(); - expect(h.runProcess).not.toHaveBeenCalled(); + expect(h.selectControl).not.toHaveBeenCalled(); expect(h.warn).toHaveBeenCalledWith( expect.stringContaining("Could not read the sandbox registry for 'my-sandbox'"), ); }); - it("fails closed when the persisted gateway binding is invalid", () => { + it("fails closed when the persisted gateway binding is invalid", async () => { const h = harness(); h.getSandbox.mockReturnValue(sandbox({ agent: "openclaw", gatewayPort: 0 })); - stopSandboxChannels("my-sandbox", h.deps); + await stopSandboxChannels("my-sandbox", h.deps); - expect(h.runDocker).not.toHaveBeenCalled(); - expect(h.resolveOpenshell).not.toHaveBeenCalled(); - expect(h.runProcess).not.toHaveBeenCalled(); + expect(h.selectControl).not.toHaveBeenCalled(); expect(h.warn.mock.calls.map((call) => call[0]).join("\n")).toContain( "Invalid persisted sandbox gateway binding", ); }); - it("fails closed when a registered non-OpenClaw agent cannot be loaded", () => { + it("fails closed when a registered non-OpenClaw agent cannot be loaded", async () => { const h = harness(); h.getSandbox.mockReturnValue(sandbox({ agent: "missing-agent" })); - stopSandboxChannels("my-sandbox", h.deps); + await stopSandboxChannels("my-sandbox", h.deps); - expect(h.runDocker).not.toHaveBeenCalled(); - expect(h.runProcess).not.toHaveBeenCalled(); + expect(h.selectControl).not.toHaveBeenCalled(); expect(h.warn).toHaveBeenCalledWith( expect.stringContaining("Could not resolve registered agent 'missing-agent'"), ); }); - it("warns when privileged shutdown reports the gateway may still be running", () => { + it("warns when sandbox exec reports the gateway may still be running", async () => { const h = harness(); - h.runDocker - .mockReturnValueOnce(spawnResult(0, "pod/my-sandbox-0\n")) - .mockReturnValueOnce(spawnResult(2, "", "205")); + h.exec.mockResolvedValueOnce(execResult(2, "", "205")); - stopSandboxChannels("my-sandbox", h.deps); + await stopSandboxChannels("my-sandbox", h.deps); const output = h.warn.mock.calls.map((call) => call[0]).join("\n"); expect(output).toContain("Could not stop OpenClaw gateway inside sandbox"); @@ -260,67 +278,11 @@ describe("stopSandboxChannels", () => { expect(output).toContain("205"); }); - it("warns when spawn returns null status", () => { - const h = harness(); - h.runDocker - .mockReturnValueOnce(spawnResult(0, "pod/my-sandbox-0\n")) - .mockReturnValueOnce(spawnResult(null)); - - stopSandboxChannels("my-sandbox", h.deps); - - expect(h.warn).toHaveBeenCalledWith(expect.stringContaining("exit unknown")); - }); - - it("warns when privileged shutdown is unavailable and openshell is not found", () => { - const h = harness(); - h.resolveOpenshell.mockReturnValue(null); - - stopSandboxChannels("my-sandbox", h.deps); - - expect(h.runProcess).not.toHaveBeenCalled(); - expect(h.warn).toHaveBeenCalledWith( - "openshell not found — cannot stop OpenClaw gateway inside sandbox.", - ); - }); - - it("routes shutdown through the sandbox's persisted non-default gateway", () => { - const h = harness(); - h.getSandbox.mockReturnValue(sandbox({ gatewayName: "nemoclaw-18080", gatewayPort: 18080 })); - - stopSandboxChannels("my-sandbox", h.deps); - - expect(h.runDocker).toHaveBeenCalledWith( - expect.arrayContaining(["exec", "openshell-cluster-nemoclaw-18080", "kubectl"]), - expect.any(Object), - ); - expect(h.runProcess).toHaveBeenCalledWith( - "/usr/local/bin/openshell", - expect.arrayContaining(["--gateway", "nemoclaw-18080"]), - expect.any(Object), - ); - }); - - it("targets launcher, re-exec, and identity-guarded bare gateway forms", () => { - const h = harness(); - h.runDocker - .mockReturnValueOnce(spawnResult(0, "pod/my-sandbox-0\n")) - .mockReturnValueOnce(spawnResult(0)); - - stopSandboxChannels("my-sandbox", h.deps); - - const script = String(h.runDocker.mock.calls[1][0].at(-1)); - expect(script).toContain("openclaw-gateway"); - expect(script).toContain("openclaw[[:space:]]+gateway"); - expect(script).toContain("openclaw[[:space:]]*$"); - expect(script).toContain("identity_files_trusted"); - }); - - it("rejects malformed sandbox names before any spawn", () => { + it("rejects malformed sandbox names before selecting a control", async () => { const h = harness(); - expect(() => stopSandboxChannels("../escape", h.deps)).toThrow("Invalid sandbox name"); + await expect(stopSandboxChannels("../escape", h.deps)).rejects.toThrow("Invalid sandbox name"); expect(h.getSandbox).not.toHaveBeenCalled(); - expect(h.runDocker).not.toHaveBeenCalled(); - expect(h.runProcess).not.toHaveBeenCalled(); + expect(h.selectControl).not.toHaveBeenCalled(); }); }); diff --git a/src/lib/tunnel/sandbox-gateway-stop.ts b/src/lib/tunnel/sandbox-gateway-stop.ts index 7176783c7b1..37704088122 100644 --- a/src/lib/tunnel/sandbox-gateway-stop.ts +++ b/src/lib/tunnel/sandbox-gateway-stop.ts @@ -2,40 +2,33 @@ // SPDX-License-Identifier: Apache-2.0 import { - type SpawnSyncOptionsWithStringEncoding, - type SpawnSyncReturns, - spawnSync, -} from "node:child_process"; - -import { dockerSpawnSync } from "../adapters/docker"; -import { getGatewayClusterContainerName } from "../adapters/openshell/gateway-drift"; -import { resolveOpenshell } from "../adapters/openshell/resolve"; + type SandboxExecRequest, + type SandboxExecResult, + validateOpenShellExecRequest, +} from "../adapters/openshell/sandbox-control"; +import { selectOpenShellSandboxControlForMutation } from "../adapters/openshell/sandbox-control-routing"; import * as agentRuntime from "../agent/runtime"; import { resolveSandboxGatewayName } from "../onboard/gateway-binding"; +import { assertNoOpenShellGatewayEndpointOverride } from "../openshell-gateway-endpoint-guard"; import * as registry from "../state/registry"; import { GATEWAY_STOP_SCRIPT } from "./gateway-stop-script"; type Reporter = (message: string) => void; -type StopAttemptResult = ReturnType; -type ProcessRunner = ( - command: string, - args: readonly string[], - options: SpawnSyncOptionsWithStringEncoding, -) => SpawnSyncReturns; export type SandboxGatewayStopDeps = { getSandbox?: typeof registry.getSandbox; getRegisteredAgent?: typeof agentRuntime.getRegisteredAgent; getAgentDisplayName?: typeof agentRuntime.getAgentDisplayName; hasGatewayRuntime?: typeof agentRuntime.hasGatewayRuntime; - resolveOpenshell?: typeof resolveOpenshell; - runDocker?: typeof dockerSpawnSync; - runProcess?: ProcessRunner; + assertNoGatewayEndpointOverride?: typeof assertNoOpenShellGatewayEndpointOverride; + selectControl?: typeof selectOpenShellSandboxControlForMutation; info?: Reporter; warn?: Reporter; }; const SAFE_NAME_RE = /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/; +const GATEWAY_STOP_TIMEOUT_MS = 20_000; +const GATEWAY_STOP_MAX_OUTPUT_BYTES = 64 * 1024; function defaultInfo(message: string): void { console.log(`[services] ${message}`); @@ -53,7 +46,10 @@ function validateSandboxName(name: string): string { } /** Stop only a proven OpenClaw gateway; supervised agents remain owned by their sandbox. */ -export function stopSandboxChannels(sandboxName: string, deps: SandboxGatewayStopDeps = {}): void { +export async function stopSandboxChannels( + sandboxName: string, + deps: SandboxGatewayStopDeps = {}, +): Promise { const info = deps.info ?? defaultInfo; const warn = deps.warn ?? defaultWarn; const validatedSandboxName = validateSandboxName(sandboxName); @@ -67,6 +63,10 @@ export function stopSandboxChannels(sandboxName: string, deps: SandboxGatewaySto ); return; } + if (!sandbox) { + warn(`Sandbox '${validatedSandboxName}' is not registered; skipping in-sandbox gateway stop.`); + return; + } const agent = (deps.getRegisteredAgent ?? agentRuntime.getRegisteredAgent)(sandbox); if (sandbox?.agent && sandbox.agent !== "openclaw" && !agent) { @@ -103,106 +103,57 @@ export function stopSandboxChannels(sandboxName: string, deps: SandboxGatewaySto } const gatewayLabel = `${agentDisplayName} gateway`; + const request: SandboxExecRequest = { + sandboxName: validatedSandboxName, + command: ["sh", "-s"], + stdin: GATEWAY_STOP_SCRIPT, + timeoutMs: GATEWAY_STOP_TIMEOUT_MS, + maxOutputBytes: GATEWAY_STOP_MAX_OUTPUT_BYTES, + }; + const validationError = validateOpenShellExecRequest(request); info(`Stopping in-sandbox ${gatewayLabel} (sandbox: ${validatedSandboxName})...`); - - const privilegedResult = stopSandboxChannelsViaKubectl( - validatedSandboxName, - gatewayName, - GATEWAY_STOP_SCRIPT, - deps.runDocker ?? dockerSpawnSync, - ); - if (reportStopResult(privilegedResult, gatewayLabel, info, warn)) return; - - const openshell = (deps.resolveOpenshell ?? resolveOpenshell)(); - if (!openshell) { - warn(`openshell not found — cannot stop ${gatewayLabel} inside sandbox.`); - return; + try { + if (validationError) throw validationError; + (deps.assertNoGatewayEndpointOverride ?? assertNoOpenShellGatewayEndpointOverride)(); + // Select once and never replay: a shutdown may commit before a transport + // failure is observable, so retrying through another path is unsafe. + const selected = (deps.selectControl ?? selectOpenShellSandboxControlForMutation)(gatewayName); + try { + const result = await selected.control.exec(request); + reportStopResult(result, gatewayLabel, info, warn); + } finally { + selected.close(); + } + } catch (error) { + warn( + `Could not stop ${gatewayLabel} inside sandbox: ${error instanceof Error ? error.message : String(error)}. ` + + "The sandbox may be unreachable or the gateway may still be running.", + ); } - - const fallbackResult = (deps.runProcess ?? spawnSync)( - openshell, - ["sandbox", "exec", "--name", validatedSandboxName, "--gateway", gatewayName, "--", "sh", "-s"], - { - encoding: "utf-8", - input: GATEWAY_STOP_SCRIPT, - stdio: ["pipe", "pipe", "pipe"], - timeout: 20000, - }, - ); - reportStopResult(fallbackResult, gatewayLabel, info, warn); -} - -function isSandboxPodName(line: string, sandboxName: string): boolean { - if (!line.startsWith("pod/")) return false; - const podName = line.slice("pod/".length); - if (podName === sandboxName) return true; - const prefix = `${sandboxName}-`; - if (!podName.startsWith(prefix)) return false; - const generatedSuffix = podName.slice(prefix.length); - return /^[a-z0-9]+$/.test(generatedSuffix); -} - -function stopSandboxChannelsViaKubectl( - sandboxName: string, - gatewayName: string, - gatewayStopScript: string, - runDocker: typeof dockerSpawnSync, -): StopAttemptResult | null { - const gatewayContainer = getGatewayClusterContainerName(gatewayName); - const podsResult = runDocker( - ["exec", gatewayContainer, "kubectl", "get", "pods", "-n", "openshell", "-o", "name"], - { encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"], timeout: 10000 }, - ); - if (podsResult.status !== 0 || !podsResult.stdout) return null; - - const podOutput = - typeof podsResult.stdout === "string" ? podsResult.stdout : podsResult.stdout.toString(); - const pod = podOutput - .split(/\r?\n/) - .map((line: string) => line.trim()) - .find((line: string) => isSandboxPodName(line, sandboxName)); - if (!pod) return null; - - return runDocker( - [ - "exec", - gatewayContainer, - "kubectl", - "exec", - "-n", - "openshell", - "-c", - "agent", - pod, - "--", - "sh", - "-lc", - gatewayStopScript, - ], - { encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"], timeout: 20000 }, - ); } function reportStopResult( - result: StopAttemptResult | null, + result: SandboxExecResult, gatewayLabel: string, info: Reporter, warn: Reporter, -): boolean { - if (!result) return false; - - if (result.status === 0) { +): void { + if (!result.error && !result.signal && result.status === 0) { info(`${gatewayLabel} stopped inside sandbox.`); - return true; + return; } - if (result.status === 1) { + if (!result.error && !result.signal && result.status === 1) { info(`${gatewayLabel} was not running inside sandbox.`); - return true; + return; } - const details = [result.stderr, result.stdout] - .map((text) => (typeof text === "string" ? text : text?.toString())) - .filter((text): text is string => Boolean(text?.trim())) + const details = [ + result.error?.message, + result.signal ? `signal ${result.signal}` : undefined, + result.stderr, + result.stdout, + ] + .filter((text): text is string => typeof text === "string" && Boolean(text.trim())) .map((text) => text.trim()) .join(" "); warn( @@ -210,5 +161,4 @@ function reportStopResult( " The sandbox may be unreachable or the gateway may still be running." + (details ? ` Details: ${details}` : ""), ); - return true; } diff --git a/src/lib/tunnel/service-command.test.ts b/src/lib/tunnel/service-command.test.ts index 93eb8d12ef3..ba9d8121254 100644 --- a/src/lib/tunnel/service-command.test.ts +++ b/src/lib/tunnel/service-command.test.ts @@ -70,18 +70,18 @@ describe("services command", () => { expect(startAll).toHaveBeenCalledWith({ sandboxName: "alpha" }); }); - it("stops services without a sandbox override when the default sandbox is unsafe", () => { - const stopAll = vi.fn(); - runStopCommand({ + it("stops services without a sandbox override when the default sandbox is unsafe", async () => { + const stopAll = vi.fn(async () => {}); + await runStopCommand({ listSandboxes: () => ({ defaultSandbox: "bad name" }), stopAll, }); expect(stopAll).toHaveBeenCalledWith({ sandboxName: undefined }); }); - it("opts the legacy full-stop command into managed gateway release", () => { - const stopAll = vi.fn(); - runStopCommand({ + it("opts the legacy full-stop command into managed gateway release", async () => { + const stopAll = vi.fn(async () => {}); + await runStopCommand({ listSandboxes: () => ({ defaultSandbox: "alpha" }), stopAll, releaseGatewayPort: true, diff --git a/src/lib/tunnel/service-command.ts b/src/lib/tunnel/service-command.ts index 2053aec8c97..370b3d9e138 100644 --- a/src/lib/tunnel/service-command.ts +++ b/src/lib/tunnel/service-command.ts @@ -12,7 +12,7 @@ export interface StartCommandDeps { export interface StopCommandDeps { listSandboxes: () => SandboxSummary; - stopAll: (options: { sandboxName?: string; releaseGatewayPort?: boolean }) => void; + stopAll: (options: { sandboxName?: string; releaseGatewayPort?: boolean }) => Promise; /** Legacy `nemoclaw stop` tears down the managed host gateway too. */ releaseGatewayPort?: boolean; } @@ -34,10 +34,10 @@ export async function runStartCommand(deps: StartCommandDeps): Promise { await deps.startAll({ sandboxName: resolveDefaultSandboxName(deps.listSandboxes) }); } -export function runStopCommand(deps: StopCommandDeps): void { +export async function runStopCommand(deps: StopCommandDeps): Promise { const options: { sandboxName?: string; releaseGatewayPort?: boolean } = { sandboxName: resolveDefaultSandboxName(deps.listSandboxes), }; if (deps.releaseGatewayPort) options.releaseGatewayPort = true; - deps.stopAll(options); + await deps.stopAll(options); } diff --git a/src/lib/tunnel/services-gateway-ownership.test.ts b/src/lib/tunnel/services-gateway-ownership.test.ts index bfc348453ae..eb3befc50cf 100644 --- a/src/lib/tunnel/services-gateway-ownership.test.ts +++ b/src/lib/tunnel/services-gateway-ownership.test.ts @@ -185,13 +185,13 @@ describe("stopAll gateway-stop wiring", () => { vi.restoreAllMocks(); }); - it("orders supervised-agent full stop as sandbox guard, forwards, then gateway release", () => { + it("orders supervised-agent full stop as sandbox guard, forwards, then gateway release", async () => { const pidDir = mkdtempSync(join(tmpdir(), "nemoclaw-gateway-stop-wiring-")); vi.stubEnv("PATH", ""); const order: string[] = []; const stopSandboxGateway = vi .spyOn(sandboxGatewayStop, "stopSandboxChannels") - .mockImplementation((_sandboxName, deps) => { + .mockImplementation(async (_sandboxName, deps) => { order.push("sandbox-guard"); deps?.info?.( "Hermes Agent gateway is managed by the sandbox; leaving it running while host forwards stop.", @@ -210,7 +210,7 @@ describe("stopAll gateway-stop wiring", () => { const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); try { - stopAll({ pidDir, sandboxName: "alpha", releaseGatewayPort: true }); + await stopAll({ pidDir, sandboxName: "alpha", releaseGatewayPort: true }); } finally { rmSync(pidDir, { recursive: true, force: true }); } @@ -233,7 +233,7 @@ describe("stopAll gateway-stop wiring", () => { expect(output).toContain("All services stopped"); }); - it("preserves the shared gateway for canonical tunnel-only stop", () => { + it("preserves the shared gateway for canonical tunnel-only stop", async () => { const pidDir = mkdtempSync(join(tmpdir(), "nemoclaw-tunnel-stop-wiring-")); vi.stubEnv("PATH", ""); const releaseForStop = vi @@ -242,10 +242,10 @@ describe("stopAll gateway-stop wiring", () => { const stopAgentForwards = vi .spyOn(agentForwardStop, "stopAgentForwardPortsForStop") .mockImplementation(() => {}); - vi.spyOn(sandboxGatewayStop, "stopSandboxChannels").mockImplementation(() => {}); + vi.spyOn(sandboxGatewayStop, "stopSandboxChannels").mockImplementation(async () => {}); try { - stopAll({ pidDir, sandboxName: "alpha" }); + await stopAll({ pidDir, sandboxName: "alpha" }); } finally { rmSync(pidDir, { recursive: true, force: true }); } diff --git a/src/lib/tunnel/services-sandbox.test.ts b/src/lib/tunnel/services-sandbox.test.ts index 3679125f0df..63d9b04ca9a 100644 --- a/src/lib/tunnel/services-sandbox.test.ts +++ b/src/lib/tunnel/services-sandbox.test.ts @@ -34,7 +34,7 @@ describe("stopAll with sandbox channels", () => { for (const name of SANDBOX_ENV_NAMES) delete process.env[name]; stopSandboxChannels = vi .spyOn(sandboxGatewayStop, "stopSandboxChannels") - .mockImplementation(() => {}); + .mockImplementation(async () => {}); }); afterEach(() => { @@ -43,10 +43,10 @@ describe("stopAll with sandbox channels", () => { vi.restoreAllMocks(); }); - it("stops in-sandbox channels when sandboxName is provided", () => { + it("stops in-sandbox channels when sandboxName is provided", async () => { const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); - stopAll({ pidDir, sandboxName: "test-sb" }); + await stopAll({ pidDir, sandboxName: "test-sb" }); expect(stopSandboxChannels).toHaveBeenCalledWith("test-sb", { info: expect.any(Function), @@ -55,10 +55,10 @@ describe("stopAll with sandbox channels", () => { expect(logSpy.mock.calls.map((call) => call[0]).join("\n")).toContain("All services stopped"); }); - it("warns when no sandbox name is available", () => { + it("warns when no sandbox name is available", async () => { const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); - stopAll({ pidDir }); + await stopAll({ pidDir }); expect(stopSandboxChannels).not.toHaveBeenCalled(); const output = logSpy.mock.calls.map((call) => call[0]).join("\n"); @@ -66,41 +66,60 @@ describe("stopAll with sandbox channels", () => { expect(output).toContain("All services stopped"); }); - it("still stops cloudflared when in-sandbox shutdown cannot stop a process", () => { + it("waits for in-sandbox shutdown before stopping cloudflared", async () => { writeFileSync(join(pidDir, "cloudflared.pid"), "999999999"); + let resolveSandboxStop: (() => void) | undefined; + stopSandboxChannels.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveSandboxStop = resolve; + }), + ); - stopAll({ pidDir, sandboxName: "test-sb" }); + const pending = stopAll({ pidDir, sandboxName: "test-sb" }); expect(stopSandboxChannels).toHaveBeenCalledTimes(1); + expect(existsSync(join(pidDir, "cloudflared.pid"))).toBe(true); + resolveSandboxStop?.(); + await pending; + expect(existsSync(join(pidDir, "cloudflared.pid"))).toBe(false); + }); + + it("skips the in-sandbox stop after sandbox deletion but still cleans host services", async () => { + writeFileSync(join(pidDir, "cloudflared.pid"), "999999999"); + + await stopAll({ pidDir, sandboxName: "test-sb", skipSandboxStop: true }); + + expect(stopSandboxChannels).not.toHaveBeenCalled(); expect(existsSync(join(pidDir, "cloudflared.pid"))).toBe(false); }); - it("reads sandbox name from NEMOCLAW_SANDBOX env when not in opts", () => { + it("reads sandbox name from NEMOCLAW_SANDBOX env when not in opts", async () => { process.env.NEMOCLAW_SANDBOX = "env-sandbox"; - stopAll({ pidDir }); + await stopAll({ pidDir }); expect(stopSandboxChannels).toHaveBeenCalledWith("env-sandbox", expect.any(Object)); }); - it("reads sandbox name from NEMOCLAW_SANDBOX_NAME when NEMOCLAW_SANDBOX is unset", () => { + it("reads sandbox name from NEMOCLAW_SANDBOX_NAME when NEMOCLAW_SANDBOX is unset", async () => { process.env.NEMOCLAW_SANDBOX_NAME = "named-sandbox"; - stopAll({ pidDir }); + await stopAll({ pidDir }); expect(stopSandboxChannels).toHaveBeenCalledWith("named-sandbox", expect.any(Object)); }); - it("prefers NEMOCLAW_SANDBOX_NAME over NEMOCLAW_SANDBOX", () => { + it("prefers NEMOCLAW_SANDBOX_NAME over NEMOCLAW_SANDBOX", async () => { process.env.NEMOCLAW_SANDBOX_NAME = "name-sandbox"; process.env.NEMOCLAW_SANDBOX = "other-sandbox"; - stopAll({ pidDir }); + await stopAll({ pidDir }); expect(stopSandboxChannels).toHaveBeenCalledWith("name-sandbox", expect.any(Object)); }); - it("uses the effective env-selected sandbox with an explicit host pidDir", () => { + it("uses the effective env-selected sandbox with an explicit host pidDir", async () => { const pidRoot = mkdtempSync(join(tmpdir(), "nemoclaw-services-pid-root-")); const effectivePidDir = join(pidRoot, "nemoclaw-services-name-sandbox"); const lowerPriorityPidDir = join(pidRoot, "nemoclaw-services-other-sandbox"); @@ -112,7 +131,7 @@ describe("stopAll with sandbox channels", () => { process.env.NEMOCLAW_SANDBOX = "other-sandbox"; try { - stopAll({ pidDir: effectivePidDir }); + await stopAll({ pidDir: effectivePidDir }); expect(stopSandboxChannels).toHaveBeenCalledWith("name-sandbox", expect.any(Object)); expect(existsSync(join(effectivePidDir, "cloudflared.pid"))).toBe(false); @@ -125,17 +144,17 @@ describe("stopAll with sandbox channels", () => { it.each([ "bad name", "../../etc/passwd", - ])("rejects malformed env sandbox name %j before in-sandbox shutdown", (invalidName) => { + ])("rejects malformed env sandbox name %j before in-sandbox shutdown", async (invalidName) => { const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); process.env.NEMOCLAW_SANDBOX_NAME = invalidName; - stopAll({ pidDir }); + await stopAll({ pidDir }); expect(stopSandboxChannels).not.toHaveBeenCalled(); expect(logSpy.mock.calls.map((call) => call[0]).join("\n")).toContain("Invalid sandbox name"); }); - it("keeps host cleanup running for a malformed explicit sandbox name", () => { + it("keeps host cleanup running for a malformed explicit sandbox name", async () => { const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); const stopAgentForwards = vi .spyOn(agentForwardStop, "stopAgentForwardPortsForStop") @@ -145,9 +164,9 @@ describe("stopAll with sandbox channels", () => { .mockImplementation(() => {}); writeFileSync(join(pidDir, "cloudflared.pid"), "999999999"); - expect(() => + await expect( stopAll({ pidDir, sandboxName: "bad name", releaseGatewayPort: true }), - ).not.toThrow(); + ).resolves.toBeUndefined(); expect(stopSandboxChannels).not.toHaveBeenCalled(); expect(stopAgentForwards).not.toHaveBeenCalled(); @@ -158,10 +177,10 @@ describe("stopAll with sandbox channels", () => { expect(output).toContain("All services stopped"); }); - it("does not stop default cloudflared for a malformed sandbox name without an explicit pidDir", () => { + it("does not stop default cloudflared for a malformed sandbox name without an explicit pidDir", async () => { const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); - expect(() => stopAll({ sandboxName: "bad name" })).not.toThrow(); + await expect(stopAll({ sandboxName: "bad name" })).resolves.toBeUndefined(); expect(stopSandboxChannels).not.toHaveBeenCalled(); const output = logSpy.mock.calls.map((call) => call[0]).join("\n"); diff --git a/src/lib/tunnel/services.test.ts b/src/lib/tunnel/services.test.ts index d863f7ae070..95d9ea50238 100644 --- a/src/lib/tunnel/services.test.ts +++ b/src/lib/tunnel/services.test.ts @@ -479,34 +479,34 @@ describe("stopAll", () => { rmSync(pidDir, { recursive: true, force: true }); }); - it("removes stale PID files", () => { + it("removes stale PID files", async () => { writeFileSync(join(pidDir, "cloudflared.pid"), "999999999"); const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); - stopAll({ pidDir }); + await stopAll({ pidDir }); logSpy.mockRestore(); expect(existsSync(join(pidDir, "cloudflared.pid"))).toBe(false); }); - it("is idempotent — calling twice does not throw", () => { + it("is idempotent — calling twice does not throw", async () => { const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); - stopAll({ pidDir }); - stopAll({ pidDir }); + await stopAll({ pidDir }); + await stopAll({ pidDir }); logSpy.mockRestore(); }); - it("logs stop messages", () => { + it("logs stop messages", async () => { const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); - stopAll({ pidDir }); + await stopAll({ pidDir }); const output = logSpy.mock.calls.map((c) => c[0]).join("\n"); expect(output).toContain("All services stopped"); logSpy.mockRestore(); }); - it("unloads Ollama models before reporting services stopped", () => { + it("unloads Ollama models before reporting services stopped", async () => { const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); - stopAll({ pidDir }); + await stopAll({ pidDir }); logSpy.mockRestore(); const psCall = spawnSyncCalls.find( diff --git a/src/lib/tunnel/services.ts b/src/lib/tunnel/services.ts index 8e80ae2ab97..9c589554aa5 100644 --- a/src/lib/tunnel/services.ts +++ b/src/lib/tunnel/services.ts @@ -45,6 +45,8 @@ export interface ServiceOptions { cloudflareTunnelToken?: string; /** Also release the managed host gateway port (legacy full-stop only). */ releaseGatewayPort?: boolean; + /** Skip the in-sandbox stop when the sandbox was already deleted. */ + skipSandboxStop?: boolean; } export interface ServiceStatus { @@ -445,7 +447,7 @@ export function showStatus(opts: ServiceOptions = {}): void { } } -export function stopAll(opts: ServiceOptions = {}): void { +export async function stopAll(opts: ServiceOptions = {}): Promise { // Resolve the target sandbox once and reuse it for in-sandbox and host-side cleanup. const rawSandboxName = opts.sandboxName ?? @@ -467,13 +469,15 @@ export function stopAll(opts: ServiceOptions = {}): void { : resolvePidDir({ ...opts, sandboxName: sandboxName ?? "default" })); if (pidDir) ensurePidDir(pidDir); - if (sandboxName) { - sandboxGatewayStop.stopSandboxChannels(sandboxName, { info, warn }); - } else if (rawSandboxName) { - warn(`Invalid sandbox name: ${JSON.stringify(rawSandboxName)} — skipping in-sandbox stop.`); - } else { - warn("No sandbox name available — cannot stop in-sandbox messaging channels."); - warn("Hint: run 'nemoclaw stop' with a registered sandbox or set NEMOCLAW_SANDBOX_NAME."); + if (!opts.skipSandboxStop) { + if (sandboxName) { + await sandboxGatewayStop.stopSandboxChannels(sandboxName, { info, warn }); + } else if (rawSandboxName) { + warn(`Invalid sandbox name: ${JSON.stringify(rawSandboxName)} — skipping in-sandbox stop.`); + } else { + warn("No sandbox name available — cannot stop in-sandbox messaging channels."); + warn("Hint: run 'nemoclaw stop' with a registered sandbox or set NEMOCLAW_SANDBOX_NAME."); + } } try { diff --git a/test/destroy-cleanup-sandbox-services.test.ts b/test/destroy-cleanup-sandbox-services.test.ts index 7e3df5163d2..2f42a500e42 100644 --- a/test/destroy-cleanup-sandbox-services.test.ts +++ b/test/destroy-cleanup-sandbox-services.test.ts @@ -23,10 +23,10 @@ function buildDeps(sandbox: SandboxLike): { "getSandbox" | "stopAll" | "unloadOllamaModels" | "runOpenshell" | "rmSync" > >; - stopAllCalls: Array<{ sandboxName: string }>; + stopAllCalls: Array<{ sandboxName: string; skipSandboxStop: true }>; unloadCalls: number; } { - const stopAllCalls: Array<{ sandboxName: string }> = []; + const stopAllCalls: Array<{ sandboxName: string; skipSandboxStop: true }> = []; let unloadCalls = 0; return { stopAllCalls, @@ -35,7 +35,7 @@ function buildDeps(sandbox: SandboxLike): { }, deps: { getSandbox: vi.fn(() => sandbox as never), - stopAll: vi.fn((opts: { sandboxName: string }) => { + stopAll: vi.fn(async (opts: { sandboxName: string; skipSandboxStop: true }) => { stopAllCalls.push(opts); }), unloadOllamaModels: vi.fn(() => { @@ -48,42 +48,73 @@ function buildDeps(sandbox: SandboxLike): { } describe("cleanupSandboxServices Ollama unload (#2717)", () => { - it("delegates GPU unload to stopAll() exactly once when stopHostServices=true", () => { + it("delegates GPU unload to stopAll() exactly once when stopHostServices=true", async () => { const harness = buildDeps({ provider: "ollama-local" }); - cleanupSandboxServices("regression-2717", { stopHostServices: true }, harness.deps); + await cleanupSandboxServices("regression-2717", { stopHostServices: true }, harness.deps); expect(harness.deps.stopAll).toHaveBeenCalledTimes(1); - expect(harness.stopAllCalls[0]).toEqual({ sandboxName: "regression-2717" }); + expect(harness.stopAllCalls[0]).toEqual({ + sandboxName: "regression-2717", + skipSandboxStop: true, + }); // stopAll() invokes unloadOllamaModels() internally — see services.ts. // cleanupSandboxServices itself must not call it again. expect(harness.deps.unloadOllamaModels).not.toHaveBeenCalled(); expect(harness.unloadCalls).toBe(0); }); - it("calls unloadOllamaModels() exactly once for an Ollama sandbox when stopHostServices=false", () => { + it("waits for host stop before removing post-delete service state", async () => { + const harness = buildDeps({ provider: "ollama-local" }); + let resolveStopAll: (() => void) | undefined; + vi.mocked(harness.deps.stopAll).mockImplementationOnce( + () => + new Promise((resolve) => { + resolveStopAll = resolve; + }), + ); + + const pending = cleanupSandboxServices( + "regression-2717", + { stopHostServices: true }, + harness.deps, + ); + + expect(harness.deps.stopAll).toHaveBeenCalledWith({ + sandboxName: "regression-2717", + skipSandboxStop: true, + }); + expect(harness.deps.rmSync).not.toHaveBeenCalled(); + expect(harness.deps.runOpenshell).not.toHaveBeenCalled(); + resolveStopAll?.(); + await pending; + expect(harness.deps.rmSync).toHaveBeenCalledOnce(); + expect(harness.deps.runOpenshell).toHaveBeenCalled(); + }); + + it("calls unloadOllamaModels() exactly once for an Ollama sandbox when stopHostServices=false", async () => { const harness = buildDeps({ provider: "ollama-local" }); - cleanupSandboxServices("regression-2717", { stopHostServices: false }, harness.deps); + await cleanupSandboxServices("regression-2717", { stopHostServices: false }, harness.deps); expect(harness.deps.stopAll).not.toHaveBeenCalled(); expect(harness.deps.unloadOllamaModels).toHaveBeenCalledTimes(1); expect(harness.unloadCalls).toBe(1); }); - it("skips unloadOllamaModels() entirely for non-Ollama providers", () => { + it("skips unloadOllamaModels() entirely for non-Ollama providers", async () => { const harness = buildDeps({ provider: "nvidia-prod" }); - cleanupSandboxServices("regression-2717", { stopHostServices: false }, harness.deps); + await cleanupSandboxServices("regression-2717", { stopHostServices: false }, harness.deps); expect(harness.deps.stopAll).not.toHaveBeenCalled(); expect(harness.deps.unloadOllamaModels).not.toHaveBeenCalled(); }); - it("removes the sandbox PID dir and tears down all messaging providers", () => { + it("removes the sandbox PID dir and tears down all messaging providers", async () => { const harness = buildDeps({ provider: "ollama-local" }); - cleanupSandboxServices("regression-2717", { stopHostServices: false }, harness.deps); + await cleanupSandboxServices("regression-2717", { stopHostServices: false }, harness.deps); expect(harness.deps.rmSync).toHaveBeenCalledWith( path.join("/tmp", "nemoclaw-services-regression-2717"), @@ -99,12 +130,12 @@ describe("cleanupSandboxServices Ollama unload (#2717)", () => { ); }); - it("rejects traversal-shaped sandbox names before any cleanup side effect", () => { + it("rejects traversal-shaped sandbox names before any cleanup side effect", async () => { const harness = buildDeps({ provider: "ollama-local" }); - expect(() => + await expect( cleanupSandboxServices("x/../../victim", { stopHostServices: true }, harness.deps), - ).toThrow("Invalid sandbox name"); + ).rejects.toThrow("Invalid sandbox name"); expect(harness.deps.getSandbox).not.toHaveBeenCalled(); expect(harness.deps.stopAll).not.toHaveBeenCalled(); diff --git a/test/helpers/destroy-flow-test-assertions.ts b/test/helpers/destroy-flow-test-assertions.ts index ac9b1bb5508..d65fe1572e1 100644 --- a/test/helpers/destroy-flow-test-assertions.ts +++ b/test/helpers/destroy-flow-test-assertions.ts @@ -66,8 +66,21 @@ export function expectSuccessfulLiveDestroy(harness: DestroyHarness, exitSpy: Mo ["sandbox", "delete", "alpha"], expect.objectContaining({ ignoreError: true }), ); - expect(harness.unloadOllamaModelsSpy).toHaveBeenCalledTimes(1); + expect(harness.stopAllSpy).toHaveBeenCalledWith({ + sandboxName: "alpha", + skipSandboxStop: true, + }); + expect(harness.unloadOllamaModelsSpy).not.toHaveBeenCalled(); expect(harness.removeSandboxSpy).toHaveBeenCalledWith("alpha"); + const deleteCall = harness.runOpenshellSpy.mock.calls.findIndex( + (call) => Array.isArray(call[0]) && call[0].join(" ") === "sandbox delete alpha", + ); + expect(harness.runOpenshellSpy.mock.invocationCallOrder[deleteCall]).toBeLessThan( + harness.stopAllSpy.mock.invocationCallOrder[0], + ); + expect(harness.stopAllSpy.mock.invocationCallOrder[0]).toBeLessThan( + harness.removeSandboxSpy.mock.invocationCallOrder[0], + ); expect(harness.cleanupGatewaySpy).toHaveBeenCalledWith("nemoclaw-19080", harness.runOpenshellSpy); expect(harness.logSpy.mock.calls.map((call) => String(call[0])).join("\n")).toContain( "Sandbox 'alpha' destroyed", diff --git a/test/helpers/destroy-flow-test-harness.ts b/test/helpers/destroy-flow-test-harness.ts index d8a1c43c4ae..51f705eb556 100644 --- a/test/helpers/destroy-flow-test-harness.ts +++ b/test/helpers/destroy-flow-test-harness.ts @@ -235,7 +235,7 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr const unloadOllamaModelsSpy = vi .spyOn(ollamaProxy, "unloadOllamaModels") .mockImplementation(() => undefined); - const stopAllSpy = vi.spyOn(tunnelServices, "stopAll").mockImplementation(() => undefined); + const stopAllSpy = vi.spyOn(tunnelServices, "stopAll").mockImplementation(async () => undefined); vi.spyOn(timerControl, "readTimerMarker").mockReturnValue( options.activeTimer ? { From 49ae590d15a3a4cc4c6dbc76f9d21e466b571351 Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Tue, 14 Jul 2026 23:51:03 -0700 Subject: [PATCH 4/6] test(openshell): execute gateway stop via stdin Signed-off-by: Aaron Erickson --- src/lib/tunnel/gateway-stop-script.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/lib/tunnel/gateway-stop-script.test.ts b/src/lib/tunnel/gateway-stop-script.test.ts index c7b2a331138..33f231f5f1f 100644 --- a/src/lib/tunnel/gateway-stop-script.test.ts +++ b/src/lib/tunnel/gateway-stop-script.test.ts @@ -19,7 +19,7 @@ import { GATEWAY_STOP_SCRIPT } from "./gateway-stop-script"; // Linux-only: execute the production shell script against real processes while // scoping its ps snapshot to PIDs created by this test. This guards all gateway // argv forms without risking unrelated developer or CI processes. -describe("GATEWAY_STOP_SCRIPT (executed)", () => { +describe("GATEWAY_STOP_SCRIPT (executed via production sh -s stdin)", () => { // eslint-disable-next-line @typescript-eslint/no-require-imports const cp = require("node:child_process"); const children: Array<{ pid?: number }> = []; @@ -81,8 +81,9 @@ ps() { fi } ${script}`; - const result = cp.spawnSync("sh", ["-lc", scopedScript], { + const result = cp.spawnSync("sh", ["-s"], { encoding: "utf-8", + input: scopedScript, timeout: 20000, }); assert(result.status !== null, `stop script did not exit: ${result.signal} ${result.stderr}`); From 17efd4918121f4809622a9b7bc12901038eed2de Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Tue, 14 Jul 2026 23:54:15 -0700 Subject: [PATCH 5/6] refactor(openshell): finish bootstrap repair removal Signed-off-by: Aaron Erickson --- scripts/checks/legacy-sandbox-transports.ts | 2 - src/lib/adapters/docker/container.ts | 4 - src/lib/onboard.ts | 39 +---- src/lib/onboard/gateway-bootstrap.test.ts | 86 ---------- src/lib/onboard/gateway-bootstrap.ts | 168 -------------------- src/lib/onboard/gateway-health-wait.test.ts | 28 +--- src/lib/onboard/gateway-health-wait.ts | 13 +- test/gateway-start-wait.test.ts | 55 +------ 8 files changed, 7 insertions(+), 388 deletions(-) delete mode 100644 src/lib/onboard/gateway-bootstrap.test.ts delete mode 100644 src/lib/onboard/gateway-bootstrap.ts diff --git a/scripts/checks/legacy-sandbox-transports.ts b/scripts/checks/legacy-sandbox-transports.ts index a9b3160a2f2..7150bf8efd2 100644 --- a/scripts/checks/legacy-sandbox-transports.ts +++ b/scripts/checks/legacy-sandbox-transports.ts @@ -39,9 +39,7 @@ const REVIEWED_SITE_TUPLES = [ ["src/lib/actions/sandbox/host-aliases.ts", "docker-exec-command", 1], ["src/lib/actions/sandbox/sessions/passthrough.ts", "grpc-cli-read-only-fallback", 1], ["src/lib/actions/sandbox/process-recovery.ts", "privileged-sandbox-exec", 2], - ["src/lib/adapters/docker/container.ts", "docker-exec-command", 1], ["src/lib/diagnostics/debug.ts", "grpc-cli-read-only-fallback", 1], - ["src/lib/onboard.ts", "docker-exec-builder", 1], ["src/lib/resources-cmd.ts", "docker-exec-command", 1], ["src/lib/sandbox/config.ts", "privileged-sandbox-exec", 2], ["src/lib/sandbox/version.ts", "grpc-cli-read-only-fallback", 1], diff --git a/src/lib/adapters/docker/container.ts b/src/lib/adapters/docker/container.ts index 9dba35695ac..c66d5b9d35e 100644 --- a/src/lib/adapters/docker/container.ts +++ b/src/lib/adapters/docker/container.ts @@ -72,7 +72,3 @@ export function dockerPort( ): string { return dockerCapture(["port", containerName, String(containerPort)], opts); } - -export function dockerExecArgv(containerName: string, cmd: readonly string[]): string[] { - return ["docker", "exec", containerName, ...cmd]; -} diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index b9d02d6831a..1ab8adbe338 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -98,11 +98,6 @@ const { }: typeof import("./onboard/web-search-support") = require("./onboard/web-search-support"); const onboardDashboard: typeof import("./onboard/dashboard") = require("./onboard/dashboard"); const dashboardRuntime: typeof import("./onboard/dashboard-runtime") = require("./onboard/dashboard-runtime"); -const { - buildGatewayBootstrapSecretsScript, - createGatewayBootstrapRepairHelpers, - getGatewayBootstrapRepairPlan, -}: typeof import("./onboard/gateway-bootstrap") = require("./onboard/gateway-bootstrap"); const { buildDirectGpuPolicyYaml, buildDirectSandboxGpuProofCommands, @@ -173,7 +168,6 @@ const { getNameValidationGuidance } = nameValidation; const docker: typeof import("./adapters/docker") = require("./adapters/docker"); const { dockerContainerInspectFormat, - dockerExecArgv, dockerInfoFormat, dockerInspect, dockerRemoveVolumesByPrefix, @@ -1380,10 +1374,6 @@ function getGatewayHealthWaitConfig(_startStatus = 0, containerState = "") { }; } -function buildGatewayClusterExecArgv(script: string): string[] { - return dockerExecArgv(getGatewayClusterContainerName(GATEWAY_NAME), ["sh", "-lc", script]); -} - function captureProcessArgs(pid: number): string { return runCapture(["ps", "-p", String(pid), "-o", "args="], { ignoreError: true, @@ -1398,13 +1388,6 @@ function getGatewayLocalEndpoint(): string { return dockerDriverGatewayEnv.getGatewayHttpsEndpoint(GATEWAY_PORT); } -const { gatewayClusterHealthcheckPassed, repairGatewayBootstrapSecrets } = - createGatewayBootstrapRepairHelpers({ - buildGatewayClusterExecArgv, - run, - runCapture, - }); - function registerDockerDriverGatewayEndpoint(): boolean { const selectExisting = runQuietOpenshell(["gateway", "select", GATEWAY_NAME]); if (selectExisting.status === 0) { @@ -1450,18 +1433,11 @@ function registerDockerDriverGatewayEndpoint(): boolean { return ok; } -function attachGatewayMetadataIfNeeded({ - forceRefresh = false, -}: { - forceRefresh?: boolean; -} = {}): boolean { +function attachGatewayMetadataIfNeeded(): boolean { const gwInfo = runCaptureOpenshell(["gateway", "info", "-g", GATEWAY_NAME], { ignoreError: true, }); - // runCaptureOpenshell may return stale-but-present gateway metadata. When - // hasStaleGateway(gwInfo) is truthy we skip runOpenshell unless a repair - // flow explicitly forces a refresh after recreating bootstrap secrets. - if (!forceRefresh && hasStaleGateway(gwInfo)) return true; + if (hasStaleGateway(gwInfo)) return true; if (isLinuxDockerDriverGatewayEnabled()) { return registerDockerDriverGatewayEndpoint(); @@ -1870,14 +1846,12 @@ async function startGatewayWithOptions( if ( await waitForGatewayHealth({ attachGatewayMetadataIfNeeded, - gatewayClusterHealthcheckPassed, gatewayName: GATEWAY_NAME, healthPollCount: healthWait.count, healthPollIntervalSeconds: healthWait.interval, isGatewayHealthy, isGatewayHttpReady: (signal) => isGatewayHttpReady(undefined, undefined, undefined, signal), - repairGatewayBootstrapSecrets, runCaptureOpenshell, sleepSeconds, }) @@ -2234,12 +2208,7 @@ async function recoverGatewayRuntime() { ? recoveryWait.interval : envInt("NEMOCLAW_HEALTH_POLL_INTERVAL", 2); for (let i = 0; i < recoveryPollCount; i++) { - const repairResult = repairGatewayBootstrapSecrets(); - if (repairResult.repaired) { - attachGatewayMetadataIfNeeded({ forceRefresh: true }); - } else if (gatewayClusterHealthcheckPassed()) { - attachGatewayMetadataIfNeeded(); - } + attachGatewayMetadataIfNeeded(); status = runCaptureOpenshell(["status"], { ignoreError: true }); if (status.includes("Connected") && isSelectedGateway(status) && (await isGatewayHttpReady())) { process.env.OPENSHELL_GATEWAY = GATEWAY_NAME; @@ -4697,7 +4666,6 @@ async function runOnboard(opts: OnboardOptions = {}): Promise { module.exports = { buildOrphanedSandboxRollbackMessage, buildProviderArgs, - buildGatewayBootstrapSecretsScript, buildCompatibleEndpointSandboxSmokeCommand, buildCompatibleEndpointSandboxSmokeScript, buildSandboxConfigSyncScript, @@ -4716,7 +4684,6 @@ module.exports = { areRequiredDockerDriverBinariesPresent, ensureOpenshellForOnboard, shouldRequireDockerDriverEnv, - getGatewayBootstrapRepairPlan, getGatewayLocalEndpoint, getGatewayStartEnv, getDockerDriverGatewayEnv, diff --git a/src/lib/onboard/gateway-bootstrap.test.ts b/src/lib/onboard/gateway-bootstrap.test.ts deleted file mode 100644 index bbf9505e154..00000000000 --- a/src/lib/onboard/gateway-bootstrap.test.ts +++ /dev/null @@ -1,86 +0,0 @@ -// 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 { - buildGatewayBootstrapSecretsScript, - createGatewayBootstrapRepairHelpers, - getGatewayBootstrapRepairPlan, -} from "./gateway-bootstrap"; - -describe("gateway bootstrap helpers", () => { - it("normalizes missing secrets into a repair plan", () => { - expect( - getGatewayBootstrapRepairPlan([ - "openshell-client-tls", - "noise", - " openshell-server-tls ", - "", - ]), - ).toEqual({ - missingSecrets: ["openshell-client-tls", "openshell-server-tls"], - needsRepair: true, - needsServerTls: true, - needsClientBundle: true, - needsHandshake: false, - }); - }); - - it("builds a no-op script when nothing is missing", () => { - expect(buildGatewayBootstrapSecretsScript([]).trim()).toBe("exit 0"); - }); - - it("builds a repair script only for requested secret classes", () => { - const script = buildGatewayBootstrapSecretsScript([ - "openshell-server-tls", - "openshell-ssh-handshake", - ]); - - expect(script).toContain("openshell-server-tls"); - expect(script).toContain("openshell-ssh-handshake"); - expect(script).toContain("if true; then"); - expect(script).toContain("if false; then"); - }); - - it("lists missing secrets through the gateway cluster executor", () => { - const buildGatewayClusterExecArgv = vi.fn((script: string) => ["docker", "exec", script]); - const runCapture = vi.fn(() => "openshell-client-tls\n\nopenshell-server-tls\n"); - const helpers = createGatewayBootstrapRepairHelpers({ - buildGatewayClusterExecArgv, - run: vi.fn(() => ({ status: 0 })), - runCapture, - }); - - expect(helpers.listMissingGatewayBootstrapSecrets()).toEqual([ - "openshell-client-tls", - "openshell-server-tls", - ]); - expect(runCapture).toHaveBeenCalledWith( - expect.arrayContaining([expect.stringContaining("kubectl -n openshell get secret")]), - { ignoreError: true }, - ); - }); - - it("repairs missing secrets and reports success when the second probe is clean", () => { - const log = vi.fn(); - const runCapture = vi - .fn() - .mockReturnValueOnce("openshell-client-tls\n") - .mockReturnValueOnce(""); - const run = vi.fn(() => ({ status: 0 })); - const helpers = createGatewayBootstrapRepairHelpers({ - buildGatewayClusterExecArgv: (script) => ["docker", "exec", script], - run, - runCapture, - log, - }); - - expect(helpers.repairGatewayBootstrapSecrets()).toEqual({ repaired: true, missingSecrets: [] }); - expect(run).toHaveBeenCalledWith(expect.any(Array), { - ignoreError: true, - suppressOutput: true, - }); - expect(log).toHaveBeenCalledWith(" ✓ OpenShell bootstrap secrets created"); - }); -}); diff --git a/src/lib/onboard/gateway-bootstrap.ts b/src/lib/onboard/gateway-bootstrap.ts deleted file mode 100644 index 311268ff8d7..00000000000 --- a/src/lib/onboard/gateway-bootstrap.ts +++ /dev/null @@ -1,168 +0,0 @@ -// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -import { shellQuote } from "../core/shell-quote"; - -export const GATEWAY_BOOTSTRAP_SECRET_NAMES = [ - "openshell-server-tls", - "openshell-server-client-ca", - "openshell-client-tls", - "openshell-ssh-handshake", -] as const; - -export type GatewayBootstrapRepairPlan = { - missingSecrets: string[]; - needsRepair: boolean; - needsServerTls: boolean; - needsClientBundle: boolean; - needsHandshake: boolean; -}; - -export type GatewayBootstrapRepairResult = { - repaired: boolean; - missingSecrets: string[]; -}; - -type RunnerOptions = { - ignoreError?: boolean; - suppressOutput?: boolean; - timeout?: number; -}; - -type RunResult = { - status: number | null; -}; - -export type GatewayBootstrapRepairDeps = { - buildGatewayClusterExecArgv: (script: string) => string[]; - run: (args: string[], opts?: RunnerOptions) => RunResult; - runCapture: (args: string[], opts?: RunnerOptions) => string; - log?: (message?: string) => void; -}; - -export function getGatewayBootstrapRepairPlan( - missingSecrets: string[] = [], -): GatewayBootstrapRepairPlan { - const allowed = new Set(GATEWAY_BOOTSTRAP_SECRET_NAMES); - const normalized = [ - ...new Set((missingSecrets || []).map((name) => String(name).trim()).filter(Boolean)), - ].filter((name) => allowed.has(name)); - const missing = new Set(normalized); - const needsClientBundle = - missing.has("openshell-server-client-ca") || missing.has("openshell-client-tls"); - - return { - missingSecrets: normalized, - needsRepair: normalized.length > 0, - needsServerTls: missing.has("openshell-server-tls"), - needsClientBundle, - needsHandshake: missing.has("openshell-ssh-handshake"), - }; -} - -export function buildGatewayBootstrapSecretsScript(missingSecrets: string[] = []): string { - const plan = getGatewayBootstrapRepairPlan(missingSecrets); - if (!plan.needsRepair) return "exit 0"; - - return ` -set -eu -export KUBECONFIG=/etc/rancher/k3s/k3s.yaml -kubectl get namespace openshell >/dev/null 2>&1 -kubectl -n openshell get statefulset/openshell >/dev/null 2>&1 -TMPDIR="$(mktemp -d)" -cleanup() { - rm -rf "$TMPDIR" -} -trap cleanup EXIT -if ${plan.needsServerTls ? "true" : "false"}; then - cat >"$TMPDIR/server-ext.cnf" <<'EOF' -subjectAltName=DNS:openshell,DNS:openshell.openshell,DNS:openshell.openshell.svc,DNS:openshell.openshell.svc.cluster.local,DNS:localhost,IP:127.0.0.1 -extendedKeyUsage=serverAuth -EOF - openssl req -nodes -newkey rsa:2048 -keyout "$TMPDIR/server.key" -out "$TMPDIR/server.csr" -subj "/CN=openshell.openshell.svc.cluster.local" >/dev/null 2>&1 - openssl x509 -req -in "$TMPDIR/server.csr" -signkey "$TMPDIR/server.key" -out "$TMPDIR/server.crt" -days 3650 -sha256 -extfile "$TMPDIR/server-ext.cnf" >/dev/null 2>&1 - kubectl create secret tls -n openshell openshell-server-tls --cert="$TMPDIR/server.crt" --key="$TMPDIR/server.key" --dry-run=client -o yaml | kubectl apply -f - -fi -if ${plan.needsClientBundle ? "true" : "false"}; then - cat >"$TMPDIR/client-ext.cnf" <<'EOF' -extendedKeyUsage=clientAuth -EOF - openssl req -x509 -nodes -newkey rsa:2048 -keyout "$TMPDIR/client-ca.key" -out "$TMPDIR/client-ca.crt" -subj "/CN=openshell-client-ca" -days 3650 >/dev/null 2>&1 - openssl req -nodes -newkey rsa:2048 -keyout "$TMPDIR/client.key" -out "$TMPDIR/client.csr" -subj "/CN=openshell-client" >/dev/null 2>&1 - openssl x509 -req -in "$TMPDIR/client.csr" -CA "$TMPDIR/client-ca.crt" -CAkey "$TMPDIR/client-ca.key" -CAcreateserial -out "$TMPDIR/client.crt" -days 3650 -sha256 -extfile "$TMPDIR/client-ext.cnf" >/dev/null 2>&1 - kubectl create secret generic -n openshell openshell-server-client-ca --from-file=ca.crt="$TMPDIR/client-ca.crt" --dry-run=client -o yaml | kubectl apply -f - - kubectl create secret generic -n openshell openshell-client-tls --from-file=tls.crt="$TMPDIR/client.crt" --from-file=tls.key="$TMPDIR/client.key" --from-file=ca.crt="$TMPDIR/client-ca.crt" --dry-run=client -o yaml | kubectl apply -f - -fi -if ${plan.needsHandshake ? "true" : "false"}; then - kubectl create secret generic -n openshell openshell-ssh-handshake --from-literal=secret="$(openssl rand -hex 32)" --dry-run=client -o yaml | kubectl apply -f - -fi -`; -} - -export function createGatewayBootstrapRepairHelpers(deps: GatewayBootstrapRepairDeps) { - const log = deps.log ?? console.log; - - function runGatewayClusterCapture(script: string, opts: RunnerOptions = {}): string { - return deps.runCapture(deps.buildGatewayClusterExecArgv(script), opts); - } - - function runGatewayCluster(script: string, opts: RunnerOptions = {}): RunResult { - return deps.run(deps.buildGatewayClusterExecArgv(script), opts); - } - - function listMissingGatewayBootstrapSecrets(): string[] { - const output = runGatewayClusterCapture( - ` -set -eu -export KUBECONFIG=/etc/rancher/k3s/k3s.yaml -kubectl get namespace openshell >/dev/null 2>&1 || exit 0 -kubectl -n openshell get statefulset/openshell >/dev/null 2>&1 || exit 0 -for name in ${GATEWAY_BOOTSTRAP_SECRET_NAMES.map((name) => shellQuote(name)).join(" ")}; do - kubectl -n openshell get secret "$name" >/dev/null 2>&1 || printf '%s\\n' "$name" -done -`, - { ignoreError: true }, - ); - return output - .split("\n") - .map((line) => line.trim()) - .filter(Boolean); - } - - function gatewayClusterHealthcheckPassed(): boolean { - const result = runGatewayCluster("/usr/local/bin/cluster-healthcheck.sh", { - ignoreError: true, - suppressOutput: true, - }); - return result.status === 0; - } - - function repairGatewayBootstrapSecrets(): GatewayBootstrapRepairResult { - const missingSecrets = listMissingGatewayBootstrapSecrets(); - const plan = getGatewayBootstrapRepairPlan(missingSecrets); - if (!plan.needsRepair) return { repaired: false, missingSecrets }; - - log(` OpenShell bootstrap secrets missing: ${plan.missingSecrets.join(", ")}. Repairing...`); - const repairResult = runGatewayCluster( - buildGatewayBootstrapSecretsScript(plan.missingSecrets), - { - ignoreError: true, - suppressOutput: true, - }, - ); - const remainingSecrets = listMissingGatewayBootstrapSecrets(); - if (repairResult.status === 0 && remainingSecrets.length === 0) { - log(" ✓ OpenShell bootstrap secrets created"); - return { repaired: true, missingSecrets: remainingSecrets }; - } - return { repaired: false, missingSecrets: remainingSecrets }; - } - - return { - runGatewayClusterCapture, - runGatewayCluster, - listMissingGatewayBootstrapSecrets, - gatewayClusterHealthcheckPassed, - repairGatewayBootstrapSecrets, - }; -} diff --git a/src/lib/onboard/gateway-health-wait.test.ts b/src/lib/onboard/gateway-health-wait.test.ts index 66ebb68316f..de1533dfd20 100644 --- a/src/lib/onboard/gateway-health-wait.test.ts +++ b/src/lib/onboard/gateway-health-wait.test.ts @@ -14,13 +14,11 @@ import { function buildOptions(overrides: Partial = {}): GatewayHealthWaitOptions { return { attachGatewayMetadataIfNeeded: vi.fn(), - gatewayClusterHealthcheckPassed: vi.fn(() => false), gatewayName: "nemoclaw", healthPollCount: 1, healthPollIntervalSeconds: 2, isGatewayHealthy: vi.fn(() => true), isGatewayHttpReady: vi.fn(async () => true), - repairGatewayBootstrapSecrets: vi.fn(() => ({ repaired: false })), runCaptureOpenshell: vi.fn((args: string[]) => { if (args[0] === "status") return "status"; if (args[0] === "gateway" && args[1] === "info" && args[2] === "-g") return "named"; @@ -50,6 +48,7 @@ describe("waitForGatewayHealth", () => { expect(isGatewayHealthy).toHaveBeenCalledTimes(2); expect(isGatewayHttpReady).toHaveBeenCalledTimes(2); + expect(options.attachGatewayMetadataIfNeeded).toHaveBeenCalledTimes(2); expect(options.sleepSeconds).toHaveBeenCalledTimes(1); expect(options.sleepSeconds).toHaveBeenCalledWith(2); }); @@ -70,30 +69,6 @@ describe("waitForGatewayHealth", () => { expect(options.sleepSeconds).toHaveBeenCalledTimes(2); }); - it("force-refreshes metadata after bootstrap secret repair", async () => { - const options = buildOptions({ - gatewayClusterHealthcheckPassed: vi.fn(() => true), - repairGatewayBootstrapSecrets: vi.fn(() => ({ repaired: true })), - }); - - await expect(waitForGatewayHealth(options)).resolves.toBe(true); - - expect(options.attachGatewayMetadataIfNeeded).toHaveBeenCalledOnce(); - expect(options.attachGatewayMetadataIfNeeded).toHaveBeenCalledWith({ forceRefresh: true }); - expect(options.gatewayClusterHealthcheckPassed).not.toHaveBeenCalled(); - }); - - it("attaches metadata without force when cluster healthcheck passes without repair", async () => { - const options = buildOptions({ - gatewayClusterHealthcheckPassed: vi.fn(() => true), - }); - - await expect(waitForGatewayHealth(options)).resolves.toBe(true); - - expect(options.attachGatewayMetadataIfNeeded).toHaveBeenCalledOnce(); - expect(options.attachGatewayMetadataIfNeeded).toHaveBeenCalledWith(); - }); - it("polls until the configured health deadline instead of stopping at the count cap (#3768)", async () => { const clock = createVirtualClock(); const isGatewayHealthy = vi.fn(() => { @@ -189,7 +164,6 @@ describe("waitForGatewayHealth", () => { await expect(waitForGatewayHealth(options)).resolves.toBe(false); - expect(options.repairGatewayBootstrapSecrets).not.toHaveBeenCalled(); expect(options.attachGatewayMetadataIfNeeded).not.toHaveBeenCalled(); expect(options.runCaptureOpenshell).not.toHaveBeenCalled(); expect(options.isGatewayHealthy).not.toHaveBeenCalled(); diff --git a/src/lib/onboard/gateway-health-wait.ts b/src/lib/onboard/gateway-health-wait.ts index 2193baff3a6..9378b0260e1 100644 --- a/src/lib/onboard/gateway-health-wait.ts +++ b/src/lib/onboard/gateway-health-wait.ts @@ -6,14 +6,12 @@ import { type WaitUntilOptions, waitUntilAsync } from "../core/wait"; type RunCaptureOpenshell = (args: string[], opts?: { ignoreError?: boolean }) => string; export interface GatewayHealthWaitOptions { - attachGatewayMetadataIfNeeded: (options?: { forceRefresh?: boolean }) => void; - gatewayClusterHealthcheckPassed: () => boolean; + attachGatewayMetadataIfNeeded: () => void; gatewayName: string; healthPollCount: number; healthPollIntervalSeconds: number; isGatewayHealthy: (status: string, namedInfo: string, currentInfo: string) => boolean; isGatewayHttpReady: (signal?: AbortSignal) => Promise; - repairGatewayBootstrapSecrets: () => { repaired: boolean }; runCaptureOpenshell: RunCaptureOpenshell; sleepSeconds: (seconds: number) => void; now?: () => number; @@ -118,13 +116,11 @@ function startAbortableGatewayHttpProbe( export async function waitForGatewayHealth({ attachGatewayMetadataIfNeeded, - gatewayClusterHealthcheckPassed, gatewayName, healthPollCount, healthPollIntervalSeconds, isGatewayHealthy, isGatewayHttpReady, - repairGatewayBootstrapSecrets, runCaptureOpenshell, sleepSeconds, now = Date.now, @@ -138,12 +134,7 @@ export async function waitForGatewayHealth({ return ( waitOptions !== null && (await waitUntilAsync(async () => { - const repairResult = repairGatewayBootstrapSecrets(); - if (repairResult.repaired) { - attachGatewayMetadataIfNeeded({ forceRefresh: true }); - } else if (gatewayClusterHealthcheckPassed()) { - attachGatewayMetadataIfNeeded(); - } + attachGatewayMetadataIfNeeded(); const httpProbe = startAbortableGatewayHttpProbe(isGatewayHttpReady); runCaptureOpenshell(["gateway", "select", gatewayName], { ignoreError: true }); const status = runCaptureOpenshell(["status"], { ignoreError: true }); diff --git a/test/gateway-start-wait.test.ts b/test/gateway-start-wait.test.ts index 394b7c0dff7..b87246044bd 100644 --- a/test/gateway-start-wait.test.ts +++ b/test/gateway-start-wait.test.ts @@ -117,7 +117,7 @@ describe("gateway startup wait config", () => { }); }); -describe("gateway bootstrap secret repair", () => { +describe("gateway endpoint metadata", () => { it("uses the configured gateway port for local metadata reattachment", () => { process.env.NEMOCLAW_GATEWAY_PORT = "9443"; const { getGatewayLocalEndpoint } = loadOnboard(); @@ -146,57 +146,4 @@ describe("gateway bootstrap secret repair", () => { OPENSHELL_SSH_GATEWAY_PORT: "9443", }); }); - - it("repairs the client CA and client TLS secrets together", () => { - const { getGatewayBootstrapRepairPlan } = loadOnboard(); - expect(getGatewayBootstrapRepairPlan(["openshell-client-tls"])).toEqual({ - missingSecrets: ["openshell-client-tls"], - needsRepair: true, - needsServerTls: false, - needsClientBundle: true, - needsHandshake: false, - }); - }); - - it("ignores unknown secret names when planning repairs", () => { - const { getGatewayBootstrapRepairPlan } = loadOnboard(); - - expect( - getGatewayBootstrapRepairPlan([ - "openshell-client-tls", - "noise", - " openshell-server-tls ", - "", - ]), - ).toEqual({ - missingSecrets: ["openshell-client-tls", "openshell-server-tls"], - needsRepair: true, - needsServerTls: true, - needsClientBundle: true, - needsHandshake: false, - }); - }); - - it("emits a script that creates all missing bootstrap secrets", () => { - const { buildGatewayBootstrapSecretsScript } = loadOnboard(); - const script = buildGatewayBootstrapSecretsScript([ - "openshell-server-tls", - "openshell-server-client-ca", - "openshell-client-tls", - "openshell-ssh-handshake", - ]); - - expect(script).toContain("openshell-server-tls"); - expect(script).toContain("openshell-server-client-ca"); - expect(script).toContain("openshell-client-tls"); - expect(script).toContain("openshell-ssh-handshake"); - expect(script).toContain("CN=openshell-client-ca"); - expect(script).toContain("CN=openshell-client"); - expect(script).toContain("subjectAltName=DNS:openshell"); - }); - - it("skips secret generation when nothing is missing", () => { - const { buildGatewayBootstrapSecretsScript } = loadOnboard(); - expect(buildGatewayBootstrapSecretsScript([]).trim()).toBe("exit 0"); - }); }); From 0df7f2b2d564da478166b722403da6791491e3cd Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Wed, 15 Jul 2026 00:31:14 -0700 Subject: [PATCH 6/6] test(openshell): cover gateway readiness recovery Signed-off-by: Aaron Erickson --- ...onboard-gateway-readiness-recovery.test.ts | 269 ++++++++++++++++++ 1 file changed, 269 insertions(+) create mode 100644 test/onboard-gateway-readiness-recovery.test.ts diff --git a/test/onboard-gateway-readiness-recovery.test.ts b/test/onboard-gateway-readiness-recovery.test.ts new file mode 100644 index 00000000000..f7e9997a44e --- /dev/null +++ b/test/onboard-gateway-readiness-recovery.test.ts @@ -0,0 +1,269 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import fs from "node:fs"; +import net from "node:net"; +import os from "node:os"; +import path from "node:path"; + +import { describe, it } from "vitest"; + +import { testTimeoutOptions } from "./helpers/timeouts"; + +function reserveFreePort(): Promise { + return new Promise((resolve, reject) => { + const probe = net.createServer(); + probe.once("error", reject); + probe.listen(0, "127.0.0.1", () => { + const address = probe.address(); + const port = typeof address === "object" && address ? address.port : 0; + probe.close(() => resolve(port)); + }); + }); +} + +function stopFixtureGateway(pidFile: string): void { + spawnSync( + "bash", + [ + "-c", + 'test ! -s "$1" || { kill "$(cat "$1")" 2>/dev/null || true; sleep 0.1; }', + "fixture-cleanup", + pidFile, + ], + { stdio: "ignore" }, + ); +} + +describe("gateway readiness recovery", () => { + it( + "cleans the failed generation before retrying and converges on selected HTTP-ready state", + testTimeoutOptions(40_000), + async () => { + const repoRoot = path.join(import.meta.dirname, ".."); + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-gateway-recovery-")); + const fakeBin = path.join(tmpDir, "bin"); + const tracePath = path.join(tmpDir, "lifecycle.trace"); + const attemptPath = path.join(tmpDir, "attempt"); + const registrationPath = path.join(tmpDir, "registration"); + const selectedPath = path.join(tmpDir, "selected"); + const ownedVolumePath = path.join(tmpDir, "owned-volume"); + const gatewayPidPath = path.join(tmpDir, "gateway.pid"); + const gatewayReadyPath = path.join(tmpDir, "gateway.ready"); + const childPath = path.join(tmpDir, "run-recovery.cjs"); + const port = await reserveFreePort(); + const gatewayName = `nemoclaw-${String(port)}`; + const onboardPath = JSON.stringify(path.join(repoRoot, "src", "lib", "onboard.ts")); + + fs.mkdirSync(fakeBin, { recursive: true }); + fs.writeFileSync(tracePath, ""); + fs.writeFileSync(attemptPath, "0"); + fs.writeFileSync( + path.join(fakeBin, "openshell"), + `#!/usr/bin/env bash +set -euo pipefail +command="$*" +printf 'openshell:%s\\n' "$command" >> "$OPENSHELL_TEST_TRACE" + +case "$command" in + "--version") + printf 'openshell 0.0.83\\n' + ;; + "gateway --help") + printf 'Commands: start\\nLegacy command: gateway destroy\\n' + ;; + gateway\\ start\\ *) + generation="$(( $(cat "$OPENSHELL_TEST_ATTEMPT") + 1 ))" + printf '%s' "$generation" > "$OPENSHELL_TEST_ATTEMPT" + case "$generation" in + 2) + test ! -e "$OPENSHELL_TEST_REGISTRATION" + test ! -e "$OPENSHELL_TEST_SELECTED" + test ! -e "$OPENSHELL_TEST_OWNED_VOLUME" + test ! -e "$OPENSHELL_TEST_GATEWAY_READY" + ;; + esac + printf '%s' "$generation" > "$OPENSHELL_TEST_REGISTRATION" + printf '%s' "$generation" > "$OPENSHELL_TEST_OWNED_VOLUME" + case "$generation" in + 1) http_status=503 ;; + *) http_status=200 ;; + esac + server_script='const fs=require("node:fs");const http=require("node:http");const ready=process.argv[1];const code=Number(process.argv[2]);const port=Number(process.argv[3]);const server=http.createServer((_req,res)=>{res.writeHead(code);res.end("gateway");});server.listen(port,"127.0.0.1",()=>fs.writeFileSync(ready,String(process.pid)));process.on("SIGTERM",()=>server.close(()=>{fs.rmSync(ready,{force:true});process.exit(0);}));' + nohup "$OPENSHELL_TEST_NODE" -e "$server_script" "$OPENSHELL_TEST_GATEWAY_READY" "$http_status" "$OPENSHELL_TEST_PORT" >/dev/null 2>&1 & + printf '%s' "$!" > "$OPENSHELL_TEST_GATEWAY_PID" + for _ in $(seq 1 500); do + test ! -s "$OPENSHELL_TEST_GATEWAY_READY" || break + sleep 0.01 + done + test -s "$OPENSHELL_TEST_GATEWAY_READY" + ;; + "gateway remove $OPENSHELL_TEST_GATEWAY_NAME") + kill "$(cat "$OPENSHELL_TEST_GATEWAY_PID")" 2>/dev/null || true + for _ in $(seq 1 500); do + test -e "$OPENSHELL_TEST_GATEWAY_READY" || break + sleep 0.01 + done + test ! -e "$OPENSHELL_TEST_GATEWAY_READY" + rm -f "$OPENSHELL_TEST_GATEWAY_PID" "$OPENSHELL_TEST_REGISTRATION" "$OPENSHELL_TEST_SELECTED" + ;; + "gateway select $OPENSHELL_TEST_GATEWAY_NAME") + printf '%s' "$OPENSHELL_TEST_GATEWAY_NAME" > "$OPENSHELL_TEST_SELECTED" + ;; + "gateway info -g $OPENSHELL_TEST_GATEWAY_NAME"|"gateway info") + generation="$(cat "$OPENSHELL_TEST_REGISTRATION" 2>/dev/null || true)" + printf 'Gateway: %s\\nGateway endpoint: http://127.0.0.1:%s\\nGeneration: %s\\n' "$OPENSHELL_TEST_GATEWAY_NAME" "$OPENSHELL_TEST_PORT" "$generation" + ;; + "status") + generation="$(cat "$OPENSHELL_TEST_REGISTRATION" 2>/dev/null || true)" + selected="$(cat "$OPENSHELL_TEST_SELECTED" 2>/dev/null || true)" + case "$generation:$selected" in + "2:$OPENSHELL_TEST_GATEWAY_NAME") printf 'Gateway: %s\\nConnected\\n' "$OPENSHELL_TEST_GATEWAY_NAME" ;; + *) printf 'Gateway: %s\\nDisconnected\\n' "$OPENSHELL_TEST_GATEWAY_NAME" ;; + esac + ;; + "doctor logs --name $OPENSHELL_TEST_GATEWAY_NAME") + printf 'fixture diagnostics\\n' + ;; + *) + ;; +esac +`, + { mode: 0o755 }, + ); + fs.writeFileSync( + path.join(fakeBin, "docker"), + `#!/usr/bin/env bash +set -euo pipefail +command="$*" +printf 'docker:%s\\n' "$command" >> "$OPENSHELL_TEST_TRACE" +case "$command" in + "volume ls -q --filter name=openshell-cluster-$OPENSHELL_TEST_GATEWAY_NAME") + test ! -e "$OPENSHELL_TEST_OWNED_VOLUME" || printf 'openshell-cluster-%s-owned\\n' "$OPENSHELL_TEST_GATEWAY_NAME" + ;; + "volume rm openshell-cluster-$OPENSHELL_TEST_GATEWAY_NAME-owned") + rm -f "$OPENSHELL_TEST_OWNED_VOLUME" + ;; + *) + ;; +esac +`, + { mode: 0o755 }, + ); + fs.writeFileSync( + childPath, + `const mod = require("module"); +const originalLoad = mod._load; +const immediateRetry = async (operation, options) => { + try { + return await operation({ attemptNumber: 1, retriesLeft: 1 }); + } catch (firstError) { + await options.onFailedAttempt(Object.assign(firstError, { attemptNumber: 1, retriesLeft: 1 })); + } + try { + return await operation({ attemptNumber: 2, retriesLeft: 0 }); + } catch (secondError) { + await options.onFailedAttempt(Object.assign(secondError, { attemptNumber: 2, retriesLeft: 0 })); + throw secondError; + } +}; +immediateRetry.AbortError = class AbortError extends Error {}; +// Keep the production HTTP probe itself; only bind its URL to this fixture's +// reserved plain-HTTP listener so endpoint selection is outside this lifecycle test. +const loadWithFixtureProbe = (request, parent, isMain) => { + const loaded = originalLoad.call(mod, request, parent, isMain); + return request === "./onboard/gateway-http-readiness" + ? { + ...loaded, + isGatewayHttpReady: (timeoutMs, _url, method, signal) => + loaded.isGatewayHttpReady( + timeoutMs, + "http://127.0.0.1:" + process.env.OPENSHELL_TEST_PORT + "/", + method, + signal, + ), + } + : loaded; +}; +mod._load = function(request, parent, isMain) { + return request === "p-retry" ? immediateRetry : loadWithFixtureProbe(request, parent, isMain); +}; +Object.defineProperty(process, "platform", { value: "darwin" }); +Object.defineProperty(process, "arch", { value: "x64" }); +const onboard = require(${onboardPath}); +(async () => { + await onboard.startGateway(null); + const status = onboard.runCaptureOpenshell(["status"], { ignoreError: true }); + const namedInfo = onboard.runCaptureOpenshell(["gateway", "info", "-g", process.env.OPENSHELL_TEST_GATEWAY_NAME], { ignoreError: true }); + const currentInfo = onboard.runCaptureOpenshell(["gateway", "info"], { ignoreError: true }); + console.log("FINAL_SELECTED=" + String(onboard.isGatewayHealthy(status, namedInfo, currentInfo, process.env.OPENSHELL_TEST_GATEWAY_NAME))); + console.log("FINAL_HTTP=" + String(await onboard.isGatewayHttpReady())); +})().catch((error) => { + console.error(error); + process.exitCode = 1; +}); +`, + ); + + let result: ReturnType; + try { + result = spawnSync(process.execPath, [childPath], { + cwd: repoRoot, + encoding: "utf-8", + timeout: 30_000, + env: { + ...process.env, + HOME: tmpDir, + PATH: `${fakeBin}:${process.env.PATH ?? ""}`, + NEMOCLAW_GATEWAY_PORT: String(port), + NEMOCLAW_GATEWAY_START_TIMEOUT: "10", + NEMOCLAW_HEALTH_POLL_COUNT: "1", + NEMOCLAW_HEALTH_POLL_INTERVAL: "0", + NEMOCLAW_NON_INTERACTIVE: "1", + OPENSHELL_TEST_ATTEMPT: attemptPath, + OPENSHELL_TEST_GATEWAY_NAME: gatewayName, + OPENSHELL_TEST_GATEWAY_PID: gatewayPidPath, + OPENSHELL_TEST_GATEWAY_READY: gatewayReadyPath, + OPENSHELL_TEST_NODE: process.execPath, + OPENSHELL_TEST_OWNED_VOLUME: ownedVolumePath, + OPENSHELL_TEST_PORT: String(port), + OPENSHELL_TEST_REGISTRATION: registrationPath, + OPENSHELL_TEST_SELECTED: selectedPath, + OPENSHELL_TEST_TRACE: tracePath, + }, + }); + const stdout = String(result.stdout ?? ""); + + assert.equal( + result.status, + 0, + `unexpected exit; stdout:\n${result.stdout}\nstderr:\n${result.stderr}\ntrace:\n${fs.readFileSync(tracePath, "utf8")}`, + ); + assert.match(stdout, /FINAL_SELECTED=true/); + assert.match(stdout, /FINAL_HTTP=true/); + assert.equal(fs.readFileSync(attemptPath, "utf8"), "2"); + assert.equal(fs.readFileSync(registrationPath, "utf8"), "2"); + assert.equal(fs.readFileSync(selectedPath, "utf8"), gatewayName); + assert.equal(fs.readFileSync(ownedVolumePath, "utf8"), "2"); + + const trace = fs.readFileSync(tracePath, "utf8").trim().split("\n"); + const starts = trace + .map((line, index) => ({ index, line })) + .filter(({ line }) => line.startsWith("openshell:gateway start ")); + const removeIndex = trace.indexOf(`openshell:gateway remove ${gatewayName}`); + const volumeCleanupIndex = trace.indexOf( + `docker:volume rm openshell-cluster-${gatewayName}-owned`, + ); + assert.equal(starts.length, 2, trace.join("\n")); + assert.ok(removeIndex > starts[0].index, trace.join("\n")); + assert.ok(volumeCleanupIndex > removeIndex, trace.join("\n")); + assert.ok(starts[1].index > volumeCleanupIndex, trace.join("\n")); + } finally { + stopFixtureGateway(gatewayPidPath); + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }, + ); +});