From 07f417f8affe10c6180cd7ec97132980f53b8a07 Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Tue, 28 Apr 2026 11:22:43 -0700 Subject: [PATCH 1/9] refactor(cli): extract docker command helpers --- src/lib/agent-onboard.ts | 7 +- src/lib/docker/container.ts | 32 ++++++++ src/lib/docker/image.ts | 18 ++++ src/lib/docker/index.test.ts | 122 ++++++++++++++++++++++++++++ src/lib/docker/index.ts | 10 +++ src/lib/docker/info.ts | 12 +++ src/lib/docker/inspect.ts | 39 +++++++++ src/lib/docker/pull.ts | 8 ++ src/lib/docker/run.ts | 19 +++++ src/lib/docker/volume.ts | 37 +++++++++ src/lib/nim.ts | 43 ++++++---- src/lib/onboard.ts | 83 ++++++++++--------- src/nemoclaw.ts | 10 +-- test/cli.test.ts | 28 +++---- test/gateway-cleanup.test.ts | 3 +- test/gateway-liveness-probe.test.ts | 4 +- test/image-cleanup.test.ts | 4 +- 17 files changed, 395 insertions(+), 84 deletions(-) create mode 100644 src/lib/docker/container.ts create mode 100644 src/lib/docker/image.ts create mode 100644 src/lib/docker/index.test.ts create mode 100644 src/lib/docker/index.ts create mode 100644 src/lib/docker/info.ts create mode 100644 src/lib/docker/inspect.ts create mode 100644 src/lib/docker/pull.ts create mode 100644 src/lib/docker/run.ts create mode 100644 src/lib/docker/volume.ts diff --git a/src/lib/agent-onboard.ts b/src/lib/agent-onboard.ts index e632ebfdea4..0cb18593293 100644 --- a/src/lib/agent-onboard.ts +++ b/src/lib/agent-onboard.ts @@ -9,7 +9,8 @@ import fs from "fs"; import os from "os"; import path from "path"; -import { ROOT, run } from "./runner"; +import { ROOT } from "./runner"; +import { dockerBuild, dockerImageInspect } from "./docker"; import { loadAgent, resolveAgentName, type AgentDefinition } from "./agent-defs"; import { getProviderSelectionConfig } from "./inference-config"; import * as onboardSession from "./onboard-session"; @@ -61,13 +62,13 @@ export function createAgentSandbox(agent: AgentDefinition): { if (baseDockerfile) { const baseImageTag = `ghcr.io/nvidia/nemoclaw/${agent.name}-sandbox-base:latest`; - const inspectResult = run(["docker", "image", "inspect", baseImageTag], { + const inspectResult = dockerImageInspect(baseImageTag, { ignoreError: true, suppressOutput: true, }); if (inspectResult.status !== 0) { console.log(` Building ${agent.displayName} base image (first time only)...`); - run(["docker", "build", "-f", baseDockerfile, "-t", baseImageTag, ROOT], { + dockerBuild(baseDockerfile, baseImageTag, ROOT, { stdio: ["ignore", "inherit", "inherit"], }); console.log(` \u2713 Base image built: ${baseImageTag}`); diff --git a/src/lib/docker/container.ts b/src/lib/docker/container.ts new file mode 100644 index 00000000000..afe8d71ec3e --- /dev/null +++ b/src/lib/docker/container.ts @@ -0,0 +1,32 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { dockerCapture, dockerRun, type DockerCaptureOptions, type DockerRunOptions } from "./run"; + +export function dockerStop(containerName: string, opts: DockerRunOptions = {}) { + return dockerRun(["stop", containerName], opts); +} + +export function dockerRm(containerName: string, opts: DockerRunOptions = {}) { + return dockerRun(["rm", containerName], opts); +} + +export function dockerForceRm(containerName: string, opts: DockerRunOptions = {}) { + return dockerRun(["rm", "-f", containerName], opts); +} + +export function dockerRunDetached(args: readonly string[], opts: DockerRunOptions = {}) { + return dockerRun(["run", "-d", ...args], opts); +} + +export function dockerPort( + containerName: string, + containerPort: string | number, + opts: DockerCaptureOptions = {}, +): string { + return dockerCapture(["port", containerName, String(containerPort)], opts); +} + +export function dockerExecArgv(containerName: string, cmd: readonly string[]): string[] { + return ["docker", "exec", containerName, ...cmd]; +} diff --git a/src/lib/docker/image.ts b/src/lib/docker/image.ts new file mode 100644 index 00000000000..eb39c63cc55 --- /dev/null +++ b/src/lib/docker/image.ts @@ -0,0 +1,18 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { ROOT } from "../runner"; +import { dockerRun, type DockerRunOptions } from "./run"; + +export function dockerBuild( + dockerfilePath: string, + tag: string, + contextDir: string = ROOT, + opts: DockerRunOptions = {}, +) { + return dockerRun(["build", "-f", dockerfilePath, "-t", tag, contextDir], opts); +} + +export function dockerRmi(imageRef: string, opts: DockerRunOptions = {}) { + return dockerRun(["rmi", imageRef], opts); +} diff --git a/src/lib/docker/index.test.ts b/src/lib/docker/index.test.ts new file mode 100644 index 00000000000..f0640d04d89 --- /dev/null +++ b/src/lib/docker/index.test.ts @@ -0,0 +1,122 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const runMock = vi.fn(); +const runCaptureMock = vi.fn(); + +vi.mock("../runner", () => ({ + ROOT: "/repo/root", + run: (...args: unknown[]) => runMock(...args), + runCapture: (...args: unknown[]) => runCaptureMock(...args), +})); + +import { + dockerBuild, + dockerContainerInspectFormat, + dockerInfoFormat, + dockerListVolumesByPrefix, + dockerPull, + dockerRemoveVolumesByPrefix, + dockerRmi, + dockerRunDetached, +} from "./index"; + +describe("docker helpers", () => { + beforeEach(() => { + runMock.mockReset(); + runCaptureMock.mockReset(); + runMock.mockReturnValue({ status: 0, stdout: "", stderr: "" }); + runCaptureMock.mockReturnValue(""); + }); + + it("prefixes docker argv for pull/build/run/rmi helpers", () => { + dockerPull("ghcr.io/example/image:latest"); + dockerBuild("Dockerfile", "example:tag", "/tmp/build"); + dockerRunDetached(["--name", "example", "busybox:latest"]); + dockerRmi("example:tag"); + + expect(runMock.mock.calls).toEqual([ + [["docker", "pull", "ghcr.io/example/image:latest"], {}], + [["docker", "build", "-f", "Dockerfile", "-t", "example:tag", "/tmp/build"], {}], + [["docker", "run", "-d", "--name", "example", "busybox:latest"], {}], + [["docker", "rmi", "example:tag"], {}], + ]); + }); + + it("prefixes docker argv for info/inspect capture helpers", () => { + dockerInfoFormat("{{.KernelVersion}}", { ignoreError: true }); + dockerContainerInspectFormat("{{.State.Status}}", "example-container", { + ignoreError: true, + }); + + expect(runCaptureMock.mock.calls).toEqual([ + [["docker", "info", "--format", "{{.KernelVersion}}"], { ignoreError: true }], + [ + [ + "docker", + "inspect", + "--type", + "container", + "--format", + "{{.State.Status}}", + "example-container", + ], + { ignoreError: true }, + ], + ]); + }); + + it("filters docker volume names by exact prefix", () => { + runCaptureMock.mockReturnValue( + [ + "openshell-cluster-nemoclaw", + "openshell-cluster-nemoclaw-cache", + "openshell-cluster-nemoclaw-2", + "not-a-match", + "", + ].join("\n"), + ); + + const names = dockerListVolumesByPrefix("openshell-cluster-nemoclaw"); + + expect(names).toEqual([ + "openshell-cluster-nemoclaw", + "openshell-cluster-nemoclaw-cache", + "openshell-cluster-nemoclaw-2", + ]); + }); + + it("removes only volumes returned by the prefix probe", () => { + runCaptureMock.mockReturnValue("openshell-cluster-nemoclaw\nopenshell-cluster-nemoclaw-cache\n"); + + const removed = dockerRemoveVolumesByPrefix("openshell-cluster-nemoclaw", { + ignoreError: true, + }); + + expect(removed).toEqual([ + "openshell-cluster-nemoclaw", + "openshell-cluster-nemoclaw-cache", + ]); + expect(runMock).toHaveBeenCalledWith( + [ + "docker", + "volume", + "rm", + "openshell-cluster-nemoclaw", + "openshell-cluster-nemoclaw-cache", + ], + { ignoreError: true }, + ); + }); + + it("skips volume removal when the probe finds no matches", () => { + runCaptureMock.mockReturnValue(""); + + const removed = dockerRemoveVolumesByPrefix("openshell-cluster-nemoclaw"); + + expect(removed).toEqual([]); + expect(runMock).not.toHaveBeenCalled(); + }); +}); diff --git a/src/lib/docker/index.ts b/src/lib/docker/index.ts new file mode 100644 index 00000000000..6fbd291b689 --- /dev/null +++ b/src/lib/docker/index.ts @@ -0,0 +1,10 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +export * from "./run"; +export * from "./pull"; +export * from "./info"; +export * from "./inspect"; +export * from "./image"; +export * from "./container"; +export * from "./volume"; diff --git a/src/lib/docker/info.ts b/src/lib/docker/info.ts new file mode 100644 index 00000000000..fbcb92571a6 --- /dev/null +++ b/src/lib/docker/info.ts @@ -0,0 +1,12 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { dockerCapture, type DockerCaptureOptions } from "./run"; + +export function dockerInfo(opts: DockerCaptureOptions = {}): string { + return dockerCapture(["info"], opts); +} + +export function dockerInfoFormat(format: string, opts: DockerCaptureOptions = {}): string { + return dockerCapture(["info", "--format", format], opts); +} diff --git a/src/lib/docker/inspect.ts b/src/lib/docker/inspect.ts new file mode 100644 index 00000000000..3ae7e5243dc --- /dev/null +++ b/src/lib/docker/inspect.ts @@ -0,0 +1,39 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { dockerCapture, dockerRun, type DockerCaptureOptions, type DockerRunOptions } from "./run"; + +export function dockerInspect(args: readonly string[], opts: DockerRunOptions = {}) { + return dockerRun(["inspect", ...args], opts); +} + +export function dockerInspectFormat( + format: string, + target: string, + opts: DockerCaptureOptions = {}, +): string { + return dockerCapture(["inspect", "--format", format, target], opts); +} + +export function dockerImageInspect( + target: string, + opts: DockerRunOptions = {}, +) { + return dockerRun(["image", "inspect", target], opts); +} + +export function dockerImageInspectFormat( + format: string, + target: string, + opts: DockerCaptureOptions = {}, +): string { + return dockerCapture(["image", "inspect", "--format", format, target], opts); +} + +export function dockerContainerInspectFormat( + format: string, + containerName: string, + opts: DockerCaptureOptions = {}, +): string { + return dockerCapture(["inspect", "--type", "container", "--format", format, containerName], opts); +} diff --git a/src/lib/docker/pull.ts b/src/lib/docker/pull.ts new file mode 100644 index 00000000000..5fff19417d5 --- /dev/null +++ b/src/lib/docker/pull.ts @@ -0,0 +1,8 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { dockerRun, type DockerRunOptions } from "./run"; + +export function dockerPull(imageRef: string, opts: DockerRunOptions = {}) { + return dockerRun(["pull", imageRef], opts); +} diff --git a/src/lib/docker/run.ts b/src/lib/docker/run.ts new file mode 100644 index 00000000000..58fb786a102 --- /dev/null +++ b/src/lib/docker/run.ts @@ -0,0 +1,19 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { run, runCapture } from "../runner"; + +export type DockerRunOptions = Parameters[1]; +export type DockerCaptureOptions = Parameters[1]; + +export function dockerArgv(args: readonly string[]): string[] { + return ["docker", ...args]; +} + +export function dockerRun(args: readonly string[], opts: DockerRunOptions = {}) { + return run(dockerArgv(args), opts); +} + +export function dockerCapture(args: readonly string[], opts: DockerCaptureOptions = {}): string { + return runCapture(dockerArgv(args), opts); +} diff --git a/src/lib/docker/volume.ts b/src/lib/docker/volume.ts new file mode 100644 index 00000000000..fdeba538d1e --- /dev/null +++ b/src/lib/docker/volume.ts @@ -0,0 +1,37 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { dockerCapture, dockerRun, type DockerCaptureOptions, type DockerRunOptions } from "./run"; + +function splitNonEmptyLines(output: string): string[] { + return output + .split(/\r?\n/) + .map((line) => line.trim()) + .filter(Boolean); +} + +export function dockerListVolumesByPrefix( + prefix: string, + opts: DockerCaptureOptions = {}, +): string[] { + const output = dockerCapture(["volume", "ls", "-q", "--filter", `name=${prefix}`], { + ignoreError: true, + ...opts, + }); + return splitNonEmptyLines(output).filter((name) => name.startsWith(prefix)); +} + +export function dockerRemoveVolumes(names: readonly string[], opts: DockerRunOptions = {}) { + if (names.length === 0) return null; + return dockerRun(["volume", "rm", ...names], opts); +} + +export function dockerRemoveVolumesByPrefix( + prefix: string, + opts: DockerRunOptions = {}, +): string[] { + const names = dockerListVolumesByPrefix(prefix); + if (names.length === 0) return names; + dockerRemoveVolumes(names, opts); + return names; +} diff --git a/src/lib/nim.ts b/src/lib/nim.ts index 01233e37c3c..addf81b482d 100644 --- a/src/lib/nim.ts +++ b/src/lib/nim.ts @@ -4,7 +4,17 @@ // NIM container management — pull, start, stop, health-check NIM images. // eslint-disable-next-line @typescript-eslint/no-require-imports -const { run, runCapture } = require("./runner"); +const { runCapture } = require("./runner"); +// eslint-disable-next-line @typescript-eslint/no-require-imports +const { + dockerForceRm, + dockerPort, + dockerPull, + dockerRm, + dockerRunDetached, + dockerStop, + dockerContainerInspectFormat, +} = require("./docker"); // eslint-disable-next-line @typescript-eslint/no-require-imports const { sleepSeconds } = require("./wait"); // eslint-disable-next-line @typescript-eslint/no-require-imports @@ -226,7 +236,7 @@ export function pullNimImage(model: string): string { process.exit(1); } console.log(` Pulling NIM image: ${image}`); - run(["docker", "pull", image]); + dockerPull(image); return image; } @@ -242,14 +252,18 @@ export function startNimContainerByName(name: string, model: string, port = VLLM process.exit(1); } - run(["docker", "rm", "-f", name], { ignoreError: true }); + dockerForceRm(name, { ignoreError: true }); console.log(` Starting NIM container: ${name}`); - run([ - "docker", "run", "-d", "--gpus", "all", - "-p", `${Number(port)}:8000`, - "--name", name, - "--shm-size", "16g", + dockerRunDetached([ + "--gpus", + "all", + "-p", + `${Number(port)}:8000`, + "--name", + name, + "--shm-size", + "16g", image, ]); return name; @@ -302,8 +316,8 @@ export function stopNimContainerByName( ): void { if (!silent) console.log(` Stopping NIM container: ${name}`); const stdio = silent ? ["ignore", "ignore", "ignore"] : undefined; - run(["docker", "stop", name], { ignoreError: true, ...(stdio && { stdio }) }); - run(["docker", "rm", name], { ignoreError: true, ...(stdio && { stdio }) }); + dockerStop(name, { ignoreError: true, ...(stdio && { stdio }) }); + dockerRm(name, { ignoreError: true, ...(stdio && { stdio }) }); } export function nimStatus(sandboxName: string, port?: number): NimStatus { @@ -313,19 +327,14 @@ export function nimStatus(sandboxName: string, port?: number): NimStatus { export function nimStatusByName(name: string, port?: number): NimStatus { try { - const state = runCapture( - ["docker", "inspect", "--format", "{{.State.Status}}", name], - { ignoreError: true }, - ); + const state = dockerContainerInspectFormat("{{.State.Status}}", name, { ignoreError: true }); if (!state) return { running: false, container: name }; let healthy = false; if (state === "running") { let resolvedHostPort = port != null ? Number(port) : 0; if (!resolvedHostPort) { - const mapping = runCapture(["docker", "port", name, "8000"], { - ignoreError: true, - }); + const mapping = dockerPort(name, "8000", { ignoreError: true }); const m = mapping && mapping.match(/:(\d+)\s*$/); resolvedHostPort = m ? Number(m[1]) : VLLM_PORT; } diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index e33082c4ccb..7aab78bfeda 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -27,6 +27,21 @@ const LOCAL_INFERENCE_TIMEOUT_SECS = envInt("NEMOCLAW_LOCAL_INFERENCE_TIMEOUT", const ANSI_RE = /\x1B(?:\[[0-?]*[ -/]*[@-~]|\][^\x07]*(?:\x07|\x1B\\)|[@-_])/g; const runner: typeof import("./runner") = require("./runner"); const { ROOT, SCRIPTS, redact, run, runCapture, runFile, shellQuote, validateName } = runner; +const docker: typeof import("./docker") = require("./docker"); +const { + dockerContainerInspectFormat, + dockerExecArgv, + dockerImageInspect, + dockerImageInspectFormat, + dockerInfo, + dockerInfoFormat, + dockerInspect, + dockerPull, + dockerRemoveVolumesByPrefix, + dockerRm, + dockerRmi, + dockerStop, +} = docker; const errnoUtils: typeof import("./errno") = require("./errno"); const { isErrnoException } = errnoUtils; @@ -232,8 +247,8 @@ const BACK_TO_SELECTION = "__NEMOCLAW_BACK_TO_SELECTION__"; */ function verifyGatewayContainerRunning() { const containerName = `openshell-cluster-${GATEWAY_NAME}`; - const result = run( - `docker inspect --type container --format '{{.State.Running}}' ${containerName}`, + const result = dockerInspect( + ["--type", "container", "--format", "{{.State.Running}}", containerName], { ignoreError: true, suppressOutput: true }, ); if (result.status === 0 && String(result.stdout || "").trim() === "true") { @@ -588,7 +603,7 @@ const SANDBOX_BASE_TAG = "latest"; function pullAndResolveBaseImageDigest(): { digest: string; ref: string } | null { const imageWithTag = `${SANDBOX_BASE_IMAGE}:${SANDBOX_BASE_TAG}`; try { - run(["docker", "pull", imageWithTag], { suppressOutput: true }); + dockerPull(imageWithTag, { suppressOutput: true }); } catch { // Pull failed — caller should fall back to unpin :latest return null; @@ -596,10 +611,9 @@ function pullAndResolveBaseImageDigest(): { digest: string; ref: string } | null let inspectOutput; try { - inspectOutput = runCapture( - ["docker", "inspect", "--format", "{{json .RepoDigests}}", imageWithTag], - { ignoreError: false }, - ); + inspectOutput = dockerImageInspectFormat("{{json .RepoDigests}}", imageWithTag, { + ignoreError: false, + }); } catch { return null; } @@ -1819,7 +1833,7 @@ function getResumeConfigConflicts( } function getContainerRuntime(): ContainerRuntime { - const info = runCapture(["docker", "info"], { ignoreError: true }); + const info = dockerInfo({ ignoreError: true }); return inferContainerRuntime(info); } @@ -1916,25 +1930,14 @@ function destroyGateway() { } // openshell gateway destroy doesn't remove Docker volumes, which leaves // corrupted cluster state that breaks the next gateway start. Clean them up. - // Shell required: pipe (|), && chaining, || fallback. - run( - `docker volume ls -q --filter "name=openshell-cluster-${GATEWAY_NAME}" | grep . && docker volume ls -q --filter "name=openshell-cluster-${GATEWAY_NAME}" | xargs docker volume rm || true`, - { ignoreError: true }, - ); + dockerRemoveVolumesByPrefix(`openshell-cluster-${GATEWAY_NAME}`, { ignoreError: true }); } function getGatewayClusterContainerState(): string { const containerName = getGatewayClusterContainerName(); - const state = runCapture( - [ - "docker", - "inspect", - "--type", - "container", - "--format", - "{{.State.Status}}{{if .State.Health}} {{.State.Health.Status}}{{end}}", - containerName, - ], + const state = dockerContainerInspectFormat( + "{{.State.Status}}{{if .State.Health}} {{.State.Health.Status}}{{end}}", + containerName, { ignoreError: true }, ) .trim() @@ -1967,7 +1970,7 @@ function getGatewayClusterContainerName(): string { } function buildGatewayClusterExecArgv(script: string): string[] { - return ["docker", "exec", getGatewayClusterContainerName(), "sh", "-lc", script]; + return dockerExecArgv(getGatewayClusterContainerName(), ["sh", "-lc", script]); } function hostCommandExists(commandName: string): boolean { @@ -2541,29 +2544,29 @@ async function preflight(): Promise> { // OpenShell has no metadata for it (gatewayReuseState === "missing"). if (gatewayReuseState === "missing") { const containerName = `openshell-cluster-${GATEWAY_NAME}`; - const inspectResult = run( - ["docker", "inspect", "--type", "container", "--format", "{{.State.Status}}", containerName], + const inspectResult = dockerInspect( + ["--type", "container", "--format", "{{.State.Status}}", containerName], { ignoreError: true, suppressOutput: true }, ); if (inspectResult.status === 0) { console.log(" Cleaning up orphaned gateway container..."); - run(["docker", "stop", containerName], { + dockerStop(containerName, { ignoreError: true, suppressOutput: true, }); - run(["docker", "rm", containerName], { + dockerRm(containerName, { ignoreError: true, suppressOutput: true, }); - const postInspectResult = run(["docker", "inspect", "--type", "container", containerName], { + const postInspectResult = dockerInspect(["--type", "container", containerName], { ignoreError: true, suppressOutput: true, }); if (postInspectResult.status !== 0) { - run( - `docker volume ls -q --filter "name=openshell-cluster-${GATEWAY_NAME}" | grep . && docker volume ls -q --filter "name=openshell-cluster-${GATEWAY_NAME}" | xargs docker volume rm 2>/dev/null || true`, - { ignoreError: true, suppressOutput: true }, - ); + dockerRemoveVolumesByPrefix(`openshell-cluster-${GATEWAY_NAME}`, { + ignoreError: true, + suppressOutput: true, + }); registry.clearAll(); console.log(" ✓ Orphaned gateway container removed"); } else { @@ -3525,7 +3528,7 @@ async function createSandbox( // Destroy old sandbox and clean up its host-side Docker image. runOpenshell(["sandbox", "delete", sandboxName], { ignoreError: true }); if (previousEntry?.imageTag) { - const rmiResult = run(["docker", "rmi", previousEntry.imageTag], { + const rmiResult = dockerRmi(previousEntry.imageTag, { ignoreError: true, suppressOutput: true, }); @@ -3727,11 +3730,11 @@ async function createSandbox( // Check if the image exists locally before falling back to unpinned :latest. // On a first-time install behind a firewall with no cached image, warn early // so the user knows the build will likely fail. - const localCheck = runCapture( - ["docker", "image", "inspect", `${SANDBOX_BASE_IMAGE}:${SANDBOX_BASE_TAG}`], - { ignoreError: true }, - ); - if (localCheck) { + const localCheck = dockerImageInspect(`${SANDBOX_BASE_IMAGE}:${SANDBOX_BASE_TAG}`, { + ignoreError: true, + suppressOutput: true, + }); + if (localCheck.status === 0) { console.warn(" Warning: could not pull base image from registry; using cached :latest."); } else { console.warn( @@ -4012,7 +4015,7 @@ async function createSandbox( try { if (process.platform === "darwin") { - const vmKernel = runCapture(["docker", "info", "--format", "{{.KernelVersion}}"], { + const vmKernel = dockerInfoFormat("{{.KernelVersion}}", { ignoreError: true, }).trim(); if (vmKernel) { diff --git a/src/nemoclaw.ts b/src/nemoclaw.ts index c9ea19ddcf6..82931de64ad 100644 --- a/src/nemoclaw.ts +++ b/src/nemoclaw.ts @@ -29,6 +29,7 @@ const { shellQuote, validateName, } = require("./lib/runner"); +const { dockerRemoveVolumesByPrefix, dockerRmi } = require("./lib/docker"); const { resolveOpenshell } = require("./lib/resolve-openshell"); const { fetchGatewayAuthTokenFromSandbox, @@ -173,10 +174,9 @@ function captureOpenshell(args: CommandArgs, opts: RunnerOptions = {}) { function cleanupGatewayAfterLastSandbox() { runOpenshell(["forward", "stop", DASHBOARD_FORWARD_PORT], { ignoreError: true }); runOpenshell(["gateway", "destroy", "-g", NEMOCLAW_GATEWAY_NAME], { ignoreError: true }); - run( - `docker volume ls -q --filter "name=openshell-cluster-${NEMOCLAW_GATEWAY_NAME}" | grep . && docker volume ls -q --filter "name=openshell-cluster-${NEMOCLAW_GATEWAY_NAME}" | xargs docker volume rm || true`, - { ignoreError: true }, - ); + dockerRemoveVolumesByPrefix(`openshell-cluster-${NEMOCLAW_GATEWAY_NAME}`, { + ignoreError: true, + }); } function hasNoLiveSandboxes() { @@ -2448,7 +2448,7 @@ function cleanupSandboxServices( function removeSandboxImage(sandboxName: string) { const sb = registry.getSandbox(sandboxName); if (!sb?.imageTag) return; - const result = run(["docker", "rmi", sb.imageTag], { ignoreError: true }); + const result = dockerRmi(sb.imageTag, { ignoreError: true }); if (result.status === 0) { console.log(` Removed Docker image ${sb.imageTag}`); } else { diff --git a/test/cli.test.ts b/test/cli.test.ts index c6db10364f6..e5616b62884 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -505,7 +505,7 @@ describe("CLI dispatch", () => { const localBin = path.join(home, "bin"); const registryDir = path.join(home, ".nemoclaw"); const openshellLog = path.join(home, "openshell.log"); - const bashLog = path.join(home, "bash.log"); + const bashLog = path.join(home, "docker.log"); fs.mkdirSync(localBin, { recursive: true }); fs.mkdirSync(registryDir, { recursive: true }); fs.writeFileSync( @@ -539,7 +539,7 @@ describe("CLI dispatch", () => { { mode: 0o755 }, ); fs.writeFileSync( - path.join(localBin, "bash"), + path.join(localBin, "docker"), [ "#!/bin/sh", `log_file=${JSON.stringify(bashLog)}`, @@ -559,7 +559,7 @@ describe("CLI dispatch", () => { expect(fs.readFileSync(openshellLog, "utf8")).toContain("NAME STATUS"); expect(fs.readFileSync(openshellLog, "utf8")).toContain("forward stop 18789"); expect(fs.readFileSync(openshellLog, "utf8")).toContain("gateway destroy -g nemoclaw"); - expect(fs.readFileSync(bashLog, "utf8")).toContain("docker volume ls -q --filter"); + expect(fs.readFileSync(bashLog, "utf8")).toContain("volume ls -q --filter"); }); it("keeps the gateway runtime when other sandboxes still exist", () => { @@ -567,7 +567,7 @@ describe("CLI dispatch", () => { const localBin = path.join(home, "bin"); const registryDir = path.join(home, ".nemoclaw"); const openshellLog = path.join(home, "openshell.log"); - const bashLog = path.join(home, "bash.log"); + const bashLog = path.join(home, "docker.log"); fs.mkdirSync(localBin, { recursive: true }); fs.mkdirSync(registryDir, { recursive: true }); fs.writeFileSync( @@ -609,7 +609,7 @@ describe("CLI dispatch", () => { { mode: 0o755 }, ); fs.writeFileSync( - path.join(localBin, "bash"), + path.join(localBin, "docker"), [ "#!/bin/sh", `log_file=${JSON.stringify(bashLog)}`, @@ -629,7 +629,7 @@ describe("CLI dispatch", () => { expect(fs.readFileSync(openshellLog, "utf8")).not.toContain("forward stop 18789"); expect(fs.readFileSync(openshellLog, "utf8")).not.toContain("gateway destroy -g nemoclaw"); if (fs.existsSync(bashLog)) { - expect(fs.readFileSync(bashLog, "utf8")).not.toContain("docker volume ls -q --filter"); + expect(fs.readFileSync(bashLog, "utf8")).not.toContain("volume ls -q --filter"); } }); @@ -638,7 +638,7 @@ describe("CLI dispatch", () => { const localBin = path.join(home, "bin"); const registryDir = path.join(home, ".nemoclaw"); const openshellLog = path.join(home, "openshell.log"); - const bashLog = path.join(home, "bash.log"); + const bashLog = path.join(home, "docker.log"); fs.mkdirSync(localBin, { recursive: true }); fs.mkdirSync(registryDir, { recursive: true }); fs.writeFileSync( @@ -673,7 +673,7 @@ describe("CLI dispatch", () => { { mode: 0o755 }, ); fs.writeFileSync( - path.join(localBin, "bash"), + path.join(localBin, "docker"), [ "#!/bin/sh", `log_file=${JSON.stringify(bashLog)}`, @@ -694,7 +694,7 @@ describe("CLI dispatch", () => { expect(fs.readFileSync(openshellLog, "utf8")).not.toContain("forward stop 18789"); expect(fs.readFileSync(openshellLog, "utf8")).not.toContain("gateway destroy -g nemoclaw"); if (fs.existsSync(bashLog)) { - expect(fs.readFileSync(bashLog, "utf8")).not.toContain("docker volume ls -q --filter"); + expect(fs.readFileSync(bashLog, "utf8")).not.toContain("volume ls -q --filter"); } }); @@ -759,7 +759,7 @@ describe("CLI dispatch", () => { const localBin = path.join(home, "bin"); const registryDir = path.join(home, ".nemoclaw"); const openshellLog = path.join(home, "openshell.log"); - const bashLog = path.join(home, "bash.log"); + const bashLog = path.join(home, "docker.log"); fs.mkdirSync(localBin, { recursive: true }); fs.mkdirSync(registryDir, { recursive: true }); fs.writeFileSync( @@ -799,7 +799,7 @@ describe("CLI dispatch", () => { { mode: 0o755 }, ); fs.writeFileSync( - path.join(localBin, "bash"), + path.join(localBin, "docker"), [ "#!/bin/sh", `log_file=${JSON.stringify(bashLog)}`, @@ -825,7 +825,7 @@ describe("CLI dispatch", () => { expect(fs.readFileSync(openshellLog, "utf8")).toContain("sandbox delete alpha"); expect(fs.readFileSync(openshellLog, "utf8")).toContain("forward stop 18789"); expect(fs.readFileSync(openshellLog, "utf8")).toContain("gateway destroy -g nemoclaw"); - expect(fs.readFileSync(bashLog, "utf8")).toContain("docker volume ls -q --filter"); + expect(fs.readFileSync(bashLog, "utf8")).toContain("volume ls -q --filter"); }); it("deletes messaging providers when destroying a sandbox", () => { @@ -833,7 +833,7 @@ describe("CLI dispatch", () => { const localBin = path.join(home, "bin"); const registryDir = path.join(home, ".nemoclaw"); const openshellLog = path.join(home, "openshell.log"); - const bashLog = path.join(home, "bash.log"); + const bashLog = path.join(home, "docker.log"); fs.mkdirSync(localBin, { recursive: true }); fs.mkdirSync(registryDir, { recursive: true }); fs.writeFileSync( @@ -867,7 +867,7 @@ describe("CLI dispatch", () => { { mode: 0o755 }, ); fs.writeFileSync( - path.join(localBin, "bash"), + path.join(localBin, "docker"), [ "#!/bin/sh", `log_file=${JSON.stringify(bashLog)}`, diff --git a/test/gateway-cleanup.test.ts b/test/gateway-cleanup.test.ts index cf29135ad17..2a5ff555047 100644 --- a/test/gateway-cleanup.test.ts +++ b/test/gateway-cleanup.test.ts @@ -16,7 +16,8 @@ const ROOT = path.resolve(import.meta.dirname, ".."); describe("gateway cleanup: Docker volumes removed on failure (#17)", () => { it("onboard.js: destroyGateway() removes Docker volumes", () => { const content = fs.readFileSync(path.join(ROOT, "src/lib/onboard.ts"), "utf-8"); - expect(content.includes("docker volume") && content.includes("openshell-cluster")).toBe(true); + expect(content).toContain("dockerRemoveVolumesByPrefix"); + expect(content).toContain("openshell-cluster"); }); it("onboard.js: volume cleanup runs on gateway start failure", () => { diff --git a/test/gateway-liveness-probe.test.ts b/test/gateway-liveness-probe.test.ts index 390c04f6fdb..f8f3ae81873 100644 --- a/test/gateway-liveness-probe.test.ts +++ b/test/gateway-liveness-probe.test.ts @@ -19,8 +19,8 @@ describe("gateway liveness probe (#2020)", () => { it("verifyGatewayContainerRunning() helper exists and checks Docker state", () => { expect(content).toContain("function verifyGatewayContainerRunning()"); - // Must use docker inspect to probe container state - expect(content).toContain("docker inspect --type container"); + // Must use a Docker inspect helper to probe container state + expect(content).toMatch(/docker(?:Inspect|ContainerInspectFormat)\(/); // Must check .State.Running, not just container existence expect(content).toContain("{{.State.Running}}"); }); diff --git a/test/image-cleanup.test.ts b/test/image-cleanup.test.ts index 4be41daa146..e8273e79447 100644 --- a/test/image-cleanup.test.ts +++ b/test/image-cleanup.test.ts @@ -15,7 +15,7 @@ describe("image cleanup: sandbox destroy removes Docker image (#2086)", () => { it("removeSandboxImage() helper exists and calls docker rmi", () => { expect(nemoclawSrc).toContain("function removeSandboxImage("); - expect(nemoclawSrc).toMatch(/docker.*rmi/); + expect(nemoclawSrc).toMatch(/dockerRmi|docker.*rmi/); }); it("sandboxDestroy calls removeSandboxImage before registry.removeSandbox", () => { @@ -78,7 +78,7 @@ describe("image cleanup: onboard records imageTag in registry (#2086)", () => { it("onboard recreate path cleans up old image", () => { // When recreating, the old image should be removed expect(onboardSrc).toMatch(/previousEntry\?\.imageTag/); - expect(onboardSrc).toMatch(/docker.*rmi.*previousEntry\.imageTag/); + expect(onboardSrc).toMatch(/dockerRmi|docker.*rmi/); }); }); From 5dd621f10de7f398b3492755174fffc2d82884ff Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Tue, 28 Apr 2026 11:42:47 -0700 Subject: [PATCH 2/9] refactor(cli): route remaining docker calls through helpers --- src/lib/debug.ts | 6 ++-- src/lib/docker/exec.ts | 22 ++++++++++++++ src/lib/docker/image.ts | 10 ++++++- src/lib/docker/index.ts | 2 ++ src/lib/docker/login.ts | 18 +++++++++++ src/lib/nim.ts | 10 ++----- src/lib/sandbox-config.ts | 8 ++--- src/lib/shields.ts | 63 ++++++++++++++++----------------------- src/nemoclaw.ts | 45 ++++++++++++---------------- 9 files changed, 105 insertions(+), 79 deletions(-) create mode 100644 src/lib/docker/exec.ts create mode 100644 src/lib/docker/login.ts diff --git a/src/lib/debug.ts b/src/lib/debug.ts index b1138228da0..6f331b2e238 100644 --- a/src/lib/debug.ts +++ b/src/lib/debug.ts @@ -3,6 +3,7 @@ import { execFileSync, spawnSync } from "node:child_process"; import { existsSync, mkdtempSync, rmSync, unlinkSync, writeFileSync } from "node:fs"; +import { dockerExecFileSync } from "./docker/exec"; import { platform, tmpdir } from "node:os"; import { basename, dirname, join } from "node:path"; @@ -255,10 +256,9 @@ function collectDocker(collectDir: string, quick: boolean): void { // NemoClaw-labelled containers if (commandExists("docker")) { try { - const output = execFileSync( - "docker", + const output = dockerExecFileSync( ["ps", "-a", "--filter", "label=com.nvidia.nemoclaw", "--format", "{{.Names}}"], - { encoding: "utf-8", timeout: TIMEOUT_MS, stdio: ["ignore", "pipe", "ignore"] }, + { timeout: TIMEOUT_MS, stdio: ["ignore", "pipe", "ignore"] }, ); const containers = output.split("\n").filter((c) => c.trim().length > 0); for (const cid of containers) { diff --git a/src/lib/docker/exec.ts b/src/lib/docker/exec.ts new file mode 100644 index 00000000000..a5a960c406f --- /dev/null +++ b/src/lib/docker/exec.ts @@ -0,0 +1,22 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { execFileSync, spawnSync, type ExecFileSyncOptionsWithStringEncoding } from "node:child_process"; + +export type DockerExecFileSyncOptions = Omit; +export type DockerSpawnSyncOptions = Parameters[2]; +export type DockerSpawnSyncResult = ReturnType; + +export function dockerExecFileSync( + args: readonly string[], + opts: DockerExecFileSyncOptions = {}, +): string { + return String(execFileSync("docker", [...args], { encoding: "utf-8", ...opts })); +} + +export function dockerSpawnSync( + args: readonly string[], + opts: DockerSpawnSyncOptions = {}, +): DockerSpawnSyncResult { + return spawnSync("docker", [...args], opts); +} diff --git a/src/lib/docker/image.ts b/src/lib/docker/image.ts index eb39c63cc55..1de29f2f847 100644 --- a/src/lib/docker/image.ts +++ b/src/lib/docker/image.ts @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { ROOT } from "../runner"; -import { dockerRun, type DockerRunOptions } from "./run"; +import { dockerCapture, dockerRun, type DockerCaptureOptions, type DockerRunOptions } from "./run"; export function dockerBuild( dockerfilePath: string, @@ -16,3 +16,11 @@ export function dockerBuild( export function dockerRmi(imageRef: string, opts: DockerRunOptions = {}) { return dockerRun(["rmi", imageRef], opts); } + +export function dockerListImagesFormat( + reference: string, + format: string, + opts: DockerCaptureOptions = {}, +): string { + return dockerCapture(["images", "--filter", `reference=${reference}`, "--format", format], opts); +} diff --git a/src/lib/docker/index.ts b/src/lib/docker/index.ts index 6fbd291b689..9e726a05775 100644 --- a/src/lib/docker/index.ts +++ b/src/lib/docker/index.ts @@ -2,9 +2,11 @@ // SPDX-License-Identifier: Apache-2.0 export * from "./run"; +export * from "./exec"; export * from "./pull"; export * from "./info"; export * from "./inspect"; export * from "./image"; export * from "./container"; export * from "./volume"; +export * from "./login"; diff --git a/src/lib/docker/login.ts b/src/lib/docker/login.ts new file mode 100644 index 00000000000..9548305ffd9 --- /dev/null +++ b/src/lib/docker/login.ts @@ -0,0 +1,18 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { dockerSpawnSync, type DockerSpawnSyncOptions, type DockerSpawnSyncResult } from "./exec"; + +export function dockerLoginPasswordStdin( + registry: string, + username: string, + password: string, + opts: DockerSpawnSyncOptions = {}, +): DockerSpawnSyncResult { + return dockerSpawnSync(["login", registry, "-u", username, "--password-stdin"], { + input: password, + encoding: "utf-8", + stdio: ["pipe", "pipe", "pipe"], + ...opts, + }); +} diff --git a/src/lib/nim.ts b/src/lib/nim.ts index addf81b482d..c8613577645 100644 --- a/src/lib/nim.ts +++ b/src/lib/nim.ts @@ -7,13 +7,14 @@ const { runCapture } = require("./runner"); // eslint-disable-next-line @typescript-eslint/no-require-imports const { + dockerContainerInspectFormat, dockerForceRm, + dockerLoginPasswordStdin, dockerPort, dockerPull, dockerRm, dockerRunDetached, dockerStop, - dockerContainerInspectFormat, } = require("./docker"); // eslint-disable-next-line @typescript-eslint/no-require-imports const { sleepSeconds } = require("./wait"); @@ -213,12 +214,7 @@ export function isNgcLoggedIn(): boolean { // NGC expects literal "$oauthtoken" as the username for API key authentication. export function dockerLoginNgc(apiKey: string): boolean { - const { spawnSync } = require("child_process"); - const result = spawnSync("docker", ["login", "nvcr.io", "-u", "$oauthtoken", "--password-stdin"], { - input: apiKey, - encoding: "utf-8", - stdio: ["pipe", "pipe", "pipe"], - }); + const result = dockerLoginPasswordStdin("nvcr.io", "$oauthtoken", apiKey); if (result.error) { console.error(` Docker error: ${result.error.message}`); return false; diff --git a/src/lib/sandbox-config.ts b/src/lib/sandbox-config.ts index e80d95888ad..527d60c8311 100644 --- a/src/lib/sandbox-config.ts +++ b/src/lib/sandbox-config.ts @@ -16,8 +16,8 @@ const readline = require("readline"); const fs = require("fs"); const os = require("os"); const path = require("path"); -const { execFileSync } = require("child_process"); const { validateName } = require("./runner"); +const { dockerExecFileSync } = require("./docker/exec"); const credentialFilter: typeof import("./credential-filter") = require("./credential-filter"); const { stripCredentials, isConfigObject, isConfigValue } = credentialFilter; const { appendAuditEntry } = require("./shields-audit"); @@ -516,8 +516,7 @@ async function configSet(sandboxName: string, opts: ConfigSetOpts = {}): Promise // Write config to sandbox via kubectl exec (bypasses Landlock) console.log(` Writing config to sandbox (${target.configPath})...`); const content = fs.readFileSync(tmpFile, "utf-8"); - execFileSync( - "docker", + dockerExecFileSync( [ "exec", "-i", @@ -540,8 +539,7 @@ async function configSet(sandboxName: string, opts: ConfigSetOpts = {}): Promise // Fix ownership via kubectl exec (bypasses Landlock) try { - execFileSync( - "docker", + dockerExecFileSync( [ "exec", K3S_CONTAINER, diff --git a/src/lib/shields.ts b/src/lib/shields.ts index 79b920064fb..fd5565641de 100644 --- a/src/lib/shields.ts +++ b/src/lib/shields.ts @@ -11,8 +11,9 @@ const fs = require("fs"); const path = require("path"); -const { fork, execFileSync } = require("child_process"); +const { fork } = require("child_process"); const { run, runCapture, validateName, shellQuote } = require("./runner"); +const { dockerExecFileSync } = require("./docker/exec"); const { buildPolicyGetCommand, buildPolicySetCommand, @@ -36,46 +37,34 @@ const STATE_DIR = path.join(process.env.HOME ?? "/tmp", ".nemoclaw", "state"); const K3S_CONTAINER = "openshell-cluster-nemoclaw"; +function kubectlExecArgv(sandboxName: string, cmd: string[]): string[] { + return [ + "exec", + K3S_CONTAINER, + "kubectl", + "exec", + "-n", + "openshell", + sandboxName, + "-c", + "agent", + "--", + ...cmd, + ]; +} + function kubectlExec(sandboxName: string, cmd: string[]): void { - execFileSync( - "docker", - [ - "exec", - K3S_CONTAINER, - "kubectl", - "exec", - "-n", - "openshell", - sandboxName, - "-c", - "agent", - "--", - ...cmd, - ], - { stdio: ["ignore", "pipe", "pipe"], timeout: 15000 }, - ); + dockerExecFileSync(kubectlExecArgv(sandboxName, cmd), { + stdio: ["ignore", "pipe", "pipe"], + timeout: 15000, + }); } function kubectlExecCapture(sandboxName: string, cmd: string[]): string { - return execFileSync( - "docker", - [ - "exec", - K3S_CONTAINER, - "kubectl", - "exec", - "-n", - "openshell", - sandboxName, - "-c", - "agent", - "--", - ...cmd, - ], - { stdio: ["ignore", "pipe", "pipe"], timeout: 15000 }, - ) - .toString() - .trim(); + return dockerExecFileSync(kubectlExecArgv(sandboxName, cmd), { + stdio: ["ignore", "pipe", "pipe"], + timeout: 15000, + }).trim(); } // Re-export for tests and external consumers diff --git a/src/nemoclaw.ts b/src/nemoclaw.ts index 82931de64ad..ce5acb352ea 100644 --- a/src/nemoclaw.ts +++ b/src/nemoclaw.ts @@ -21,15 +21,13 @@ const R = _useColor ? "\x1b[0m" : ""; const _RD = _useColor ? "\x1b[1;31m" : ""; const YW = _useColor ? "\x1b[1;33m" : ""; +const { ROOT, run, runInteractive, shellQuote, validateName } = require("./lib/runner"); const { - ROOT, - run, - runCapture: _runCapture, - runInteractive, - shellQuote, - validateName, -} = require("./lib/runner"); -const { dockerRemoveVolumesByPrefix, dockerRmi } = require("./lib/docker"); + dockerCapture, + dockerListImagesFormat, + dockerRemoveVolumesByPrefix, + dockerRmi, +} = require("./lib/docker"); const { resolveOpenshell } = require("./lib/resolve-openshell"); const { fetchGatewayAuthTokenFromSandbox, @@ -3163,8 +3161,7 @@ function renderSnapshotTable( function resolveSrcPodImage(srcName: string): string | null { const gatewayContainer = `openshell-cluster-${NEMOCLAW_GATEWAY_NAME}`; try { - const result = spawnSync( - "docker", + const output = dockerCapture( [ "exec", gatewayContainer, @@ -3177,10 +3174,9 @@ function resolveSrcPodImage(srcName: string): string | null { "-o", 'jsonpath={.spec.containers[?(@.name=="agent")].image}', ], - { encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"], timeout: 10000 }, + { ignoreError: true, timeout: 10000 }, ); - if (result.status !== 0) return null; - const img = (result.stdout || "").trim().split(/\s+/)[0]; + const img = output.trim().split(/\s+/)[0]; return img || null; } catch { return null; @@ -3540,23 +3536,18 @@ async function garbageCollectImages(args: string[] = []): Promise { const skipConfirm = args.includes("--yes") || args.includes("--force"); // 1. List all openshell/sandbox-from images on the host - const imagesResult = spawnSync( - "docker", - [ - "images", - "--filter", - "reference=openshell/sandbox-from", - "--format", + let imagesOutput = ""; + try { + imagesOutput = dockerListImagesFormat( + "openshell/sandbox-from", "{{.Repository}}:{{.Tag}}\t{{.Size}}", - ], - { encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"] }, - ); - if (imagesResult.status !== 0) { + ); + } catch { console.error(" Failed to query Docker images. Is Docker running?"); process.exit(1); } - const allImages = (imagesResult.stdout || "") + const allImages = imagesOutput .split("\n") .map((line: string) => line.trim()) .filter(Boolean) @@ -3612,9 +3603,11 @@ async function garbageCollectImages(args: string[] = []): Promise { let removed = 0; let failed = 0; for (const img of orphans) { - const rmiResult = spawnSync("docker", ["rmi", img.tag], { + const rmiResult = dockerRmi(img.tag, { encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"], + ignoreError: true, + suppressOutput: true, }); if (rmiResult.status === 0) { console.log(` ${G}✓${R} Removed ${img.tag}`); From 1d18a31cdacf3c5d13f8cb3f7ed2155b5e4a818e Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Tue, 28 Apr 2026 11:55:46 -0700 Subject: [PATCH 3/9] fix(cli): harden docker helper migration --- src/lib/agent-onboard.ts | 2 +- src/lib/docker/index.test.ts | 18 +++++++++++++++++- src/lib/docker/volume.ts | 24 +++++++++++++++++++++--- src/lib/onboard.ts | 17 ++++++----------- test/gateway-cleanup.test.ts | 7 +++++-- test/gateway-liveness-probe.test.ts | 8 +++++--- test/image-cleanup.test.ts | 12 ++++++++---- 7 files changed, 63 insertions(+), 25 deletions(-) diff --git a/src/lib/agent-onboard.ts b/src/lib/agent-onboard.ts index 0cb18593293..efe5151a1d7 100644 --- a/src/lib/agent-onboard.ts +++ b/src/lib/agent-onboard.ts @@ -9,7 +9,7 @@ import fs from "fs"; import os from "os"; import path from "path"; -import { ROOT } from "./runner"; +import { ROOT, run } from "./runner"; import { dockerBuild, dockerImageInspect } from "./docker"; import { loadAgent, resolveAgentName, type AgentDefinition } from "./agent-defs"; import { getProviderSelectionConfig } from "./inference-config"; diff --git a/src/lib/docker/index.test.ts b/src/lib/docker/index.test.ts index f0640d04d89..0011a226c71 100644 --- a/src/lib/docker/index.test.ts +++ b/src/lib/docker/index.test.ts @@ -91,7 +91,7 @@ describe("docker helpers", () => { it("removes only volumes returned by the prefix probe", () => { runCaptureMock.mockReturnValue("openshell-cluster-nemoclaw\nopenshell-cluster-nemoclaw-cache\n"); - const removed = dockerRemoveVolumesByPrefix("openshell-cluster-nemoclaw", { + const removed = dockerRemoveVolumesByPrefix(" openshell-cluster-nemoclaw ", { ignoreError: true, }); @@ -111,6 +111,22 @@ describe("docker helpers", () => { ); }); + it("rejects empty volume prefixes", () => { + expect(() => dockerListVolumesByPrefix(" ")).toThrow(/prefix must be a non-empty string/); + expect(() => dockerRemoveVolumesByPrefix("\t")).toThrow(/prefix must be a non-empty string/); + }); + + it("treats failed volume probes as empty when ignoreError is set", () => { + runCaptureMock.mockImplementation(() => { + throw new Error("docker unavailable"); + }); + + expect(dockerRemoveVolumesByPrefix("openshell-cluster-nemoclaw", { ignoreError: true })).toEqual( + [], + ); + expect(runMock).not.toHaveBeenCalled(); + }); + it("skips volume removal when the probe finds no matches", () => { runCaptureMock.mockReturnValue(""); diff --git a/src/lib/docker/volume.ts b/src/lib/docker/volume.ts index fdeba538d1e..3ff5a7d9f95 100644 --- a/src/lib/docker/volume.ts +++ b/src/lib/docker/volume.ts @@ -10,15 +10,24 @@ function splitNonEmptyLines(output: string): string[] { .filter(Boolean); } +function normalizeVolumePrefix(prefix: string): string { + const normalized = prefix.trim(); + if (!normalized) { + throw new Error("prefix must be a non-empty string"); + } + return normalized; +} + export function dockerListVolumesByPrefix( prefix: string, opts: DockerCaptureOptions = {}, ): string[] { - const output = dockerCapture(["volume", "ls", "-q", "--filter", `name=${prefix}`], { + const normalized = normalizeVolumePrefix(prefix); + const output = dockerCapture(["volume", "ls", "-q", "--filter", `name=${normalized}`], { ignoreError: true, ...opts, }); - return splitNonEmptyLines(output).filter((name) => name.startsWith(prefix)); + return splitNonEmptyLines(output).filter((name) => name.startsWith(normalized)); } export function dockerRemoveVolumes(names: readonly string[], opts: DockerRunOptions = {}) { @@ -30,7 +39,16 @@ export function dockerRemoveVolumesByPrefix( prefix: string, opts: DockerRunOptions = {}, ): string[] { - const names = dockerListVolumesByPrefix(prefix); + const normalized = normalizeVolumePrefix(prefix); + let names: string[]; + try { + names = dockerListVolumesByPrefix(normalized, { + ignoreError: opts.ignoreError, + }); + } catch (error) { + if (opts.ignoreError) return []; + throw error; + } if (names.length === 0) return names; dockerRemoveVolumes(names, opts); return names; diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 11e08c7cdd7..39b249e6ad5 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -602,21 +602,16 @@ const SANDBOX_BASE_TAG = "latest"; */ function pullAndResolveBaseImageDigest(): { digest: string; ref: string } | null { const imageWithTag = `${SANDBOX_BASE_IMAGE}:${SANDBOX_BASE_TAG}`; - try { - dockerPull(imageWithTag, { suppressOutput: true }); - } catch { + const pullResult = dockerPull(imageWithTag, { ignoreError: true, suppressOutput: true }); + if (pullResult.status !== 0) { // Pull failed — caller should fall back to unpin :latest return null; } - let inspectOutput; - try { - inspectOutput = dockerImageInspectFormat("{{json .RepoDigests}}", imageWithTag, { - ignoreError: false, - }); - } catch { - return null; - } + const inspectOutput = dockerImageInspectFormat("{{json .RepoDigests}}", imageWithTag, { + ignoreError: true, + }); + if (!inspectOutput) return null; // RepoDigests is a JSON array like ["ghcr.io/nvidia/nemoclaw/sandbox-base@sha256:abc..."]. // Filter to the entry matching our registry — index ordering is not guaranteed. diff --git a/test/gateway-cleanup.test.ts b/test/gateway-cleanup.test.ts index 2a5ff555047..20dd13eddbd 100644 --- a/test/gateway-cleanup.test.ts +++ b/test/gateway-cleanup.test.ts @@ -16,8 +16,11 @@ const ROOT = path.resolve(import.meta.dirname, ".."); describe("gateway cleanup: Docker volumes removed on failure (#17)", () => { it("onboard.js: destroyGateway() removes Docker volumes", () => { const content = fs.readFileSync(path.join(ROOT, "src/lib/onboard.ts"), "utf-8"); - expect(content).toContain("dockerRemoveVolumesByPrefix"); - expect(content).toContain("openshell-cluster"); + const match = content.match(/function destroyGateway\([\s\S]*?^}/m); + expect(match).toBeTruthy(); + if (!match) throw new Error("Expected destroyGateway() in src/lib/onboard.ts"); + expect(match[0]).toContain("dockerRemoveVolumesByPrefix"); + expect(match[0]).toContain("openshell-cluster"); }); it("onboard.js: volume cleanup runs on gateway start failure", () => { diff --git a/test/gateway-liveness-probe.test.ts b/test/gateway-liveness-probe.test.ts index f8f3ae81873..b69096bc664 100644 --- a/test/gateway-liveness-probe.test.ts +++ b/test/gateway-liveness-probe.test.ts @@ -18,11 +18,13 @@ describe("gateway liveness probe (#2020)", () => { const content = fs.readFileSync(path.join(ROOT, "src/lib/onboard.ts"), "utf-8"); it("verifyGatewayContainerRunning() helper exists and checks Docker state", () => { - expect(content).toContain("function verifyGatewayContainerRunning()"); + const match = content.match(/function verifyGatewayContainerRunning\([\s\S]*?^}/m); + expect(match).toBeTruthy(); + if (!match) throw new Error("Expected verifyGatewayContainerRunning() in src/lib/onboard.ts"); // Must use a Docker inspect helper to probe container state - expect(content).toMatch(/docker(?:Inspect|ContainerInspectFormat)\(/); + expect(match[0]).toMatch(/docker(?:Inspect|ContainerInspectFormat)\(/); // Must check .State.Running, not just container existence - expect(content).toContain("{{.State.Running}}"); + expect(match[0]).toContain("{{.State.Running}}"); }); it("preflight probes the container when gatewayReuseState is 'healthy'", () => { diff --git a/test/image-cleanup.test.ts b/test/image-cleanup.test.ts index e8273e79447..1e541247d00 100644 --- a/test/image-cleanup.test.ts +++ b/test/image-cleanup.test.ts @@ -14,8 +14,10 @@ describe("image cleanup: sandbox destroy removes Docker image (#2086)", () => { const nemoclawSrc = fs.readFileSync(path.join(ROOT, "src/nemoclaw.ts"), "utf-8"); it("removeSandboxImage() helper exists and calls docker rmi", () => { - expect(nemoclawSrc).toContain("function removeSandboxImage("); - expect(nemoclawSrc).toMatch(/dockerRmi|docker.*rmi/); + const match = nemoclawSrc.match(/function removeSandboxImage[\s\S]*?^}/m); + expect(match).toBeTruthy(); + if (!match) throw new Error("Expected removeSandboxImage() in src/nemoclaw.ts"); + expect(match[0]).toMatch(/dockerRmi\(|docker.*\.rmi\(/); }); it("sandboxDestroy calls removeSandboxImage before registry.removeSandbox", () => { @@ -77,8 +79,10 @@ describe("image cleanup: onboard records imageTag in registry (#2086)", () => { it("onboard recreate path cleans up old image", () => { // When recreating, the old image should be removed - expect(onboardSrc).toMatch(/previousEntry\?\.imageTag/); - expect(onboardSrc).toMatch(/dockerRmi|docker.*rmi/); + const match = onboardSrc.match(/if \(previousEntry\?\.imageTag\)[\s\S]*?^\s*}/m); + expect(match).toBeTruthy(); + if (!match) throw new Error("Expected previousEntry image cleanup block in src/lib/onboard.ts"); + expect(match[0]).toMatch(/dockerRmi\(|docker.*\.rmi\(/); }); }); From 38b4a2b2e9f2ca989b1dc4f5e1b5492917d503d6 Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Tue, 28 Apr 2026 13:18:42 -0700 Subject: [PATCH 4/9] test(cli): guard docker abstraction boundary --- test/docker-abstraction-guard.test.ts | 65 +++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 test/docker-abstraction-guard.test.ts diff --git a/test/docker-abstraction-guard.test.ts b/test/docker-abstraction-guard.test.ts new file mode 100644 index 00000000000..00ee6758da1 --- /dev/null +++ b/test/docker-abstraction-guard.test.ts @@ -0,0 +1,65 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { spawnSync } from "node:child_process"; +import path from "node:path"; + +import { describe, expect, it } from "vitest"; + +const REPO_ROOT = path.join(import.meta.dirname, ".."); +const TSX = path.join(REPO_ROOT, "node_modules", ".bin", "tsx"); +const INVENTORY_SCRIPT = path.join(REPO_ROOT, "scripts", "list-command-helper-uses.ts"); +const DOCKER_ABSTRACTION_PREFIX = "src/lib/docker/"; + +type CommandUse = { + filePath: string; + line: number; + column: number; + kind: "call" | "assign"; + name: string; + expression: string; + commandHead: string | null; + snippet: string; +}; + +function listCommandUses(): CommandUse[] { + const result = spawnSync( + TSX, + [INVENTORY_SCRIPT, "--json", "--list-calls", "src"], + { + cwd: REPO_ROOT, + encoding: "utf-8", + }, + ); + expect(result.status).toBe(0); + return JSON.parse(result.stdout) as CommandUse[]; +} + +describe("Docker command abstraction guard", () => { + it("keeps direct docker process launches inside src/lib/docker", () => { + const directDockerCalls = listCommandUses().filter( + (entry) => + entry.kind === "call" && + entry.commandHead === "docker" && + !entry.filePath.startsWith(DOCKER_ABSTRACTION_PREFIX), + ); + + if (directDockerCalls.length > 0) { + const formatted = directDockerCalls + .map( + (entry) => + ` - ${entry.filePath}:${String(entry.line)}:${String(entry.column)} ${entry.expression}: ${entry.snippet}`, + ) + .join("\n"); + throw new Error( + `Direct docker process launches must use the src/lib/docker abstractions.\n` + + `Do not add calls like run([\"docker\", ...]), runCapture([\"docker\", ...]), ` + + `spawnSync(\"docker\", ...), or execFileSync(\"docker\", ...) outside ${DOCKER_ABSTRACTION_PREFIX}.\n` + + `Add or reuse a helper under ${DOCKER_ABSTRACTION_PREFIX} (for example dockerRun(), dockerCapture(), dockerPull(), dockerRmi(), dockerExecFileSync(), or dockerSpawnSync()) and call that instead.\n` + + `Violations:\n${formatted}`, + ); + } + + expect(directDockerCalls).toEqual([]); + }); +}); From 0b6fb134dd880bed506e8f29fb26823c26d10b83 Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Tue, 28 Apr 2026 13:27:17 -0700 Subject: [PATCH 5/9] fix: update test/docker-abstraction-guard.test.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- test/docker-abstraction-guard.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/docker-abstraction-guard.test.ts b/test/docker-abstraction-guard.test.ts index 00ee6758da1..8673ab18cb3 100644 --- a/test/docker-abstraction-guard.test.ts +++ b/test/docker-abstraction-guard.test.ts @@ -53,8 +53,8 @@ describe("Docker command abstraction guard", () => { .join("\n"); throw new Error( `Direct docker process launches must use the src/lib/docker abstractions.\n` + - `Do not add calls like run([\"docker\", ...]), runCapture([\"docker\", ...]), ` + - `spawnSync(\"docker\", ...), or execFileSync(\"docker\", ...) outside ${DOCKER_ABSTRACTION_PREFIX}.\n` + + `Do not add calls like run(["docker", ...]), runCapture(["docker", ...]), ` + + `spawnSync("docker", ...), or execFileSync("docker", ...) outside ${DOCKER_ABSTRACTION_PREFIX}.\n` + `Add or reuse a helper under ${DOCKER_ABSTRACTION_PREFIX} (for example dockerRun(), dockerCapture(), dockerPull(), dockerRmi(), dockerExecFileSync(), or dockerSpawnSync()) and call that instead.\n` + `Violations:\n${formatted}`, ); From e5e208d0370a782e86a0a1353ae36ac70481b6b6 Mon Sep 17 00:00:00 2001 From: Deepak Jain Date: Tue, 28 Apr 2026 13:27:59 -0700 Subject: [PATCH 6/9] fix(skill): clarify plugin install errors (Fixes #2536) (#2585) ## Summary `nemoclaw skill install --help` was treated like a path, and plugin-shaped directories only got a generic missing SKILL.md error. This PR makes the help path work and gives plugin users a clearer next step. ## Changes - Print skill install usage when `--help`, `-h`, or `help` follows `install`. - Detect OpenClaw plugin-shaped directories through `openclaw.plugin.json` or `package.json` metadata. - Add a targeted hint that plugins should be baked into a custom sandbox image with `nemoclaw onboard --from`. - Add CLI tests for both flows. ## Testing - `npm run build:cli` passed. - `npm run typecheck:cli` passed. - `npm test -- test/cli.test.ts` passed: 61 tests. - Full `npm test -- --reporter=dot` was attempted. In this local checkout it still fails outside this change in installer/uninstall/onboard/build-context tests, including temp-source generated-dist lookup and a few timeout/status checks. ## Evidence it works The CLI test now verifies that `skill install --help` returns usage without a missing-file error and that plugin-shaped directories get the OpenClaw plugin hint. Fixes #2536 Signed-off-by: Deepak Jain ## Summary by CodeRabbit * **Bug Fixes** * Improved error detection and messaging for `skill install` when given plugin-shaped directories, and added a clear suggestion to use the sandbox onboarding workflow instead. * Enhanced `--help` handling to display skill-install usage immediately, including when `--help` is passed as the positional path. * **Tests** * Expanded CLI tests for `skill install` help and plugin-detection scenarios, and restored related start-test coverage. Signed-off-by: Deepak Jain --- src/nemoclaw.ts | 81 ++++++++++++++++++++----- test/cli.test.ts | 152 +++++++++++++++++++++++++++++++++++------------ 2 files changed, 178 insertions(+), 55 deletions(-) diff --git a/src/nemoclaw.ts b/src/nemoclaw.ts index 603310e9518..c57d5ca942a 100644 --- a/src/nemoclaw.ts +++ b/src/nemoclaw.ts @@ -36,10 +36,7 @@ const { ensureOllamaAuthProxy, isNonInteractive, } = require("./lib/onboard"); -const { - parseGatewayTokenArgs, - runGatewayTokenCommand, -} = require("./lib/gateway-token-command"); +const { parseGatewayTokenArgs, runGatewayTokenCommand } = require("./lib/gateway-token-command"); const { getCredential, deleteCredential, @@ -1863,7 +1860,9 @@ async function sandboxPolicyAdd(sandboxName: string, args: string[] = []): Promi } const files = fs .readdirSync(absDir, { withFileTypes: true }) - .filter((ent: { name: string; isFile(): boolean }) => ent.isFile() && /\.ya?ml$/i.test(ent.name)) + .filter( + (ent: { name: string; isFile(): boolean }) => ent.isFile() && /\.ya?ml$/i.test(ent.name), + ) .map((ent: { name: string }) => path.join(absDir, ent.name)) .sort(); if (files.length === 0) { @@ -2190,6 +2189,56 @@ async function sandboxChannelsStart(sandboxName: string, args: string[] = []): P await sandboxChannelsSetEnabled(sandboxName, args, false); } +function printSkillInstallUsage(): void { + console.log(""); + console.log(" Usage: nemoclaw skill install "); + console.log(""); + console.log(" Deploy a skill directory to a running sandbox."); + console.log( + " must be a skill directory containing a SKILL.md (with 'name:' frontmatter),", + ); + console.log( + " or a direct path to a SKILL.md file. All non-dot files in the directory are uploaded.", + ); + console.log(""); +} + +function looksLikeOpenClawPlugin(candidatePath: string): boolean { + const dir = + fs.existsSync(candidatePath) && fs.statSync(candidatePath).isDirectory() + ? candidatePath + : path.dirname(candidatePath); + if (!fs.existsSync(dir)) return false; + if (fs.existsSync(path.join(dir, "openclaw.plugin.json"))) return true; + + const packageJsonPath = path.join(dir, "package.json"); + if (!fs.existsSync(packageJsonPath)) return false; + try { + const packageJson = JSON.parse(fs.readFileSync(packageJsonPath, "utf-8")); + const openclawBlock = packageJson?.openclaw; + return Boolean( + packageJson?.["openclaw.plugin"] === true || + openclawBlock === true || + (typeof openclawBlock === "object" && + openclawBlock !== null && + (openclawBlock.plugin === true || + typeof openclawBlock.entry === "string" || + typeof openclawBlock.main === "string" || + (Array.isArray(openclawBlock.extensions) && openclawBlock.extensions.length > 0))), + ); + } catch { + return false; + } +} + +function printPluginInstallHint(): void { + console.error(" This looks like an OpenClaw plugin, not a SKILL.md agent skill."); + console.error(" `skill install` only accepts skill directories or direct SKILL.md paths."); + console.error( + " To use an OpenClaw plugin today, bake it into a custom sandbox image with `nemoclaw onboard --from `.", + ); +} + /** * Install or update a local skill directory into a live sandbox and perform * any agent-specific post-install refresh needed for the new content to load. @@ -2197,17 +2246,7 @@ async function sandboxChannelsStart(sandboxName: string, args: string[] = []): P async function sandboxSkillInstall(sandboxName: string, args: string[] = []): Promise { const sub = args[0]; if (!sub || sub === "help" || sub === "--help" || sub === "-h") { - console.log(""); - console.log(" Usage: nemoclaw skill install "); - console.log(""); - console.log(" Deploy a skill directory to a running sandbox."); - console.log( - " must be a skill directory containing a SKILL.md (with 'name:' frontmatter),", - ); - console.log( - " or a direct path to a SKILL.md file. All non-dot files in the directory are uploaded.", - ); - console.log(""); + printSkillInstallUsage(); return; } @@ -2219,6 +2258,10 @@ async function sandboxSkillInstall(sandboxName: string, args: string[] = []): Pr const skillPath = args[1]; const extraArgs = args.slice(2); + if (skillPath === "--help" || skillPath === "-h" || skillPath === "help") { + printSkillInstallUsage(); + return; + } if (extraArgs.length > 0) { console.error(` Unknown argument(s) for skill install: ${extraArgs.join(", ")}`); console.error(" Usage: nemoclaw skill install "); @@ -2244,12 +2287,18 @@ async function sandboxSkillInstall(sandboxName: string, args: string[] = []): Pr } else { console.error(` No SKILL.md found at '${resolvedPath}'.`); console.error(" must be a skill directory or a direct path to SKILL.md."); + if (looksLikeOpenClawPlugin(resolvedPath)) { + printPluginInstallHint(); + } process.exit(1); } if (!fs.existsSync(skillMdPath)) { console.error(` No SKILL.md found in '${skillDir}'.`); console.error(" The skill directory must contain a SKILL.md file."); + if (looksLikeOpenClawPlugin(skillDir)) { + printPluginInstallHint(); + } process.exit(1); } diff --git a/test/cli.test.ts b/test/cli.test.ts index e5616b62884..cfeba520ab8 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -90,6 +90,27 @@ function readRecordedArgs(markerFile: string): string[] { return fs.readFileSync(markerFile, "utf8").trim().split(/\s+/); } +function writeSandboxRegistry(home: string): void { + const registryDir = path.join(home, ".nemoclaw"); + 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 }, + ); +} + describe("CLI dispatch", () => { it("help exits 0 and shows sections", () => { const r = run("help"); @@ -136,53 +157,106 @@ describe("CLI dispatch", () => { expect(r.out.includes("No sandboxes")).toBeTruthy(); }); - it("start does not prompt for NVIDIA_API_KEY before launching local services", { timeout: 35000 }, () => { - const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-cli-start-no-key-")); - const localBin = path.join(home, "bin"); - const registryDir = path.join(home, ".nemoclaw"); - const markerFile = path.join(home, "start-args"); - fs.mkdirSync(localBin, { recursive: true }); - fs.mkdirSync(registryDir, { recursive: true }); + it("shows skill install help when --help follows install", () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-cli-skill-help-")); + writeSandboxRegistry(home); + + const r = runWithEnv("alpha skill install --help", { HOME: home }); + + expect(r.code).toBe(0); + expect(r.out).toContain("Usage: nemoclaw skill install "); + expect(r.out).toContain("Deploy a skill directory"); + expect(r.out).not.toContain("--help"); + expect(r.out).not.toContain("No SKILL.md found"); + }); + + it("points plugin-shaped directories away from skill install", () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-cli-plugin-hint-")); + const pluginDir = path.join(home, "openclaw-plugin"); + fs.mkdirSync(pluginDir, { recursive: true }); + writeSandboxRegistry(home); 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 }, + path.join(pluginDir, "package.json"), + JSON.stringify({ name: "demo-plugin", openclaw: { extensions: ["./dist/index.js"] } }), ); + + const r = runWithEnv(`alpha skill install ${JSON.stringify(pluginDir)}`, { HOME: home }); + + expect(r.code).toBe(1); + expect(r.out).toContain("No SKILL.md found in"); + expect(r.out).toContain("This looks like an OpenClaw plugin"); + expect(r.out).toContain("nemoclaw onboard --from "); + }); + + it("detects openclaw.plugin.json as a plugin marker", () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-cli-plugin-marker-")); + const pluginDir = path.join(home, "openclaw-plugin"); + fs.mkdirSync(pluginDir, { recursive: true }); + writeSandboxRegistry(home); fs.writeFileSync( - path.join(localBin, "bash"), - [ - "#!/bin/sh", - `marker_file=${JSON.stringify(markerFile)}`, - 'printf \'%s\\n\' "$@" > "$marker_file"', - "exit 0", - ].join("\n"), - { mode: 0o755 }, + path.join(pluginDir, "openclaw.plugin.json"), + JSON.stringify({ name: "demo" }), ); - const r = runWithEnv("start", { - HOME: home, - PATH: `${localBin}:${process.env.PATH || ""}`, - NVIDIA_API_KEY: "", - TELEGRAM_BOT_TOKEN: "", - }); + const r = runWithEnv(`alpha skill install ${JSON.stringify(pluginDir)}`, { HOME: home }); - expect(r.code).toBe(0); - expect(r.out).not.toContain("NVIDIA API Key required"); - // Services module now runs in-process (no bash shelling) - expect(r.out).toContain("NemoClaw Services"); + expect(r.code).toBe(1); + expect(r.out).toContain("No SKILL.md found in"); + expect(r.out).toContain("This looks like an OpenClaw plugin"); + expect(r.out).toContain("nemoclaw onboard --from "); }); + it( + "start does not prompt for NVIDIA_API_KEY before launching local services", + { timeout: 35000 }, + () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-cli-start-no-key-")); + const localBin = path.join(home, "bin"); + const registryDir = path.join(home, ".nemoclaw"); + const markerFile = path.join(home, "start-args"); + 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, "bash"), + [ + "#!/bin/sh", + `marker_file=${JSON.stringify(markerFile)}`, + 'printf \'%s\\n\' "$@" > "$marker_file"', + "exit 0", + ].join("\n"), + { mode: 0o755 }, + ); + + const r = runWithEnv("start", { + HOME: home, + PATH: `${localBin}:${process.env.PATH || ""}`, + NVIDIA_API_KEY: "", + TELEGRAM_BOT_TOKEN: "", + }); + + expect(r.code).toBe(0); + expect(r.out).not.toContain("NVIDIA API Key required"); + // Services module now runs in-process (no bash shelling) + expect(r.out).toContain("NemoClaw Services"); + }, + ); + it("onboard --help exits 0 and shows usage", () => { const r = run("onboard --help"); expect(r.code).toBe(0); From de1b2d2f3d7f5a533f04b5cf63dfc65d19cc9721 Mon Sep 17 00:00:00 2001 From: Deepak Jain Date: Tue, 28 Apr 2026 13:31:51 -0700 Subject: [PATCH 7/9] docs: add OpenClaw plugin install guide (Fixes #2538) (#2587) ## Summary NemoClaw docs did not explain how OpenClaw plugins should be installed under NemoClaw. This adds a deployment guide that points plugin users to the supported custom-sandbox-image path. ## Changes - Add an OpenClaw plugin install guide with a build-directory layout and Dockerfile example. - Link the guide from the docs deployment index. - Add a note under `skill install` that OpenClaw plugins are different from SKILL.md agent skills. - Regenerate the generated `nemoclaw-user` skills content. ## Testing - `python3 scripts/docs-to-skills.py docs/ .agents/skills/ --prefix nemoclaw-user` passed. - `npm test -- test/skills-frontmatter.test.ts` passed: 38 tests. - `npm run build:cli` passed. - `npx prettier --check docs/deployment/install-openclaw-plugins.md docs/index.md docs/reference/commands.md .agents/skills/nemoclaw-user-deploy-remote/SKILL.md .agents/skills/nemoclaw-user-reference/references/commands.md` passed. - Full `npm test -- --reporter=dot` was attempted. In this local checkout it still fails in unrelated installer/uninstall/onboard tests and generated-dist lookup paths. ## Evidence it works The docs generator picked up the new plugin install page and updated the generated deployment skill content. Fixes #2538 Signed-off-by: Deepak Jain ## Summary by CodeRabbit * **Documentation** * Added comprehensive OpenClaw plugins deployment guide with Dockerfile-based preparation, build and onboarding workflow, configuration notes, and troubleshooting. * Clarified that OpenClaw plugins are distinct from agent skills and require a separate installation process; updated command reference links accordingly. * Updated documentation navigation and renumbered onboarding steps to account for the new plugin content. Signed-off-by: Deepak Jain Co-authored-by: Carlos Villela --- .../nemoclaw-user-deploy-remote/SKILL.md | 95 ++++++++++++++- .../references/commands.md | 1 + docs/deployment/install-openclaw-plugins.md | 113 ++++++++++++++++++ docs/index.md | 1 + docs/reference/commands.md | 1 + 5 files changed, 205 insertions(+), 6 deletions(-) create mode 100644 docs/deployment/install-openclaw-plugins.md diff --git a/.agents/skills/nemoclaw-user-deploy-remote/SKILL.md b/.agents/skills/nemoclaw-user-deploy-remote/SKILL.md index c24687d4f70..afddff5e305 100644 --- a/.agents/skills/nemoclaw-user-deploy-remote/SKILL.md +++ b/.agents/skills/nemoclaw-user-deploy-remote/SKILL.md @@ -1,6 +1,6 @@ --- name: "nemoclaw-user-deploy-remote" -description: "Explains how to run NemoClaw on a remote GPU instance, including the deprecated Brev compatibility path and the preferred installer plus onboard flow. Use when deploying NemoClaw to a remote VM, onboarding a Brev instance, or migrating away from the legacy `nemoclaw deploy` wrapper. Trigger keywords - deploy nemoclaw remote gpu, nemoclaw brev cloud deployment, nemoclaw sandbox hardening, container security, docker capabilities, process limits, nemoclaw telegram, telegram bot openclaw agent, openshell channel messaging." +description: "Explains how to run NemoClaw on a remote GPU instance, including the deprecated Brev compatibility path and the preferred installer plus onboard flow. Use when deploying NemoClaw to a remote VM, onboarding a Brev instance, or migrating away from the legacy `nemoclaw deploy` wrapper. Trigger keywords - deploy nemoclaw remote gpu, nemoclaw brev cloud deployment, nemoclaw plugins, openclaw plugins, install openclaw plugin, nemoclaw onboard from dockerfile, nemoclaw sandbox hardening, container security, docker capabilities, process limits, nemoclaw telegram, telegram bot openclaw agent, openshell channel messaging." --- @@ -138,18 +138,101 @@ $ nemoclaw deploy --- +OpenClaw plugins extend the OpenClaw runtime with hooks, services, tools, or +provider integrations. They are different from NemoClaw-managed agent skills: + +- **Plugins** are code packages loaded by OpenClaw. +- **Skills** are `SKILL.md` directories that teach an agent how to perform a task. +- **Policy presets** are network-egress rules that control what sandboxed code can reach. + +Today, the supported NemoClaw path for OpenClaw plugins is to bake the plugin +into a custom sandbox image and onboard from that Dockerfile. + +## Step 9: Prepare a Build Directory + +Put the Dockerfile and everything it needs to `COPY` in one directory. +`nemoclaw onboard --from ` uses the Dockerfile's parent directory as +the Docker build context. + +```text +my-plugin-sandbox/ +├── Dockerfile +└── my-plugin/ + ├── package.json + └── src/ +``` + +## Step 10: Example Dockerfile + +Use the custom image to copy the plugin into the OpenClaw extensions directory +and let OpenClaw refresh its config before NemoClaw starts the sandbox. + +```dockerfile +ARG SANDBOX_BASE=ghcr.io/nvidia/nemoclaw/sandbox-base:latest +FROM ${SANDBOX_BASE} + +COPY my-plugin/ /opt/my-plugin/ +WORKDIR /opt/my-plugin +RUN npm ci --no-audit --no-fund && npm run build + +RUN mkdir -p /sandbox/.openclaw-data/extensions \ + && cp -a /opt/my-plugin /sandbox/.openclaw-data/extensions/my-plugin \ + && openclaw doctor --fix + +WORKDIR /opt/nemoclaw +``` + +If the plugin needs configuration in `openclaw.json`, apply it after +`openclaw doctor --fix` so the base config exists first. + +## Step 11: Create the Sandbox + +Point `nemoclaw onboard --from` at the Dockerfile in the build directory. + +```console +$ nemoclaw onboard --from ./my-plugin-sandbox/Dockerfile +``` + +If you need a second sandbox alongside an existing one, use a dedicated build +directory and rerun onboarding with the sandbox name and ports you intend to +use. + +## Step 12: Network Access + +Plugins still run inside the sandbox policy boundary. If a plugin needs network +egress, add or update a policy preset for the required hostnames and binaries +before rebuilding the sandbox. + +For example, see Network Policies (use the `nemoclaw-user-reference` skill) for +policy concepts and Customize Network Policy (use the `nemoclaw-user-manage-policy` skill) +for custom preset workflows. + +## Step 13: Common Mistakes + +These are the most common places where plugin installation gets mixed up with +other NemoClaw extension paths. + +- Do not use `nemoclaw skill install` for OpenClaw plugins. That + command only installs `SKILL.md` agent skills. +- Do not put a Dockerfile in a broad directory such as `/tmp` unless you intend + to send that whole directory as the Docker build context. +- Keep plugin dependencies in the build stage or plugin directory; avoid copying + unrelated host files into the sandbox image. + +--- + Telegram, Discord, and Slack reach your agent through OpenShell-managed processes and gateway constructs. NemoClaw configures those channels during `nemoclaw onboard`. Tokens are registered with OpenShell providers, channel configuration is baked into the sandbox image, and runtime delivery stays under OpenShell control. `nemoclaw tunnel start` does not start Telegram (or other chat bridges). It only starts optional host services such as the cloudflared tunnel when that binary is present. (`nemoclaw start` is kept as a deprecated alias.) For details, refer to Commands (use the `nemoclaw-user-reference` skill). -## Step 9: Create a Telegram Bot +## Step 14: Create a Telegram Bot Open Telegram and send `/newbot` to [@BotFather](https://t.me/BotFather). Follow the prompts to create a bot and copy the bot token. -## Step 10: Provide the Bot Token and Optional Allowlist +## Step 15: Provide the Bot Token and Optional Allowlist Onboarding reads Telegram credentials from either host environment variables or the NemoClaw credential store (`getCredential` / `saveCredential` in the onboard flow). You do not have to export variables if you enter the token when the wizard asks. @@ -174,7 +257,7 @@ If `TELEGRAM_ALLOWED_IDS` is not set, the wizard can prompt for allowed sender I NemoClaw applies that allowlist to Telegram DMs only. Group chats stay open by default so rebuilt sandboxes do not silently drop Telegram group messages because of an empty group allowlist. -## Step 11: Run `nemoclaw onboard` +## Step 16: Run `nemoclaw onboard` Complete the rest of the wizard so the blueprint can create OpenShell providers (for example `-telegram-bridge`), bake channel configuration into the image (`NEMOCLAW_MESSAGING_CHANNELS_B64`), and start the sandbox. @@ -192,12 +275,12 @@ If you enable a messaging channel and another sandbox already uses the same toke For a full first-time flow, refer to Quickstart (use the `nemoclaw-user-get-started` skill). -## Step 12: Confirm Delivery +## Step 17: Confirm Delivery After the sandbox is running, send a message to your bot in Telegram. If something fails, use `openshell term` on the host, check gateway logs, and verify network policy allows the Telegram API (see Customize the Network Policy (use the `nemoclaw-user-manage-policy` skill) and the `telegram` preset). -## Step 13: `nemoclaw tunnel start` (cloudflared Only) +## Step 18: `nemoclaw tunnel start` (cloudflared Only) `nemoclaw tunnel start` starts cloudflared when it is installed, which can expose the dashboard with a public URL. It does not affect Telegram connectivity. The older `nemoclaw start` still works as a deprecated alias. diff --git a/.agents/skills/nemoclaw-user-reference/references/commands.md b/.agents/skills/nemoclaw-user-reference/references/commands.md index 439ef8f1bcf..5370e634125 100644 --- a/.agents/skills/nemoclaw-user-reference/references/commands.md +++ b/.agents/skills/nemoclaw-user-reference/references/commands.md @@ -459,6 +459,7 @@ $ nemoclaw my-assistant skill install ./my-skill/ The skill directory must contain a `SKILL.md` file with YAML frontmatter that includes a `name` field. Skill names must contain only alphanumeric characters, dots, hyphens, and underscores. +OpenClaw plugins are a different kind of extension. To install an OpenClaw plugin, see Install OpenClaw Plugins (use the `nemoclaw-user-deploy-remote` skill). Files with names starting with `.` (dotfiles) are skipped and listed in the output. Files with unsafe path characters are rejected to prevent shell injection. diff --git a/docs/deployment/install-openclaw-plugins.md b/docs/deployment/install-openclaw-plugins.md new file mode 100644 index 00000000000..150a021cc99 --- /dev/null +++ b/docs/deployment/install-openclaw-plugins.md @@ -0,0 +1,113 @@ +--- +title: + page: "Install OpenClaw Plugins" + nav: "OpenClaw Plugins" +description: + main: "How to install OpenClaw plugins in a NemoClaw-managed sandbox today." + agent: "Explains the difference between OpenClaw plugins and agent skills, and shows the current Dockerfile-based workflow for baking a plugin into a NemoClaw sandbox. Use when users ask how to install, build, or configure OpenClaw plugins under NemoClaw." +keywords: ["nemoclaw plugins", "openclaw plugins", "install openclaw plugin", "nemoclaw onboard from dockerfile"] +topics: ["generative_ai", "ai_agents"] +tags: ["nemoclaw", "openclaw", "plugins", "sandbox"] +content: + type: how_to + difficulty: technical_intermediate + audience: ["developer", "engineer"] +status: published +--- + + + +# Install OpenClaw Plugins + +OpenClaw plugins extend the OpenClaw runtime with hooks, services, tools, or +provider integrations. They are different from NemoClaw-managed agent skills: + +- **Plugins** are code packages loaded by OpenClaw. +- **Skills** are `SKILL.md` directories that teach an agent how to perform a task. +- **Policy presets** are network-egress rules that control what sandboxed code can reach. + +Today, the supported NemoClaw path for OpenClaw plugins is to bake the plugin +into a custom sandbox image and onboard from that Dockerfile. + +## Prepare a Build Directory + +Put the Dockerfile and everything it needs to `COPY` in one directory. +`nemoclaw onboard --from ` uses the Dockerfile's parent directory as +the Docker build context. + +```text +my-plugin-sandbox/ +├── Dockerfile +└── my-plugin/ + ├── package.json + └── src/ +``` + +## Example Dockerfile + +Use the custom image to copy the plugin into the OpenClaw extensions directory +and let OpenClaw refresh its config before NemoClaw starts the sandbox. + +```dockerfile +ARG SANDBOX_BASE=ghcr.io/nvidia/nemoclaw/sandbox-base:latest +FROM ${SANDBOX_BASE} + +COPY my-plugin/ /opt/my-plugin/ +WORKDIR /opt/my-plugin +RUN npm ci --no-audit --no-fund && npm run build + +RUN mkdir -p /sandbox/.openclaw-data/extensions \ + && cp -a /opt/my-plugin /sandbox/.openclaw-data/extensions/my-plugin \ + && openclaw doctor --fix + +WORKDIR /opt/nemoclaw +``` + +If the plugin needs configuration in `openclaw.json`, apply it after +`openclaw doctor --fix` so the base config exists first. + +## Create the Sandbox + +Point `nemoclaw onboard --from` at the Dockerfile in the build directory. + +```console +$ nemoclaw onboard --from ./my-plugin-sandbox/Dockerfile +``` + +If you need a second sandbox alongside an existing one, use a dedicated build +directory and rerun onboarding with the sandbox name and ports you intend to +use. + +## Network Access + +Plugins still run inside the sandbox policy boundary. If a plugin needs network +egress, add or update a policy preset for the required hostnames and binaries +before rebuilding the sandbox. + +For example, see [Network Policies](../reference/network-policies.md) for +policy concepts and [Customize Network Policy](../network-policy/customize-network-policy.md) +for custom preset workflows. + +## Common Mistakes + +These are the most common places where plugin installation gets mixed up with +other NemoClaw extension paths. + +- Do not use `nemoclaw skill install` for OpenClaw plugins. That + command only installs `SKILL.md` agent skills. +- Do not put a Dockerfile in a broad directory such as `/tmp` unless you intend + to send that whole directory as the Docker build context. +- Keep plugin dependencies in the build stage or plugin directory; avoid copying + unrelated host files into the sandbox image. + +## Next Steps + +- Review [Sandbox Hardening](sandbox-hardening.md) before adding plugin code to a + shared or long-lived sandbox. +- Review [Network Policies](../reference/network-policies.md) to plan plugin + egress rules. +- Follow [Customize Network Policy](../network-policy/customize-network-policy.md) + if the plugin needs a custom preset. diff --git a/docs/index.md b/docs/index.md index b8fabc7871f..1632e841ec5 100644 --- a/docs/index.md +++ b/docs/index.md @@ -332,6 +332,7 @@ OpenClaw Controls Deploy to a Remote GPU Instance Set Up Telegram +Install OpenClaw Plugins Sandbox Hardening ``` diff --git a/docs/reference/commands.md b/docs/reference/commands.md index 8aca2ae4561..d32c9d744ed 100644 --- a/docs/reference/commands.md +++ b/docs/reference/commands.md @@ -489,6 +489,7 @@ $ nemoclaw my-assistant skill install ./my-skill/ The skill directory must contain a `SKILL.md` file with YAML frontmatter that includes a `name` field. Skill names must contain only alphanumeric characters, dots, hyphens, and underscores. +OpenClaw plugins are a different kind of extension. To install an OpenClaw plugin, see [Install OpenClaw Plugins](../deployment/install-openclaw-plugins.md). Files with names starting with `.` (dotfiles) are skipped and listed in the output. Files with unsafe path characters are rejected to prevent shell injection. From a8725c21869e66cb87e4b3e1b2bfa5df38639bef Mon Sep 17 00:00:00 2001 From: Deepak Jain Date: Tue, 28 Apr 2026 13:33:38 -0700 Subject: [PATCH 8/9] fix(cli): recognize BuildKit sandbox build progress (Fixes #2311) (#2404) ## Summary NemoClaw's sandbox create stream only recognized the legacy Docker builder format, so BuildKit output would not be treated as active build progress once OpenShell emits it. This adds BuildKit progress markers to the same parser path as the existing legacy builder output. It keeps the current legacy behavior and makes `#1 [internal] ...`, `#2 CACHED`, and `#3 DONE ...` visible as build progress. ## Changes - `src/lib/sandbox-create-stream.ts`: recognize BuildKit step and completion lines while tracking the build phase. - `src/lib/sandbox-create-stream.test.ts`: cover BuildKit progress output and verify it is streamed to the user. ## Testing - `npm run build:cli` passed - `npm run typecheck:cli` passed - `npm test -- src/lib/sandbox-create-stream.test.ts` passed - `npm test` was also attempted. The full suite is not green on current main in this environment; failures are in existing installer/onboard/legacy-guard tests outside this change. ## Evidence it works The new focused test feeds BuildKit-style output into `streamSandboxCreate` and verifies that the lines are logged, collected in output, and mark sandbox creation as having seen progress. Fixes #2311 Signed-off-by: Deepak Jain ## Summary by CodeRabbit * **Bug Fixes** * Improved detection and display of BuildKit and upload progress so progress markers and completion states are recognized reliably. * **Refactor** * Centralized progress-detection logic for more consistent handling of build and upload output. * **Tests** * Added a test ensuring BuildKit-formatted progress lines are captured, included in output, and reported to the log callback. Signed-off-by: Deepak Jain Co-authored-by: Carlos Villela --- src/lib/sandbox-create-stream.test.ts | 26 ++++++++++++++ src/lib/sandbox-create-stream.ts | 52 ++++++++++++++++----------- 2 files changed, 58 insertions(+), 20 deletions(-) diff --git a/src/lib/sandbox-create-stream.test.ts b/src/lib/sandbox-create-stream.test.ts index 6165fa43093..6b0fc50cfc5 100644 --- a/src/lib/sandbox-create-stream.test.ts +++ b/src/lib/sandbox-create-stream.test.ts @@ -68,6 +68,32 @@ describe("sandbox-create-stream", () => { expect(logLine).toHaveBeenCalledWith("Created sandbox: demo"); }); + it("streams BuildKit progress lines as build output", async () => { + const child = new FakeChild(); + const logLine = vi.fn(); + const promise = streamSandboxCreate("echo create", process.env, { + logLine, + spawnImpl: () => child as never, + heartbeatIntervalMs: 1_000, + silentPhaseMs: 10_000, + }); + + child.stdout.emit( + "data", + Buffer.from("#1 [internal] load build definition from Dockerfile\n#2 CACHED\n#3 DONE 0.1s\n"), + ); + child.emit("close", 0); + + await expect(promise).resolves.toMatchObject({ + status: 0, + sawProgress: true, + output: expect.stringContaining("#1 [internal] load build definition from Dockerfile"), + }); + expect(logLine).toHaveBeenCalledWith("#1 [internal] load build definition from Dockerfile"); + expect(logLine).toHaveBeenCalledWith("#2 CACHED"); + expect(logLine).toHaveBeenCalledWith("#3 DONE 0.1s"); + }); + it("forces success when the sandbox becomes ready before the stream exits", async () => { vi.useFakeTimers(); diff --git a/src/lib/sandbox-create-stream.ts b/src/lib/sandbox-create-stream.ts index e4494ba12f7..72c561dc87f 100644 --- a/src/lib/sandbox-create-stream.ts +++ b/src/lib/sandbox-create-stream.ts @@ -48,6 +48,35 @@ export interface StreamableChildProcess { on(event: "close", listener: (code: number | null) => void): this; } +export const BUILD_PROGRESS_PATTERNS: readonly RegExp[] = [ + /^ {2}Building image /, + /^ {2}Step \d+\/\d+ : /, + /^#\d+ \[/, + /^#\d+ (DONE|CACHED)\b/, +]; + +const UPLOAD_PROGRESS_PATTERNS: readonly RegExp[] = [ + /^ {2}Pushing image /, + /^\s*\[progress\]/, + /^ {2}Image .*available in the gateway/, +]; + +const VISIBLE_PROGRESS_PATTERNS: readonly RegExp[] = [ + ...BUILD_PROGRESS_PATTERNS, + /^ {2}Context: /, + /^ {2}Gateway: /, + /^Successfully built /, + /^Successfully tagged /, + /^ {2}Built image /, + ...UPLOAD_PROGRESS_PATTERNS, + /^Created sandbox: /, + /^✓ /, +]; + +function matchesAny(line: string, patterns: readonly RegExp[]) { + return patterns.some((pattern) => pattern.test(line)); +} + export function streamSandboxCreate( command: string, env: NodeJS.ProcessEnv = process.env, @@ -124,13 +153,9 @@ export function streamSandboxCreate( if (!line) return; lines.push(line); lastOutputAt = Date.now(); - if (/^ {2}Building image /.test(line) || /^ {2}Step \d+\/\d+ : /.test(line)) { + if (matchesAny(line, BUILD_PROGRESS_PATTERNS)) { setPhase("build"); - } else if ( - /^ {2}Pushing image /.test(line) || - /^\s*\[progress\]/.test(line) || - /^ {2}Image .*available in the gateway/.test(line) - ) { + } else if (matchesAny(line, UPLOAD_PROGRESS_PATTERNS)) { setPhase("upload"); } else if (/^Created sandbox: /.test(line)) { setPhase("create"); @@ -142,20 +167,7 @@ export function streamSandboxCreate( } function shouldShowLine(line: string) { - return ( - /^ {2}Building image /.test(line) || - /^ {2}Step \d+\/\d+ : /.test(line) || - /^ {2}Context: /.test(line) || - /^ {2}Gateway: /.test(line) || - /^Successfully built /.test(line) || - /^Successfully tagged /.test(line) || - /^ {2}Built image /.test(line) || - /^ {2}Pushing image /.test(line) || - /^\s*\[progress\]/.test(line) || - /^ {2}Image .*available in the gateway/.test(line) || - /^Created sandbox: /.test(line) || - /^✓ /.test(line) - ); + return matchesAny(line, VISIBLE_PROGRESS_PATTERNS); } function onChunk(chunk: Buffer | string) { From 45daf0fd7078a33378ba781e8f610195b2e1cb78 Mon Sep 17 00:00:00 2001 From: Truong Nguyen Date: Wed, 29 Apr 2026 03:33:51 +0700 Subject: [PATCH 9/9] fix(ci): restore cloud-experimental-e2e to nightly pipeline (#2609) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Restore the `cloud-experimental-e2e` job that was accidentally deleted from `nightly-e2e.yaml` in PR #2472. ## Related Issue Fixes #2570 ## Changes Restores the `cloud-experimental-e2e` job that tests: - Landlock read-only enforcement (8 assertions on .bashrc, .profile, .openclaw, .openclaw-data, /tmp) - API key leak detection in process list - `openclaw tui` smoke test inside sandbox - Live chat via `openclaw agent` - Skill injection + agent verification - `inference.local` HTTPS probe The job runs unconditionally (no feature-flag gate). Added to `notify-on-failure` needs list. Removed the old `skip/05-network-policy.sh` step (now covered by the dedicated `network-policy-e2e` job). ## Type of Change - Code change (feature, bug fix, or refactor) ## Verification - YAML validated on fork: all jobs parse correctly - Verified on fork CI: cloud-experimental-e2e PASS in 14m 5s ## AI Disclosure - AI-assisted — tool: Cursor --- Signed-off-by: Truong Nguyen Made with [Cursor](https://cursor.com) ## Summary by CodeRabbit * **Tests** * Added nightly cloud experimental end-to-end tests to broaden coverage. * Made the experimental job selectable from the manual job list for targeted runs. * Always-check documentation during these runs for improved QA. * Ensure experimental sandbox is torn down and verified after tests. * Upload an install-log artifact when the experimental job fails to aid troubleshooting. * Include the experimental job in failure notifications and PR reporting so results are tracked. Signed-off-by: Truong Nguyen --- .github/workflows/nightly-e2e.yaml | 81 +++++++++++++++++++++++++++--- 1 file changed, 74 insertions(+), 7 deletions(-) diff --git a/.github/workflows/nightly-e2e.yaml b/.github/workflows/nightly-e2e.yaml index 5dac8efc621..495a2963e11 100644 --- a/.github/workflows/nightly-e2e.yaml +++ b/.github/workflows/nightly-e2e.yaml @@ -39,13 +39,13 @@ on: jobs: description: >- Comma-separated job names to run (empty = all). - Valid: cloud-e2e, messaging-providers-e2e, token-rotation-e2e, - sandbox-survival-e2e, hermes-e2e, skip-permissions-e2e, - sandbox-operations-e2e, inference-routing-e2e, network-policy-e2e, - deployment-services-e2e, diagnostics-e2e, snapshot-commands-e2e, - shields-config-e2e, rebuild-openclaw-e2e, upgrade-stale-sandbox-e2e, - rebuild-hermes-e2e, double-onboard-e2e, onboard-repair-e2e, - onboard-resume-e2e, runtime-overrides-e2e, + Valid: cloud-e2e, cloud-experimental-e2e, messaging-providers-e2e, + token-rotation-e2e, sandbox-survival-e2e, hermes-e2e, + skip-permissions-e2e, sandbox-operations-e2e, inference-routing-e2e, + network-policy-e2e, deployment-services-e2e, diagnostics-e2e, + snapshot-commands-e2e, shields-config-e2e, rebuild-openclaw-e2e, + upgrade-stale-sandbox-e2e, rebuild-hermes-e2e, double-onboard-e2e, + onboard-repair-e2e, onboard-resume-e2e, runtime-overrides-e2e, credential-sanitization-e2e, telegram-injection-e2e, overlayfs-autofix-e2e, gpu-e2e required: false @@ -90,6 +90,71 @@ jobs: path: /tmp/nemoclaw-e2e-install.log if-no-files-found: ignore + # ── Cloud Experimental E2E ──────────────────────────────────── + # Landlock read-only enforcement, API key leak detection, openclaw tui smoke, + # live chat via openclaw agent, skill injection, inference.local HTTPS. + # Restored after accidental deletion in #2472. + cloud-experimental-e2e: + if: >- + github.repository == 'NVIDIA/NemoClaw' && + (github.event_name != 'workflow_dispatch' || + inputs.jobs == '' || + contains(format(',{0},', inputs.jobs), ',cloud-experimental-e2e,')) + runs-on: ubuntu-latest + timeout-minutes: 90 + steps: + - name: Checkout + uses: actions/checkout@v6 + + - name: Run cloud-experimental E2E test + env: + NVIDIA_API_KEY: ${{ secrets.NVIDIA_API_KEY }} + GITHUB_TOKEN: ${{ github.token }} + RUN_E2E_CLOUD_EXPERIMENTAL_INTERACTIVE_INSTALL: "0" + NEMOCLAW_NON_INTERACTIVE: "1" + NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE: "1" + NEMOCLAW_RECREATE_SANDBOX: "1" + NEMOCLAW_POLICY_MODE: "custom" + NEMOCLAW_POLICY_PRESETS: "npm,pypi" + RUN_E2E_CLOUD_EXPERIMENTAL_SKIP_CHECK_DOCS: "1" + RUN_E2E_CLOUD_EXPERIMENTAL_SKIP_FINAL_CLEANUP: "1" + run: bash test/e2e/test-e2e-cloud-experimental.sh + + - name: Documentation checks (check-docs.sh) + if: always() + env: + GITHUB_TOKEN: ${{ github.token }} + run: | + set -euo pipefail + # shellcheck source=/dev/null + [ -f "$HOME/.bashrc" ] && source "$HOME/.bashrc" 2>/dev/null || true + export NVM_DIR="${NVM_DIR:-$HOME/.nvm}" + [ -s "$NVM_DIR/nvm.sh" ] && . "$NVM_DIR/nvm.sh" + [ -d "$HOME/.local/bin" ] && [[ ":$PATH:" != *":$HOME/.local/bin:"* ]] && export PATH="$HOME/.local/bin:$PATH" + bash test/e2e/e2e-cloud-experimental/check-docs.sh + + - name: Tear down cloud-experimental sandbox (always) + if: always() + env: + SANDBOX_NAME: e2e-cloud-experimental + NEMOCLAW_SANDBOX_NAME: e2e-cloud-experimental + run: | + set -euo pipefail + # shellcheck source=/dev/null + [ -f "$HOME/.bashrc" ] && source "$HOME/.bashrc" 2>/dev/null || true + export NVM_DIR="${NVM_DIR:-$HOME/.nvm}" + [ -s "$NVM_DIR/nvm.sh" ] && . "$NVM_DIR/nvm.sh" + [ -d "$HOME/.local/bin" ] && [[ ":$PATH:" != *":$HOME/.local/bin:"* ]] && export PATH="$HOME/.local/bin:$PATH" + bash test/e2e/e2e-cloud-experimental/cleanup.sh --verify + + - name: Upload install log on failure + if: failure() + uses: actions/upload-artifact@v4 + with: + name: install-log-cloud-experimental + path: /tmp/nemoclaw-e2e-cloud-experimental-install.log + if-no-files-found: ignore + # ── Messaging Providers E2E ────────────────────────────────── # Validates the full provider/placeholder/L7-proxy chain for messaging # credentials (Telegram, Discord). Uses fake tokens by default — the L7 @@ -975,6 +1040,7 @@ jobs: needs: [ cloud-e2e, + cloud-experimental-e2e, messaging-providers-e2e, token-rotation-e2e, sandbox-survival-e2e, @@ -1050,6 +1116,7 @@ jobs: needs: [ cloud-e2e, + cloud-experimental-e2e, messaging-providers-e2e, token-rotation-e2e, sandbox-survival-e2e,