fix(onboard): prefer CDI GPU mode over --gpus on CDI hosts

src/lib/onboard/docker-gpu-patch-mode-selection.test.ts

@@ -0,0 +1,147 @@
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+import { describe, expect, it, vi } from "vitest";
+
+import {
+  buildDockerGpuModeCandidates,
+  type DockerContainerInspect,
+  type DockerGpuPatchDeps,
+  recreateOpenShellDockerSandboxWithGpu,
+  selectDockerGpuPatchMode,
+} from "../../../dist/lib/onboard/docker-gpu-patch";
+
+// Deps that surface an NVIDIA CDI spec at /etc/cdi/nvidia.yaml so
+// `dockerReportsNvidiaCdiDevices` reports CDI as available (the #4948 host
+// shape). Probe behavior is supplied per-test via `dockerRun`.
+function cdiHostDeps(): DockerGpuPatchDeps {
+  return {
+    dockerCapture: vi.fn(() => JSON.stringify(["/etc/cdi"])),
+    readDir: (dir: string) => (dir === "/etc/cdi" ? ["nvidia.yaml"] : null),
+    readFile: (file: string) =>
+      file === "/etc/cdi/nvidia.yaml"
+        ? "cdiVersion: 0.6.0\nkind: nvidia.com/gpu\ndevices:\n  - name: all\n"
+        : null,
+    dockerRm: vi.fn(() => ({ status: 0 })),
+  };
+}
+
+function inspectFixture(): DockerContainerInspect {
+  return {
+    Id: "old-container-id",
+    Name: "/openshell-alpha",
+    Config: {
+      Image: "openshell/sandbox:abc",
+      Labels: {
+        "openshell.ai/managed-by": "openshell",
+        "openshell.ai/sandbox-name": "alpha",
+      },
+    },
+    HostConfig: { NetworkMode: "openshell-docker" },
+  };
+}
+
+describe("docker-gpu-patch CDI-first mode selection (#4948)", () => {
+  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
+    // create-only probe passes and `--gpus all` was selected. OpenShell's
+    // gateway injects GPUs via the CDI spec, so the legacy --gpus injection
+    // path diverges from how the supervisor expects the container to be wired
+    // and never reconnects. When a CDI spec is present we must select the CDI
+    // mode (`--device nvidia.com/gpu=all`) ahead of --gpus.
+    expect(buildDockerGpuModeCandidates("all", { cdiAvailable: true }).map((m) => m.kind)).toEqual([
+      "cdi",
+      "gpus",
+      "nvidia-runtime",
+    ]);
+
+    // Every probe (including --gpus) would succeed on this host, yet CDI wins.
+    const dockerRun = vi.fn(() => ({ status: 0, stdout: "probe-id" }));
+    const selected = selectDockerGpuPatchMode(
+      { image: "openshell/sandbox:abc" },
+      { ...cdiHostDeps(), dockerRun },
+    );
+
+    expect(selected.mode?.kind).toBe("cdi");
+    expect(selected.attempts[0].mode.kind).toBe("cdi");
+  });
+
+  it("falls back to --gpus when the CDI probe fails on a CDI host", () => {
+    // CDI is preferred first, but if `docker create --device nvidia.com/gpu=all`
+    // is rejected the selection must continue down the fallback chain rather
+    // than leaving the host with no usable GPU mode.
+    const dockerRun = vi.fn((args: readonly string[]) =>
+      args.includes("--device")
+        ? { status: 1, stderr: "could not select device driver" }
+        : { status: 0, stdout: "probe-id" },
+    );
+    const selected = selectDockerGpuPatchMode(
+      { image: "openshell/sandbox:abc" },
+      { ...cdiHostDeps(), dockerRun },
+    );
+
+    expect(selected.mode?.kind).toBe("gpus");
+    expect(selected.attempts.map((attempt) => attempt.mode.kind)).toEqual(["cdi", "gpus"]);
+    expect(selected.attempts[0].ok).toBe(false);
+  });
+
+  it("falls back to the NVIDIA runtime when both CDI and --gpus probes fail", () => {
+    const dockerRun = vi.fn((args: readonly string[]) =>
+      args.includes("--device") || args.includes("--gpus")
+        ? { status: 1, stderr: "probe rejected" }
+        : { status: 0, stdout: "probe-id" },
+    );
+    const selected = selectDockerGpuPatchMode(
+      { image: "openshell/sandbox:abc" },
+      { ...cdiHostDeps(), dockerRun },
+    );
+
+    expect(selected.mode?.kind).toBe("nvidia-runtime");
+    expect(selected.attempts.map((attempt) => attempt.mode.kind)).toEqual([
+      "cdi",
+      "gpus",
+      "nvidia-runtime",
+    ]);
+  });
+
+  it("passes the CDI --device flag to docker run when recreating on a CDI host", () => {
+    // Proves the selected CDI mode propagates into the actual recreate command
+    // (`dockerRunDetached`), not just the selection result. This is the create
+    // option that the issue's product log surfaces as `patched_create_option`.
+    const dockerCapture = vi.fn((args: readonly string[]) => {
+      if (args[0] === "ps") return "old-container-id\n";
+      if (args[0] === "inspect") return JSON.stringify([inspectFixture()]);
+      if (args[0] === "info") return JSON.stringify(["/etc/cdi"]);
+      return "";
+    });
+    const dockerRunDetached = vi.fn(() => ({ status: 0, stdout: "new-container-id\n" }));
+    const host = cdiHostDeps();
+
+    const result = recreateOpenShellDockerSandboxWithGpu(
+      { sandboxName: "alpha", timeoutSecs: 1 },
+      {
+        dockerCapture,
+        readDir: host.readDir,
+        readFile: host.readFile,
+        dockerRun: vi.fn(() => ({ status: 0, stdout: "probe-id\n" })),
+        dockerRunDetached,
+        dockerRename: vi.fn(() => ({ status: 0 })),
+        dockerStop: vi.fn(() => ({ status: 0 })),
+        dockerRm: vi.fn(() => ({ status: 0 })),
+        runOpenshell: vi.fn(() => ({ status: 0 })),
+        sleep: vi.fn(),
+        now: () => new Date("2026-05-12T00:00:00Z"),
+      },
+    );
+
+    expect(result.mode.kind).toBe("cdi");
+    expect(dockerRunDetached).toHaveBeenCalledWith(
+      expect.arrayContaining(["--name", "openshell-alpha", "--device", "nvidia.com/gpu=all"]),
+      expect.objectContaining({ ignoreError: true }),
+    );
+    // The legacy --gpus flag must NOT appear on a CDI host recreate.
+    const detachedArgs = (dockerRunDetached.mock.calls[0] as unknown[])[0] as string[];
+    expect(detachedArgs).not.toContain("--gpus");
+  });
+});

src/lib/onboard/docker-gpu-patch.test.ts

@@ -385,6 +385,8 @@ describe("docker-gpu-patch", () => {
         dockerCapture: vi.fn(() => ""),
         dockerRun,
         dockerRm: vi.fn(() => ({ status: 0 })),
+        readDir: vi.fn(() => null),
+        readFile: vi.fn(() => null),
       },
     );
 
@@ -423,14 +425,16 @@ describe("docker-gpu-patch", () => {
     expect(dockerCapture).not.toHaveBeenCalled();
   });
 
-  it("tries CDI only when Docker reports readable NVIDIA CDI specs", () => {
+  it("prefers CDI only when Docker reports readable NVIDIA CDI specs", () => {
     expect(buildDockerGpuModeCandidates("all", { cdiAvailable: false }).map((m) => m.kind)).toEqual(
       ["gpus", "nvidia-runtime"],
     );
+    // When a CDI spec is present, CDI is preferred first (see #4948); --gpus
+    // and the NVIDIA runtime remain as fallbacks.
     expect(buildDockerGpuModeCandidates("all", { cdiAvailable: true }).map((m) => m.kind)).toEqual([
+      "cdi",
       "gpus",
       "nvidia-runtime",
-      "cdi",
     ]);
 
     const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-docker-cdi-"));
@@ -454,8 +458,7 @@ describe("docker-gpu-patch", () => {
     // case: `docker info` returns an empty CDISpecDirs list, but Docker is
     // still reading specs from its well-known default /etc/cdi. The detector
     // should mirror Docker's behavior and surface cdi as available so the
-    // candidate list keeps `cdi` for fallback after `--gpus all` trips the
-    // AMD-CDI bug.
+    // candidate list prefers `cdi` ahead of `--gpus all` on CDI hosts (#4948).
     const readDir = vi.fn((dirPath: string) => (dirPath === "/etc/cdi" ? ["nvidia.yaml"] : null));
     const readFile = vi.fn((filePath: string) =>
       filePath === "/etc/cdi/nvidia.yaml"
@@ -535,6 +538,8 @@ describe("docker-gpu-patch", () => {
         runOpenshell,
         sleep: vi.fn(),
         now: () => new Date("2026-05-12T00:00:00Z"),
+        readDir: vi.fn(() => null),
+        readFile: vi.fn(() => null),
       },
     );
 

src/lib/onboard/docker-gpu-patch.ts

@@ -16,21 +16,22 @@ import {
 } from "../adapters/docker";
 import { reconcileSupervisorReconnect } from "./docker-gpu-patch-finalize";
 import {
-  type DockerGpuSupervisorReconnectDeps,
   DOCKER_GPU_SUPERVISOR_RECONNECT_ERROR_DEBOUNCE_ENV,
   DOCKER_GPU_SUPERVISOR_RECONNECT_TIMEOUT_ENV,
+  type DockerGpuSupervisorReconnectDeps,
   getDockerGpuSupervisorReconnectErrorDebouncePolls,
   getDockerGpuSupervisorReconnectTimeoutSecs,
   waitForOpenShellSupervisorReconnect,
 } from "./docker-gpu-supervisor-reconnect";
+
+export type { DockerGpuSupervisorReconnectDeps };
 export {
   DOCKER_GPU_SUPERVISOR_RECONNECT_ERROR_DEBOUNCE_ENV,
   DOCKER_GPU_SUPERVISOR_RECONNECT_TIMEOUT_ENV,
   getDockerGpuSupervisorReconnectErrorDebouncePolls,
   getDockerGpuSupervisorReconnectTimeoutSecs,
   waitForOpenShellSupervisorReconnect,
 };
-export type { DockerGpuSupervisorReconnectDeps };
 
 export const OPENSHELL_MANAGED_BY_LABEL = "openshell.ai/managed-by";
 export const OPENSHELL_MANAGED_BY_VALUE = "openshell";
@@ -500,11 +501,18 @@ export function buildDockerGpuModeCandidates(
   if (options.backend === "jetson") {
     return [buildDockerGpuMode("nvidia-runtime", device, { backend: "jetson" })];
   }
-  const candidates = [
-    buildDockerGpuMode("gpus", device),
-    buildDockerGpuMode("nvidia-runtime", device),
-  ];
+  // When the host advertises an NVIDIA CDI spec, prefer the CDI mode
+  // (`--device nvidia.com/gpu=all`) ahead of --gpus. OpenShell's gateway owns
+  // supervisor GPU injection and wires Docker-CDI hosts from that spec; this
+  // NemoClaw patch only chooses the recreate mode while matching that source
+  // boundary. On Docker-CDI hosts `docker create --gpus all` is accepted (the
+  // create-only probe passes), but the legacy --gpus injection diverges from
+  // gateway wiring and the supervisor never reconnects (#4948). Keep --gpus
+  // and the NVIDIA runtime as fallbacks until OpenShell exposes an
+  // authoritative GPU mode contract that can replace CDI-spec probing.
+  const candidates: DockerGpuPatchMode[] = [];
   if (options.cdiAvailable) candidates.push(buildDockerGpuMode("cdi", device));
+  candidates.push(buildDockerGpuMode("gpus", device), buildDockerGpuMode("nvidia-runtime", device));
   return candidates;
 }
 

src/lib/onboard/docker-gpu-supervisor-reconnect.test.ts

@@ -96,8 +96,34 @@ describe("docker-gpu-supervisor-reconnect Error-phase debounce", () => {
     expect(runOpenshell).toHaveBeenCalledTimes(6);
   });
 
-  it("defaults the debounce to 15 polls and honors the env override", () => {
-    expect(getDockerGpuSupervisorReconnectErrorDebouncePolls({})).toBe(15);
+  it("absorbs a Docker-CDI Error phase longer than the old 30s window", () => {
+    // #4948 runtime validation on the Docker-CDI GPU runner showed the
+    // sandbox-list row can remain Error for roughly a minute after the CDI
+    // recreate (`--device nvidia.com/gpu=all`) while the supervisor is still
+    // reconnecting. The default debounce must therefore outlive the old
+    // 15-poll / ~30s fast-fail window.
+    let polls = 0;
+    const runOpenshell = vi.fn(() => {
+      polls += 1;
+      return polls <= 30 ? { status: 1, stderr: "sandbox not ready" } : { status: 0 };
+    });
+    const runCaptureOpenshell = vi.fn(() =>
+      polls <= 30 ? "alpha   Error   1s ago" : "alpha   Ready   65s ago",
+    );
+    const sleep = vi.fn();
+
+    const ok = waitForOpenShellSupervisorReconnect("alpha", 600, {
+      runOpenshell,
+      runCaptureOpenshell,
+      sleep,
+    });
+
+    expect(ok).toBe(true);
+    expect(runOpenshell).toHaveBeenCalledTimes(31);
+  });
+
+  it("defaults the debounce to 60 polls and honors the env override", () => {
+    expect(getDockerGpuSupervisorReconnectErrorDebouncePolls({})).toBe(60);
     expect(
       getDockerGpuSupervisorReconnectErrorDebouncePolls({
         NEMOCLAW_DOCKER_GPU_SUPERVISOR_RECONNECT_ERROR_DEBOUNCE: "2",
@@ -149,9 +175,9 @@ describe("docker-gpu-supervisor-reconnect Error-phase debounce", () => {
       });
 
       expect(ok).toBe(false);
-      // Default K=15 from the env-backed helper: 15 polls + 14 sleeps before fast-fail.
-      expect(runOpenshell).toHaveBeenCalledTimes(15);
-      expect(sleep).toHaveBeenCalledTimes(14);
+      // Default K=60 from the env-backed helper: 60 polls + 59 sleeps before fast-fail.
+      expect(runOpenshell).toHaveBeenCalledTimes(60);
+      expect(sleep).toHaveBeenCalledTimes(59);
     }
   });
 });

src/lib/onboard/docker-gpu-supervisor-reconnect.ts

@@ -29,11 +29,11 @@ import { envInt } from "./env";
 const DOCKER_GPU_PATCH_TIMEOUT_MS = 30_000;
 const DOCKER_GPU_SUPERVISOR_RECONNECT_MIN_SECS = 900;
 // Default consecutive Error-phase polls required before fast-fail. With a
-// 2-second poll interval this is ~30s of sustained Error, leaving headroom
-// for slower hosts (Docker Desktop on WSL2, DGX Spark cached re-onboard)
-// whose sandbox list cache divergence was observed at ~12s in the original
-// repro. Hosts that genuinely crashed on startup hit the rollback path in
-// applyDockerGpuPatch rather than waiting out the full window.
+// 2-second poll interval this is ~2 minutes of sustained Error, leaving
+// headroom for Docker-CDI GPU runners whose OpenShell sandbox-list row can
+// stay Error for longer than the original ~30s window while the recreated
+// container is still reconnecting (#4948). Hosts that genuinely crashed on
+// startup still hit the rollback path well before the full reconnect timeout.
 //
 // Alternative considered: branching on Docker State.Status + Health.Status
 // to keep retrying when the patched container reports Status=running plus
@@ -44,10 +44,10 @@ const DOCKER_GPU_SUPERVISOR_RECONNECT_MIN_SECS = 900;
 // the user keeps the pre-patch CPU sandbox on reconnect failure, which a
 // Health-aware retry alone would not provide. If a future repro shows
 // Status=running + Health=starting genuinely failing reconnect after this
-// 30s window, switch to a Health-aware retry, but extract Docker health
+// default window, switch to a Health-aware retry, but extract Docker health
 // probing into a separate observation channel first rather than overloading
 // this one.
-const DOCKER_GPU_SUPERVISOR_RECONNECT_ERROR_PHASE_DEFAULT_DEBOUNCE_POLLS = 15;
+const DOCKER_GPU_SUPERVISOR_RECONNECT_ERROR_PHASE_DEFAULT_DEBOUNCE_POLLS = 60;
 
 export const DOCKER_GPU_SUPERVISOR_RECONNECT_TIMEOUT_ENV =
   "NEMOCLAW_DOCKER_GPU_SUPERVISOR_RECONNECT_TIMEOUT";