diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index f1a3348144d..537e4990b34 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -555,8 +555,8 @@ import { type SandboxGpuConfig, type SandboxGpuFlag, } from "./onboard/sandbox-gpu-mode"; -import { filterSlackSelectionByValidation } from "./onboard/slack-validation"; import type { SelectionDrift } from "./onboard/selection-drift"; +import { filterSlackSelectionByValidation } from "./onboard/slack-validation"; import { formatOnboardConfigSummary, formatSandboxBuildEstimateNote } from "./onboard/summary"; import type { ModelValidationResult, ValidationFailureLike } from "./onboard/types"; import type { ContainerRuntime } from "./platform"; @@ -3682,26 +3682,25 @@ async function createSandbox( // without this gate, NemoClaw registers a phantom sandbox that // causes "sandbox not found" on every subsequent connect/status call. console.log(" Waiting for sandbox to become ready..."); - const ready = sandboxReadinessTracing.waitForCreatedSandboxReadyWithTrace({ + const readiness = sandboxReadinessTracing.waitForCreatedSandboxReadyWithTrace({ sandboxName, timeoutSecs: sandboxReadyTimeoutSecs, runCaptureOpenshell, isSandboxReady, + getSandboxFailurePhase: gatewayState.getSandboxFailurePhase, sleep: sleepSeconds, }); const restoreBackupPath = pendingStateRestore?.manifest?.backupPath ?? pendingStateRestoreBackupPath; - if (!ready) { + if (!readiness.ready) { const diagnostics = sandboxCreateFailureDiagnostics.collectSandboxCreateFailureDiagnostics( sandboxName, { backupPath: restoreBackupPath }, ); console.error(""); - console.error( - ` Sandbox '${sandboxName}' was created but did not become ready within ${sandboxReadyTimeoutSecs}s.`, - ); + sandboxReadinessTracing.printReadinessFailure(readiness, sandboxName, sandboxReadyTimeoutSecs); if (diagnostics) { console.error(` Diagnostics saved: ${diagnostics.dir}`); if (diagnostics.summaryLines.length > 0) { @@ -3715,11 +3714,7 @@ async function createSandbox( } } if (useDockerGpuPatch) { - dockerGpuPatch.printDockerGpuReadinessFailure( - sandboxName, - dockerGpuCreatePatch.selectedMode(), - { runCaptureOpenshell }, - ); + dockerGpuCreatePatch.printReadinessFailureIfEnabled(); } else { // Clean up non-GPU failures after preserving local diagnostics so the // next onboard retry with the same name does not fail on "sandbox already exists". @@ -3749,13 +3744,14 @@ async function createSandbox( }); if (effectiveSandboxGpuConfig.sandboxGpuEnabled) { - // Runs the GPU proof, then (when the Docker GPU patch recreated the - // container) gates on host-network local inference reachability (#4509). + // Runs the GPU proof, preserving Docker-GPU patch Error-phase diagnostics + // when applicable, then gates host-network local inference reachability (#4509). dockerGpuLocalInference.verifyGpuSandboxAfterReady(effectiveSandboxGpuConfig, provider, { sandboxName, dockerDriverGateway: isLinuxDockerDriverGatewayEnabled(), useDockerGpuPatch, verifyDirectSandboxGpu, + verifyGpuOrExit: dockerGpuCreatePatch.verifyGpuOrExit, selectedMode: dockerGpuCreatePatch.selectedMode, runCaptureOpenshell, log: console.log, diff --git a/src/lib/onboard/docker-gpu-local-inference.test.ts b/src/lib/onboard/docker-gpu-local-inference.test.ts index 65d60a3baa0..5b9a4b77b07 100644 --- a/src/lib/onboard/docker-gpu-local-inference.test.ts +++ b/src/lib/onboard/docker-gpu-local-inference.test.ts @@ -286,6 +286,43 @@ describe("verifyGpuSandboxAfterReady", () => { expect(log).toHaveBeenCalledWith(expect.stringContaining("reachable from sandbox")); }); + it("uses Docker GPU patch verifier when supplied", () => { + const verifyDirectSandboxGpu = vi.fn(); + const verifyGpuOrExit = vi.fn((proof: (sandboxName: string) => void) => proof("alpha")); + verifyGpuSandboxAfterReady( + GPU_CONFIG, + "vllm-local", + baseOptions({ + verifyDirectSandboxGpu, + verifyGpuOrExit, + deps: { + findContainerIds: () => ["container-abc"], + dockerCapture: vi.fn(() => inspectWithNetworkMode("host")), + dockerRun: dockerRunWithCurl({ status: 0 }), + sleep: vi.fn(), + }, + }), + ); + expect(verifyGpuOrExit).toHaveBeenCalledWith(verifyDirectSandboxGpu); + expect(verifyDirectSandboxGpu).toHaveBeenCalledWith("alpha"); + }); + + it("does not duplicate proof diagnostics when Docker GPU patch verifier handles them", () => { + const proofError = new Error("process.exit"); + const verifyGpuOrExit = vi.fn(() => { + throw proofError; + }); + const logError = vi.fn(); + expect(() => + verifyGpuSandboxAfterReady( + GPU_CONFIG, + "ollama-local", + baseOptions({ verifyGpuOrExit, logError }), + ), + ).toThrow(proofError); + expect(logError).not.toHaveBeenCalled(); + }); + it("routes failure diagnostics through the provided error sink and exits", () => { const logError = vi.fn(); const exitSpy = vi.spyOn(process, "exit").mockImplementation((() => { diff --git a/src/lib/onboard/docker-gpu-local-inference.ts b/src/lib/onboard/docker-gpu-local-inference.ts index d93bd686c53..933e9e730fe 100644 --- a/src/lib/onboard/docker-gpu-local-inference.ts +++ b/src/lib/onboard/docker-gpu-local-inference.ts @@ -370,6 +370,7 @@ export type GpuSandboxAfterReadyOptions = { dockerDriverGateway: boolean; useDockerGpuPatch: boolean; verifyDirectSandboxGpu: (sandboxName: string) => void; + verifyGpuOrExit?: (verifyDirectSandboxGpu: (sandboxName: string) => void) => void; selectedMode: () => DockerGpuPatchMode | null; runCaptureOpenshell: (args: string[], opts?: Record) => string; env?: NodeJS.ProcessEnv; @@ -392,11 +393,20 @@ export function verifyGpuSandboxAfterReady( options: GpuSandboxAfterReadyOptions, ): void { try { - options.verifyDirectSandboxGpu(options.sandboxName); + if (options.verifyGpuOrExit) { + options.verifyGpuOrExit(options.verifyDirectSandboxGpu); + } else { + options.verifyDirectSandboxGpu(options.sandboxName); + } } catch (error) { - printDockerGpuProofFailure(options.sandboxName, error, options.selectedMode(), { - runCaptureOpenshell: options.runCaptureOpenshell, - }); + // `verifyGpuOrExit` is supplied by the Docker GPU create patch and already + // prints the richer Error-phase / patched-container diagnostics before + // rethrowing. Avoid a second generic proof-failure block in that path. + if (!options.verifyGpuOrExit) { + printDockerGpuProofFailure(options.sandboxName, error, options.selectedMode(), { + runCaptureOpenshell: options.runCaptureOpenshell, + }); + } throw error; } diff --git a/src/lib/onboard/docker-gpu-patch.test.ts b/src/lib/onboard/docker-gpu-patch.test.ts index 3462bcf0030..cf560e7e4ff 100644 --- a/src/lib/onboard/docker-gpu-patch.test.ts +++ b/src/lib/onboard/docker-gpu-patch.test.ts @@ -12,6 +12,8 @@ import { buildDockerGpuCloneRunOptions, buildDockerGpuMode, buildDockerGpuModeCandidates, + captureDockerGpuPatchSandboxSnapshot, + classifyDockerGpuPatchFailure, collectDockerGpuPatchDiagnostics, type DockerContainerInspect, detectSandboxFallbackDns, @@ -22,7 +24,13 @@ import { recreateOpenShellDockerSandboxWithGpu, selectDockerGpuPatchMode, shouldApplyDockerGpuPatch, + waitForOpenShellSupervisorReconnect, } from "../../../dist/lib/onboard/docker-gpu-patch"; +import { waitForCreatedSandboxReadyWithTrace } from "../../../dist/lib/onboard/sandbox-readiness-tracing"; +import { + getSandboxFailurePhase, + isSandboxReady, +} from "../../../dist/lib/state/gateway"; function inspectFixture(): DockerContainerInspect { return { @@ -777,3 +785,419 @@ describe("docker-gpu-patch sandbox DNS fallback (#3579)", () => { expect(args).toEqual(expect.arrayContaining(["--dns", "8.8.8.8"])); }); }); + +// Regression coverage for NemoClaw issue #4316: the Docker GPU patch path +// must distinguish "sandbox never became executable" (Error phase / dead +// container) from "GPU proof failed inside an executable sandbox", and the +// readiness wait must short-circuit on a terminal failure phase instead of +// burning the full timeout window. +describe("docker-gpu-patch Error-phase diagnostics (#4316)", () => { + it("detects terminal failure phases in `openshell sandbox list` output", () => { + const errorList = "my-sandbox Error 2s ago"; + expect(getSandboxFailurePhase(errorList, "my-sandbox")).toBe("Error"); + expect(getSandboxFailurePhase("my-sandbox CrashLoopBackOff 3s ago", "my-sandbox")) + .toBe("CrashLoopBackOff"); + expect(getSandboxFailurePhase("my-sandbox Failed 3s ago", "my-sandbox")).toBe("Failed"); + + expect(getSandboxFailurePhase("my-sandbox Ready 3s ago", "my-sandbox")).toBeNull(); + expect(getSandboxFailurePhase("other Error 3s ago", "my-sandbox")).toBeNull(); + expect(getSandboxFailurePhase("", "my-sandbox")).toBeNull(); + }); + + it("short-circuits the readiness wait when the sandbox enters Error phase", () => { + const outputs = [ + "my-sandbox Provisioning 1s ago", + "my-sandbox Error 3s ago", + ]; + let i = 0; + const runCaptureOpenshell = vi.fn(() => outputs[Math.min(i++, outputs.length - 1)]); + const sleep = vi.fn(); + + const ready = waitForCreatedSandboxReadyWithTrace({ + sandboxName: "my-sandbox", + // 600 / 2 = 300 readyAttempts. Without short-circuit we'd loop 300 + // times. With short-circuit we should bail out after the 2nd poll. + timeoutSecs: 600, + runCaptureOpenshell, + isSandboxReady, + getSandboxFailurePhase, + sleep, + }); + + expect(ready).toEqual({ + ready: false, + reason: "terminal_failure_phase", + failurePhase: "Error", + }); + expect(runCaptureOpenshell).toHaveBeenCalledTimes(2); + // Should not sleep after detecting the terminal phase. + expect(sleep).toHaveBeenCalledTimes(1); + }); + + it("short-circuits the supervisor-reconnect wait when the sandbox enters Error phase", () => { + // Without the short-circuit, a patched container that crashes on startup + // leaves users waiting the full 900s+ supervisor-reconnect timeout before + // any Error-phase diagnostics run (#4316). + const runOpenshell = vi.fn(() => ({ status: 1, stderr: "sandbox not ready" })); + const listOutputs = [ + "alpha Provisioning 1s ago", + "alpha Error 3s ago", + ]; + let i = 0; + const runCaptureOpenshell = vi.fn( + () => listOutputs[Math.min(i++, listOutputs.length - 1)], + ); + const sleep = vi.fn(); + + const ok = waitForOpenShellSupervisorReconnect("alpha", 600, { + runOpenshell, + runCaptureOpenshell, + sleep, + }); + + expect(ok).toBe(false); + // Without short-circuit we'd loop ~300 iterations. With it, the second + // iteration's list output shows Error and the wait bails out. + expect(runOpenshell).toHaveBeenCalledTimes(2); + expect(sleep).toHaveBeenCalledTimes(1); + }); + + it("prefers `sandbox list` phase over `sandbox get` when both are present (stale get)", () => { + // Regression guard for #4316 CodeRabbit feedback: when `sandbox get` + // returns a stale Phase (e.g. Provisioning while the gateway has already + // transitioned the row to Error), the list-derived phase must take + // precedence so the classifier doesn't act on stale data. + const runCaptureOpenshell = vi.fn((args: readonly string[]) => { + if (args[0] === "sandbox" && args[1] === "get") { + return "Name: alpha\nPhase: Provisioning\n"; + } + if (args[0] === "sandbox" && args[1] === "list") { + return "alpha Error 2s ago\n"; + } + return ""; + }); + + const snapshot = captureDockerGpuPatchSandboxSnapshot( + "alpha", + { patchedContainerId: null }, + { runCaptureOpenshell }, + ); + + expect(snapshot.sandboxPhase).toBe("Error"); + expect(snapshot.sandboxListLine).toContain("Error"); + }); + + it("uses the list-derived phase whenever the sandbox row is present", () => { + // Regression guard for CodeRabbit feedback: `sandbox list` reflects the + // gateway's table row and should be the phase used by the failure + // classifier whenever that row is available, even if `sandbox get` reports + // a different phase. + const runCaptureOpenshell = vi.fn((args: readonly string[]) => { + if (args[0] === "sandbox" && args[1] === "get") { + return "Name: alpha\nPhase: Error\nReason: ContainerCannotRun\n"; + } + if (args[0] === "sandbox" && args[1] === "list") { + return "alpha Ready 1m ago\n"; + } + return ""; + }); + + const snapshot = captureDockerGpuPatchSandboxSnapshot( + "alpha", + { patchedContainerId: null }, + { runCaptureOpenshell }, + ); + + expect(snapshot.sandboxPhase).toBe("Ready"); + expect(snapshot.sandboxListLine).toContain("Ready"); + }); + + it("keeps the get-derived phase when the sandbox row is absent from list output", () => { + // Complement to the precedence test: if `sandbox list` has no row for + // the named sandbox (e.g. the gateway lost track of it), the get-derived + // phase is the only signal we have — don't drop it. + const runCaptureOpenshell = vi.fn((args: readonly string[]) => { + if (args[0] === "sandbox" && args[1] === "get") { + return "Name: alpha\nPhase: Terminated\n"; + } + if (args[0] === "sandbox" && args[1] === "list") { + return "other-box Ready 2s ago\n"; + } + return ""; + }); + + const snapshot = captureDockerGpuPatchSandboxSnapshot( + "alpha", + { patchedContainerId: null }, + { runCaptureOpenshell }, + ); + + expect(snapshot.sandboxPhase).toBe("Terminated"); + expect(snapshot.sandboxListLine).toBeNull(); + }); + + it("captures sandbox phase and patched container State via the snapshot helper", () => { + const runCaptureOpenshell = vi.fn((args: readonly string[]) => { + if (args[0] === "sandbox" && args[1] === "get") { + return "Name: alpha\nPhase: Error\nReason: ContainerExit\n"; + } + if (args[0] === "sandbox" && args[1] === "list") { + return "alpha Error 1m ago\n"; + } + return ""; + }); + const dockerCapture = vi.fn((args: readonly string[]) => { + if (args[0] === "inspect" && args[1] === "--format" && args[2] === "{{json .State}}") { + return JSON.stringify({ + Status: "exited", + Running: false, + ExitCode: 125, + Error: 'could not select device driver "nvidia" with capabilities: [[gpu]]', + OOMKilled: false, + StartedAt: "2026-05-12T00:00:00Z", + FinishedAt: "2026-05-12T00:00:01Z", + }); + } + return ""; + }); + + const snapshot = captureDockerGpuPatchSandboxSnapshot( + "alpha", + { patchedContainerId: "new-container-id" }, + { runCaptureOpenshell, dockerCapture }, + ); + + expect(snapshot.sandboxPhase).toBe("Error"); + expect(snapshot.sandboxListLine).toBe("alpha Error 1m ago"); + expect(snapshot.patchedContainerState?.ExitCode).toBe(125); + expect(snapshot.patchedContainerState?.Error).toContain("could not select device driver"); + }); + + it("classifies a dead patched container as patched_container_failed with the failed mode", () => { + const result = classifyDockerGpuPatchFailure( + { + sandboxPhase: "Error", + sandboxListLine: "alpha Error 1m ago", + patchedContainerState: { + Status: "exited", + ExitCode: 125, + Error: 'could not select device driver "nvidia" with capabilities: [[gpu]]', + }, + }, + buildDockerGpuMode("gpus"), + ); + + expect(result.kind).toBe("patched_container_failed"); + expect(result.headline).toContain("Patched GPU container exited with code 125"); + expect(result.headline).toContain("--gpus all"); + const flat = result.summaryLines.join("\n"); + expect(flat).toContain("sandbox_phase=Error"); + expect(flat).toContain("patched_container_exit_code=125"); + expect(flat).toContain("could not select device driver"); + expect(flat).toContain("patched_create_option=--gpus all"); + }); + + it("classifies an Error-phase sandbox with unknown container state as sandbox_error_phase", () => { + const result = classifyDockerGpuPatchFailure( + { + sandboxPhase: "Error", + sandboxListLine: null, + patchedContainerState: null, + }, + buildDockerGpuMode("gpus"), + ); + + expect(result.kind).toBe("sandbox_error_phase"); + expect(result.headline).toContain("OpenShell sandbox entered Error phase"); + }); + + it("classifies a live container but timed-out supervisor as supervisor_unreachable", () => { + const result = classifyDockerGpuPatchFailure( + { + sandboxPhase: "Provisioning", + sandboxListLine: "alpha Provisioning 30s ago", + patchedContainerState: { Status: "running", Running: true, ExitCode: 0 }, + }, + buildDockerGpuMode("gpus"), + ); + + expect(result.kind).toBe("supervisor_unreachable"); + expect(result.headline).toContain("Provisioning"); + }); + + it("prefers supervisor_unreachable over proof_failure when the sandbox is non-live but non-terminal", () => { + // Regression guard for #4316 review: a proof failing while the sandbox is + // still in a transient/non-live phase (Provisioning, NotReady) is really + // a lifecycle failure — classifying it as proof_failure would tell users + // `nvidia-smi` failed inside an executable sandbox, which masks the real + // cause. + const result = classifyDockerGpuPatchFailure( + { + sandboxPhase: "Provisioning", + sandboxListLine: "alpha Provisioning 30s ago", + patchedContainerState: null, + }, + buildDockerGpuMode("gpus"), + { proofError: new Error("openshell sandbox exec refused: sandbox not ready") }, + ); + + expect(result.kind).toBe("supervisor_unreachable"); + expect(result.headline).toContain("Provisioning"); + expect(result.summaryLines.join("\n")).toContain("proof_error="); + }); + + it("does not blame the supervisor when the patch failed before a container existed", () => { + // Regression guard for #4316 review: an early patch failure (e.g. all GPU + // mode probes were rejected, or detached `docker run` failed) leaves no + // patched container. If the original sandbox happens to still be in a + // transient phase like Provisioning, the classifier must not point at + // an OpenShell supervisor reconnect issue. + const result = classifyDockerGpuPatchFailure( + { + sandboxPhase: "Provisioning", + sandboxListLine: "alpha Provisioning 3s ago", + patchedContainerState: null, + }, + null, + ); + + expect(result.kind).toBe("unknown"); + expect(result.headline).not.toMatch(/supervisor/i); + }); + + it("treats proof failures inside a Ready sandbox as proof_failure, not patched_container_failed", () => { + const result = classifyDockerGpuPatchFailure( + { + sandboxPhase: "Ready", + sandboxListLine: "alpha Ready 30s ago", + patchedContainerState: { Status: "running", Running: true, ExitCode: 0 }, + }, + buildDockerGpuMode("gpus"), + { proofError: new Error("nvidia-smi exited with status 9") }, + ); + + expect(result.kind).toBe("proof_failure"); + expect(result.summaryLines.join("\n")).toContain("proof_error=nvidia-smi exited with status 9"); + }); + + it("preserves the default Docker capture when callers omit dockerCapture from deps", () => { + // Regression guard for #4316 review: passing `dockerCapture: undefined` + // through to `depsWithDefaults` would shadow the module's real Docker + // adapter. The print/diagnostic helpers must NOT forward an explicit + // `undefined` — they should let the default flow through so `docker ps` + // and `docker inspect ` still run. + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-docker-gpu-default-")); + try { + const dockerCapture = vi.fn((_args: readonly string[]) => ""); + const dockerLogs = vi.fn(() => ""); + collectDockerGpuPatchDiagnostics( + "alpha", + { + context: { + sandboxName: "alpha", + newContainerId: "new-container-id", + selectedMode: buildDockerGpuMode("gpus"), + }, + }, + { + // `runCaptureOpenshell` intentionally omitted — exercises the + // "caller has no openshell capture either" path. + dockerCapture, + dockerLogs, + homedir: () => tmpDir, + now: () => new Date("2026-05-12T00:00:00Z"), + }, + ); + + // Without the fix, `depsWithDefaults` would still see `dockerCapture` as + // a function here (the explicit one), so this is more of a structural + // sanity check. The substantive regression is exercised at the print- + // helper level (printDockerGpuPatchFailureAndExit must not pass + // `dockerCapture: undefined`). Here we just confirm collect() invokes + // the supplied dockerCapture for ps/inspect. + expect( + dockerCapture.mock.calls.some(([args]) => args?.[0] === "ps"), + ).toBe(true); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it("does not inspect the original/backup container when newContainerId is missing", () => { + // Regression guard for #4316 review: when `recreateOpenShellDockerSandboxWithGpu` + // throws before the patched container exists, only `oldContainerId` is set + // in the failure context. The snapshot must NOT inspect the old/backup + // container as if it were the patched one — that would mis-attribute the + // patched container's State. + const dockerCapture = vi.fn((args: readonly string[]) => { + if (args[0] === "inspect" && args[1] === "--format" && args[2] === "{{json .State}}") { + // If this is called for old-container-id, return State that *looks* + // like a failed patch; the test would then incorrectly classify it. + return JSON.stringify({ Status: "exited", ExitCode: 1 }); + } + return ""; + }); + + const snapshot = captureDockerGpuPatchSandboxSnapshot( + "alpha", + { patchedContainerId: null }, + { dockerCapture }, + ); + + expect(snapshot.patchedContainerState).toBeNull(); + // The `--format '{{json .State}}'` invocation should not have happened. + expect( + dockerCapture.mock.calls.some( + ([args]) => args[0] === "inspect" && args[1] === "--format", + ), + ).toBe(false); + }); + + it("writes patched-container-state.json and surfaces failure_kind/sandbox_phase in the summary", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-docker-gpu-4316-")); + try { + const snapshot = { + sandboxPhase: "Error", + sandboxListLine: "alpha Error 1m ago", + patchedContainerState: { + Status: "exited", + ExitCode: 125, + Error: 'could not select device driver "nvidia"', + }, + }; + const classification = classifyDockerGpuPatchFailure(snapshot, buildDockerGpuMode("gpus")); + const diagnostics = collectDockerGpuPatchDiagnostics( + "alpha", + { + context: { + sandboxName: "alpha", + newContainerId: "new-container-id", + selectedMode: buildDockerGpuMode("gpus"), + }, + selectedMode: buildDockerGpuMode("gpus"), + snapshot, + classification, + }, + { + dockerCapture: vi.fn(() => ""), + dockerLogs: vi.fn(() => ""), + homedir: () => tmpDir, + now: () => new Date("2026-05-12T00:00:00Z"), + }, + ); + + expect(diagnostics?.dir).toBeTruthy(); + const summary = fs.readFileSync(path.join(diagnostics?.dir || "", "summary.txt"), "utf-8"); + expect(summary).toContain("failure_kind=patched_container_failed"); + expect(summary).toContain("sandbox_phase=Error"); + expect(summary).toContain("patched_container_exit_code=125"); + const state = fs.readFileSync( + path.join(diagnostics?.dir || "", "patched-container-state.json"), + "utf-8", + ); + expect(state).toContain("could not select device driver"); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); +}); diff --git a/src/lib/onboard/docker-gpu-patch.ts b/src/lib/onboard/docker-gpu-patch.ts index bc3546291d3..5e9dd16a130 100644 --- a/src/lib/onboard/docker-gpu-patch.ts +++ b/src/lib/onboard/docker-gpu-patch.ts @@ -118,6 +118,53 @@ export type DockerGpuPatchDiagnostics = { summaryLines: string[]; }; +/** + * Subset of `docker inspect --format '{{json .State}}'` fields surfaced when + * the patched GPU sandbox container fails to become executable. We capture + * just the runtime/exit/health state — not the full inspect — because that + * is what tells the user *why* the patched create option broke (e.g. a + * non-zero ExitCode with `Error: "could not select device driver"`). + */ +export type DockerContainerState = { + Status?: string; + Running?: boolean; + Paused?: boolean; + Restarting?: boolean; + OOMKilled?: boolean; + Dead?: boolean; + ExitCode?: number; + Error?: string; + StartedAt?: string; + FinishedAt?: string; + Health?: { Status?: string; FailingStreak?: number } | null; +}; + +/** + * Snapshot of "is the patched sandbox even runnable?" — sandbox phase from + * OpenShell plus the patched Docker container's State. This is the data the + * caller needs to tell the user whether the failure is at the OpenShell + * sandbox layer (Error phase) vs. the Docker container layer (non-zero exit + * with a driver/runtime error) — see #4316. + */ +export type DockerGpuPatchSandboxSnapshot = { + sandboxPhase: string | null; + sandboxListLine: string | null; + patchedContainerState: DockerContainerState | null; +}; + +export type DockerGpuPatchFailureKind = + | "patched_container_failed" + | "sandbox_error_phase" + | "supervisor_unreachable" + | "proof_failure" + | "unknown"; + +export type DockerGpuPatchFailureClassification = { + kind: DockerGpuPatchFailureKind; + headline: string; + summaryLines: string[]; +}; + export type DockerContainerInspect = { Id?: string; Name?: string; @@ -786,6 +833,24 @@ function waitForNewContainerId( return null; } +function sandboxListShowsErrorPhase( + sandboxName: string, + runCaptureOpenshell: NonNullable, +): boolean { + try { + const list = runCaptureOpenshell(["sandbox", "list"], { + ignoreError: true, + suppressOutput: true, + timeout: DOCKER_GPU_PATCH_TIMEOUT_MS, + }); + return SANDBOX_FAILURE_PHASE_TOKENS.has( + parseSandboxPhaseFromListOutput(list, sandboxName) ?? "", + ); + } catch { + return false; + } +} + function waitForOpenShellSandboxExec( sandboxName: string, timeoutSecs: number, @@ -800,6 +865,16 @@ function waitForOpenShellSandboxExec( { ignoreError: true, suppressOutput: true, timeout: DOCKER_GPU_PATCH_TIMEOUT_MS }, ); if (isZeroStatus(result)) return true; + // Short-circuit the supervisor-reconnect wait when the sandbox enters a + // terminal failure phase. Without this, a patched container that exits + // on startup leaves the user staring at the supervisor-reconnect + // timeout (default 900s) before any Error-phase diagnostics run (#4316). + if ( + deps.runCaptureOpenshell && + sandboxListShowsErrorPhase(sandboxName, deps.runCaptureOpenshell) + ) { + return false; + } d.sleep(2); } return false; @@ -997,26 +1072,67 @@ export function applyDockerGpuPatchOrExit( } } +function printDockerGpuPatchClassificationLines( + classification: DockerGpuPatchFailureClassification | null, +): void { + if (!classification) return; + if (classification.headline) console.error(` ${classification.headline}`); + for (const line of classification.summaryLines) console.error(` ${line}`); +} + +function patchedContainerIdFromContext( + context?: DockerGpuPatchFailureContext | null, +): string | null { + // Snapshot only the newly created GPU-enabled container. Falling back to + // `oldContainerId` here would inspect the original (or its renamed backup) + // and mis-attribute its State as the patched container's — see #4316 + // review feedback. + if (!context) return null; + return context.newContainerId || null; +} + +function snapshotInspectDeps( + deps: Pick, +): Pick { + // `depsWithDefaults` spreads the caller's `deps`, so passing an explicit + // `dockerCapture: undefined` would shadow the module's default Docker + // adapter and disable downstream `docker ps`/`inspect`/`logs` capture. + // Build the inner deps object with only the keys the caller actually + // supplied so defaults stay in place. + const inner: Pick = {}; + if (deps.runCaptureOpenshell) inner.runCaptureOpenshell = deps.runCaptureOpenshell; + if (deps.dockerCapture) inner.dockerCapture = deps.dockerCapture; + return inner; +} + export function printDockerGpuPatchFailureAndExit( sandboxName: string, error: unknown, - deps: Pick & { + deps: Pick & { context?: DockerGpuPatchFailureContext | null; selectedMode?: DockerGpuPatchMode | null; }, ): never { + const context = deps.context || getDockerGpuPatchFailureContext(error) || null; + const selectedMode = deps.selectedMode || context?.selectedMode || null; + const inspectDeps = snapshotInspectDeps(deps); + const snapshot = captureDockerGpuPatchSandboxSnapshot( + sandboxName, + { patchedContainerId: patchedContainerIdFromContext(context) }, + inspectDeps, + ); + const classification = classifyDockerGpuPatchFailure(snapshot, selectedMode); const diagnostics = collectDockerGpuPatchDiagnostics( sandboxName, - { error, context: deps.context, selectedMode: deps.selectedMode }, - { - runCaptureOpenshell: deps.runCaptureOpenshell, - }, + { error, context, selectedMode, snapshot, classification }, + inspectDeps, ); console.error(""); console.error(" Docker GPU patch failed."); if (error instanceof Error && error.message) { console.error(` ${error.message}`); } + printDockerGpuPatchClassificationLines(classification); if (diagnostics) { console.error(` Diagnostics saved: ${diagnostics.dir}`); } @@ -1028,15 +1144,24 @@ export function printDockerGpuPatchFailureAndExit( export function printDockerGpuReadinessFailure( sandboxName: string, selectedMode: DockerGpuPatchMode | null, - deps: Pick, + deps: Pick & { + context?: DockerGpuPatchFailureContext | null; + }, ): void { + const context = deps.context ?? null; + const inspectDeps = snapshotInspectDeps(deps); + const snapshot = captureDockerGpuPatchSandboxSnapshot( + sandboxName, + { patchedContainerId: patchedContainerIdFromContext(context) }, + inspectDeps, + ); + const classification = classifyDockerGpuPatchFailure(snapshot, selectedMode); const diagnostics = collectDockerGpuPatchDiagnostics( sandboxName, - { selectedMode }, - { - runCaptureOpenshell: deps.runCaptureOpenshell, - }, + { selectedMode, context, snapshot, classification }, + inspectDeps, ); + printDockerGpuPatchClassificationLines(classification); if (diagnostics) { console.error(` Docker GPU diagnostics saved: ${diagnostics.dir}`); } @@ -1047,15 +1172,26 @@ export function printDockerGpuProofFailure( sandboxName: string, error: unknown, selectedMode: DockerGpuPatchMode | null, - deps: Pick, + deps: Pick & { + context?: DockerGpuPatchFailureContext | null; + }, ): void { + const context = deps.context ?? null; + const inspectDeps = snapshotInspectDeps(deps); + const snapshot = captureDockerGpuPatchSandboxSnapshot( + sandboxName, + { patchedContainerId: patchedContainerIdFromContext(context) }, + inspectDeps, + ); + const classification = classifyDockerGpuPatchFailure(snapshot, selectedMode, { + proofError: error, + }); const diagnostics = collectDockerGpuPatchDiagnostics( sandboxName, - { error, selectedMode }, - { - runCaptureOpenshell: deps.runCaptureOpenshell, - }, + { error, selectedMode, context, snapshot, classification }, + inspectDeps, ); + printDockerGpuPatchClassificationLines(classification); if (diagnostics) { console.error(` Diagnostics saved: ${diagnostics.dir}`); } @@ -1126,12 +1262,255 @@ export function formatDockerInspectNetworkSummary( return lines.join("\n"); } +const SANDBOX_FAILURE_PHASE_TOKENS = new Set([ + "Error", + "Failed", + "CrashLoopBackOff", +]); + +const SANDBOX_LIVE_PHASE_TOKENS = new Set(["Ready", "Running"]); + +const ANSI_RE = /\x1b\[[0-9;]*m/g; + +function stripAnsi(value: string): string { + return value.replace(ANSI_RE, ""); +} + +function parseSandboxRowForName(output: string, sandboxName: string): string[] | null { + if (typeof output !== "string") return null; + for (const line of stripAnsi(output).split("\n")) { + const cols = line.trim().split(/\s+/); + if (cols[0] === sandboxName) return cols; + } + return null; +} + +function findSandboxListLine(output: string, sandboxName: string): string | null { + if (typeof output !== "string") return null; + for (const line of stripAnsi(output).split("\n")) { + if (line.trim().split(/\s+/)[0] === sandboxName) return line.trim(); + } + return null; +} + +function parseSandboxPhaseFromGetOutput(output: string): string | null { + if (typeof output !== "string") return null; + const match = stripAnsi(output).match(/^\s*Phase:\s+(\S+)/m); + return match ? match[1] : null; +} + +function parseSandboxPhaseFromListOutput(output: string, sandboxName: string): string | null { + const cols = parseSandboxRowForName(output, sandboxName); + if (!cols) return null; + return ( + cols.find((col) => SANDBOX_FAILURE_PHASE_TOKENS.has(col)) ?? + cols.find((col) => SANDBOX_LIVE_PHASE_TOKENS.has(col)) ?? + cols[1] ?? + null + ); +} + +function isFailurePhase(phase: string | null | undefined): boolean { + return typeof phase === "string" && SANDBOX_FAILURE_PHASE_TOKENS.has(phase); +} + +function parseDockerContainerState(json: string): DockerContainerState | null { + if (!json.trim()) return null; + try { + const parsed = JSON.parse(json); + // `docker inspect --format '{{json .State}}'` returns the State object + // directly; `docker inspect ` returns an array of full container + // descriptors with `.State` nested. Accept both shapes. + if (parsed && typeof parsed === "object") { + if ("Status" in parsed || "ExitCode" in parsed || "Running" in parsed) { + return parsed as DockerContainerState; + } + const first = Array.isArray(parsed) ? parsed[0] : parsed; + if (first && typeof first === "object" && "State" in first) { + const state = (first as { State?: unknown }).State; + if (state && typeof state === "object") return state as DockerContainerState; + } + } + } catch { + /* fall through */ + } + return null; +} + +/** + * Capture the current sandbox phase from OpenShell and the patched + * container's runtime State from Docker. Either field may be null when the + * external CLI is unavailable or the named target no longer exists; callers + * (notably `classifyDockerGpuPatchFailure`) treat null defensively. + * + * When `deps.dockerCapture` is not supplied, this helper falls back to the + * module's default Docker adapter so the patched-container State is still + * captured in production paths that only thread `runCaptureOpenshell` + * through (e.g. `applyDockerGpuPatchOrExit`). + */ +export function captureDockerGpuPatchSandboxSnapshot( + sandboxName: string, + options: { + patchedContainerId?: string | null; + } = {}, + deps: Pick = {}, +): DockerGpuPatchSandboxSnapshot { + let sandboxPhase: string | null = null; + let sandboxListLine: string | null = null; + if (deps.runCaptureOpenshell) { + try { + const getOutput = deps.runCaptureOpenshell(["sandbox", "get", sandboxName], { + ignoreError: true, + timeout: DOCKER_GPU_PATCH_TIMEOUT_MS, + }); + sandboxPhase = parseSandboxPhaseFromGetOutput(getOutput); + } catch { + /* best effort */ + } + try { + const listOutput = deps.runCaptureOpenshell(["sandbox", "list"], { + ignoreError: true, + timeout: DOCKER_GPU_PATCH_TIMEOUT_MS, + }); + sandboxListLine = findSandboxListLine(listOutput, sandboxName); + // Prefer the `sandbox list` phase whenever the named row is present. + // The list row is the operator-facing gateway state and avoids letting + // a stale `sandbox get` response drive the Docker-GPU failure + // classification (#4316 CodeRabbit feedback). + if (sandboxListLine) { + const listPhase = parseSandboxPhaseFromListOutput(listOutput, sandboxName); + if (listPhase) sandboxPhase = listPhase; + } + } catch { + /* best effort */ + } + } + + let patchedContainerState: DockerContainerState | null = null; + const target = String(options.patchedContainerId || "").trim(); + if (target) { + const capture = deps.dockerCapture ?? dockerCapture; + try { + const stateJson = capture( + ["inspect", "--format", "{{json .State}}", target], + { ignoreError: true, timeout: DOCKER_GPU_PATCH_TIMEOUT_MS }, + ); + patchedContainerState = parseDockerContainerState(stateJson); + } catch { + /* best effort */ + } + } + + return { sandboxPhase, sandboxListLine, patchedContainerState }; +} + +function describePatchedContainerState(state: DockerContainerState | null): string[] { + if (!state) return []; + const lines: string[] = []; + if (state.Status) lines.push(`patched_container_status=${state.Status}`); + if (typeof state.ExitCode === "number") lines.push(`patched_container_exit_code=${state.ExitCode}`); + if (state.OOMKilled) lines.push("patched_container_oom_killed=true"); + if (state.Error) lines.push(`patched_container_error=${state.Error}`); + if (state.Health?.Status) lines.push(`patched_container_health=${state.Health.Status}`); + if (state.FinishedAt && state.FinishedAt !== "0001-01-01T00:00:00Z") { + lines.push(`patched_container_finished_at=${state.FinishedAt}`); + } + return lines; +} + +function patchedContainerLooksFailed(state: DockerContainerState | null): boolean { + if (!state) return false; + if (state.Dead === true) return true; + if (state.OOMKilled === true) return true; + if (typeof state.ExitCode === "number" && state.ExitCode !== 0) return true; + if (state.Error && state.Error.length > 0) return true; + // `exited`/`dead`/`removing` indicate a container that did not stay up. + // `running` and `restarting` are live states we do not classify as failed. + if (typeof state.Status === "string") { + const status = state.Status.toLowerCase(); + if (status === "exited" || status === "dead" || status === "removing") return true; + } + return false; +} + +/** + * Turn the snapshot + selected GPU mode into a user-facing classification + * that distinguishes "the patched container itself died" from "the sandbox + * never reached a live phase" from "the OpenShell supervisor cannot reach + * the container" from "the GPU proof itself reported a runtime failure". + * + * This is the contract NemoClaw uses to tell users *which* part of the + * GPU patch path broke — not just "something failed" (#4316). + */ +export function classifyDockerGpuPatchFailure( + snapshot: DockerGpuPatchSandboxSnapshot, + selectedMode: DockerGpuPatchMode | null, + options: { proofError?: unknown } = {}, +): DockerGpuPatchFailureClassification { + const lines: string[] = []; + if (snapshot.sandboxPhase) lines.push(`sandbox_phase=${snapshot.sandboxPhase}`); + if (snapshot.sandboxListLine) lines.push(`sandbox_list_row=${snapshot.sandboxListLine}`); + lines.push(...describePatchedContainerState(snapshot.patchedContainerState)); + if (selectedMode) lines.push(`patched_create_option=${selectedMode.label}`); + + const containerFailed = patchedContainerLooksFailed(snapshot.patchedContainerState); + const sandboxInErrorPhase = isFailurePhase(snapshot.sandboxPhase); + const sandboxNotLive = + !!snapshot.sandboxPhase && !SANDBOX_LIVE_PHASE_TOKENS.has(snapshot.sandboxPhase); + + let kind: DockerGpuPatchFailureKind = "unknown"; + let headline: string; + if (containerFailed) { + kind = "patched_container_failed"; + const exit = snapshot.patchedContainerState?.ExitCode; + const opt = selectedMode ? ` (${selectedMode.label})` : ""; + headline = + typeof exit === "number" && exit !== 0 + ? `Patched GPU container exited with code ${exit}${opt}.` + : `Patched GPU container is not running${opt}.`; + } else if (sandboxInErrorPhase) { + kind = "sandbox_error_phase"; + headline = `OpenShell sandbox entered ${snapshot.sandboxPhase} phase before the GPU proof could run.`; + } else if (sandboxNotLive && (snapshot.patchedContainerState || options.proofError)) { + // Cover the non-live-but-non-terminal case (e.g. Provisioning / NotReady) + // BEFORE the proof-error branch — a proof failing while the sandbox + // never reached Ready/Running is really a lifecycle failure, not a + // proof failure. Classifying it as proof_failure would tell users + // `nvidia-smi` failed inside an executable sandbox, which is the + // wrong story (#4316 review feedback). + // + // Gate this on evidence that the patched container actually existed + // (either we inspected its State, or we got far enough to attempt the + // proof). Otherwise an early patch failure (e.g. mode probes rejected, + // detached `docker run` failing) would mislabel a still-Provisioning + // original sandbox as a supervisor reconnect issue. + kind = "supervisor_unreachable"; + headline = `OpenShell supervisor did not reach Ready (last phase: ${snapshot.sandboxPhase}).`; + } else if (options.proofError) { + kind = "proof_failure"; + headline = "GPU proof failed inside an executable sandbox."; + } else { + headline = "Docker GPU patch did not complete successfully."; + } + + if (options.proofError) { + const proofText = + options.proofError instanceof Error + ? options.proofError.message + : String(options.proofError); + if (proofText) lines.push(`proof_error=${proofText}`); + } + return { kind, headline, summaryLines: lines }; +} + export function collectDockerGpuPatchDiagnostics( sandboxName: string, options: { error?: unknown; context?: DockerGpuPatchFailureContext | null; selectedMode?: DockerGpuPatchMode | null; + snapshot?: DockerGpuPatchSandboxSnapshot | null; + classification?: DockerGpuPatchFailureClassification | null; } = {}, deps: DockerGpuPatchDeps = {}, ): DockerGpuPatchDiagnostics | null { @@ -1158,6 +1537,8 @@ export function collectDockerGpuPatchDiagnostics( ? String(options.error) : "none"; const selectedMode = options.selectedMode || context?.selectedMode || null; + const snapshot = options.snapshot ?? null; + const classification = options.classification ?? null; const summaryLines = [ `created_at=${now.toISOString()}`, `sandbox_name=${sandboxName}`, @@ -1177,7 +1558,23 @@ export function collectDockerGpuPatchDiagnostics( ); } } + if (classification) { + summaryLines.push(`failure_kind=${classification.kind}`); + if (classification.headline) summaryLines.push(`failure_headline=${classification.headline}`); + } + if (snapshot) { + if (snapshot.sandboxPhase) summaryLines.push(`sandbox_phase=${snapshot.sandboxPhase}`); + if (snapshot.sandboxListLine) summaryLines.push(`sandbox_list_row=${snapshot.sandboxListLine}`); + summaryLines.push(...describePatchedContainerState(snapshot.patchedContainerState)); + } writeTextFile(dir, "summary.txt", summaryLines.join("\n")); + if (snapshot?.patchedContainerState) { + writeTextFile( + dir, + "patched-container-state.json", + JSON.stringify(snapshot.patchedContainerState, null, 2), + ); + } try { const ps = d.dockerCapture( diff --git a/src/lib/onboard/docker-gpu-sandbox-create.ts b/src/lib/onboard/docker-gpu-sandbox-create.ts index cb8f657c181..00e7dd3fbd0 100644 --- a/src/lib/onboard/docker-gpu-sandbox-create.ts +++ b/src/lib/onboard/docker-gpu-sandbox-create.ts @@ -4,6 +4,7 @@ import type { DockerGpuPatchBackend, DockerGpuPatchDeps, + DockerGpuPatchFailureContext, DockerGpuPatchMode, DockerGpuPatchResult, } from "./docker-gpu-patch"; @@ -12,14 +13,17 @@ import { findOpenShellDockerSandboxContainerIds, getDockerGpuSupervisorReconnectTimeoutSecs, printDockerGpuPatchFailureAndExit, + printDockerGpuProofFailure, + printDockerGpuReadinessFailure, recreateOpenShellDockerSandboxWithGpu, shouldApplyDockerGpuPatch, waitForOpenShellSupervisorReconnect, } from "./docker-gpu-patch"; +import { getSandboxFailurePhase } from "../state/gateway"; type DockerGpuSandboxCreateDeps = Pick< DockerGpuPatchDeps, - "runOpenshell" | "runCaptureOpenshell" | "sleep" + "runOpenshell" | "runCaptureOpenshell" | "sleep" | "dockerCapture" >; type DockerGpuSandboxCreatePatchOptions = { @@ -50,6 +54,19 @@ export type DockerGpuSandboxCreatePatch = { ensureApplied: () => void; waitForSupervisorReconnectIfNeeded: () => void; selectedMode: () => DockerGpuPatchMode | null; + /** + * Print the Docker GPU readiness-failure block (including the Error-phase + * classification + patched container State diagnostics) when the + * post-create readiness wait times out. No-op when the patch is disabled. + */ + printReadinessFailureIfEnabled: () => void; + /** + * Run the GPU proof while distinguishing "sandbox in terminal phase" from + * "proof failed inside a live sandbox". Calls `process.exit(1)` for the + * former and rethrows after printing diagnostics for the latter so the + * onboarding flow surfaces the right failure cause (#4316). + */ + verifyGpuOrExit: (verifyDirectSandboxGpu: (sandboxName: string) => void) => void; }; export function createDockerGpuSandboxCreatePatch( @@ -96,6 +113,7 @@ export function createDockerGpuSandboxCreatePatch( if (!patchError) return; printDockerGpuPatchFailureAndExit(options.sandboxName, patchError, { runCaptureOpenshell: options.deps.runCaptureOpenshell, + dockerCapture: options.deps.dockerCapture, }); }, @@ -115,7 +133,15 @@ export function createDockerGpuSandboxCreatePatch( const supervisorReady = waitForOpenShellSupervisorReconnect( options.sandboxName, supervisorReconnectTimeoutSecs, - { runOpenshell: options.deps.runOpenshell, sleep: options.deps.sleep }, + { + runOpenshell: options.deps.runOpenshell, + // Pass `runCaptureOpenshell` so the supervisor-reconnect wait can + // short-circuit on a terminal sandbox phase instead of burning + // the full reconnect timeout window when the patched container + // crashed on startup (#4316). + runCaptureOpenshell: options.deps.runCaptureOpenshell, + sleep: options.deps.sleep, + }, ); if (supervisorReady) return; printDockerGpuPatchFailureAndExit( @@ -123,6 +149,7 @@ export function createDockerGpuSandboxCreatePatch( new Error("OpenShell supervisor did not reconnect to the GPU-enabled container."), { runCaptureOpenshell: options.deps.runCaptureOpenshell, + dockerCapture: options.deps.dockerCapture, context: { sandboxName: options.sandboxName, oldContainerId: result?.oldContainerId, @@ -137,6 +164,77 @@ export function createDockerGpuSandboxCreatePatch( selectedMode() { return result?.mode ?? null; }, + + printReadinessFailureIfEnabled() { + if (!options.enabled) return; + printDockerGpuReadinessFailure(options.sandboxName, result?.mode ?? null, { + runCaptureOpenshell: options.deps.runCaptureOpenshell, + dockerCapture: options.deps.dockerCapture, + context: buildFailureContext(options.sandboxName, result), + }); + }, + + verifyGpuOrExit(verifyDirectSandboxGpu) { + // Before issuing GPU proof commands through `openshell sandbox exec`, + // confirm the sandbox is still in a live phase. A sandbox that + // transitioned to Error after the readiness wait succeeded (e.g. the + // patched GPU container crashed mid-startup) would make the proof step + // fail with an exec error that looks like an `nvidia-smi` failure — + // masking the real cause. When that happens, surface the patched- + // container/Error-phase classification instead of running the proof + // (#4316). + const sandboxName = options.sandboxName; + const failureContext = buildFailureContext(sandboxName, result); + if (options.enabled && options.deps.runCaptureOpenshell) { + const list = options.deps.runCaptureOpenshell(["sandbox", "list"], { + ignoreError: true, + }); + const phase = getSandboxFailurePhase(list, sandboxName); + if (phase) { + console.error(""); + console.error(` Skipping GPU proof: sandbox '${sandboxName}' is in ${phase} phase.`); + printDockerGpuProofFailure( + sandboxName, + new Error( + `Sandbox '${sandboxName}' entered ${phase} phase after readiness; GPU proof skipped.`, + ), + result?.mode ?? null, + { + runCaptureOpenshell: options.deps.runCaptureOpenshell, + dockerCapture: options.deps.dockerCapture, + context: failureContext, + }, + ); + process.exit(1); + } + } + try { + verifyDirectSandboxGpu(sandboxName); + } catch (error) { + printDockerGpuProofFailure(sandboxName, error, result?.mode ?? null, { + runCaptureOpenshell: options.deps.runCaptureOpenshell, + dockerCapture: options.deps.dockerCapture, + context: options.enabled ? failureContext : null, + }); + throw error; + } + }, + }; +} + +function buildFailureContext( + sandboxName: string, + result: DockerGpuPatchResult | null, +): DockerGpuPatchFailureContext { + return { + sandboxName, + // `oldContainerId` is retained alongside `newContainerId` so the + // before/after pair lands in `patched-container-state.json` and + // `docker-network-summary.txt`, matching the supervisor-reconnect path. + oldContainerId: result?.oldContainerId ?? null, + newContainerId: result?.newContainerId ?? null, + backupContainerName: result?.backupContainerName ?? null, + selectedMode: result?.mode ?? null, }; } diff --git a/src/lib/onboard/sandbox-readiness-tracing.ts b/src/lib/onboard/sandbox-readiness-tracing.ts index 33d49ef8b9b..66fdf2e27e5 100644 --- a/src/lib/onboard/sandbox-readiness-tracing.ts +++ b/src/lib/onboard/sandbox-readiness-tracing.ts @@ -8,6 +8,11 @@ type RunCaptureOpenshell = ( options?: { ignoreError?: boolean }, ) => string; +export type CreatedSandboxReadinessResult = + | { ready: true; reason: "ready"; failurePhase: null } + | { ready: false; reason: "terminal_failure_phase"; failurePhase: string | null } + | { ready: false; reason: "timeout"; failurePhase: null }; + export function waitForSandboxReadyWithTrace(options: { sandboxName: string; attempts: number; @@ -72,24 +77,70 @@ export function waitForCreatedSandboxReadyWithTrace(options: { timeoutSecs: number; runCaptureOpenshell: RunCaptureOpenshell; isSandboxReady: (output: string, sandboxName: string) => boolean; + /** + * Optional terminal-failure-phase classifier. When provided, the waiter + * short-circuits as soon as the sandbox enters a terminal failure phase + * (e.g. Error / Failed / CrashLoopBackOff) rather than burning the full + * timeout window before reporting "did not become ready" (#4316). + */ + getSandboxFailurePhase?: (output: string, sandboxName: string) => string | null; sleep: (seconds: number) => void; -}): boolean { - const { sandboxName, timeoutSecs, runCaptureOpenshell, isSandboxReady, sleep } = options; +}): CreatedSandboxReadinessResult { + const { + sandboxName, + timeoutSecs, + runCaptureOpenshell, + isSandboxReady, + getSandboxFailurePhase, + sleep, + } = options; return withSandboxReadinessTrace(sandboxName, { timeout_seconds: timeoutSecs }, () => { const readyAttempts = Math.max(1, Math.ceil(timeoutSecs / 2)); for (let i = 0; i < readyAttempts; i++) { const list = runCaptureOpenshell(["sandbox", "list"], { ignoreError: true }); if (isSandboxReady(list, sandboxName)) { addTraceEvent("ready", { attempt: i + 1 }); - return true; + return { ready: true, reason: "ready", failurePhase: null }; + } + const failurePhase = getSandboxFailurePhase?.(list, sandboxName) ?? null; + if (failurePhase) { + addTraceEvent("terminal_failure_phase", { attempt: i + 1, failure_phase: failurePhase }); + return { ready: false, reason: "terminal_failure_phase", failurePhase }; } if (i < readyAttempts - 1) sleep(2); } addTraceEvent("not_ready", { attempts: readyAttempts }); - return false; + return { ready: false, reason: "timeout", failurePhase: null }; }); } +/** + * Format the user-facing readiness failure message based on whether the + * waiter short-circuited on a terminal sandbox phase or actually timed out. + * Keeps the message branching close to the readiness contract so callers + * (notably onboard.ts) stay thin (#4316 codebase-growth guardrail). + */ +export function formatCreatedSandboxReadinessFailureMessage( + sandboxName: string, + readiness: CreatedSandboxReadinessResult, + timeoutSecs: number, +): string { + if (readiness.reason === "terminal_failure_phase") { + const phase = readiness.failurePhase ?? "a terminal failure"; + return ` Sandbox '${sandboxName}' entered ${phase} phase before it became ready (waited up to ${timeoutSecs}s).`; + } + return ` Sandbox '${sandboxName}' was created but did not become ready within ${timeoutSecs}s.`; +} + +export function printReadinessFailure( + readiness: CreatedSandboxReadinessResult, + sandboxName: string, + timeoutSecs: number, + logError: (message: string) => void = (message) => console.error(message), +): void { + logError(formatCreatedSandboxReadinessFailureMessage(sandboxName, readiness, timeoutSecs)); +} + export function waitForDashboardReadyWithTrace(options: { sandboxName: string; port: string | number; diff --git a/src/lib/state/gateway.ts b/src/lib/state/gateway.ts index 172241c4235..5eef5f907b3 100644 --- a/src/lib/state/gateway.ts +++ b/src/lib/state/gateway.ts @@ -57,6 +57,30 @@ export function isSandboxReady(output: string, sandboxName: string): boolean { return (cols.includes("Ready") || cols.includes("Running")) && !cols.includes("NotReady"); } +/** + * Terminal failure phases reported by `openshell sandbox list`/`get` for a + * sandbox whose underlying container is dead or unrecoverable. We treat these + * as short-circuit signals during readiness waits so onboarding fails fast + * with a clear phase rather than waiting out the full timeout window + * (NemoClaw issue #4316 — Docker GPU patch leaves the sandbox in Error). + */ +const TERMINAL_SANDBOX_FAILURE_PHASES = new Set([ + "Error", + "Failed", + "CrashLoopBackOff", +]); + +/** + * Return the failure phase token from `openshell sandbox list` if the row + * is in a terminal failure phase, otherwise null. Useful for distinguishing + * "Error" from "Failed"/"CrashLoopBackOff" in user-facing diagnostics. + */ +export function getSandboxFailurePhase(output: string, sandboxName: string): string | null { + const cols = parseSandboxRow(output, sandboxName); + if (!cols) return null; + return cols.find((col) => TERMINAL_SANDBOX_FAILURE_PHASES.has(col)) ?? null; +} + /** * Determine whether stale NemoClaw gateway output indicates a previous * session that should be cleaned up before the port preflight check.