Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions src/lib/onboard/docker-gpu-patch-mode-selection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,40 @@ function inspectFixture(): DockerContainerInspect {
}

describe("docker-gpu-patch CDI-first mode selection (#4948)", () => {
it("keeps Docker Desktop WSL on the compatibility selector path instead of trusting host-visible CDI", () => {
// #5512: Docker Desktop WSL may expose CDI spec directories while the WSL
// distro cannot resolve a usable nvidia.com/gpu spec. The patch must stay
// enabled, the OpenShell create --gpu flag must stay suppressed, and mode
// probing must still fall back to Docker's --gpus compatibility path if a
// speculative CDI probe fails.
expect(buildDockerGpuModeCandidates("all", { cdiAvailable: true }).map((m) => m.kind)).toEqual([
"cdi",
"gpus",
"nvidia-runtime",
]);

const dockerRun = vi.fn((args: readonly string[]) =>
args.includes("--device")
? {
status: 1,
stderr: "CDI device injection failed: unresolvable CDI devices nvidia.com/gpu=all",
}
: { status: 0, stdout: "probe-id" },
);
const selected = selectDockerGpuPatchMode(
{ image: "openshell/sandbox:abc" },
{ ...cdiHostDeps(), dockerRun },
);

expect(selected.mode?.kind).toBe("gpus");
expect(selected.mode?.label).toBe("--gpus all");
expect(selected.attempts.map((attempt) => [attempt.mode.kind, attempt.ok])).toEqual([
["cdi", false],
["gpus", true],
]);
expect(selected.attempts[0]?.error).toContain("unresolvable CDI devices");
});

it("prefers CDI over --gpus when the host advertises an NVIDIA CDI spec", () => {
// Repro for #4948: on a Docker-CDI GPU host (e.g. Ubuntu 24.04 with
// /etc/cdi/nvidia.yaml), `docker create --gpus all` is *accepted* so the
Expand Down
31 changes: 31 additions & 0 deletions src/lib/onboard/docker-gpu-patch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -794,6 +794,37 @@ describe("docker-gpu-patch sandbox DNS fallback (#3579)", () => {
expect(args).not.toEqual(expect.arrayContaining(["--network", "host"]));
expect(args).toEqual(expect.arrayContaining(["--dns", "8.8.8.8"]));
});

it("keeps the DGX Spark DNS workaround diagnostic-only unless the host exposes an upstream resolver", () => {
const loopbackFiles: Record<string, string> = {
"/etc/resolv.conf": "nameserver 127.0.0.53\n",
"/run/systemd/resolve/resolv.conf": "nameserver 8.8.8.8\n",
};
const sparkFiles: Record<string, string> = {
"/etc/resolv.conf": "nameserver 10.18.1.96\n",
};
const loopbackOnly = detectSandboxFallbackDns({
readFile: (p: string): string | null => loopbackFiles[p] ?? null,
});
const sparkManagedDns = detectSandboxFallbackDns({
readFile: (p: string): string | null => sparkFiles[p] ?? null,
});

expect(loopbackOnly).toBe("8.8.8.8");
expect(sparkManagedDns).toBeNull();

const inspect = inspectFixture();
const preservedArgs = buildDockerGpuCloneRunArgs(inspect, buildDockerGpuMode("gpus"), {
sandboxFallbackDns: sparkManagedDns,
});
expect(preservedArgs).not.toEqual(expect.arrayContaining(["--dns"]));
expect(preservedArgs).not.toEqual(expect.arrayContaining(["--network", "host"]));

const patchedArgs = buildDockerGpuCloneRunArgs(inspect, buildDockerGpuMode("gpus"), {
sandboxFallbackDns: loopbackOnly,
});
expect(patchedArgs).toEqual(expect.arrayContaining(["--dns", "8.8.8.8"]));
});
});

// Jetson `/dev/nvmap` group-permission propagation (#4231). The reporter's
Expand Down
20 changes: 20 additions & 0 deletions src/lib/onboard/gateway-sandbox-reachability.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,26 @@ describe("formatSandboxBridgeUnreachableMessage", () => {
expect(msg).toContain("host-gateway");
expect(msg).not.toContain("ufw allow");
});

it("keeps Docker Desktop WSL host-gateway failures retryable instead of suggesting UFW", () => {
const msg = formatSandboxBridgeUnreachableMessage(
{
ok: false,
reason: "tcp_failed",
routeKind: "host_gateway",
networkName: "openshell-docker",
subnet: "172.19.0.0/16",
detail: "sandbox container could not reach host.openshell.internal:8080",
},
8080,
{ isWsl: true },
);

expect(msg).toContain("host-gateway");
expect(msg).toContain("Restart Docker and the OpenShell gateway");
expect(msg).not.toContain("ufw allow");
expect(msg).not.toContain("127.0.0.1");
});
});

describe("tryAutoApplyUfwRule (#4265)", () => {
Expand Down
18 changes: 18 additions & 0 deletions src/lib/onboard/preflight-cdi.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -340,4 +340,22 @@ describe("planHostRemediation — CDI", () => {
),
).toBe(true);
});

it("still blocks with install_nvidia_container_toolkit when nvidia-smi is unavailable but PCI GPU context exists", () => {
const actions = planHostRemediation(
baseAssessment({
cdiNvidiaGpuSpecMissing: true,
hasNvidiaGpu: true,
nvidiaContainerToolkitInstalled: false,
}),
);
Comment on lines +344 to +351

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Test title references nvidia-smi, but the assessment drives the branch via the toolkit flag.

The behavior being exercised is nvidiaContainerToolkitInstalled: false (plus cdiNvidiaGpuSpecMissing: true), which is what selects the install_nvidia_container_toolkit branch in planHostRemediation. The name "when nvidia-smi is unavailable" isn't modeled by any field here, so it may confuse the parity-coverage mapping this PR documents. Also, hasNvidiaGpu: true duplicates the baseAssessment default and can be dropped.

✏️ Suggested title/override tidy-up
-  it("still blocks with install_nvidia_container_toolkit when nvidia-smi is unavailable but PCI GPU context exists", () => {
+  it("still blocks with install_nvidia_container_toolkit when the toolkit is not installed but a GPU is present", () => {
     const actions = planHostRemediation(
       baseAssessment({
         cdiNvidiaGpuSpecMissing: true,
-        hasNvidiaGpu: true,
         nvidiaContainerToolkitInstalled: false,
       }),
     );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it("still blocks with install_nvidia_container_toolkit when nvidia-smi is unavailable but PCI GPU context exists", () => {
const actions = planHostRemediation(
baseAssessment({
cdiNvidiaGpuSpecMissing: true,
hasNvidiaGpu: true,
nvidiaContainerToolkitInstalled: false,
}),
);
it("still blocks with install_nvidia_container_toolkit when the toolkit is not installed but a GPU is present", () => {
const actions = planHostRemediation(
baseAssessment({
cdiNvidiaGpuSpecMissing: true,
nvidiaContainerToolkitInstalled: false,
}),
);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/onboard/preflight-cdi.test.ts` around lines 344 - 351, Update the
test in preflight-cdi.test.ts so the title matches the actual inputs driving
planHostRemediation: it should describe the install_nvidia_container_toolkit
branch triggered by cdiNvidiaGpuSpecMissing and nvidiaContainerToolkitInstalled
being false, not nvidia-smi availability. Remove the redundant hasNvidiaGpu:
true override if baseAssessment already sets it, and keep the assertion focused
on the remediation action selected by planHostRemediation.


const actionIds = actions.map((entry) => entry.id);
expect(actionIds.indexOf("install_nvidia_container_toolkit")).toBeGreaterThanOrEqual(0);
expect(actionIds).not.toContain("generate_nvidia_cdi_spec");
const toolkitAction = actions.find((entry) => entry.id === "install_nvidia_container_toolkit");
expect(toolkitAction?.blocking).toBe(true);
expect(toolkitAction?.commands.join("\n")).toContain("nvidia-container-toolkit");
expect(toolkitAction?.commands.join("\n")).toContain("nvidia-ctk cdi generate");
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

import { describe, expect, it } from "vitest";

import {
evaluateE2eVitestWorkflowDispatchSelectors,
readFreeStandingJobsInventory,
validateE2eVitestScenariosWorkflowBoundary,
} from "../../../tools/e2e-scenarios/workflow-boundary.mts";
import { canonicalScenarios } from "../scenarios/scenarios/baseline.ts";

describe("P0-E platform parity workflow coverage", () => {
it("keeps the WSL platform scenario on a Windows WSL runner boundary", () => {
const wsl = canonicalScenarios().find((entry) => entry.id === "wsl-repo-cloud-openclaw");
expect(wsl).toBeDefined();

expect(wsl.environment).toMatchObject({

Check failure on line 18 in test/e2e-scenario/support-tests/platform-parity-workflow-boundary.test.ts

View workflow job for this annotation

GitHub Actions / build-typecheck

'wsl' is possibly 'undefined'.
platform: "wsl-local",
runtime: "docker-running",
onboarding: "cloud-openclaw",
});
expect(wsl.runnerRequirements).toEqual(expect.arrayContaining(["windows-latest", "wsl2"]));

Check failure on line 23 in test/e2e-scenario/support-tests/platform-parity-workflow-boundary.test.ts

View workflow job for this annotation

GitHub Actions / build-typecheck

'wsl' is possibly 'undefined'.
expect(wsl.suiteIds).toContain("platform-wsl");

Check failure on line 24 in test/e2e-scenario/support-tests/platform-parity-workflow-boundary.test.ts

View workflow job for this annotation

GitHub Actions / build-typecheck

'wsl' is possibly 'undefined'.
expect(wsl.requiredSecrets).toContain("NVIDIA_INFERENCE_API_KEY");

Check failure on line 25 in test/e2e-scenario/support-tests/platform-parity-workflow-boundary.test.ts

View workflow job for this annotation

GitHub Actions / build-typecheck

'wsl' is possibly 'undefined'.
Comment on lines +15 to +25

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the target test file and relevant TypeScript/Vitest context.
git ls-files 'test/e2e-scenario/support-tests/platform-parity-workflow-boundary.test.ts' 'tsconfig*.json' 'package.json' 'vitest.config.*' 'test/**/support-tests/*.test.ts' | sed 's#^`#FILE` #'

echo
echo "---- target file ----"
cat -n test/e2e-scenario/support-tests/platform-parity-workflow-boundary.test.ts

echo
echo "---- search for similar patterns ----"
rg -n "toBeDefined\(\).*possible|expect\\(.*\\)\\.toBeDefined\\(\\).*\\n.*if \\(!.*\\)" test src -U || true

echo
echo "---- search for config / test runner deps ----"
rg -n "\"vitest\"|expectTypeOf|toBeDefined" package.json tsconfig*.json vitest.config.* test -g '!**/node_modules/**' || true

Repository: NVIDIA/NemoClaw

Length of output: 50371


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read the file and typecheck-relevant configuration in smaller slices if needed.
stat -c '%n %s bytes' test/e2e-scenario/support-tests/platform-parity-workflow-boundary.test.ts
echo
sed -n '1,220p' test/e2e-scenario/support-tests/platform-parity-workflow-boundary.test.ts

echo
echo "---- tsconfig files ----"
for f in tsconfig*.json; do
  [ -f "$f" ] && echo "### $f" && cat "$f" && echo
done

echo
echo "---- package.json snippets ----"
sed -n '1,240p' package.json

Repository: NVIDIA/NemoClaw

Length of output: 195


Add a runtime guard before dereferencing wsl.
expect(wsl).toBeDefined() doesn’t narrow the find() result for TypeScript, so the later property reads still fail type-checking. Add if (!wsl) throw new Error("Missing canonical scenario: wsl-repo-cloud-openclaw"); after the expectation.

🧰 Tools
🪛 GitHub Check: build-typecheck

[failure] 25-25:
'wsl' is possibly 'undefined'.


[failure] 24-24:
'wsl' is possibly 'undefined'.


[failure] 23-23:
'wsl' is possibly 'undefined'.


[failure] 18-18:
'wsl' is possibly 'undefined'.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/e2e-scenario/support-tests/platform-parity-workflow-boundary.test.ts`
around lines 15 - 25, The `wsl` lookup in `canonicalScenarios().find(...)` is
still treated as possibly undefined after `expect(wsl).toBeDefined()`, so the
later property access in the same test is not type-safe. In
`platform-parity-workflow-boundary.test.ts`, add an explicit runtime guard
immediately after the `expect(wsl).toBeDefined()` assertion in the
`wsl-repo-cloud-openclaw` block so TypeScript can narrow the value before
reading `environment`, `runnerRequirements`, `suiteIds`, and `requiredSecrets`.

Source: Linters/SAST tools


expect(
evaluateE2eVitestWorkflowDispatchSelectors({ scenarios: "wsl-repo-cloud-openclaw" }),
).toMatchObject({
valid: true,
liveScenariosRuns: true,
registryScenarios: ["wsl-repo-cloud-openclaw"],
selectedFreeStandingJobs: [],
});
});

it("keeps GPU, Spark, and Jetson platform selectors explicit and routable", () => {
expect(validateE2eVitestScenariosWorkflowBoundary()).toEqual([]);

const inventory = readFreeStandingJobsInventory();
expect(inventory.scenarioToJob.get("gpu-e2e")).toBe("gpu-e2e-vitest");
expect(inventory.scenarioToJob.get("gpu-double-onboard")).toBe("gpu-double-onboard-vitest");
expect(inventory.scenarioToJob.get("spark-install")).toBe("spark-install-vitest");
expect(inventory.scenarioToJob.get("jetson-nvmap-gpu")).toBe("jetson-nvmap-gpu-vitest");

expect(evaluateE2eVitestWorkflowDispatchSelectors({ scenarios: "gpu-e2e" })).toMatchObject({
valid: true,
liveScenariosRuns: false,
selectedFreeStandingJobs: ["gpu-e2e-vitest"],
registryScenarios: [],
});
expect(
evaluateE2eVitestWorkflowDispatchSelectors({ scenarios: "gpu-double-onboard" }),
).toMatchObject({
valid: true,
liveScenariosRuns: false,
selectedFreeStandingJobs: ["gpu-double-onboard-vitest"],
registryScenarios: [],
});
expect(
evaluateE2eVitestWorkflowDispatchSelectors({ scenarios: "spark-install" }),
).toMatchObject({
valid: true,
liveScenariosRuns: false,
selectedFreeStandingJobs: ["spark-install-vitest"],
registryScenarios: [],
});

// Jetson/Tegra hardware is still an explicit-only route; do not imply the
// default full-suite dispatch proved /dev/nvmap group behavior.
expect(evaluateE2eVitestWorkflowDispatchSelectors({}).selectedFreeStandingJobs).not.toContain(
"jetson-nvmap-gpu-vitest",
);
expect(
evaluateE2eVitestWorkflowDispatchSelectors({ scenarios: "jetson-nvmap-gpu" }),
).toMatchObject({
valid: true,
liveScenariosRuns: false,
selectedFreeStandingJobs: ["jetson-nvmap-gpu-vitest"],
registryScenarios: [],
});
});
});
Loading