diff --git a/.github/workflows/macos-e2e.yaml b/.github/workflows/macos-e2e.yaml index 56f7f1e8931..55f9b741b00 100644 --- a/.github/workflows/macos-e2e.yaml +++ b/.github/workflows/macos-e2e.yaml @@ -75,23 +75,25 @@ jobs: npx vitest run --project integration test/tunnel-gateway-port-release-runtime.test.ts test/onboard-gateway-prelaunch-cutover.test.ts + test/onboard-gateway-legacy-identity-upgrade-runtime.test.ts - name: Detect Docker availability id: docker run: | + set -euo pipefail if docker info >/dev/null 2>&1; then echo "docker_ok=true" >> "$GITHUB_OUTPUT" echo "Docker is available" docker version - else - echo "docker_ok=false" >> "$GITHUB_OUTPUT" - echo "Docker is not available on this runner" + exit 0 fi + echo "docker_ok=false" >> "$GITHUB_OUTPUT" + echo "Docker is unavailable on the Apple Silicon runner." - name: Run macOS full E2E - if: steps.docker.outputs.docker_ok == 'true' + if: steps.docker.outputs.docker_ok == 'true' && github.ref == 'refs/heads/main' && github.event_name != 'pull_request' env: - NVIDIA_INFERENCE_API_KEY: ${{ secrets.NVIDIA_INFERENCE_API_KEY }} + NVIDIA_INFERENCE_API_KEY: ${{ github.ref == 'refs/heads/main' && github.event_name != 'pull_request' && secrets.NVIDIA_INFERENCE_API_KEY || '' }} GITHUB_TOKEN: ${{ github.token }} NEMOCLAW_NON_INTERACTIVE: "1" NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE: "1" @@ -100,17 +102,95 @@ jobs: run: | NEMOCLAW_RUN_LIVE_E2E=1 npx vitest run --project e2e-live test/e2e/live/full-e2e.test.ts --silent=false --reporter=default - - name: Explain skipped full E2E - if: steps.docker.outputs.docker_ok != 'true' + - name: Explain skipped macOS live E2E + if: steps.docker.outputs.docker_ok != 'true' || github.ref != 'refs/heads/main' || github.event_name == 'pull_request' run: | - echo 'Skipping macOS full E2E because Docker is unavailable on this runner.' + if [ "${{ github.event_name }}" = "pull_request" ]; then + echo 'Skipping secret-bearing macOS live E2E on pull_request; use trusted workflow_dispatch/push evidence for live validation.' + elif [ "${{ github.ref }}" != "refs/heads/main" ]; then + echo 'Skipping secret-bearing macOS live E2E outside the trusted main branch.' + elif [ "${{ steps.docker.outputs.docker_ok }}" != "true" ]; then + echo 'Skipping macOS live E2E because Docker is unavailable on this runner.' + fi echo 'The workflow still validated the NemoClaw build on macOS (Apple Silicon).' - name: Upload logs on failure - if: failure() + if: failure() && github.event_name == 'pull_request' uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: macos-e2e-logs path: | /tmp/nemoclaw-e2e-*.log + ${{ github.workspace }}/e2e-artifacts/live + if-no-files-found: ignore + + # docker/setup-docker-action supports Intel macOS, while the primary Apple + # Silicon job validates the same gateway lifecycle regressions without Docker. + # Keep the secret-bearing real Docker proof on reviewed main-branch code only. + macos-docker-final-destroy: + if: github.ref == 'refs/heads/main' && github.event_name != 'pull_request' + permissions: + contents: read + runs-on: macos-15-intel + timeout-minutes: 90 + steps: + - name: Checkout + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + + - name: Setup Node.js + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6 + with: + node-version: "22" + cache: npm + + - name: Set up pinned Docker Engine + uses: docker/setup-docker-action@6d7cfa65f60a9dda7b46e5513fa982536f3c9877 # v5.3.0 + with: + version: v27.4.0 + env: + LIMA_START_ARGS: --cpus 4 --memory 8 + + - name: Install root dependencies + run: npm ci --ignore-scripts + + - name: Build CLI TypeScript modules + run: npm run build:cli + + - name: Install and build plugin + run: | + set -euo pipefail + cd nemoclaw + npm ci --ignore-scripts + npm run build + + - name: Install OpenShell CLI + run: bash scripts/install-openshell.sh + + - name: Run macOS Docker final-destroy E2E + env: + E2E_TARGET_ID: "sandbox-operations" + E2E_ARTIFACT_DIR: ${{ github.workspace }}/e2e-artifacts/live/macos-sandbox-operations + NEMOCLAW_CLI_BIN: ${{ github.workspace }}/bin/nemoclaw.js + NEMOCLAW_RUN_LIVE_E2E: "1" + NEMOCLAW_E2E_USE_HOSTED_INFERENCE: "1" + NEMOCLAW_NON_INTERACTIVE: "1" + NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE: "1" + NEMOCLAW_POLICY_TIER: "open" + NVIDIA_INFERENCE_API_KEY: ${{ secrets.NVIDIA_INFERENCE_API_KEY }} + OPENSHELL_GATEWAY: "nemoclaw" + run: | + set -euo pipefail + export PATH="$HOME/.local/bin:$HOME/.npm-global/bin:$PATH" + npx vitest run --project e2e-live \ + test/e2e/live/sandbox-operations.test.ts \ + --silent=false --reporter=default + + - name: Upload macOS Docker logs on failure + if: failure() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: macos-docker-final-destroy-logs + path: | + /tmp/nemoclaw-e2e-*.log + ${{ github.workspace }}/e2e-artifacts/live if-no-files-found: ignore diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index 4d878f2c6d0..416260dce42 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -1506,7 +1506,7 @@ If you want to upgrade the sandbox while preserving state, use `$$nemoclaw If another terminal has an active SSH session to the sandbox, `destroy` prints an active-session warning and requires a second confirmation before it proceeds. -Pass `--yes`, `-y`, or `--force` to skip the prompt in scripted workflows. +Pass `--yes`, `-y`, or `--force`, or set `NEMOCLAW_NON_INTERACTIVE=1`, to authorize deletion without prompting in scripted workflows. @@ -1521,10 +1521,14 @@ It restores and verifies lockdown and revokes the active timer before deletion. It clears the remaining local shields state only after deletion succeeds. If hardening fails, the command refuses deletion and leaves the timer authority available to retry lockdown. If deletion fails after hardening, the command keeps the surviving sandbox's locked shields state instead of cleaning it up as though deletion succeeded. -By default, `destroy` preserves the shared NemoClaw gateway. -Pass `--cleanup-gateway` to remove the shared gateway when destroying the last sandbox, or `--no-cleanup-gateway` to force preservation when environment defaults request cleanup. +By default, unattended final-sandbox destroys (`--yes`, `--force`, or `NEMOCLAW_NON_INTERACTIVE=1`) remove the shared NemoClaw gateway on macOS so the host listener is released, while Linux preserves it for reuse. +Pass `--cleanup-gateway` to force removal, or `--no-cleanup-gateway` to force preservation. +These flags always override both `NEMOCLAW_CLEANUP_GATEWAY` and the platform default. If the pre-delete workspace wipe cannot run, use a different sandbox name for a clean start. -When this is the last sandbox, pass `--cleanup-gateway` to purge the shared cluster volume that retains the per-name persistent volume. +Cleaning up the gateway after the last sandbox also purges the shared cluster volume that retains the per-name persistent volume. +If final gateway cleanup finds a live PID-file process whose command line does not prove it owns the target gateway, `destroy` exits non-zero after sandbox and registry deletion and skips gateway and volume removal. +NemoClaw preserves the per-gateway PID file and runtime marker so you can inspect the process. +Stop only the listener that matches the target gateway, then rerun `destroy` to converge cleanup. If the OpenShell gateway is unreachable and the sandbox has no managed MCP ownership state, `--force` removes only NemoClaw's local registry entry and local artifacts. Gateway-side deletion remains unconfirmed, shared host-service and gateway teardown are skipped, and the sandbox and retained volume may still exist if the gateway returns. Start the gateway with `$$nemoclaw status` and retry destroy when you need a confirmed deletion. @@ -3501,7 +3505,7 @@ The following flags change defaults for commands that manage existing sandboxes. | Variable | Format | Effect | |----------|--------|--------| -| `NEMOCLAW_CLEANUP_GATEWAY` | `1`, `true`, or `yes` to enable; `0`, `false`, or `no` to disable | Sets the default for whether `$$nemoclaw destroy` removes the shared gateway when destroying the last sandbox. Command-line `--cleanup-gateway` and `--no-cleanup-gateway` still take precedence. | +| `NEMOCLAW_CLEANUP_GATEWAY` | `1`, `true`, or `yes` to enable; `0`, `false`, or `no` to disable | Overrides the platform default (macOS unattended: cleanup; Linux/Windows: preserve) for whether `$$nemoclaw destroy` removes the shared gateway when destroying the last sandbox. Command-line `--cleanup-gateway` and `--no-cleanup-gateway` still take precedence. | | `NEMOCLAW_CONFIRM_LEGACY_MANAGED_RECREATE` | Exact JSON array of sandbox names | Confirms to the installer that the exact listed set of pre-fingerprint OpenClaw or Hermes sandboxes used NemoClaw-managed images, allowing recovery onto the current managed image. The normalized names must exactly match the installer's printed array. Set it only after verifying every named sandbox. Recorded custom-image evidence remains blocked. | | `NEMOCLAW_DISABLE_INFERENCE_ROUTE_REPAIR` | `1` to enable | Skips the automatic DNS-proxy repair for stale `inference.local` routes during `$$nemoclaw connect` and `$$nemoclaw connect --probe-only`. Use only as a troubleshooting escape hatch. | | `NEMOCLAW_SHIELDS_ACCEPT_LEGACY_BASELINE` | `1` to opt in | Allows advanced immutable-config verification to trust the current on-disk bytes for older or partial content baselines. Use only after you have rebuilt or manually inspected the sandbox state and accepted that the baseline is operator-approved. | diff --git a/src/commands/sandbox/destroy.ts b/src/commands/sandbox/destroy.ts index 1522a11fa83..2de6df1ca46 100644 --- a/src/commands/sandbox/destroy.ts +++ b/src/commands/sandbox/destroy.ts @@ -26,7 +26,7 @@ export default class DestroyCliCommand extends NemoClawCommand { force: forceFlag(), "cleanup-gateway": Flags.boolean({ description: - "When destroying the last sandbox, also tear down the shared NemoClaw gateway. Default: preserve. NEMOCLAW_CLEANUP_GATEWAY=1 sets the same default.", + "When destroying the last sandbox, also tear down the shared NemoClaw gateway. Default: preserve on Linux; cleanup for unattended macOS destroys. NEMOCLAW_CLEANUP_GATEWAY overrides the platform default.", allowNo: true, }), }; diff --git a/src/lib/actions/sandbox/destroy-flow.test.ts b/src/lib/actions/sandbox/destroy-flow.test.ts index 3e3ce11d0c7..7e3c1694d13 100644 --- a/src/lib/actions/sandbox/destroy-flow.test.ts +++ b/src/lib/actions/sandbox/destroy-flow.test.ts @@ -37,6 +37,7 @@ describe("destroySandbox flow", () => { ? delete process.env.OPENSHELL_GATEWAY : (process.env.OPENSHELL_GATEWAY = originalGatewayEnv); vi.restoreAllMocks(); + vi.unstubAllEnvs(); resetDestroyModuleCache(); }); @@ -54,6 +55,30 @@ describe("destroySandbox flow", () => { expectSuccessfulLiveDestroy(harness, exitSpy); }); + it.each([ + ["--yes", "darwin", { yes: true }, "", true], + ["NEMOCLAW_NON_INTERACTIVE=1", "darwin", {}, "1", true], + [ + "an explicit preservation override", + "darwin", + { yes: true, cleanupGateway: false }, + "", + false, + ], + ["NEMOCLAW_NON_INTERACTIVE=1", "linux", {}, "1", false], + ] as const)("applies the final-gateway default for %s on %s (#4662)", async (_scenario, platform, options, nonInteractive, cleanupExpected) => { + vi.spyOn(process, "platform", "get").mockReturnValue(platform); + vi.stubEnv("NEMOCLAW_NON_INTERACTIVE", nonInteractive); + const harness = createDestroyHarness(); + + await expect(harness.destroySandbox("alpha", options)).resolves.toBeUndefined(); + + expect(harness.promptSpy).not.toHaveBeenCalled(); + expect(harness.cleanupGatewaySpy.mock.calls).toEqual( + cleanupExpected ? [["nemoclaw-19080", harness.runOpenshellSpy]] : [], + ); + }); + it("stops before local cleanup when OpenShell fails to delete the live sandbox", async () => { const harness = createDestroyHarness({ deleteStatus: 7, diff --git a/src/lib/actions/sandbox/destroy-gateway-cleanup.test.ts b/src/lib/actions/sandbox/destroy-gateway-cleanup.test.ts new file mode 100644 index 00000000000..85e69d9d10b --- /dev/null +++ b/src/lib/actions/sandbox/destroy-gateway-cleanup.test.ts @@ -0,0 +1,133 @@ +// 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 { hasNoLiveSandboxes } from "../../domain/sandbox/destroy"; +import { + collectLiveSandboxProbeSnapshot, + shouldCleanupGatewayAfterConfirmedFinalDestroy, +} from "./destroy-gateway-cleanup"; + +describe("shouldCleanupGatewayAfterConfirmedFinalDestroy", () => { + it("defers live probes until the local registry is empty", () => { + const liveSandboxProbe = vi.fn(() => true); + + expect( + shouldCleanupGatewayAfterConfirmedFinalDestroy( + { + deleteSucceededOrAlreadyGone: true, + removedRegistryEntry: true, + }, + { + listSandboxes: () => ({ sandboxes: [{}] }), + liveSandboxProbe, + }, + ), + ).toBe(false); + expect(liveSandboxProbe).not.toHaveBeenCalled(); + }); + + it("requires confirmed delete, registry removal, and no live sandboxes", () => { + expect( + shouldCleanupGatewayAfterConfirmedFinalDestroy( + { + deleteSucceededOrAlreadyGone: true, + removedRegistryEntry: true, + }, + { + listSandboxes: () => ({ sandboxes: [] }), + liveSandboxProbe: () => true, + }, + ), + ).toBe(true); + + expect( + shouldCleanupGatewayAfterConfirmedFinalDestroy( + { + deleteSucceededOrAlreadyGone: true, + removedRegistryEntry: true, + }, + { + listSandboxes: () => ({ sandboxes: [] }), + liveSandboxProbe: () => false, + }, + ), + ).toBe(false); + }); + + it("preserves the gateway when a live sandbox appears after the empty-registry check", () => { + const events: string[] = []; + expect( + shouldCleanupGatewayAfterConfirmedFinalDestroy( + { + deleteSucceededOrAlreadyGone: true, + removedRegistryEntry: true, + }, + { + listSandboxes: () => { + events.push("registry-empty"); + return { sandboxes: [] }; + }, + liveSandboxProbe: () => { + events.push("live-sandbox-observed"); + // False means the host probe observed a sandbox during the TOCTOU window. + return false; + }, + }, + ), + ).toBe(false); + expect(events).toEqual(["registry-empty", "live-sandbox-observed"]); + }); + + it("collects OpenShell and Docker live-sandbox snapshots in the action layer", () => { + const captureOpenshell = vi.fn(() => ({ + status: 0, + output: + "NAME CREATED PHASE\nnpmtest now Error\n", + })); + const dockerCapture = vi.fn(() => "openshell-npmtest-e487d1bd\n"); + + const snapshot = collectLiveSandboxProbeSnapshot({ + captureOpenshell, + dockerCapture, + timeoutMs: 1_000, + }); + + expect(captureOpenshell).toHaveBeenCalledWith(["sandbox", "list"], { + ignoreError: true, + timeout: 1_000, + }); + expect(dockerCapture).toHaveBeenCalledWith( + ["ps", "--filter", "name=openshell-npmtest-", "--format", "{{.Names}}"], + { + timeout: 1_000, + }, + ); + expect(hasNoLiveSandboxes(snapshot)).toBe(false); + }); + + it("records failed Docker probes as fail-closed snapshots", () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined); + const snapshot = collectLiveSandboxProbeSnapshot({ + captureOpenshell: () => ({ + status: 0, + output: + "NAME CREATED PHASE\nnpmtest now Failed\n", + }), + dockerCapture: () => { + throw new Error("docker unavailable"); + }, + timeoutMs: 1_000, + }); + + expect(hasNoLiveSandboxes(snapshot)).toBe(false); + expect(snapshot.dockerContainersBySandboxName.get("npmtest")).toEqual({ + output: "", + probeFailed: true, + }); + expect(warn).toHaveBeenCalledWith( + "Docker container probe failed for sandbox 'npmtest'; preserving shared gateway: Error: docker unavailable", + ); + }); +}); diff --git a/src/lib/actions/sandbox/destroy-gateway-cleanup.ts b/src/lib/actions/sandbox/destroy-gateway-cleanup.ts new file mode 100644 index 00000000000..d6d2fc7ed4e --- /dev/null +++ b/src/lib/actions/sandbox/destroy-gateway-cleanup.ts @@ -0,0 +1,131 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { OPENSHELL_PROBE_TIMEOUT_MS } from "../../adapters/openshell/timeouts"; +import { + type DockerSandboxContainerSnapshot, + dockerSandboxContainerNamePrefix, + getLiveSandboxNames, + hasNoLiveSandboxes, + type LiveSandboxListSnapshot, + shouldCleanupGatewayAfterDestroy, +} from "../../domain/sandbox/destroy"; +import * as registry from "../../state/registry"; + +type SandboxListProvider = () => { sandboxes: unknown[] }; + +type LiveSandboxListProbe = ( + args: string[], + opts?: { ignoreError?: boolean; timeout?: number }, +) => LiveSandboxListSnapshot; + +type DockerCaptureProbe = (args: string[], opts?: Record) => string; + +type LiveSandboxProbe = (deps?: { + captureOpenshell?: LiveSandboxListProbe; + dockerCapture?: DockerCaptureProbe; + timeoutMs?: number; +}) => boolean; + +type FinalDestroyGatewayCleanupInput = { + deleteSucceededOrAlreadyGone: boolean; + removedRegistryEntry: boolean; +}; + +type FinalDestroyGatewayCleanupDeps = { + listSandboxes?: SandboxListProvider; + liveSandboxProbe?: LiveSandboxProbe; + timeoutMs?: number; +}; + +function captureLiveSandboxes(...args: Parameters) { + const { captureOpenshell } = require("../../adapters/openshell/runtime") as { + captureOpenshell: LiveSandboxListProbe; + }; + return captureOpenshell(...args); +} + +function captureDockerContainers(...args: Parameters) { + const { dockerCapture } = require("../../adapters/docker/run") as { + dockerCapture: DockerCaptureProbe; + }; + return dockerCapture(...args); +} + +export function collectLiveSandboxProbeSnapshot( + deps: { + captureOpenshell?: LiveSandboxListProbe; + dockerCapture?: DockerCaptureProbe; + timeoutMs?: number; + } = {}, +): Parameters[0] { + // Both host probes are synchronous so this produces one ordered snapshot + // after the registry check and before the cleanup decision. + const captureOpenshell = deps.captureOpenshell ?? captureLiveSandboxes; + const dockerCapture = deps.dockerCapture ?? captureDockerContainers; + const timeoutMs = deps.timeoutMs ?? OPENSHELL_PROBE_TIMEOUT_MS; + const liveList = captureOpenshell(["sandbox", "list"], { + ignoreError: true, + timeout: timeoutMs, + }); + const dockerContainersBySandboxName = new Map(); + for (const sandboxName of getLiveSandboxNames(liveList)) { + try { + dockerContainersBySandboxName.set(sandboxName, { + output: dockerCapture( + [ + "ps", + "--filter", + `name=${dockerSandboxContainerNamePrefix(sandboxName)}`, + "--format", + "{{.Names}}", + ], + { + timeout: timeoutMs, + }, + ), + }); + } catch (error) { + // SOURCE_OF_TRUTH: this host Docker CLI probe follows a terminal OpenShell + // row and must attest that its backing container is absent. An exception + // leaves live-sandbox state unknown, so preserve the shared gateway. + // NemoClaw cannot manufacture that container-runtime attestation here; + // destroy-gateway-cleanup.test.ts locks this fail-closed behavior. Remove + // it only when final cleanup has one authoritative sandbox/container state + // source; see the OpenShell listener-removal boundary tracked in #6639. + console.warn( + `Docker container probe failed for sandbox '${sandboxName}'; preserving shared gateway: ${String(error)}`, + ); + dockerContainersBySandboxName.set(sandboxName, { output: "", probeFailed: true }); + } + } + return { liveList, dockerContainersBySandboxName }; +} + +function hasNoLiveSandboxesFromHost(deps?: Parameters[0]): boolean { + return hasNoLiveSandboxes(collectLiveSandboxProbeSnapshot(deps)); +} + +export function shouldCleanupGatewayAfterConfirmedFinalDestroy( + input: FinalDestroyGatewayCleanupInput, + deps: FinalDestroyGatewayCleanupDeps = {}, +): boolean { + const listSandboxes = deps.listSandboxes ?? registry.listSandboxes; + const liveSandboxProbe = deps.liveSandboxProbe ?? hasNoLiveSandboxesFromHost; + const timeoutMs = deps.timeoutMs ?? OPENSHELL_PROBE_TIMEOUT_MS; + const noRegisteredSandboxes = listSandboxes().sandboxes.length === 0; + const noLiveSandboxes = + input.deleteSucceededOrAlreadyGone && + input.removedRegistryEntry && + noRegisteredSandboxes && + liveSandboxProbe({ + timeoutMs, + }); + + return shouldCleanupGatewayAfterDestroy({ + deleteSucceededOrAlreadyGone: input.deleteSucceededOrAlreadyGone, + removedRegistryEntry: input.removedRegistryEntry, + noRegisteredSandboxes, + noLiveSandboxes, + }); +} diff --git a/src/lib/actions/sandbox/destroy-gateway-runtime-evidence.test.ts b/src/lib/actions/sandbox/destroy-gateway-runtime-evidence.test.ts new file mode 100644 index 00000000000..4639f645cee --- /dev/null +++ b/src/lib/actions/sandbox/destroy-gateway-runtime-evidence.test.ts @@ -0,0 +1,92 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + dockerRemoveVolumesByPrefix: vi.fn(), + spawnSync: vi.fn(), + stopStaleDashboardListeners: vi.fn(), +})); + +vi.mock("node:child_process", () => ({ + spawnSync: mocks.spawnSync, +})); +vi.mock("../../adapters/docker/volume", () => ({ + dockerRemoveVolumesByPrefix: mocks.dockerRemoveVolumesByPrefix, +})); +vi.mock("../../onboard/stale-gateway-cleanup", () => ({ + stopStaleDashboardListeners: mocks.stopStaleDashboardListeners, +})); + +import { cleanupGatewayAfterLastSandbox } from "./destroy-gateway"; + +describe("cleanupGatewayAfterLastSandbox runtime evidence", () => { + let stateDir: string; + + beforeEach(() => { + stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-destroy-gateway-evidence-")); + vi.stubEnv("NEMOCLAW_OPENSHELL_GATEWAY_STATE_DIR", stateDir); + }); + + afterEach(() => { + vi.restoreAllMocks(); + vi.clearAllMocks(); + vi.unstubAllEnvs(); + fs.rmSync(stateDir, { force: true, recursive: true }); + }); + + it("preserves unverifiable PID evidence so final cleanup can converge on retry (#4662)", () => { + const pid = 456; + let pidIsAlive = true; + const pidFile = path.join(stateDir, "openshell-gateway.pid"); + const runtimeMarker = path.join(stateDir, "runtime.json"); + fs.writeFileSync(pidFile, `${pid}\n`); + fs.writeFileSync(runtimeMarker, '{"evidence":"keep-until-safe"}\n'); + vi.spyOn(process, "platform", "get").mockReturnValue("linux"); + const missingProcess = () => ({ status: 1, stdout: "", stderr: "" }); + const processResponses = new Map([ + [ + `ps -p ${pid} -o pid=`, + () => ({ + status: pidIsAlive ? 0 : 1, + stdout: pidIsAlive ? `${pid}\n` : "", + stderr: "", + }), + ], + [`ps -p ${pid} -o args=`, () => ({ status: 0, stdout: "openclaw-gateway\n", stderr: "" })], + ]); + mocks.spawnSync.mockImplementation((command: string, args: string[]) => + (processResponses.get(`${command} ${args.map(String).join(" ")}`) ?? missingProcess)(), + ); + const runOpenshell = vi.fn(() => ({ status: 0, stdout: "", stderr: "" })); + + expect(() => cleanupGatewayAfterLastSandbox("nemoclaw-8081", runOpenshell)).toThrow( + /PID-file process\(es\) 456.*do not prove ownership/, + ); + expect(fs.readFileSync(pidFile, "utf-8")).toBe(`${pid}\n`); + expect(fs.readFileSync(runtimeMarker, "utf-8")).toContain("keep-until-safe"); + expect(runOpenshell).not.toHaveBeenCalledWith( + ["gateway", "remove", "nemoclaw-8081"], + expect.anything(), + ); + expect(mocks.dockerRemoveVolumesByPrefix).not.toHaveBeenCalled(); + + pidIsAlive = false; + expect(() => cleanupGatewayAfterLastSandbox("nemoclaw-8081", runOpenshell)).not.toThrow(); + expect(fs.existsSync(pidFile)).toBe(false); + expect(fs.existsSync(runtimeMarker)).toBe(false); + expect(runOpenshell).toHaveBeenCalledWith(["gateway", "remove", "nemoclaw-8081"], { + ignoreError: true, + stdio: ["ignore", "pipe", "pipe"], + }); + expect(mocks.dockerRemoveVolumesByPrefix).toHaveBeenCalledWith( + "openshell-cluster-nemoclaw-8081", + { ignoreError: true }, + ); + }); +}); diff --git a/src/lib/actions/sandbox/destroy-gateway.test.ts b/src/lib/actions/sandbox/destroy-gateway.test.ts new file mode 100644 index 00000000000..ac47fe5d7fd --- /dev/null +++ b/src/lib/actions/sandbox/destroy-gateway.test.ts @@ -0,0 +1,217 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import os from "node:os"; +import path from "node:path"; + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + dockerRemoveVolumesByPrefix: vi.fn(), + stopHostGatewayProcesses: vi.fn(), + stopStaleDashboardListeners: vi.fn(), +})); + +vi.mock("../../adapters/docker/volume", () => ({ + dockerRemoveVolumesByPrefix: mocks.dockerRemoveVolumesByPrefix, +})); +vi.mock("../../onboard/host-gateway-process", () => ({ + stopHostGatewayProcesses: mocks.stopHostGatewayProcesses, +})); +vi.mock("../../onboard/stale-gateway-cleanup", () => ({ + stopStaleDashboardListeners: mocks.stopStaleDashboardListeners, +})); + +import { cleanupGatewayAfterLastSandbox } from "./destroy-gateway"; + +describe("cleanupGatewayAfterLastSandbox", () => { + beforeEach(() => { + mocks.stopHostGatewayProcesses.mockReturnValue({ + failed: [], + skippedDeadPids: [], + skippedNonMatchingPids: [], + stopped: [], + sudoRemediationPids: [], + }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + vi.clearAllMocks(); + delete process.env.NEMOCLAW_OPENSHELL_GATEWAY_STATE_DIR; + }); + + it("uses the PID-file-scoped host gateway reaper for macOS final destroy (#4662)", () => { + vi.spyOn(process, "platform", "get").mockReturnValue("darwin"); + vi.spyOn(os, "homedir").mockReturnValue("/home/tester"); + const runOpenshell = vi.fn(() => ({ status: 0, stdout: "", stderr: "" })); + const stateDir = path.join( + "/home/tester", + ".local", + "state", + "nemoclaw", + "openshell-docker-gateway-8081", + ); + + cleanupGatewayAfterLastSandbox("nemoclaw-8081", runOpenshell); + + expect(mocks.stopStaleDashboardListeners).toHaveBeenCalledOnce(); + expect(mocks.stopHostGatewayProcesses).toHaveBeenCalledWith( + {}, + { + usePgrepFallback: false, + stateDir, + pidFile: path.join(stateDir, "openshell-gateway.pid"), + openShellGatewayName: "nemoclaw-8081", + openShellGatewayPort: 8081, + preserveRuntimeFilesOnNonMatching: true, + }, + ); + expect(runOpenshell).toHaveBeenCalledWith(["gateway", "remove", "nemoclaw-8081"], { + ignoreError: true, + stdio: ["ignore", "pipe", "pipe"], + }); + expect(mocks.dockerRemoveVolumesByPrefix).toHaveBeenCalledWith( + "openshell-cluster-nemoclaw-8081", + { + ignoreError: true, + }, + ); + }); + + it("keeps the PID-file-scoped host gateway reaper active for Linux final destroy", () => { + vi.spyOn(process, "platform", "get").mockReturnValue("linux"); + vi.spyOn(os, "homedir").mockReturnValue("/home/tester"); + const runOpenshell = vi.fn(() => ({ status: 0, stdout: "", stderr: "" })); + const stateDir = path.join( + "/home/tester", + ".local", + "state", + "nemoclaw", + "openshell-docker-gateway-8081", + ); + + cleanupGatewayAfterLastSandbox("nemoclaw-8081", runOpenshell); + + expect(mocks.stopHostGatewayProcesses).toHaveBeenCalledWith( + {}, + { + usePgrepFallback: false, + stateDir, + pidFile: path.join(stateDir, "openshell-gateway.pid"), + openShellGatewayName: "nemoclaw-8081", + openShellGatewayPort: 8081, + preserveRuntimeFilesOnNonMatching: true, + }, + ); + expect(runOpenshell).toHaveBeenCalledWith(["gateway", "remove", "nemoclaw-8081"], { + ignoreError: true, + stdio: ["ignore", "pipe", "pipe"], + }); + expect(mocks.dockerRemoveVolumesByPrefix).toHaveBeenCalledWith( + "openshell-cluster-nemoclaw-8081", + { + ignoreError: true, + }, + ); + }); + + it("keeps host gateway reaping disabled for non-Docker-driver platforms", () => { + vi.spyOn(process, "platform", "get").mockReturnValue("win32"); + const runOpenshell = vi.fn(() => ({ status: 0, stdout: "", stderr: "" })); + + cleanupGatewayAfterLastSandbox("nemoclaw", runOpenshell); + + expect(mocks.stopHostGatewayProcesses).not.toHaveBeenCalled(); + expect(runOpenshell).toHaveBeenCalledWith(["gateway", "remove", "nemoclaw"], { + ignoreError: true, + stdio: ["ignore", "pipe", "pipe"], + }); + }); + + it("fails before gateway and volume removal when the owned host listener survives (#4662)", () => { + vi.spyOn(process, "platform", "get").mockReturnValue("darwin"); + vi.spyOn(os, "homedir").mockReturnValue("/home/tester"); + mocks.stopHostGatewayProcesses.mockReturnValue({ + failed: [123], + skippedDeadPids: [], + skippedNonMatchingPids: [], + stopped: [], + sudoRemediationPids: [123], + }); + const runOpenshell = vi.fn(() => ({ status: 0, stdout: "", stderr: "" })); + + expect(() => cleanupGatewayAfterLastSandbox("nemoclaw-8081", runOpenshell)).toThrow( + /PID\(s\) 123.*rerun destroy/, + ); + expect(runOpenshell).not.toHaveBeenCalledWith( + ["gateway", "remove", "nemoclaw-8081"], + expect.anything(), + ); + expect(mocks.dockerRemoveVolumesByPrefix).not.toHaveBeenCalled(); + }); + + it("fails before gateway and volume removal when PID-file ownership is unverifiable (#4662)", () => { + vi.spyOn(process, "platform", "get").mockReturnValue("darwin"); + vi.spyOn(os, "homedir").mockReturnValue("/home/tester"); + mocks.stopHostGatewayProcesses.mockReturnValue({ + failed: [], + skippedDeadPids: [], + skippedNonMatchingPids: [456], + stopped: [], + sudoRemediationPids: [], + }); + const runOpenshell = vi.fn(() => ({ status: 0, stdout: "", stderr: "" })); + + expect(() => cleanupGatewayAfterLastSandbox("nemoclaw-8081", runOpenshell)).toThrow( + /PID-file process\(es\) 456.*do not prove ownership.*rerun destroy/, + ); + expect(runOpenshell).not.toHaveBeenCalledWith( + ["gateway", "remove", "nemoclaw-8081"], + expect.anything(), + ); + expect(mocks.dockerRemoveVolumesByPrefix).not.toHaveBeenCalled(); + }); + + it.each([ + [ + "host reaper", + () => + mocks.stopHostGatewayProcesses.mockImplementationOnce(() => { + throw new Error("injected host reaper failure"); + }), + ], + [ + "gateway remove", + (runOpenshell: ReturnType) => + runOpenshell + .mockImplementationOnce(() => ({ status: 0, stdout: "", stderr: "" })) + .mockImplementationOnce(() => { + throw new Error("injected gateway remove failure"); + }), + ], + [ + "volume cleanup", + () => + mocks.dockerRemoveVolumesByPrefix.mockImplementationOnce(() => { + throw new Error("injected volume cleanup failure"); + }), + ], + ] as const)("converges on retry after a partial %s failure (#4662)", (_stage, injectFailure) => { + vi.spyOn(process, "platform", "get").mockReturnValue("darwin"); + vi.spyOn(os, "homedir").mockReturnValue("/home/tester"); + const runOpenshell = vi.fn(() => ({ status: 0, stdout: "", stderr: "" })); + injectFailure(runOpenshell); + + expect(() => cleanupGatewayAfterLastSandbox("nemoclaw-8081", runOpenshell)).toThrow(); + expect(() => cleanupGatewayAfterLastSandbox("nemoclaw-8081", runOpenshell)).not.toThrow(); + expect(runOpenshell).toHaveBeenCalledWith(["gateway", "remove", "nemoclaw-8081"], { + ignoreError: true, + stdio: ["ignore", "pipe", "pipe"], + }); + expect(mocks.dockerRemoveVolumesByPrefix).toHaveBeenCalledWith( + "openshell-cluster-nemoclaw-8081", + { ignoreError: true }, + ); + }); +}); diff --git a/src/lib/actions/sandbox/destroy-gateway.ts b/src/lib/actions/sandbox/destroy-gateway.ts index 267abae86a3..f5d2b02efcc 100644 --- a/src/lib/actions/sandbox/destroy-gateway.ts +++ b/src/lib/actions/sandbox/destroy-gateway.ts @@ -4,6 +4,7 @@ import os from "node:os"; import path from "node:path"; +import { dockerRemoveVolumesByPrefix } from "../../adapters/docker/volume"; import { OPENSHELL_OPERATION_TIMEOUT_MS } from "../../adapters/openshell/timeouts"; import { DASHBOARD_PORT } from "../../core/ports"; import { @@ -26,14 +27,23 @@ const DASHBOARD_FORWARD_PORT = String(DASHBOARD_PORT); // `nemoclaw-` sandbox would read the default instance's pid file and // stop the wrong host gateway process. Returns null when the gateway name is // outside the NemoClaw namespace (the caller then keeps the defaults). -function resolvePerGatewayStateDir(gatewayName: string): string | null { +function resolvePerGatewayState(gatewayName: string): { port: number; stateDir: string } | null { const port = resolveGatewayPortFromName(gatewayName); if (port === null) return null; const configured = process.env.NEMOCLAW_OPENSHELL_GATEWAY_STATE_DIR; if (configured && configured.trim()) { - return path.resolve(configured.trim()); + return { port, stateDir: path.resolve(configured.trim()) }; } - return path.join(os.homedir(), ".local", "state", "nemoclaw", resolveGatewayStateDirName(port)); + return { + port, + stateDir: path.join( + os.homedir(), + ".local", + "state", + "nemoclaw", + resolveGatewayStateDirName(port), + ), + }; } export function selectGatewayForSandboxDestroy( @@ -66,9 +76,6 @@ export function cleanupGatewayAfterLastSandbox( runOpenshell ?? (require("../../adapters/openshell/runtime") as { runOpenshell: DestroyRunOpenshell }) .runOpenshell; - const { dockerRemoveVolumesByPrefix } = require("../../adapters/docker") as { - dockerRemoveVolumesByPrefix: (prefix: string, opts?: { ignoreError?: boolean }) => void; - }; openshell(["forward", "stop", DASHBOARD_FORWARD_PORT], { ignoreError: true, @@ -79,23 +86,51 @@ export function cleanupGatewayAfterLastSandbox( // ports the live openshell tracks; this catches orphans whose openshell // record was lost across upgrades or failed onboards. stopStaleDashboardListeners(); - if (process.platform === "linux") { + if (process.platform === "linux" || process.platform === "darwin") { // Sandbox destroy is conservative: only stop the host gateway whose PID // file we wrote during onboard. Disable the pgrep sweep so a stray // openshell-gateway under another user/project on the same host (rare but // possible on shared hosts) is not torn down by a NemoClaw `destroy`. // The uninstall path keeps the broader sweep on (run-plan.ts). The state // dir is per-gateway-name so a destroy of `nemoclaw-` reads the - // per-port pid file rather than defaulting to the bare instance's. - const perGatewayStateDir = resolvePerGatewayStateDir(gatewayName); - const stopOptions: { usePgrepFallback: false; stateDir?: string; pidFile?: string } = { + // per-port pid file rather than defaulting to the bare instance's. The + // expected gateway name and port also gate `openshell gateway start` + // cmdlines so a stale pid file cannot kill another gateway instance. + const perGatewayState = resolvePerGatewayState(gatewayName); + const stopOptions: { + openShellGatewayName?: string; + openShellGatewayPort?: number; + preserveRuntimeFilesOnNonMatching: true; + usePgrepFallback: false; + stateDir?: string; + pidFile?: string; + } = { + preserveRuntimeFilesOnNonMatching: true, usePgrepFallback: false, }; - if (perGatewayStateDir) { - stopOptions.stateDir = perGatewayStateDir; - stopOptions.pidFile = path.join(perGatewayStateDir, "openshell-gateway.pid"); + if (perGatewayState) { + stopOptions.stateDir = perGatewayState.stateDir; + stopOptions.pidFile = path.join(perGatewayState.stateDir, "openshell-gateway.pid"); + stopOptions.openShellGatewayName = gatewayName; + stopOptions.openShellGatewayPort = perGatewayState.port; + } + const stopResult = stopHostGatewayProcesses({}, stopOptions); + const unverifiablePids = [...new Set(stopResult.skippedNonMatchingPids)]; + if (unverifiablePids.length > 0) { + throw new Error( + `Refusing cleanup because PID-file process(es) ${unverifiablePids.join(", ")} do not prove ownership of gateway '${gatewayName}'. Inspect the process and per-gateway PID file, stop only the matching gateway listener, then rerun destroy.`, + ); + } + const failedPids = [...new Set([...stopResult.failed, ...stopResult.sudoRemediationPids])]; + if (failedPids.length > 0) { + const remediation = + stopResult.sudoRemediationPids.length > 0 + ? ` Retry with sufficient permissions for PID(s) ${stopResult.sudoRemediationPids.join(", ")}, then rerun destroy.` + : " Retry destroy after stopping the listed process(es)."; + throw new Error( + `Failed to stop the owned host gateway process(es) for '${gatewayName}': ${failedPids.join(", ")}.${remediation}`, + ); } - stopHostGatewayProcesses({}, stopOptions); } /** * SOURCE_OF_TRUTH @@ -104,15 +139,21 @@ export function cleanupGatewayAfterLastSandbox( * an existing installation is being recovered or removed. * Source-fix constraint: NemoClaw cannot add the modern verb to historical * OpenShell builds, so cleanup tries their legacy verb best-effort. - * Regression proof: destroy-gateway-cleanup.test.ts covers successful remove - * and remove-nonzero fallback while preserving Docker-volume cleanup. + * Regression proof: test/cli/destroy-gateway-cleanup.test.ts covers successful + * remove and remove-nonzero fallback while preserving Docker-volume cleanup. * Removal condition: remove the fallback when every supported recovery and * teardown entry point upgrades OpenShell to the blueprint minimum (currently * 0.0.72) before this function can run. * * macOS previously ran only `gateway destroy`, which current OpenShell * rejects as an unrecognized subcommand (#6569). The host-process stop above - * remains Linux-only. + * now uses the same PID-file-scoped reaper as Linux so final unattended + * macOS destroys release the Docker-driver gateway listener (#4662). + * Removal tracker: #6639. Remove the macOS reliance on this host-process + * fallback after OpenShell releases the Docker-driver listener fix, NemoClaw + * raises its supported OpenShell floor to that fixed build, and a real macOS + * Docker-driver sandbox-operations run proves final unattended destroy + * releases the gateway port without this fallback. */ const removeResult = openshell(["gateway", "remove", gatewayName], { ignoreError: true, diff --git a/src/lib/actions/sandbox/destroy.ts b/src/lib/actions/sandbox/destroy.ts index e182902699b..071b8605e87 100644 --- a/src/lib/actions/sandbox/destroy.ts +++ b/src/lib/actions/sandbox/destroy.ts @@ -4,16 +4,16 @@ import fs from "node:fs"; import path from "node:path"; -import { OPENSHELL_PROBE_TIMEOUT_MS } from "../../adapters/openshell/timeouts"; import { CLI_NAME } from "../../cli/branding"; import { G, R, YW } from "../../cli/terminal-style"; +import { isNonInteractiveEnv } from "../../core/non-interactive"; import { prompt as askPrompt } from "../../credentials/store"; import { type DestroySandboxOptions, normalizeDestroySandboxOptions, } from "../../domain/lifecycle/options"; import { - shouldCleanupGatewayAfterDestroy, + resolveDestroyGatewayCleanupDecision, shouldStopHostServicesAfterDestroy, } from "../../domain/sandbox/destroy"; import { @@ -21,7 +21,6 @@ import { SANDBOX_PROVIDER_SUFFIXES, } from "../../onboard/sandbox-provider-cleanup"; import { validateName } from "../../runner"; -import { parseLiveSandboxNames } from "../../runtime-recovery"; import { killTimer as defaultKillShieldsTimer } from "../../shields/timer-control"; import { withMcpLifecycleLock } from "../../state/mcp-lifecycle-lock"; import type { Session } from "../../state/onboard-session"; @@ -30,6 +29,7 @@ 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 { prepareSandboxDestroy } from "./destroy-preflight"; import { type WipeSandboxStateDeps, wipeSandboxState } from "./wipe-state"; @@ -79,30 +79,14 @@ type RemoveShieldsStateDeps = { warn?: (message: string) => void; }; -// Mirrors the body of `isNonInteractive()` in src/lib/onboard.ts. Duplicated -// here to avoid an awkward sibling-action -> onboard import; the canonical -// helper should be lifted to src/lib/core/ so this and the lazy requires in -// policy-channel.ts and inference/ollama/proxy.ts can all share one source. -function isNonInteractive(): boolean { - return process.env.NEMOCLAW_NON_INTERACTIVE === "1"; -} - -/** - * Decide whether to tear down the shared NemoClaw gateway after destroying - * the last sandbox. Default is to preserve it (#2166); explicit opt-in via - * `cleanupGateway: true` (which `normalizeDestroySandboxOptions` also reads - * from `--cleanup-gateway` / `NEMOCLAW_CLEANUP_GATEWAY`). - * - * Prompt rules: - * - explicit `cleanupGateway` set → honour it without prompting - * - non-interactive or `--yes` / `--force` → preserve gateway (safe default) - * - interactive without `--yes` → prompt the user - */ async function resolveCleanupGatewayDecision(options: DestroySandboxOptions): Promise { - if (options.cleanupGateway === true) return true; - if (options.cleanupGateway === false) return false; - if (options.yes === true || options.force === true) return false; - if (isNonInteractive()) return false; + const decision = resolveDestroyGatewayCleanupDecision(options, { + nonInteractive: isNonInteractiveEnv(), + platform: process.platform, + }); + if (decision === "cleanup") return true; + if (decision === "preserve") return false; + console.log(` ${YW}This was the last sandbox.${R}`); console.log( " Also destroy the shared NemoClaw gateway (port forward, gateway pod, cluster volumes)?", @@ -115,23 +99,6 @@ async function resolveCleanupGatewayDecision(options: DestroySandboxOptions): Pr return trimmed === "y" || trimmed === "yes"; } -function hasNoLiveSandboxes(): boolean { - const { captureOpenshell } = require("../../adapters/openshell/runtime") as { - captureOpenshell: ( - args: string[], - opts?: { ignoreError?: boolean; timeout?: number }, - ) => { status: number | null; output: string }; - }; - const liveList = captureOpenshell(["sandbox", "list"], { - ignoreError: true, - timeout: OPENSHELL_PROBE_TIMEOUT_MS, - }); - if (liveList.status !== 0) { - return false; - } - return parseLiveSandboxNames(liveList.output).size === 0; -} - export function cleanupSandboxServices( sandboxName: string, { stopHostServices = false }: { stopHostServices?: boolean } = {}, @@ -428,11 +395,9 @@ async function destroySandboxUnlocked( }); } if ( - shouldCleanupGatewayAfterDestroy({ + shouldCleanupGatewayAfterConfirmedFinalDestroy({ deleteSucceededOrAlreadyGone, removedRegistryEntry: removed, - noRegisteredSandboxes: registry.listSandboxes().sandboxes.length === 0, - noLiveSandboxes: hasNoLiveSandboxes(), }) ) { const shouldCleanupGateway = await resolveCleanupGatewayDecision(normalized); diff --git a/src/lib/actions/sandbox/policy-channel.ts b/src/lib/actions/sandbox/policy-channel.ts index 0762679a99c..99bb7809047 100644 --- a/src/lib/actions/sandbox/policy-channel.ts +++ b/src/lib/actions/sandbox/policy-channel.ts @@ -6,6 +6,7 @@ import path from "node:path"; import { runOpenshell } from "../../adapters/openshell/runtime"; import { type AgentDefinition, loadAgent } from "../../agent/defs"; import { CLI_DISPLAY_NAME, CLI_NAME } from "../../cli/branding"; +import { isNonInteractiveEnv } from "../../core/non-interactive"; import { prompt as askPrompt, getCredential } from "../../credentials/store"; import { type PolicyAddOptions, @@ -60,9 +61,7 @@ import { policyChannelDependencies } from "./policy-channel-dependencies"; import { refreshSandboxPolicyContextFile } from "./policy-context-refresh"; import { executeSandboxCommand, executeSandboxExecCommand } from "./process-recovery"; -function isNonInteractive(): boolean { - return process.env.NEMOCLAW_NON_INTERACTIVE === "1"; -} +const isNonInteractive = isNonInteractiveEnv; type ChannelMutationOptions = { channel?: string; @@ -166,7 +165,7 @@ async function addSandboxPolicyUnlocked( } answer = preset.name; } else { - if (process.env.NEMOCLAW_NON_INTERACTIVE === "1") { + if (isNonInteractive()) { console.error(" Non-interactive mode requires a preset name."); console.error(` Usage: ${CLI_NAME} policy-add [--yes] [--dry-run]`); process.exit(1); @@ -1504,9 +1503,7 @@ async function removeSandboxPolicyUnlocked( options: PolicyRemoveOptions, ): Promise { const dryRun = Boolean(options.dryRun); - const skipConfirm = Boolean( - options.yes || options.force || process.env.NEMOCLAW_NON_INTERACTIVE === "1", - ); + const skipConfirm = Boolean(options.yes || options.force || isNonInteractive()); // Remove-able presets = built-in presets + custom presets applied via // --from-file / --from-dir (tracked in registry.customPolicies). @@ -1533,7 +1530,7 @@ async function removeSandboxPolicyUnlocked( } answer = preset.name; } else { - if (process.env.NEMOCLAW_NON_INTERACTIVE === "1") { + if (isNonInteractive()) { console.error(" Non-interactive mode requires a preset name."); console.error(` Usage: ${CLI_NAME} policy-remove [--yes] [--dry-run]`); process.exit(1); diff --git a/src/lib/core/non-interactive.test.ts b/src/lib/core/non-interactive.test.ts new file mode 100644 index 00000000000..2da282fe2d8 --- /dev/null +++ b/src/lib/core/non-interactive.test.ts @@ -0,0 +1,29 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { isNonInteractiveEnv } from "./non-interactive"; + +afterEach(() => { + vi.unstubAllEnvs(); +}); + +describe("non-interactive environment detection", () => { + it("treats only the canonical value as non-interactive", () => { + expect(isNonInteractiveEnv({ NEMOCLAW_NON_INTERACTIVE: "1" } as NodeJS.ProcessEnv)).toBe(true); + expect(isNonInteractiveEnv({ NEMOCLAW_NON_INTERACTIVE: "true" } as NodeJS.ProcessEnv)).toBe( + false, + ); + expect(isNonInteractiveEnv({ NEMOCLAW_NON_INTERACTIVE: "" } as NodeJS.ProcessEnv)).toBe(false); + expect(isNonInteractiveEnv({} as NodeJS.ProcessEnv)).toBe(false); + }); + + it("reads process.env when called without an argument", () => { + vi.stubEnv("NEMOCLAW_NON_INTERACTIVE", "1"); + expect(isNonInteractiveEnv()).toBe(true); + + vi.stubEnv("NEMOCLAW_NON_INTERACTIVE", "true"); + expect(isNonInteractiveEnv()).toBe(false); + }); +}); diff --git a/src/lib/core/non-interactive.ts b/src/lib/core/non-interactive.ts new file mode 100644 index 00000000000..72f71c4a876 --- /dev/null +++ b/src/lib/core/non-interactive.ts @@ -0,0 +1,6 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +export function isNonInteractiveEnv(env: NodeJS.ProcessEnv = process.env): boolean { + return env.NEMOCLAW_NON_INTERACTIVE === "1"; +} diff --git a/src/lib/domain/lifecycle/options.test.ts b/src/lib/domain/lifecycle/options.test.ts index a58fcd489ad..8bb7b530952 100644 --- a/src/lib/domain/lifecycle/options.test.ts +++ b/src/lib/domain/lifecycle/options.test.ts @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { normalizeDestroySandboxOptions, @@ -11,6 +11,14 @@ import { } from "./options"; describe("lifecycle option normalization", () => { + beforeEach(() => { + vi.stubEnv("NEMOCLAW_NON_INTERACTIVE", ""); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + }); + it("preserves typed destroy options and still accepts compatibility argv", () => { expect(normalizeDestroySandboxOptions({ yes: true })).toEqual({ yes: true }); expect(normalizeDestroySandboxOptions(["--yes", "--force"])).toEqual({ @@ -19,6 +27,13 @@ describe("lifecycle option normalization", () => { }); }); + it("normalizes the shared non-interactive environment into destroy confirmation", () => { + vi.stubEnv("NEMOCLAW_NON_INTERACTIVE", "1"); + + expect(normalizeDestroySandboxOptions([])).toEqual({ force: false, yes: true }); + expect(normalizeDestroySandboxOptions({})).toEqual({ yes: true }); + }); + describe("destroy cleanupGateway resolution (#2166)", () => { const ENV_KEY = "NEMOCLAW_CLEANUP_GATEWAY"; let original: string | undefined; diff --git a/src/lib/domain/lifecycle/options.ts b/src/lib/domain/lifecycle/options.ts index b5b7cd3e891..93572fa0da4 100644 --- a/src/lib/domain/lifecycle/options.ts +++ b/src/lib/domain/lifecycle/options.ts @@ -1,6 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import { isNonInteractiveEnv } from "../../core/non-interactive"; import { DCODE_AUTO_APPROVAL_MODES, type DcodeAutoApprovalMode, @@ -18,8 +19,8 @@ export interface DestroySandboxOptions { /** * When the sandbox being destroyed is the last one, also tear down the * shared NemoClaw gateway (port forward, gateway pod, cluster volumes). - * Default `false` — gateway is preserved so the next `nemoclaw onboard` - * can reuse it without a full re-bootstrap. See #2166. + * Unattended macOS destroys default to cleanup so the host listener is + * released; Linux preserves the gateway for reuse. See #4662 and #2166. * * Resolution order during normalization: explicit option, then * `--cleanup-gateway` argv flag, then `NEMOCLAW_CLEANUP_GATEWAY=1` env @@ -61,6 +62,7 @@ export function normalizeDestroySandboxOptions( options: string[] | DestroySandboxOptions = {}, ): DestroySandboxOptions { const envCleanupGateway = readCleanupGatewayEnv(); + const nonInteractive = isNonInteractiveEnv(); if (Array.isArray(options)) { const yesIdx = options.lastIndexOf("--cleanup-gateway"); const noIdx = options.lastIndexOf("--no-cleanup-gateway"); @@ -68,12 +70,13 @@ export function normalizeDestroySandboxOptions( yesIdx === -1 && noIdx === -1 ? envCleanupGateway : yesIdx > noIdx; return { force: options.includes("--force"), - yes: options.includes("--yes"), + yes: options.includes("--yes") || nonInteractive, ...(cleanupGateway === undefined ? {} : { cleanupGateway }), }; } return { ...options, + ...(nonInteractive ? { yes: true } : {}), ...(options.cleanupGateway === undefined && envCleanupGateway !== undefined ? { cleanupGateway: envCleanupGateway } : {}), diff --git a/src/lib/domain/sandbox/destroy.test.ts b/src/lib/domain/sandbox/destroy.test.ts index c6852727615..da887b51d91 100644 --- a/src/lib/domain/sandbox/destroy.test.ts +++ b/src/lib/domain/sandbox/destroy.test.ts @@ -4,9 +4,13 @@ import { describe, expect, it } from "vitest"; import { + dockerSandboxContainerNamePrefix, getSandboxDeleteOutcome, + hasNoLiveSandboxes, + hasRunningDockerSandboxContainer, isGatewayUnreachableDeleteOutput, isMissingSandboxDeleteOutput, + resolveDestroyGatewayCleanupDecision, shouldCleanupGatewayAfterDestroy, shouldStopHostServicesAfterDestroy, } from "./destroy"; @@ -90,4 +94,119 @@ describe("sandbox destroy helpers", () => { }), ).toBe(false); }); + + it("resolves final-gateway cleanup defaults without prompting when unattended (#4662)", () => { + expect( + resolveDestroyGatewayCleanupDecision( + { cleanupGateway: true }, + { nonInteractive: false, platform: "linux" }, + ), + ).toBe("cleanup"); + expect( + resolveDestroyGatewayCleanupDecision( + { cleanupGateway: false }, + { nonInteractive: true, platform: "darwin" }, + ), + ).toBe("preserve"); + expect( + resolveDestroyGatewayCleanupDecision( + { yes: true }, + { nonInteractive: false, platform: "darwin" }, + ), + ).toBe("cleanup"); + expect( + resolveDestroyGatewayCleanupDecision( + { force: true }, + { nonInteractive: false, platform: "linux" }, + ), + ).toBe("preserve"); + expect( + resolveDestroyGatewayCleanupDecision({}, { nonInteractive: true, platform: "darwin" }), + ).toBe("cleanup"); + expect( + resolveDestroyGatewayCleanupDecision({}, { nonInteractive: true, platform: "linux" }), + ).toBe("preserve"); + expect( + resolveDestroyGatewayCleanupDecision({}, { nonInteractive: true, platform: "win32" }), + ).toBe("preserve"); + expect( + resolveDestroyGatewayCleanupDecision({}, { nonInteractive: false, platform: "darwin" }), + ).toBe("prompt"); + }); + + it("treats only terminal OpenShell rows without Docker containers as no live sandboxes (#4662)", () => { + const liveListOutput = + "NAME CREATED PHASE\nnpmtest 2026-06-01 00:00:00 Error\n"; + expect( + hasNoLiveSandboxes({ + liveList: { status: 0, output: liveListOutput }, + dockerContainersBySandboxName: new Map([["npmtest", { output: "" }]]), + }), + ).toBe(true); + expect( + hasNoLiveSandboxes({ + liveList: { status: 0, output: liveListOutput }, + dockerContainersBySandboxName: new Map([ + ["npmtest", { output: "openshell-npmtest-e487d1bd\n" }], + ]), + }), + ).toBe(false); + expect( + hasNoLiveSandboxes({ + liveList: { + status: 0, + output: + "NAME CREATED PHASE\nnpmtest now Ready\n", + }, + dockerContainersBySandboxName: new Map([["npmtest", { output: "" }]]), + }), + ).toBe(false); + }); + + it("fails closed when a Docker live-container probe snapshot is missing or failed (#4662)", () => { + expect(hasRunningDockerSandboxContainer("npmtest", undefined)).toBe(true); + expect(hasRunningDockerSandboxContainer("npmtest", { output: "", probeFailed: true })).toBe( + true, + ); + expect( + hasNoLiveSandboxes({ + liveList: { + status: 0, + output: + "NAME CREATED PHASE\nnpmtest now Failed\n", + }, + dockerContainersBySandboxName: new Map([["npmtest", { output: "", probeFailed: true }]]), + }), + ).toBe(false); + }); + + it("fails closed when OpenShell cannot report live sandbox state (#4662)", () => { + expect( + hasNoLiveSandboxes({ + liveList: { status: 1, output: "" }, + dockerContainersBySandboxName: new Map(), + }), + ).toBe(false); + }); + + it("matches Docker sandbox containers with a literal name prefix (#4662)", () => { + expect(dockerSandboxContainerNamePrefix("npmtest")).toBe("openshell-npmtest-"); + expect( + hasRunningDockerSandboxContainer("npmtest", { + output: "prefix-openshell-npmtest-e487d1bd\nopenshell-npmtest-e487d1bd\n", + }), + ).toBe(true); + expect( + hasRunningDockerSandboxContainer("npmtest[", { + output: "openshell-npmtest[-e487d1bd\n", + }), + ).toBe(true); + expect( + hasRunningDockerSandboxContainer( + "npmtest", + { output: "prefix-openshell-npmtest-e487d1bd\nopenshell-npmtest-extra-e487d1bd\n" }, + ["npmtest", "npmtest-extra"], + ), + ).toBe(false); + }); }); diff --git a/src/lib/domain/sandbox/destroy.ts b/src/lib/domain/sandbox/destroy.ts index 1dfd35cc1c7..4f02020c7ef 100644 --- a/src/lib/domain/sandbox/destroy.ts +++ b/src/lib/domain/sandbox/destroy.ts @@ -1,7 +1,10 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import { parseLiveSandboxEntries } from "../../runtime-recovery"; + const ANSI_RE = /\x1b\[[0-9;]*m/g; +const TERMINAL_OPEN_SHELL_SANDBOX_PHASES = new Set(["Error", "Failed"]); function stripAnsi(value = ""): string { return String(value).replace(ANSI_RE, ""); @@ -13,6 +16,34 @@ export type SpawnLikeResult = { stderr?: string; }; +export type DestroyGatewayCleanupDecision = "cleanup" | "preserve" | "prompt"; + +export type DestroyGatewayCleanupOptions = { + cleanupGateway?: boolean; + yes?: boolean; + force?: boolean; +}; + +export type DestroyGatewayCleanupContext = { + nonInteractive: boolean; + platform: NodeJS.Platform; +}; + +export type LiveSandboxListSnapshot = { + status: number | null; + output: string; +}; + +export type DockerSandboxContainerSnapshot = { + output: string; + probeFailed?: boolean; +}; + +export type LiveSandboxProbeSnapshot = { + liveList: LiveSandboxListSnapshot; + dockerContainersBySandboxName: ReadonlyMap; +}; + export function isMissingSandboxDeleteOutput(output = ""): boolean { return /\bNotFound\b|\bNot Found\b|sandbox not found|sandbox .* not found|sandbox .* not present|sandbox does not exist|no such sandbox/i.test( stripAnsi(output), @@ -72,3 +103,97 @@ export function shouldCleanupGatewayAfterDestroy(input: { input.noLiveSandboxes ); } + +/** + * Decide the non-UI gateway cleanup path for a final sandbox destroy. + * + * Linux preserves the shared gateway by default for reuse (#2166), while + * unattended macOS destroys clean it up so the leaked host listener is released + * (#4662). Track removal in #6639: drop the macOS default after OpenShell + * releases the Docker-driver listener fix, NemoClaw raises its supported + * OpenShell floor to that fixed version, and live macOS final destroys release + * the listener without forced gateway cleanup. + * Native win32 hosts keep the conservative non-macOS default because supported + * Windows runs go through WSL2 and report `linux`. + */ +export function resolveDestroyGatewayCleanupDecision( + options: DestroyGatewayCleanupOptions, + context: DestroyGatewayCleanupContext, +): DestroyGatewayCleanupDecision { + if (options.cleanupGateway === true) return "cleanup"; + if (options.cleanupGateway === false) return "preserve"; + if (options.yes === true || options.force === true || context.nonInteractive) { + return context.platform === "darwin" ? "cleanup" : "preserve"; + } + return "prompt"; +} + +export function dockerSandboxContainerNamePrefix(sandboxName: string): string { + return `openshell-${sandboxName}-`; +} + +function dockerContainerNames(output: string): string[] { + return output + .split(/\r?\n/u) + .map((name) => name.trim()) + .filter(Boolean); +} + +function ownsDockerSandboxContainer( + containerName: string, + sandboxName: string, + knownSandboxNames: Iterable, +): boolean { + const exactName = `openshell-${sandboxName}`; + const containerNamePrefix = `${exactName}-`; + if (containerName === exactName) return true; + if (!containerName.startsWith(containerNamePrefix)) return false; + const known = new Set(knownSandboxNames); + known.add(sandboxName); + const stripped = containerName.replace(/^openshell-/, ""); + const owner = [...known] + .filter((name) => stripped === name || stripped.startsWith(`${name}-`)) + .sort((a, b) => b.length - a.length)[0]; + return owner === sandboxName; +} + +export function hasRunningDockerSandboxContainer( + sandboxName: string, + snapshot: DockerSandboxContainerSnapshot | undefined, + knownSandboxNames: Iterable = [sandboxName], +): boolean { + if (!snapshot || snapshot.probeFailed) { + return true; + } + return dockerContainerNames(snapshot.output).some((name) => + ownsDockerSandboxContainer(name, sandboxName, knownSandboxNames), + ); +} + +export function getLiveSandboxNames(liveList: LiveSandboxListSnapshot): string[] { + if (liveList.status !== 0) { + return []; + } + return parseLiveSandboxEntries(liveList.output).map((entry) => entry.name); +} + +export function hasNoLiveSandboxes({ + liveList, + dockerContainersBySandboxName, +}: LiveSandboxProbeSnapshot): boolean { + // Fail closed: if OpenShell cannot report authoritative sandbox state, + // preserve the shared gateway so a sandbox never loses its listener. + if (liveList.status !== 0) { + return false; + } + const entries = parseLiveSandboxEntries(liveList.output); + const sandboxNames = entries.map((entry) => entry.name); + return entries.every((entry) => { + if (!TERMINAL_OPEN_SHELL_SANDBOX_PHASES.has(entry.phase ?? "")) return false; + return !hasRunningDockerSandboxContainer( + entry.name, + dockerContainersBySandboxName.get(entry.name), + sandboxNames, + ); + }); +} diff --git a/src/lib/inference/ollama/proxy.ts b/src/lib/inference/ollama/proxy.ts index 4258e48427e..3f84e6a1ecc 100644 --- a/src/lib/inference/ollama/proxy.ts +++ b/src/lib/inference/ollama/proxy.ts @@ -11,6 +11,8 @@ const path = require("path"); const { spawn, spawnSync } = require("child_process"); const { ROOT, SCRIPTS, redact, run, runCapture, shellQuote } = require("../../runner"); const { OLLAMA_PORT, OLLAMA_PROXY_PORT } = require("../../core/ports"); +const { isNonInteractiveEnv }: typeof import("../../core/non-interactive") = + require("../../core/non-interactive"); const { waitForPort } = require("../../core/wait"); const { ensurePulledOllamaModel }: typeof import("./model-discovery") = require("./model-discovery"); @@ -793,7 +795,7 @@ async function promptProxyYesNo(question: string, defaultIsYes: boolean): Promis } const defaultOllamaToolCapabilityInteraction: OllamaToolCapabilityInteraction = { - isNonInteractive: () => process.env.NEMOCLAW_NON_INTERACTIVE === "1", + isNonInteractive: isNonInteractiveEnv, isAutoYes: () => process.env.NEMOCLAW_YES === "1", confirm: promptProxyYesNo, }; diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 20c06ccbdc7..1c74dcfe27e 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -2,12 +2,12 @@ // SPDX-License-Identifier: Apache-2.0 // // Interactive onboarding wizard — 8 steps from zero to running sandbox. -// Supports non-interactive mode via --non-interactive flag or -// NEMOCLAW_NON_INTERACTIVE=1 env var for CI/CD pipelines. const { envInt, LOCAL_INFERENCE_TIMEOUT_SECS, }: typeof import("./onboard/env") = require("./onboard/env"); +const { isNonInteractiveEnv }: typeof import("./core/non-interactive") = + require("./core/non-interactive"); const { agentProductName, cliDisplayName, @@ -687,7 +687,7 @@ function getOnboardDashboardPort(): number { } function isNonInteractive(): boolean { - return NON_INTERACTIVE || process.env.NEMOCLAW_NON_INTERACTIVE === "1"; + return NON_INTERACTIVE || isNonInteractiveEnv(); } function isRecreateSandbox(requested = false): boolean { @@ -2084,20 +2084,16 @@ async function startDockerDriverGateway({ }, ); if (cutover === "reused") return; - if (!gatewayBin) throw new Error("OpenShell gateway binary missing after cutover"); + if (!gatewayBin || !gatewayLaunch) { + throw new Error("OpenShell gateway launch missing after cutover"); + } fs.mkdirSync(stateDir, { recursive: true, mode: 0o700 }); const logPath = path.join(stateDir, "openshell-gateway.log"); const logFd = dockerDriverGatewayLaunch.openDockerDriverGatewayLog(logPath, { exitOnFailure }); console.log(" Starting OpenShell Docker-driver gateway..."); console.log(` Gateway log: ${logPath}`); - const launch = gatewayLaunch ?? { - command: gatewayBin, - args: [], - env: { ...process.env, ...gatewayEnv }, - mode: "host" as const, - processGatewayBin: gatewayBin, - }; + const launch = gatewayLaunch; dockerDriverGatewayLaunch.prepareAndLogDockerDriverGatewayLaunch(launch); const child = dockerDriverGatewayLaunch.spawnDockerDriverGateway(launch, logFd); const childExit = trackChildExit(child); // #3111 zombie-safe liveness @@ -4073,7 +4069,7 @@ async function runOnboard(opts: OnboardOptions = {}): Promise { authoritativeGateway?.name ?? GATEWAY_NAME, ); setOnboardBrandingAgent(opts.agent || process.env.NEMOCLAW_AGENT || null); - NON_INTERACTIVE = opts.nonInteractive || process.env.NEMOCLAW_NON_INTERACTIVE === "1"; + NON_INTERACTIVE = opts.nonInteractive || isNonInteractiveEnv(); RECREATE_SANDBOX = opts.recreateSandbox || process.env.NEMOCLAW_RECREATE_SANDBOX === "1"; AUTO_YES = opts.autoYes === true || process.env.NEMOCLAW_YES === "1"; _preflightDashboardPort = diff --git a/src/lib/onboard/docker-driver-gateway-launch.test.ts b/src/lib/onboard/docker-driver-gateway-launch.test.ts index 2c5eec75327..0938f654d8f 100644 --- a/src/lib/onboard/docker-driver-gateway-launch.test.ts +++ b/src/lib/onboard/docker-driver-gateway-launch.test.ts @@ -15,6 +15,7 @@ import { resolveDriftGatewayBin, shouldUseContainerizedGateway, } from "./docker-driver-gateway-launch"; +import { gatewayProcessCmdlineMatches } from "./gateway-process-identity"; function withTempBinaries( fn: (paths: { dir: string; gatewayBin: string; sandboxBin: string }) => T, @@ -177,6 +178,38 @@ describe("docker-driver-gateway-launch", () => { }); }); + it("binds the real no-argument host launch identity to its gateway target", () => { + withTempBinaries(({ dir, gatewayBin }) => { + const launch = buildDockerDriverGatewayLaunch({ + gatewayBin, + gatewayName: "nemoclaw-8081", + stateDir: dir, + platform: "linux", + env: {}, + hostGlibcVersion: "2.39", + requiredGlibcVersions: ["2.39"], + gatewayEnv: { + OPENSHELL_DRIVERS: "docker", + OPENSHELL_GRPC_ENDPOINT: "https://127.0.0.1:8081", + }, + }); + const cmdline = [launch.argv0, ...launch.args].filter(Boolean).join(" "); + + expect(launch.args).toEqual([]); + expect(launch.argv0).toBe("openshell-gateway[nemoclaw=nemoclaw-8081;port=8081]"); + expect( + gatewayProcessCmdlineMatches(cmdline, gatewayBin, { + expectedOpenShellGateway: { name: "nemoclaw-8081", port: 8081 }, + }), + ).toBe(true); + expect( + gatewayProcessCmdlineMatches(cmdline, gatewayBin, { + expectedOpenShellGateway: { name: "nemoclaw", port: 8080 }, + }), + ).toBe(false); + }); + }); + it("scrubs stale auth-disable env from direct host gateway launches", () => { withTempBinaries(({ dir, gatewayBin }) => { const launch = buildDockerDriverGatewayLaunch({ diff --git a/src/lib/onboard/docker-driver-gateway-launch.ts b/src/lib/onboard/docker-driver-gateway-launch.ts index 1564d1ec982..271875c8ee2 100644 --- a/src/lib/onboard/docker-driver-gateway-launch.ts +++ b/src/lib/onboard/docker-driver-gateway-launch.ts @@ -22,6 +22,7 @@ import { buildDockerDriverGatewayLocalTlsEnv, ensureDockerDriverGatewayLocalTlsBundle, } from "./docker-driver-gateway-local-tls"; +import { buildOwnedHostGatewayArgv0 } from "./gateway-process-identity"; export { compareDottedVersions, @@ -37,6 +38,7 @@ export { buildDockerDriverGatewayConfigToml }; export type DockerDriverGatewayLaunch = { command: string; args: string[]; + argv0?: string; env: NodeJS.ProcessEnv; mode: "host" | "container"; processGatewayBin: string | null; @@ -74,6 +76,7 @@ export function spawnDockerDriverGateway( ): ChildProcess { try { return spawn(launch.command, launch.args, { + argv0: launch.argv0, detached: true, stdio: ["ignore", logFd, logFd], env: launch.env, @@ -144,6 +147,7 @@ export function buildDockerDriverGatewayLaunch( return { command: options.gatewayBin, args: [], + argv0: buildOwnedHostGatewayArgv0(options.gatewayName) ?? undefined, env, mode: "host", processGatewayBin: options.gatewayBin, diff --git a/src/lib/onboard/docker-driver-gateway-process-identity.test.ts b/src/lib/onboard/docker-driver-gateway-process-identity.test.ts new file mode 100644 index 00000000000..1a4cf6418fb --- /dev/null +++ b/src/lib/onboard/docker-driver-gateway-process-identity.test.ts @@ -0,0 +1,32 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; + +import { getDockerDriverGatewayTargetIdentityDrift } from "./docker-driver-gateway-process-identity"; + +const normalizeGatewayExecutablePath = (value: string | null | undefined) => value ?? null; + +describe("Docker-driver gateway target identity", () => { + it("requires replacement of a legacy untagged gateway before reuse", () => { + expect( + getDockerDriverGatewayTargetIdentityDrift({ + gatewayBin: "/opt/openshell/openshell-gateway", + gatewayPort: 8081, + identity: "/opt/openshell/openshell-gateway", + normalizeGatewayExecutablePath, + })?.reason, + ).toContain("lacks target-bound cleanup identity for nemoclaw-8081 on port 8081"); + }); + + it("accepts the owned target-bound gateway launched after cutover", () => { + expect( + getDockerDriverGatewayTargetIdentityDrift({ + gatewayBin: "/opt/openshell/openshell-gateway", + gatewayPort: 8081, + identity: "openshell-gateway[nemoclaw=nemoclaw-8081;port=8081]", + normalizeGatewayExecutablePath, + }), + ).toBeNull(); + }); +}); diff --git a/src/lib/onboard/docker-driver-gateway-process-identity.ts b/src/lib/onboard/docker-driver-gateway-process-identity.ts new file mode 100644 index 00000000000..ca8ce3e2dc2 --- /dev/null +++ b/src/lib/onboard/docker-driver-gateway-process-identity.ts @@ -0,0 +1,50 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import fs from "node:fs"; + +import { resolveGatewayName } from "./gateway-binding"; +import { + gatewayProcessCmdlineMatches, + OPENSHELL_GATEWAY_PROCESS_NAMES, +} from "./gateway-process-identity"; + +type NormalizeGatewayExecutablePath = (value: string | null | undefined) => string | null; + +export function readDockerDriverGatewayProcessIdentity( + pid: number, + captureProcessArgs: (pid: number) => string, +): string { + const procCmdlinePath = `/proc/${pid}/cmdline`; + try { + if (fs.existsSync(procCmdlinePath)) { + const identity = fs.readFileSync(procCmdlinePath, "utf-8").replace(/\0/g, " ").trim(); + if (identity) return identity; + } + } catch { + // Fall through to ps on hosts without readable procfs. + } + return captureProcessArgs(pid); +} + +export function getDockerDriverGatewayTargetIdentityDrift(input: { + gatewayBin?: string | null; + gatewayPort: number; + identity: string; + normalizeGatewayExecutablePath: NormalizeGatewayExecutablePath; +}): { reason: string } | null { + const gatewayName = resolveGatewayName(input.gatewayPort); + const matchesTarget = gatewayProcessCmdlineMatches(input.identity, input.gatewayBin, { + expectedOpenShellGateway: { name: gatewayName, port: input.gatewayPort }, + processNames: OPENSHELL_GATEWAY_PROCESS_NAMES, + resolveExecutablePath: input.normalizeGatewayExecutablePath, + }); + if (matchesTarget) return null; + + // Legacy untagged launches cannot prove which gateway they own. Onboarding + // treats this drift as a mandatory cutover before reuse; targeted destroy + // remains fail-closed instead of guessing from a stale PID file. + return { + reason: `gateway process lacks target-bound cleanup identity for ${gatewayName} on port ${input.gatewayPort}`, + }; +} diff --git a/src/lib/onboard/docker-driver-gateway-runtime.test.ts b/src/lib/onboard/docker-driver-gateway-runtime.test.ts index 8d656385481..f5ae8357322 100644 --- a/src/lib/onboard/docker-driver-gateway-runtime.test.ts +++ b/src/lib/onboard/docker-driver-gateway-runtime.test.ts @@ -199,15 +199,18 @@ describe("docker-driver gateway runtime helpers", () => { NEMOCLAW_OPENSHELL_GATEWAY_STATE_DIR: stateDir, }, () => { + const processOutput = new Map([ + [`ps -p ${pid} -o args=`, "openshell-gateway[nemoclaw=nemoclaw-18080;port=18080]\n"], + [ + "ps -axo pid=,ppid=,command=", + [ + `${pid} 1 ${gatewayBin}`, + `${pid + 1} ${pid} /usr/local/bin/openshell-driver-vm --bind-socket /tmp/vm.sock`, + ].join("\n"), + ], + ]); const { helpers, runCapture } = makeHelpers({ - runCapture: vi.fn((args) => - args.join(" ") === "ps -axo pid=,ppid=,command=" - ? [ - `${pid} 1 ${gatewayBin}`, - `${pid + 1} ${pid} /usr/local/bin/openshell-driver-vm --bind-socket /tmp/vm.sock`, - ].join("\n") - : "", - ), + runCapture: vi.fn((args) => processOutput.get(args.join(" ")) ?? ""), }); const desiredEnv = helpers.getDockerDriverGatewayEnv(null, "darwin"); writeDockerDriverGatewayRuntimeMarkerForStateDir(stateDir, { @@ -357,7 +360,9 @@ describe("docker-driver gateway runtime helpers", () => { const identityGatewayBin = "/opt/openshell/openshell-gateway"; const replacementGatewayBin = "/opt/openshell/replaced/openshell-gateway"; const desiredEnv = { OPENSHELL_DRIVERS: "docker" }; - const { helpers } = makeHelpers(); + const { helpers } = makeHelpers({ + runCapture: vi.fn(() => "openshell-gateway[nemoclaw=nemoclaw-18080;port=18080]\n"), + }); const originalExistsSync = fs.existsSync.bind(fs); const originalReadFileSync = fs.readFileSync.bind(fs); const originalReadlinkSync = fs.readlinkSync.bind(fs); diff --git a/src/lib/onboard/docker-driver-gateway-runtime.ts b/src/lib/onboard/docker-driver-gateway-runtime.ts index 8daddc9eb6f..ece61d0c1b1 100644 --- a/src/lib/onboard/docker-driver-gateway-runtime.ts +++ b/src/lib/onboard/docker-driver-gateway-runtime.ts @@ -12,6 +12,10 @@ import { type DockerDriverGatewayPortListenerOptions, type DockerDriverGatewayPortListenerScan, } from "./docker-driver-gateway-port-listener"; +import { + getDockerDriverGatewayTargetIdentityDrift, + readDockerDriverGatewayProcessIdentity, +} from "./docker-driver-gateway-process-identity"; import * as dockerDriverGatewayRuntimeMarker from "./docker-driver-gateway-runtime-marker"; import * as gatewayBinding from "./gateway-binding"; import { @@ -337,6 +341,13 @@ export function createDockerDriverGatewayRuntimeHelpers(deps: DockerDriverGatewa gatewayBin?: string | null, platform: NodeJS.Platform = process.platform, ): DockerDriverGatewayRuntimeDrift | null { + const identityDrift = getDockerDriverGatewayTargetIdentityDrift({ + gatewayBin, + gatewayPort: currentGatewayPort(), + identity: readDockerDriverGatewayProcessIdentity(pid, captureProcessArgs), + normalizeGatewayExecutablePath, + }); + if (identityDrift) return identityDrift; if (platform === "darwin" && desiredEnv.OPENSHELL_DRIVERS === "docker") { const markerDrift = dockerDriverGatewayRuntimeMarker.getDockerDriverGatewayRuntimeMarkerDriftForStateDir( @@ -382,18 +393,7 @@ export function createDockerDriverGatewayRuntimeHelpers(deps: DockerDriverGatewa gatewayBin?: string | null, opts: { requireDockerDriverEnv?: boolean } = {}, ): boolean { - const procCmdlinePath = `/proc/${pid}/cmdline`; - let identity = ""; - try { - if (fs.existsSync(procCmdlinePath)) { - identity = fs.readFileSync(procCmdlinePath, "utf-8").replace(/\0/g, " ").trim(); - } - } catch { - identity = ""; - } - if (!identity) { - identity = captureProcessArgs(pid); - } + const identity = readDockerDriverGatewayProcessIdentity(pid, captureProcessArgs); if (!identity) return false; const matchesGatewayBinary = processIdentityMatchesGatewayBinary(identity, gatewayBin); if (!matchesGatewayBinary) return false; diff --git a/src/lib/onboard/gateway-process-identity.ts b/src/lib/onboard/gateway-process-identity.ts index d2c03262e63..4f221fe186c 100644 --- a/src/lib/onboard/gateway-process-identity.ts +++ b/src/lib/onboard/gateway-process-identity.ts @@ -3,6 +3,19 @@ import path from "node:path"; +import { + dockerCompatGatewayMatchesTarget, + gatewayTargetMatches, + type OpenShellGatewayProcessTarget, + openShellGatewayMatchesTarget, + ownedHostGatewayTarget, +} from "./gateway-process-target-identity"; + +export { + buildOwnedHostGatewayArgv0, + type OpenShellGatewayProcessTarget, +} from "./gateway-process-target-identity"; + export const HOST_GATEWAY_PROCESS_NAMES = new Set(["openshell-gateway", "openclaw-gateway"]); export const OPENSHELL_GATEWAY_PROCESS_NAMES = new Set(["openshell-gateway"]); @@ -28,6 +41,7 @@ export function gatewayProcessCmdlineMatches( cmdline: string, gatewayBin: string | null | undefined, opts: { + expectedOpenShellGateway?: OpenShellGatewayProcessTarget; processNames?: ReadonlySet; resolveExecutablePath?: ResolveExecutablePath; } = {}, @@ -38,13 +52,43 @@ export function gatewayProcessCmdlineMatches( const processNames = opts.processNames ?? HOST_GATEWAY_PROCESS_NAMES; const base = path.basename(argv0); - if (processNames.has(base)) return true; + const ownedTarget = ownedHostGatewayTarget(base); + if (ownedTarget && processNames.has("openshell-gateway")) { + return gatewayTargetMatches(ownedTarget, opts.expectedOpenShellGateway); + } + if (processNames.has(base)) { + if (processNames.has("openshell-gateway") && base === "openshell-gateway") { + return openShellGatewayMatchesTarget(tokens, opts.expectedOpenShellGateway, { + requireExpectedFlags: false, + }); + } + if (base === "openclaw-gateway") { + return openShellGatewayMatchesTarget(tokens, opts.expectedOpenShellGateway, { + requireExpectedFlags: true, + }); + } + return true; + } + if ( + processNames.has("openshell-gateway") && + base === "openshell" && + tokens[1] === "gateway" && + tokens[2] === "start" + ) { + return openShellGatewayMatchesTarget(tokens, opts.expectedOpenShellGateway, { + requireExpectedFlags: true, + }); + } if (typeof gatewayBin === "string" && gatewayBin.length > 0) { const normalize = opts.resolveExecutablePath ?? ((value: string) => path.resolve(value)); const actual = normalize(argv0); const expected = normalize(gatewayBin); - if (actual && expected && actual === expected) return true; + if (actual && expected && actual === expected) { + return openShellGatewayMatchesTarget(tokens, opts.expectedOpenShellGateway, { + requireExpectedFlags: false, + }); + } } // Docker compatibility mode: argv0 basename must be a known container @@ -54,7 +98,7 @@ export function gatewayProcessCmdlineMatches( DOCKER_DRIVER_GATEWAY_CONTAINER_RUNTIME_NAMES.has(base) && tokens.slice(1).includes(DOCKER_DRIVER_GATEWAY_COMPAT_MOUNT_PATH) ) { - return true; + return dockerCompatGatewayMatchesTarget(tokens, opts.expectedOpenShellGateway); } return false; @@ -63,6 +107,9 @@ export function gatewayProcessCmdlineMatches( export function hostGatewayCmdlineMatches( cmdline: string, gatewayBin: string | null | undefined, + expectedOpenShellGateway?: OpenShellGatewayProcessTarget, ): boolean { - return gatewayProcessCmdlineMatches(cmdline, gatewayBin); + return gatewayProcessCmdlineMatches(cmdline, gatewayBin, { + expectedOpenShellGateway, + }); } diff --git a/src/lib/onboard/gateway-process-target-identity.ts b/src/lib/onboard/gateway-process-target-identity.ts new file mode 100644 index 00000000000..e446c6be2b8 --- /dev/null +++ b/src/lib/onboard/gateway-process-target-identity.ts @@ -0,0 +1,103 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { + resolveGatewayCompatContainerName, + resolveGatewayName, + resolveGatewayPortFromName, +} from "./gateway-binding"; + +export interface OpenShellGatewayProcessTarget { + name?: string | null; + port?: number | string | null; +} + +const OWNED_HOST_GATEWAY_ARGV0_RE = + /^openshell-gateway\[nemoclaw=(nemoclaw(?:-\d+)?);port=(\d+)\]$/; + +export function buildOwnedHostGatewayArgv0(gatewayName: string | null | undefined): string | null { + if (!gatewayName) return null; + const port = resolveGatewayPortFromName(gatewayName); + if (port === null) return null; + return `openshell-gateway[nemoclaw=${gatewayName};port=${port}]`; +} + +export function ownedHostGatewayTarget(argv0: string): { name: string; port: number } | null { + const match = OWNED_HOST_GATEWAY_ARGV0_RE.exec(argv0); + if (!match) return null; + const name = match[1]; + const port = Number(match[2]); + if (resolveGatewayPortFromName(name) !== port || resolveGatewayName(port) !== name) return null; + return { name, port }; +} + +export function gatewayTargetMatches( + actual: { name: string; port: number }, + expected: OpenShellGatewayProcessTarget | undefined, +): boolean { + if (!expected || (!expected.name && (expected.port === undefined || expected.port === null))) { + return true; + } + if (expected.name && expected.name !== actual.name) return false; + if (expected.port !== undefined && expected.port !== null) { + return String(expected.port) === String(actual.port); + } + return true; +} + +function cliFlagValue(tokens: string[], names: string[]): string | null { + for (let index = 0; index < tokens.length; index += 1) { + const token = tokens[index]; + for (const name of names) { + if (token === name) return tokens[index + 1] ?? null; + if (token.startsWith(`${name}=`)) return token.slice(name.length + 1); + } + } + return null; +} + +export function openShellGatewayMatchesTarget( + tokens: string[], + target: OpenShellGatewayProcessTarget | undefined, + opts: { requireExpectedFlags: boolean }, +): boolean { + if (!target || (!target.name && (target.port === undefined || target.port === null))) { + return true; + } + + let matchedComparableFlag = false; + if (target.name) { + const actualName = cliFlagValue(tokens, ["--name"]); + if (actualName === null) { + if (opts.requireExpectedFlags) return false; + } else { + if (actualName !== target.name) return false; + matchedComparableFlag = true; + } + } + if (target.port !== undefined && target.port !== null) { + const actualPort = cliFlagValue(tokens, ["--port"]); + if (actualPort === null) { + if (opts.requireExpectedFlags) return false; + } else { + if (actualPort !== String(target.port)) return false; + matchedComparableFlag = true; + } + } + return matchedComparableFlag; +} + +export function dockerCompatGatewayMatchesTarget( + tokens: string[], + target: OpenShellGatewayProcessTarget | undefined, +): boolean { + if (!target || (!target.name && (target.port === undefined || target.port === null))) { + return true; + } + if (target.port === undefined || target.port === null) return false; + + const port = Number(target.port); + if (!Number.isInteger(port) || port < 1 || port > 65535) return false; + if (target.name && target.name !== resolveGatewayName(port)) return false; + return cliFlagValue(tokens, ["--name"]) === resolveGatewayCompatContainerName(port); +} diff --git a/src/lib/onboard/host-gateway-process-target.test.ts b/src/lib/onboard/host-gateway-process-target.test.ts new file mode 100644 index 00000000000..80d23009608 --- /dev/null +++ b/src/lib/onboard/host-gateway-process-target.test.ts @@ -0,0 +1,184 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { describe, expect, it, vi } from "vitest"; + +import { + HOST_GATEWAY_PGREP_PATTERN, + type HostGatewayProcessDeps, + type RunResult, + stopHostGatewayProcesses, +} from "./host-gateway-process"; + +const PGREP_KEY = `pgrep -f ${HOST_GATEWAY_PGREP_PATTERN}`; + +type RunResponse = (args: string[]) => RunResult; + +function ok(stdout = ""): RunResult { + return { status: 0, stdout, stderr: "" }; +} + +function notFound(): RunResult { + return { status: 1, stdout: "", stderr: "" }; +} + +function staticResponse(result: RunResult): RunResponse { + return () => result; +} + +function commandKey(command: string, args: string[]): string { + return `${command} ${args.join(" ")}`; +} + +function makeRun(responses: Map): HostGatewayProcessDeps["run"] { + const fallback = staticResponse(notFound()); + return (command, args) => (responses.get(commandKey(command, args)) ?? fallback)(args); +} + +function psResponses( + pid: number, + opts: { + cmdline: string; + exited: Set; + }, +): [string, RunResponse][] { + return [ + [`ps -p ${pid} -o pid=`, () => (opts.exited.has(pid) ? notFound() : ok(`${pid}\n`))], + [`ps -p ${pid} -o user=`, staticResponse(ok("tester\n"))], + [`ps -p ${pid} -o args=`, staticResponse(ok(opts.cmdline))], + ]; +} + +function stopTargetedPid(pid: number, cmdline: string, targeted = true) { + const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-host-gateway-target-")); + const pidFile = path.join(stateDir, "openshell-gateway.pid"); + fs.writeFileSync(pidFile, `${pid}\n`); + const exited = new Set(); + const responses = new Map([ + [PGREP_KEY, staticResponse(notFound())], + ...psResponses(pid, { cmdline, exited }), + ]); + const kill = vi.fn((killedPid, signal) => { + switch (signal) { + case "SIGTERM": + exited.add(killedPid); + break; + } + return true; + }); + + const result = stopHostGatewayProcesses( + { + run: makeRun(responses), + kill, + env: { USER: "tester" }, + commandExists: () => true, + log: vi.fn(), + }, + { + ...(targeted ? { openShellGatewayName: "nemoclaw-8081", openShellGatewayPort: 8081 } : {}), + stateDir, + }, + ); + + return { kill, pidFile, result }; +} + +describe("stopHostGatewayProcesses target filtering", () => { + it("accepts a matching OpenShell CLI gateway-start process for the cleanup target", () => { + const { kill, pidFile, result } = stopTargetedPid( + 9999553, + "/Users/test/.local/bin/openshell gateway start --name nemoclaw-8081 --port 8081\n", + ); + + expect(result.stopped).toEqual([9999553]); + expect(kill).toHaveBeenCalledWith(9999553, "SIGTERM"); + expect(fs.existsSync(pidFile)).toBe(false); + }); + + it("skips a stale PID-file OpenShell CLI gateway-start process for another gateway", () => { + const { kill, pidFile, result } = stopTargetedPid( + 9999554, + "/Users/test/.local/bin/openshell gateway start --name other --port 9999\n", + ); + + expect(result.skippedNonMatchingPids).toEqual([9999554]); + expect(kill).not.toHaveBeenCalled(); + expect(fs.existsSync(pidFile)).toBe(false); + }); + + it("skips a bare openclaw-gateway process when cleanup supplies a target", () => { + const { kill, pidFile, result } = stopTargetedPid(9999560, "openclaw-gateway\n"); + + expect(result.skippedNonMatchingPids).toEqual([9999560]); + expect(kill).not.toHaveBeenCalled(); + expect(fs.existsSync(pidFile)).toBe(false); + }); + + it("keeps legacy openclaw-gateway matching when cleanup has no target", () => { + const { kill, pidFile, result } = stopTargetedPid(9999561, "openclaw-gateway\n", false); + + expect(result.stopped).toEqual([9999561]); + expect(kill).toHaveBeenCalledWith(9999561, "SIGTERM"); + expect(fs.existsSync(pidFile)).toBe(false); + }); + + it("accepts the owned no-argument host launch for the cleanup target", () => { + const { kill, pidFile, result } = stopTargetedPid( + 9999555, + "openshell-gateway[nemoclaw=nemoclaw-8081;port=8081]\n", + ); + + expect(result.stopped).toEqual([9999555]); + expect(kill).toHaveBeenCalledWith(9999555, "SIGTERM"); + expect(fs.existsSync(pidFile)).toBe(false); + }); + + it("skips an untagged legacy no-argument launch until onboarding migrates it", () => { + const { kill, pidFile, result } = stopTargetedPid( + 9999559, + "/opt/openshell/openshell-gateway\n", + ); + + expect(result.skippedNonMatchingPids).toEqual([9999559]); + expect(kill).not.toHaveBeenCalled(); + expect(fs.existsSync(pidFile)).toBe(false); + }); + + it("skips an owned no-argument host launch for another port", () => { + const { kill, pidFile, result } = stopTargetedPid( + 9999556, + "openshell-gateway[nemoclaw=nemoclaw;port=8080]\n", + ); + + expect(result.skippedNonMatchingPids).toEqual([9999556]); + expect(kill).not.toHaveBeenCalled(); + expect(fs.existsSync(pidFile)).toBe(false); + }); + + it("accepts a Docker compatibility gateway with the cleanup target container name", () => { + const { kill, pidFile, result } = stopTargetedPid( + 9999557, + "docker run --rm --name nemoclaw-openshell-gateway-8081 ubuntu:24.04 /opt/nemoclaw/openshell-gateway\n", + ); + + expect(result.stopped).toEqual([9999557]); + expect(kill).toHaveBeenCalledWith(9999557, "SIGTERM"); + expect(fs.existsSync(pidFile)).toBe(false); + }); + + it("skips a stale PID-file Docker compatibility gateway for another port", () => { + const { kill, pidFile, result } = stopTargetedPid( + 9999558, + "docker run --rm --name nemoclaw-openshell-gateway ubuntu:24.04 /opt/nemoclaw/openshell-gateway\n", + ); + + expect(result.skippedNonMatchingPids).toEqual([9999558]); + expect(kill).not.toHaveBeenCalled(); + expect(fs.existsSync(pidFile)).toBe(false); + }); +}); diff --git a/src/lib/onboard/host-gateway-process.test.ts b/src/lib/onboard/host-gateway-process.test.ts index 1fb74cce07b..29c87ca9ffd 100644 --- a/src/lib/onboard/host-gateway-process.test.ts +++ b/src/lib/onboard/host-gateway-process.test.ts @@ -8,11 +8,14 @@ import path from "node:path"; import { describe, expect, it, vi } from "vitest"; import { + HOST_GATEWAY_PGREP_PATTERN, type HostGatewayProcessDeps, type RunResult, stopHostGatewayProcesses, } from "./host-gateway-process"; +const PGREP_KEY = `pgrep -f ${HOST_GATEWAY_PGREP_PATTERN}`; + interface RunArgs { args: string[]; command: string; @@ -67,12 +70,12 @@ describe("stopHostGatewayProcesses", () => { it("uses pgrep fallback when the Docker-driver gateway PID file is missing", () => { const exited = new Set(); const responses = new Map RunResult)>([ - ["pgrep -f ^(/[^ ]*/)?openshell-gateway( |$)", ok("9999887\n")], + [PGREP_KEY, ok("9999887\n")], ...psResponses(9999887, { exited }), ]); const { run } = makeRun(responses); - const kill = vi.fn((pid, signal) => { - if (signal === "SIGTERM") exited.add(pid); + const kill = vi.fn((pid) => { + exited.add(pid); return true; }); const log = vi.fn(); @@ -92,7 +95,7 @@ describe("stopHostGatewayProcesses", () => { const signals: Array = []; let pidChecks = 0; const responses = new Map RunResult)>([ - ["pgrep -f ^(/[^ ]*/)?openshell-gateway( |$)", ok(`${pid}\n`)], + [PGREP_KEY, ok(`${pid}\n`)], [`ps -p ${pid} -o user=`, ok("tester\n")], [`ps -p ${pid} -o args=`, ok("/home/test/.local/bin/openshell-gateway --port 8080\n")], [ @@ -129,7 +132,7 @@ describe("stopHostGatewayProcesses", () => { fs.writeFileSync(pidFile, "9999551\n"); const exited = new Set(); const responses = new Map RunResult)>([ - ["pgrep -f ^(/[^ ]*/)?openshell-gateway( |$)", notFound()], + [PGREP_KEY, notFound()], ...psResponses(9999551, { cmdline: "/usr/bin/docker run --rm --name nemoclaw-openshell-gateway --network host /opt/nemoclaw/openshell-gateway\n", @@ -138,7 +141,11 @@ describe("stopHostGatewayProcesses", () => { ]); const { run } = makeRun(responses); const kill = vi.fn((pid, signal) => { - if (signal === "SIGTERM") exited.add(pid); + switch (signal) { + case "SIGTERM": + exited.add(pid); + break; + } return true; }); @@ -152,12 +159,44 @@ describe("stopHostGatewayProcesses", () => { expect(fs.existsSync(pidFile)).toBe(false); }); + it("accepts the OpenShell CLI gateway-start process recorded in the PID file", () => { + const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-host-gateway-")); + const pidFile = path.join(stateDir, "openshell-gateway.pid"); + fs.writeFileSync(pidFile, "9999552\n"); + const exited = new Set(); + const responses = new Map RunResult)>([ + [PGREP_KEY, notFound()], + ...psResponses(9999552, { + cmdline: "/Users/test/.local/bin/openshell gateway start --name nemoclaw --port 8080\n", + exited, + }), + ]); + const { run } = makeRun(responses); + const kill = vi.fn((pid, signal) => { + switch (signal) { + case "SIGTERM": + exited.add(pid); + break; + } + return true; + }); + + const result = stopHostGatewayProcesses( + { run, kill, env: { USER: "tester" }, commandExists: () => true, log: vi.fn() }, + { stateDir }, + ); + + expect(result.stopped).toEqual([9999552]); + expect(kill).toHaveBeenCalledWith(9999552, "SIGTERM"); + expect(fs.existsSync(pidFile)).toBe(false); + }); + it("rejects a PID whose argv0 is not docker even if it touches the mount path", () => { const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-host-gateway-")); const pidFile = path.join(stateDir, "openshell-gateway.pid"); fs.writeFileSync(pidFile, "9999662\n"); const responses = new Map RunResult)>([ - ["pgrep -f ^(/[^ ]*/)?openshell-gateway( |$)", notFound()], + [PGREP_KEY, notFound()], ...psResponses(9999662, { cmdline: "/usr/bin/vim /opt/nemoclaw/openshell-gateway\n", exited: new Set(), @@ -204,7 +243,7 @@ describe("stopHostGatewayProcesses", () => { it("ignores unrelated command lines that merely mention openshell-gateway", () => { const exited = new Set(); const responses = new Map RunResult)>([ - ["pgrep -f ^(/[^ ]*/)?openshell-gateway( |$)", ok("9999111\n9999222\n")], + [PGREP_KEY, ok("9999111\n9999222\n")], ...psResponses(9999111, { exited }), ...psResponses(9999222, { cmdline: "node /home/test/.npm-global/bin/codex issue text mentions openshell-gateway\n", @@ -229,7 +268,7 @@ describe("stopHostGatewayProcesses", () => { it("prints sudo remediation when a privileged host gateway cannot be killed", () => { const responses = new Map RunResult)>([ - ["pgrep -f ^(/[^ ]*/)?openshell-gateway( |$)", ok("9999042\n")], + [PGREP_KEY, ok("9999042\n")], ...psResponses(9999042, { exited: new Set(), owner: "root" }), ]); const { run } = makeRun(responses); @@ -299,7 +338,7 @@ describe("stopHostGatewayProcesses", () => { fs.writeFileSync(pidFile, "9999123\n"); const exited = new Set(); const responses = new Map RunResult)>([ - ["pgrep -f ^(/[^ ]*/)?openshell-gateway( |$)", ok("9999456\n")], + [PGREP_KEY, ok("9999456\n")], ...(psResponses(9999123, { exited: new Set() }).map(([key, value]) => key === "ps -p 9999123 -o pid=" ? [key, notFound()] : [key, value], ) as [string, RunResult | ((args: string[]) => RunResult)][]), diff --git a/src/lib/onboard/host-gateway-process.ts b/src/lib/onboard/host-gateway-process.ts index d0bb226d6be..af72f53fdde 100644 --- a/src/lib/onboard/host-gateway-process.ts +++ b/src/lib/onboard/host-gateway-process.ts @@ -8,7 +8,10 @@ import path from "node:path"; import { waitUntil } from "../core/wait"; import { clearDockerDriverGatewayRuntimeMarker } from "./docker-driver-gateway-runtime-marker"; -import { hostGatewayCmdlineMatches as sharedHostGatewayCmdlineMatches } from "./gateway-process-identity"; +import { + hostGatewayCmdlineMatches as sharedHostGatewayCmdlineMatches, + type OpenShellGatewayProcessTarget, +} from "./gateway-process-identity"; export interface RunResult { status: number | null; @@ -31,9 +34,13 @@ export interface StopHostGatewayOptions { gatewayBin?: string | null; killWaitMs?: number; logNoProcesses?: boolean; + openShellGatewayName?: string; + openShellGatewayPort?: number | string; pids?: Iterable; pidFile?: string; pollIntervalMs?: number; + /** Keep PID/runtime evidence when a PID-file process does not match the cleanup target. */ + preserveRuntimeFilesOnNonMatching?: boolean; stateDir?: string; termWaitMs?: number; /** Whether to read and act on the resolved pid file. */ @@ -55,7 +62,8 @@ export interface StopHostGatewayResult { // path could match unrelated commands. The compat parent is rediscovered via // the PID file written at launch time. /** Anchored pgrep pattern for direct host openshell-gateway processes. */ -export const HOST_GATEWAY_PGREP_PATTERN = "^(/[^ ]*/)?openshell-gateway( |$)"; +export const HOST_GATEWAY_PGREP_PATTERN = + "^(/[^ ]*/)?openshell-gateway(\\[nemoclaw=nemoclaw(-[0-9]+)?;port=[0-9]+\\]| |$)"; const DEFAULT_TERM_WAIT_MS = 1000; const DEFAULT_KILL_WAIT_MS = 1000; const DEFAULT_POLL_INTERVAL_MS = 50; @@ -161,7 +169,13 @@ function pidOwner(pid: number, deps: HostGatewayProcessDeps): string | null { return result.stdout.trim() || null; } -export const hostGatewayCmdlineMatches = sharedHostGatewayCmdlineMatches; +export function hostGatewayCmdlineMatches( + cmdline: string, + gatewayBin: string | null | undefined, + expectedOpenShellGateway?: OpenShellGatewayProcessTarget, +): boolean { + return sharedHostGatewayCmdlineMatches(cmdline, gatewayBin, expectedOpenShellGateway); +} function waitForExit( pid: number, @@ -287,6 +301,13 @@ export function stopHostGatewayProcesses( pollIntervalMs: options.pollIntervalMs ?? DEFAULT_POLL_INTERVAL_MS, termWaitMs: options.termWaitMs ?? DEFAULT_TERM_WAIT_MS, }; + const expectedOpenShellGateway = + options.openShellGatewayName || options.openShellGatewayPort !== undefined + ? { + name: options.openShellGatewayName, + port: options.openShellGatewayPort, + } + : undefined; let clearedRuntimeFiles = false; for (const [pid, sources] of candidates) { if (!pidExists(pid, deps)) { @@ -297,9 +318,20 @@ export function stopHostGatewayProcesses( } continue; } - if (!hostGatewayCmdlineMatches(processArgs(pid, deps), options.gatewayBin)) { + if ( + !hostGatewayCmdlineMatches( + processArgs(pid, deps), + options.gatewayBin, + expectedOpenShellGateway, + ) + ) { result.skippedNonMatchingPids.push(pid); - if (clearRuntimeState && sources.has("pid-file") && !clearedRuntimeFiles) { + if ( + clearRuntimeState && + !options.preserveRuntimeFilesOnNonMatching && + sources.has("pid-file") && + !clearedRuntimeFiles + ) { clearRuntimeFiles(pidFile, stateDir); clearedRuntimeFiles = true; } diff --git a/src/lib/onboard/machine/handlers/provider-inference.test.ts b/src/lib/onboard/machine/handlers/provider-inference.test.ts index 16eb98a5a84..5536a97b535 100644 --- a/src/lib/onboard/machine/handlers/provider-inference.test.ts +++ b/src/lib/onboard/machine/handlers/provider-inference.test.ts @@ -771,6 +771,7 @@ describe("handleProviderInferenceState", () => { it("revalidates recovered identity before reusing a gateway credential on messaging resume", async () => { const session = createSession({ + sandboxName: "my-assistant", provider: "compatible-endpoint", model: "nvidia/nemotron", endpointUrl: "https://integrate.api.nvidia.com/v1", @@ -823,10 +824,21 @@ describe("handleProviderInferenceState", () => { await handleProviderInferenceState({ ...baseOptions(deps, session), resume: true, + authoritativeResumeConfig: true, sandboxName: "my-assistant", }); expect(setupNim).toHaveBeenCalledOnce(); + expect(setupNim).toHaveBeenCalledWith( + { type: "nvidia" }, + "my-assistant", + null, + true, + "nemoclaw", + expect.any(Function), + expect.any(Function), + session.sessionId, + ); expect(calls.setupInference).toHaveBeenCalledWith( "my-assistant", "nvidia/nemotron", diff --git a/test/cli/destroy-gateway-cleanup.test.ts b/test/cli/destroy-gateway-cleanup.test.ts index 52efbe29453..f923c24aa93 100644 --- a/test/cli/destroy-gateway-cleanup.test.ts +++ b/test/cli/destroy-gateway-cleanup.test.ts @@ -9,76 +9,78 @@ import { describe, expect, it } from "vitest"; import { runWithEnv, testTimeoutOptions } from "./helpers"; describe("CLI dispatch", () => { - it("preserves the gateway runtime by default when the last sandbox is destroyed (#2166)", () => { - const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-cli-destroy-last-")); - const localBin = path.join(home, "bin"); - const registryDir = path.join(home, ".nemoclaw"); - const openshellLog = path.join(home, "openshell.log"); - const bashLog = path.join(home, "docker.log"); - fs.mkdirSync(localBin, { recursive: true }); - fs.mkdirSync(registryDir, { recursive: true }); - fs.writeFileSync( - path.join(registryDir, "sandboxes.json"), - JSON.stringify({ - sandboxes: { - alpha: { - name: "alpha", - model: "test-model", - provider: "nvidia-prod", - gpuEnabled: false, - policies: [], + it( + "uses the platform gateway default when the last sandbox is destroyed (#2166, #4662)", + testTimeoutOptions(30_000), + () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-cli-destroy-last-")); + const localBin = path.join(home, "bin"); + const registryDir = path.join(home, ".nemoclaw"); + const openshellLog = path.join(home, "openshell.log"); + const bashLog = path.join(home, "docker.log"); + fs.mkdirSync(localBin, { recursive: true }); + fs.mkdirSync(registryDir, { recursive: true }); + fs.writeFileSync( + path.join(registryDir, "sandboxes.json"), + JSON.stringify({ + sandboxes: { + alpha: { + name: "alpha", + model: "test-model", + provider: "nvidia-prod", + gpuEnabled: false, + policies: [], + }, }, - }, - defaultSandbox: "alpha", - }), - { mode: 0o600 }, - ); - fs.writeFileSync( - path.join(localBin, "openshell"), - [ - "#!/bin/sh", - `log_file=${JSON.stringify(openshellLog)}`, - 'if [ "$1" = "sandbox" ] && [ "$2" = "list" ]; then', - ' printf "NAME STATUS\\n" >> "$log_file"', - " exit 0", - "fi", - 'printf \'%s\\n\' "$*" >> "$log_file"', - "exit 0", - ].join("\n"), - { mode: 0o755 }, - ); - fs.writeFileSync( - path.join(localBin, "docker"), - [ - "#!/bin/sh", - `log_file=${JSON.stringify(bashLog)}`, - 'printf \'%s\\n\' "$*" >> "$log_file"', - "exit 0", - ].join("\n"), - { mode: 0o755 }, - ); + defaultSandbox: "alpha", + }), + { mode: 0o600 }, + ); + fs.writeFileSync( + path.join(localBin, "openshell"), + [ + "#!/bin/sh", + `log_file=${JSON.stringify(openshellLog)}`, + 'if [ "$1" = "sandbox" ] && [ "$2" = "list" ]; then', + ' printf "NAME STATUS\\n" >> "$log_file"', + " exit 0", + "fi", + 'printf \'%s\\n\' "$*" >> "$log_file"', + "exit 0", + ].join("\n"), + { mode: 0o755 }, + ); + fs.writeFileSync( + path.join(localBin, "docker"), + [ + "#!/bin/sh", + `log_file=${JSON.stringify(bashLog)}`, + 'printf \'%s\\n\' "$*" >> "$log_file"', + "exit 0", + ].join("\n"), + { mode: 0o755 }, + ); - const r = runWithEnv("alpha destroy -y", { - HOME: home, - PATH: `${localBin}:${process.env.PATH || ""}`, - }); + const r = runWithEnv("alpha destroy -y", { + HOME: home, + PATH: `${localBin}:${process.env.PATH || ""}`, + }); - expect(r.code).toBe(0); - const openshellOutput = fs.readFileSync(openshellLog, "utf8"); - expect(openshellOutput).toContain("sandbox delete alpha"); - expect(openshellOutput).toContain("NAME STATUS"); - // Gateway preservation is now the default. `--yes` confirms only the - // sandbox; the shared NemoClaw gateway must stay up so the next - // `nemoclaw onboard` reuses it. - expect(openshellOutput).not.toContain("forward stop 18789"); - expect(openshellOutput).not.toContain("gateway destroy -g nemoclaw"); - expect(openshellOutput).not.toContain("gateway remove nemoclaw"); - expect(fs.readFileSync(bashLog, "utf8")).not.toContain("volume ls -q --filter"); - // The preserved-gateway hint must recommend the real subcommand - // (`gateway remove`), never the nonexistent `gateway destroy` (#6569). - expect(r.out).toContain("openshell gateway remove nemoclaw"); - expect(r.out).not.toContain("gateway destroy"); - }); + expect(r.code).toBe(0); + const openshellOutput = fs.readFileSync(openshellLog, "utf8"); + const dockerOutput = fs.readFileSync(bashLog, "utf8"); + const shouldCleanupGateway = process.platform === "darwin"; + expect(openshellOutput).toContain("sandbox delete alpha"); + expect(openshellOutput).toContain("NAME STATUS"); + expect(openshellOutput.includes("forward stop 18789")).toBe(shouldCleanupGateway); + expect(openshellOutput.includes("gateway remove nemoclaw")).toBe(shouldCleanupGateway); + expect(dockerOutput.includes("volume ls -q --filter name=openshell-cluster-nemoclaw")).toBe( + shouldCleanupGateway, + ); + expect(r.out.includes("openshell gateway remove nemoclaw")).toBe(!shouldCleanupGateway); + expect(r.out).not.toContain("gateway destroy"); + }, + ); it( "falls back to legacy gateway destroy and still cleans volumes when remove fails (#6569)", @@ -675,79 +677,89 @@ describe("CLI dispatch", () => { expect(fs.readFileSync(openshellLog, "utf8")).not.toContain("gateway remove nemoclaw"); }); - it("treats an already-missing sandbox as destroyed and clears the stale registry entry", () => { - const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-cli-destroy-missing-")); - const localBin = path.join(home, "bin"); - const registryDir = path.join(home, ".nemoclaw"); - const openshellLog = path.join(home, "openshell.log"); - const bashLog = path.join(home, "docker.log"); - fs.mkdirSync(localBin, { recursive: true }); - fs.mkdirSync(registryDir, { recursive: true }); - fs.writeFileSync( - path.join(registryDir, "sandboxes.json"), - JSON.stringify({ - sandboxes: { - alpha: { - name: "alpha", - model: "test-model", - provider: "nvidia-prod", - gpuEnabled: false, - policies: [], + it( + "treats an already-missing sandbox as destroyed using the platform gateway default (#4662)", + testTimeoutOptions(30_000), + () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-cli-destroy-missing-")); + const localBin = path.join(home, "bin"); + const registryDir = path.join(home, ".nemoclaw"); + const openshellLog = path.join(home, "openshell.log"); + const bashLog = path.join(home, "docker.log"); + fs.mkdirSync(localBin, { recursive: true }); + fs.mkdirSync(registryDir, { recursive: true }); + fs.writeFileSync( + path.join(registryDir, "sandboxes.json"), + JSON.stringify({ + sandboxes: { + alpha: { + name: "alpha", + model: "test-model", + provider: "nvidia-prod", + gpuEnabled: false, + policies: [], + }, }, - }, - defaultSandbox: "alpha", - }), - { mode: 0o600 }, - ); - fs.writeFileSync( - path.join(localBin, "openshell"), - [ - "#!/bin/sh", - `log_file=${JSON.stringify(openshellLog)}`, - 'if [ "$1" = "sandbox" ] && [ "$2" = "delete" ]; then', - ' printf \'%s\\n\' "$*" >> "$log_file"', - ' echo "Error: status: Not Found, message: \\"sandbox not found\\"" >&2', - " exit 1", - "fi", - 'if [ "$1" = "sandbox" ] && [ "$2" = "list" ]; then', - ' printf "NAME STATUS\\n" >> "$log_file"', - ' printf "NAME STATUS\\n"', - " exit 0", - "fi", - 'printf \'%s\\n\' "$*" >> "$log_file"', - "exit 0", - ].join("\n"), - { mode: 0o755 }, - ); - fs.writeFileSync( - path.join(localBin, "docker"), - [ - "#!/bin/sh", - `log_file=${JSON.stringify(bashLog)}`, - 'printf \'%s\\n\' "$*" >> "$log_file"', - "exit 0", - ].join("\n"), - { mode: 0o755 }, - ); + defaultSandbox: "alpha", + }), + { mode: 0o600 }, + ); + fs.writeFileSync( + path.join(localBin, "openshell"), + [ + "#!/bin/sh", + `log_file=${JSON.stringify(openshellLog)}`, + 'if [ "$1" = "sandbox" ] && [ "$2" = "delete" ]; then', + ' printf \'%s\\n\' "$*" >> "$log_file"', + ' echo "Error: status: Not Found, message: \\"sandbox not found\\"" >&2', + " exit 1", + "fi", + 'if [ "$1" = "sandbox" ] && [ "$2" = "list" ]; then', + ' printf "NAME STATUS\\n" >> "$log_file"', + ' printf "NAME STATUS\\n"', + " exit 0", + "fi", + 'printf \'%s\\n\' "$*" >> "$log_file"', + "exit 0", + ].join("\n"), + { mode: 0o755 }, + ); + fs.writeFileSync( + path.join(localBin, "docker"), + [ + "#!/bin/sh", + `log_file=${JSON.stringify(bashLog)}`, + 'printf \'%s\\n\' "$*" >> "$log_file"', + "exit 0", + ].join("\n"), + { mode: 0o755 }, + ); - const r = runWithEnv("alpha destroy --yes", { - HOME: home, - PATH: `${localBin}:${process.env.PATH || ""}`, - }); + const r = runWithEnv("alpha destroy --yes", { + HOME: home, + PATH: `${localBin}:${process.env.PATH || ""}`, + }); - expect(r.code).toBe(0); - expect(r.out).toContain("already absent from the live gateway"); - expect(r.out).toContain("Sandbox 'alpha' destroyed"); + expect(r.code).toBe(0); + expect(r.out).toContain("already absent from the live gateway"); + expect(r.out).toContain("Sandbox 'alpha' destroyed"); - const registryAfter = JSON.parse( - fs.readFileSync(path.join(registryDir, "sandboxes.json"), "utf8"), - ); - expect(registryAfter.sandboxes.alpha).toBeFalsy(); - expect(fs.readFileSync(openshellLog, "utf8")).toContain("sandbox delete alpha"); - expect(fs.readFileSync(openshellLog, "utf8")).not.toContain("forward stop 18789"); - expect(fs.readFileSync(openshellLog, "utf8")).not.toContain("gateway destroy -g nemoclaw"); - expect(fs.readFileSync(bashLog, "utf8")).not.toContain("volume ls -q --filter"); - }); + const registryAfter = JSON.parse( + fs.readFileSync(path.join(registryDir, "sandboxes.json"), "utf8"), + ); + expect(registryAfter.sandboxes.alpha).toBeFalsy(); + expect(fs.readFileSync(openshellLog, "utf8")).toContain("sandbox delete alpha"); + const openshellOutput = fs.readFileSync(openshellLog, "utf8"); + const dockerOutput = fs.readFileSync(bashLog, "utf8"); + const shouldCleanupGateway = process.platform === "darwin"; + expect(openshellOutput.includes("forward stop 18789")).toBe(shouldCleanupGateway); + expect(openshellOutput.includes("gateway remove nemoclaw")).toBe(shouldCleanupGateway); + expect(dockerOutput.includes("volume ls -q --filter name=openshell-cluster-nemoclaw")).toBe( + shouldCleanupGateway, + ); + expect(openshellOutput).not.toContain("gateway destroy -g nemoclaw"); + }, + ); it("deletes messaging providers when destroying a sandbox", () => { const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-cli-destroy-providers-")); diff --git a/test/cli/doctor-gateway-token.test.ts b/test/cli/doctor-gateway-token.test.ts index fc975da3f4f..26a539286c8 100644 --- a/test/cli/doctor-gateway-token.test.ts +++ b/test/cli/doctor-gateway-token.test.ts @@ -299,7 +299,9 @@ describe("CLI dispatch", () => { expect(report.status).toBe("ok"); const calls = fs.readFileSync(hostCalls, "utf8"); - expect(calls).toContain("pgrep:-f ^(/[^ ]*/)?openshell-gateway( |$)"); + expect(calls).toContain( + "pgrep:-f ^(/[^ ]*/)?openshell-gateway(\\[nemoclaw=nemoclaw(-[0-9]+)?;port=[0-9]+\\]| |$)", + ); expect(calls).not.toContain("pgrep:-af openshell-gateway"); expect(calls).not.toContain("docker:port"); }, diff --git a/test/e2e/live/sandbox-operations.test.ts b/test/e2e/live/sandbox-operations.test.ts index 446228c5fbd..523f58ebe7a 100644 --- a/test/e2e/live/sandbox-operations.test.ts +++ b/test/e2e/live/sandbox-operations.test.ts @@ -34,6 +34,7 @@ const SANDBOX_A = "e2e-sbx-a"; const SANDBOX_B = "e2e-sbx-b"; const REGISTRY_FILE = path.join(process.env.HOME ?? os.homedir(), ".nemoclaw", "sandboxes.json"); const GATEWAY_CONTAINER = "openshell-cluster-nemoclaw"; +const GATEWAY_PORT = process.env.NEMOCLAW_GATEWAY_PORT ?? "8080"; async function onboardSandbox( host: HostCliClient, @@ -503,13 +504,20 @@ async function assertDestroyRemovesSandbox( host: HostCliClient, sandbox: SandboxClient, sandboxName: string, + options: { cleanupGateway?: boolean } = {}, ): Promise { - const destroy = await host.nemoclaw([sandboxName, "destroy", "--yes"], { + const destroyArgs = [ + sandboxName, + "destroy", + "--yes", + ...(options.cleanupGateway ? ["--cleanup-gateway"] : []), + ]; + const destroy = await host.nemoclaw(destroyArgs, { artifactName: `tc-sbx-05-destroy-${sandboxName}`, env: buildAvailabilityProbeEnv(), timeoutMs: 15 * 60_000, }); - expectExitZero(destroy, `nemoclaw ${sandboxName} destroy --yes`); + expectExitZero(destroy, `nemoclaw ${destroyArgs.join(" ")}`); const list = await host.nemoclaw(["list"], { artifactName: `tc-sbx-05-nemoclaw-list-after-destroy-${sandboxName}`, @@ -526,6 +534,29 @@ async function assertDestroyRemovesSandbox( expect(outputContainsSandbox(openshellList, sandboxName), resultText(openshellList)).toBe(false); } +async function expectHostPortFree( + host: HostCliClient, + port: string, + artifactName: string, + timeoutMs = 90_000, +): Promise { + const probe = await host.command( + "node", + [ + "-e", + 'const net=require("node:net"); const port=Number(process.argv[1]); const deadline=Date.now()+Number(process.argv[2]); const attempt=()=>{ const server=net.createServer(); server.once("error", error => { if (Date.now() >= deadline) { console.error(error.code || "bind failed"); process.exit(1); } setTimeout(attempt, 2000); }); server.listen(port, "127.0.0.1", () => server.close(error => { if (error) { console.error(error.message); process.exit(1); } console.log("available"); })); }; attempt();', + port, + String(timeoutMs), + ], + { + artifactName, + env: buildAvailabilityProbeEnv(), + timeoutMs: timeoutMs + 30_000, + }, + ); + expectExitZero(probe, `gateway port ${port} remained occupied after final destroy`); +} + type GatewayRecoveryOutcome = | "recovered-before-status" | "recovered-by-status" @@ -609,6 +640,7 @@ test( "TC-SBX-09 tmux and PTY lifecycle work inside sandbox", "TC-SBX-10 two sandboxes list with model/provider metadata", "TC-SBX-11 sandboxes cannot reach each other by hostname", + "TC-SBX-12 destroying the non-final sandbox preserves the survivor and final destroy releases the gateway port through the macOS default or explicit non-macOS cleanup", ], }); @@ -643,12 +675,22 @@ test( await assertNetworkIsolation(sandbox, SANDBOX_A, SANDBOX_B, "tc-sbx-11-a-cannot-reach-b"); await assertNetworkIsolation(sandbox, SANDBOX_B, SANDBOX_A, "tc-sbx-11-b-cannot-reach-a"); await assertDestroyRemovesSandbox(host, sandbox, SANDBOX_B); + await expectListed(host, SANDBOX_A, "tc-sbx-12-survivor-listed-after-destroy-b"); + await assertAgentCanAnswer(host, SANDBOX_A, "tc-sbx-12-survivor-agent-after-destroy-b"); const gatewayRecovery = await assertGatewayRecovery(host, SANDBOX_A); + const finalDestroyCleanupMode = + process.platform === "darwin" ? "macos-default" : "explicit-non-macos"; + await assertDestroyRemovesSandbox(host, sandbox, SANDBOX_A, { + cleanupGateway: finalDestroyCleanupMode === "explicit-non-macos", + }); + await expectHostPortFree(host, GATEWAY_PORT, "tc-sbx-12-final-destroy-gateway-port-free"); await artifacts.target.complete({ id: "sandbox-operations", status: "passed", + finalDestroyCleanupMode, + finalGatewayPortReleased: true, gatewayRecovery, legacySource: "test/e2e/test-sandbox-operations.sh", }); diff --git a/test/e2e/mock-parity.json b/test/e2e/mock-parity.json index b63d32ed77d..a49eb7ad179 100644 --- a/test/e2e/mock-parity.json +++ b/test/e2e/mock-parity.json @@ -21,6 +21,10 @@ "test/e2e/support/openclaw-tui-run-classification.test.ts" ] }, + { + "live": "test/e2e/live/sandbox-operations.test.ts", + "liveOnlyReason": "The final-destroy gateway release contract needs two real Docker/OpenShell sandboxes plus host 127.0.0.1 port binding; a fast mock cannot prove the shared-gateway ownership boundary." + }, { "live": "test/e2e/live/onboard-repair.test.ts", "fast": [ diff --git a/test/helpers/destroy-flow-test-harness.ts b/test/helpers/destroy-flow-test-harness.ts index 0f107ea7e0e..d8a1c43c4ae 100644 --- a/test/helpers/destroy-flow-test-harness.ts +++ b/test/helpers/destroy-flow-test-harness.ts @@ -14,7 +14,9 @@ const destroyModulePath = "./destroy.js"; export type DestroyHarness = { cleanupGatewaySpy: MockInstance; + captureOpenshellSpy: MockInstance; destroySandbox: DestroySandbox; + dockerCaptureSpy: MockInstance; errorSpy: MockInstance; events: string[]; finalizeMcpBridgesAfterSandboxDeleteSpy: MockInstance; @@ -25,6 +27,7 @@ export type DestroyHarness = { logSpy: MockInstance; prepareMcpBridgesForAbsentSandboxDestroySpy: MockInstance; prepareMcpBridgesForDestroySpy: MockInstance; + promptSpy: MockInstance; removeSandboxSpy: MockInstance; restoreMcpBridgesAfterDestroyAbortSpy: MockInstance; runOpenshellSpy: MockInstance; @@ -40,9 +43,12 @@ type DestroyHarnessOptions = { agent?: "openclaw" | "hermes"; deleteOutput?: string; deleteStatus?: number; + dockerPsOutput?: string; finalizeMcpError?: string; + liveListOutput?: string; mcpAddState?: "prepared"; mcpServers?: string[]; + promptResponses?: string[]; registeredSandboxCount?: number; restoreMcpError?: string; sandboxPresent?: boolean; @@ -103,6 +109,7 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr const resolve = requireDist("../../adapters/openshell/resolve.js"); const runtime = requireDist("../../adapters/openshell/runtime.js"); const destroyGateway = requireDist("./destroy-gateway.js"); + const credentialStore = requireDist("../../credentials/store.js"); const sandboxProviderCleanup = requireDist("../../onboard/sandbox-provider-cleanup.js"); const nim = requireDist("../../inference/nim.js"); const ollamaProxy = requireDist("../../inference/ollama/proxy.js"); @@ -113,8 +120,13 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr const shields = requireDist("../../shields/index.js"); const timerControl = requireDist("../../shields/timer-control.js"); const mcpBridge = requireDist("./mcp-bridge.js"); + const dockerRun = requireDist("../../adapters/docker/run.js"); vi.spyOn(resolve, "resolveOpenshell").mockReturnValue("/usr/bin/openshell"); + const promptSpy = vi.spyOn(credentialStore, "prompt").mockResolvedValue("yes"); + for (const response of options.promptResponses ?? []) { + promptSpy.mockResolvedValueOnce(response); + } vi.spyOn(sandboxSession, "getActiveSandboxSessions").mockReturnValue({ detected: true, sessions: [{ pid: 1 }], @@ -182,10 +194,24 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr return { status: 0, stdout: "", stderr: "" }; } }); - vi.spyOn(runtime, "captureOpenshell").mockReturnValue({ + const captureOpenshellSpy = vi.spyOn(runtime, "captureOpenshell").mockReturnValue({ status: 0, - output: "", + output: options.liveListOutput ?? "", }); + const dockerCaptureSpy = vi + .spyOn(dockerRun, "dockerCapture") + .mockImplementation((args: unknown) => { + const argv = Array.isArray(args) ? args.map(String) : []; + if (argv[0] !== "ps") return ""; + const filterIndex = argv.indexOf("--filter"); + const filterValue = filterIndex >= 0 ? argv[filterIndex + 1] : undefined; + const nameFilter = filterValue?.startsWith("name=") ? filterValue.slice(5) : undefined; + const names = (options.dockerPsOutput ?? "").split("\n").filter(Boolean); + const matchedNames = nameFilter + ? names.filter((name) => `/${name}`.includes(nameFilter)) + : names; + return matchedNames.length > 0 ? `${matchedNames.join("\n")}\n` : ""; + }); const selectGatewaySpy = vi .spyOn(destroyGateway, "selectGatewayForSandboxDestroy") .mockImplementation(() => undefined); @@ -278,6 +304,8 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr return { cleanupGatewaySpy, + captureOpenshellSpy, + dockerCaptureSpy, destroySandbox: requireDist(destroyModulePath).destroySandbox, errorSpy, events, @@ -289,6 +317,7 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr logSpy, prepareMcpBridgesForAbsentSandboxDestroySpy, prepareMcpBridgesForDestroySpy, + promptSpy, removeSandboxSpy, restoreMcpBridgesAfterDestroyAbortSpy, runOpenshellSpy, diff --git a/test/macos-e2e-workflow-boundary.test.ts b/test/macos-e2e-workflow-boundary.test.ts new file mode 100644 index 00000000000..c030f129977 --- /dev/null +++ b/test/macos-e2e-workflow-boundary.test.ts @@ -0,0 +1,106 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import fs from "node:fs"; +import path from "node:path"; + +import { describe, expect, it } from "vitest"; +import YAML from "yaml"; + +type WorkflowStep = { + name?: string; + if?: string; + env?: Record; + run?: string; + uses?: string; + with?: Record; +}; + +type WorkflowJob = { + if?: string; + permissions?: Record; + "runs-on"?: string; + "timeout-minutes"?: number; + steps?: WorkflowStep[]; +}; + +type Workflow = { + jobs?: Record; + on?: Record; +}; + +function readMacosWorkflow(): Workflow { + return YAML.parse( + fs.readFileSync(path.join(process.cwd(), ".github", "workflows", "macos-e2e.yaml"), "utf8"), + ) as Workflow; +} + +function jobNamed(name: string): WorkflowJob { + const job = readMacosWorkflow().jobs?.[name]; + expect(job).toBeDefined(); + return job!; +} + +function stepNamed(name: string, jobName = "macos-e2e"): WorkflowStep { + const step = jobNamed(jobName).steps?.find((candidate) => candidate.name === name); + expect(step).toBeDefined(); + return step!; +} + +describe("macOS E2E workflow boundary", () => { + it("keeps secret-bearing live E2E on trusted main-branch code", () => { + expect(readMacosWorkflow().on?.pull_request).toBeDefined(); + + expect(stepNamed("Run macOS full E2E").if).toContain("github.event_name != 'pull_request'"); + expect(stepNamed("Run macOS full E2E").if).toContain("github.ref == 'refs/heads/main'"); + + expect(String(stepNamed("Run macOS full E2E").env?.NVIDIA_INFERENCE_API_KEY)).toContain( + "github.event_name != 'pull_request'", + ); + expect(String(stepNamed("Run macOS full E2E").env?.NVIDIA_INFERENCE_API_KEY)).toContain( + "github.ref == 'refs/heads/main'", + ); + expect(jobNamed("macos-docker-final-destroy").if).toContain( + "github.event_name != 'pull_request'", + ); + expect(jobNamed("macos-docker-final-destroy").if).toContain("github.ref == 'refs/heads/main'"); + }); + + it("runs final-destroy against a commit-pinned Docker setup on trusted Intel macOS", () => { + const job = jobNamed("macos-docker-final-destroy"); + const docker = stepNamed("Set up pinned Docker Engine", "macos-docker-final-destroy"); + const live = stepNamed("Run macOS Docker final-destroy E2E", "macos-docker-final-destroy"); + + expect(job["runs-on"]).toBe("macos-15-intel"); + expect(job.permissions).toEqual({ contents: "read" }); + expect(docker.uses).toBe("docker/setup-docker-action@6d7cfa65f60a9dda7b46e5513fa982536f3c9877"); + expect(docker.with?.version).toBe("v27.4.0"); + expect(String(docker.env?.LIMA_START_ARGS)).toContain("--cpus 4 --memory 8"); + expect(live.run).toContain("test/e2e/live/sandbox-operations.test.ts"); + expect(live.env?.NEMOCLAW_NON_INTERACTIVE).toBe("1"); + }); + + it("uploads live macOS E2E artifacts when the workflow fails", () => { + const upload = stepNamed("Upload logs on failure"); + const dockerUpload = stepNamed( + "Upload macOS Docker logs on failure", + "macos-docker-final-destroy", + ); + + expect(upload.if).toBe("failure() && github.event_name == 'pull_request'"); + expect(String(upload.with?.path)).toContain("/tmp/nemoclaw-e2e-*.log"); + expect(String(upload.with?.path)).toContain("${{ github.workspace }}/e2e-artifacts/live"); + + expect(dockerUpload.if).toBe("failure()"); + expect(dockerUpload.uses).toBe( + "actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a", + ); + expect(String(dockerUpload.with?.path)).toContain("/tmp/nemoclaw-e2e-*.log"); + expect(String(dockerUpload.with?.path)).toContain("${{ github.workspace }}/e2e-artifacts/live"); + }); + + it("bounds the fast and real-Docker macOS jobs independently", () => { + expect(jobNamed("macos-e2e")["timeout-minutes"]).toBe(30); + expect(jobNamed("macos-docker-final-destroy")["timeout-minutes"]).toBe(90); + }); +}); diff --git a/test/onboard-gateway-legacy-identity-upgrade-runtime.test.ts b/test/onboard-gateway-legacy-identity-upgrade-runtime.test.ts new file mode 100644 index 00000000000..2d9486f77bb --- /dev/null +++ b/test/onboard-gateway-legacy-identity-upgrade-runtime.test.ts @@ -0,0 +1,276 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import assert from "node:assert/strict"; +import { spawn, 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 { afterEach, describe, expect, it, vi } from "vitest"; + +import { waitUntil } from "../src/lib/core/wait"; +import { + type DockerDriverGatewayCutoverDeps, + type DockerDriverGatewayCutoverInput, + runDockerDriverGatewayCutover, +} from "../src/lib/onboard/docker-driver-gateway-cutover"; +import { reapHostGatewayBeforeLaunchOrFail } from "../src/lib/onboard/docker-driver-gateway-prelaunch"; +import { createDockerDriverGatewayRuntimeHelpers } from "../src/lib/onboard/docker-driver-gateway-runtime"; +import { resolveGatewayName, resolveGatewayStateDirName } from "../src/lib/onboard/gateway-binding"; +import { buildOwnedHostGatewayArgv0 } from "../src/lib/onboard/gateway-process-identity"; +import { stopHostGatewayProcesses } from "../src/lib/onboard/host-gateway-process"; + +const posix = process.platform !== "win32"; +const hasLsof = posix && !spawnSync("lsof", ["-v"], { stdio: "ignore" }).error; + +const livePids = new Set(); +let tmpHome: string | null = null; +let scriptSequence = 0; + +function killQuietly(pid: number): void { + try { + process.kill(pid, "SIGKILL"); + } catch { + // Already stopped. + } +} + +function isAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +afterEach(() => { + for (const pid of livePids) killQuietly(pid); + livePids.clear(); + tmpHome && fs.rmSync(tmpHome, { recursive: true, force: true }); + tmpHome = null; +}); + +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 canBind(port: number): Promise { + return new Promise((resolve) => { + const server = net.createServer(); + server.once("error", () => resolve(false)); + server.listen(port, "127.0.0.1", () => server.close(() => resolve(true))); + }); +} + +function readPid(pidFile: string): number { + try { + return Number.parseInt(fs.readFileSync(pidFile, "utf-8").trim(), 10) || 0; + } catch { + return 0; + } +} + +function launchOrphanGateway(options: { + argv0: string; + env: NodeJS.ProcessEnv; + pidFile: string; + port: number; +}): number { + assert.ok(tmpHome, "temporary home is not initialized"); + scriptSequence += 1; + const gatewayFile = path.join(tmpHome, `gateway-${scriptSequence}.cjs`); + fs.writeFileSync( + gatewayFile, + `const net=require("node:net");const fs=require("node:fs");` + + `const server=net.createServer();` + + `server.listen(${String(options.port)},"127.0.0.1",()=>fs.writeFileSync(${JSON.stringify(options.pidFile)},String(process.pid)));` + + `process.on("SIGTERM",()=>process.exit(0));`, + ); + const launcherScript = + `const {spawn}=require("node:child_process");` + + `spawn(process.argv[1],[process.argv[2]],{argv0:process.argv[3],detached:true,stdio:"ignore",env:JSON.parse(process.argv[4])}).unref();`; + spawn( + process.execPath, + [ + "-e", + launcherScript, + process.execPath, + gatewayFile, + options.argv0, + JSON.stringify(options.env), + ], + { stdio: "ignore" }, + ); + + let pid = 0; + const started = waitUntil( + () => { + pid = readPid(options.pidFile); + return pid > 0 && isAlive(pid); + }, + { + deadlineMs: Date.now() + 10_000, + initialIntervalMs: 25, + maxIntervalMs: 25, + backoffFactor: 1, + }, + ); + assert.ok(started, "gateway fixture did not start"); + livePids.add(pid); + return pid; +} + +function runCapture(args: string[]): string { + const result = spawnSync(args[0], args.slice(1), { encoding: "utf-8" }); + return result.status === 0 ? result.stdout : ""; +} + +function runCaptureEx(args: readonly string[]): { + stdout: string; + exitCode: number | null; + timedOut: boolean; +} { + const result = spawnSync(args[0], args.slice(1), { encoding: "utf-8", timeout: 5000 }); + return { + stdout: result.stdout ?? "", + exitCode: result.status, + timedOut: Boolean(result.error && "code" in result.error && result.error.code === "ETIMEDOUT"), + }; +} + +describe("legacy Docker-driver gateway identity upgrade", () => { + it.skipIf(!posix || !hasLsof)( + "retires a reused legacy process, launches target-bound identity, and releases its port", + async () => { + const port = await reserveFreePort(); + const gatewayName = resolveGatewayName(port); + tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-gateway-upgrade-")); + const stateDir = path.join( + tmpHome, + ".local", + "state", + "nemoclaw", + resolveGatewayStateDirName(port), + ); + fs.mkdirSync(stateDir, { recursive: true }); + const pidFile = path.join(stateDir, "openshell-gateway.pid"); + const gatewayBin = path.join(tmpHome, "openshell-gateway"); + const driftEnv = { OPENSHELL_DRIVERS: "docker" }; + const childEnv = { ...process.env, ...driftEnv }; + + const legacyPid = launchOrphanGateway({ + argv0: gatewayBin, + env: childEnv, + pidFile, + port, + }); + await expect(canBind(port)).resolves.toBe(false); + + const runtime = createDockerDriverGatewayRuntimeHelpers({ + gatewayPort: port, + getCachedOpenshellBinary: () => null, + getBlueprintMaxOpenshellVersion: () => null, + getInstalledOpenshellVersion: () => "0.0.72", + isOpenshellDevVersion: () => false, + runCapture, + runCaptureEx, + shouldUseOpenshellDevChannel: () => false, + supportedOpenshellFallbackVersion: "0.0.72", + }); + const listenerScan = runtime.getDockerDriverGatewayPortListenerScan( + { ok: false, process: "openshell-gateway", pid: legacyPid }, + { gatewayBin, platform: process.platform }, + ); + expect(listenerScan).toEqual({ complete: true, pids: [legacyPid] }); + + const restartReasons: string[] = []; + const verifyBridge = vi.fn(async () => undefined); + const input: DockerDriverGatewayCutoverInput = { + gatewayBin, + identityGatewayBin: gatewayBin, + driftGatewayBin: gatewayBin, + driftGatewayEnv: driftEnv, + exitOnFailure: false, + skipSandboxBridgeReachability: false, + stateDir, + portListenerScan: listenerScan, + pidFileGatewayPid: legacyPid, + initialHealth: { + status: "Gateway: active", + namedInfo: `Gateway: ${gatewayName}`, + activeInfo: `Gateway: ${gatewayName}`, + }, + }; + const deps: DockerDriverGatewayCutoverDeps = { + isDockerDriverGatewayProcessAlive: () => isAlive(legacyPid), + isGatewayHealthy: () => true, + getDockerDriverGatewayRuntimeDrift: (pid, env, binary) => + runtime.getDockerDriverGatewayRuntimeDrift(pid, env, binary, process.platform), + logDockerDriverGatewayRestart: (reason) => restartReasons.push(reason), + registerDockerDriverGatewayEndpoint: () => true, + isDockerDriverGatewayHttpReady: async () => true, + verifySandboxBridgeGatewayReachableOrExit: verifyBridge, + readGatewayHealth: () => input.initialHealth, + rememberDockerDriverGatewayPid: runtime.rememberDockerDriverGatewayPid, + reapDuplicateHostGatewaysExceptOrFail: () => undefined, + reapHostGatewayBeforeLaunchOrFail: (options) => reapHostGatewayBeforeLaunchOrFail(options), + isGatewayPortAvailable: () => canBind(port), + reportUntrustedGatewayPort: (message) => { + throw new Error(message); + }, + reportMissingGatewayBinary: () => { + throw new Error("gateway binary missing"); + }, + log: () => undefined, + }; + + await expect(runDockerDriverGatewayCutover(input, deps)).resolves.toBe("launch"); + livePids.delete(legacyPid); + expect(restartReasons).toContainEqual( + expect.stringContaining("target-bound cleanup identity"), + ); + expect(verifyBridge).not.toHaveBeenCalled(); + expect(isAlive(legacyPid)).toBe(false); + await expect(canBind(port)).resolves.toBe(true); + + const argv0 = buildOwnedHostGatewayArgv0(gatewayName); + expect(argv0).not.toBeNull(); + const freshPid = launchOrphanGateway({ + argv0: argv0 as string, + env: childEnv, + pidFile, + port, + }); + const stopped = stopHostGatewayProcesses( + { env: { ...process.env, HOME: tmpHome } }, + { + gatewayBin, + openShellGatewayName: gatewayName, + openShellGatewayPort: port, + pidFile, + stateDir, + usePgrepFallback: false, + }, + ); + livePids.delete(freshPid); + + expect(stopped.stopped).toContain(freshPid); + expect(stopped.skippedNonMatchingPids).toEqual([]); + expect(isAlive(freshPid)).toBe(false); + expect(fs.existsSync(pidFile)).toBe(false); + await expect(canBind(port)).resolves.toBe(true); + }, + 30000, + ); +});