diff --git a/docs/reference/troubleshooting.mdx b/docs/reference/troubleshooting.mdx index caabd8d84a6..82fd0a4d9be 100644 --- a/docs/reference/troubleshooting.mdx +++ b/docs/reference/troubleshooting.mdx @@ -643,6 +643,12 @@ sudo ufw allow from "$SUBNET" to any port 8080 proto tcp $$nemoclaw onboard ``` +This reachability check uses a disposable Docker probe and does not create or replace a sandbox. +If Docker GPU compatibility recreation fails later, follow [GPU routing or compatibility patch failed](#gpu-routing-or-compatibility-patch-failed). +That path can restore the pre-patch sandbox. +If its diagnostics report manual cleanup, use only the printed exact-container command. +That command targets the failed replacement and preserves the restored sandbox. + ### Custom OpenClaw image creates without a gateway or dashboard @@ -657,6 +663,19 @@ For the version-pinned plugin workflow, refer to [Install OpenClaw Plugins](../m If the sandbox is unreachable or the managed runtime paths are present, NemoClaw retains the existing generic gateway-log and host OpenShell-log guidance because the base-only failure is not proven. +A custom image without the managed runtime can fail while NemoClaw starts the sandbox container. +NemoClaw reports exit code 127 without assigning a cause unless captured logs contain the exact `env` error for missing `nemoclaw-start`. +When that error is present, the failure output identifies the missing managed startup command and gives the same rebuild guidance. +If NemoClaw saves pre-rollback diagnostics, the reported directory contains the captured container logs. +If rollback succeeds, NemoClaw restores and starts the pre-patch sandbox container. +It does not print a sandbox deletion command for the restored sandbox. +If the failed replacement container remains, NemoClaw prints an exact-container Docker cleanup command. +If NemoClaw cannot confirm whether the replacement remains but retains its validated exact ID, it prints the same target-safe command. +Without a validated exact ID, it reports cleanup as unknown and prints no deletion command. +If rollback fails, sandbox and container state can be uncertain. +If NemoClaw reports a diagnostics directory, inspect it. +Inspect the diagnostics before removing any container. + @@ -2667,7 +2686,7 @@ Identify the matching path before applying the recovery guidance. | --- | --- | --- | | Native `--gpu` is rejected, host runtime evidence identifies GPU injection failure, or an explicit driver proof fails and host configuration confirms no GPU attachment | Ordinary Linux native attempt | The default native-only route stops. Retry with `NEMOCLAW_DOCKER_GPU_PATCH=fallback` only if you explicitly accept one bounded compatibility retry, or use `=1` to select compatibility before creation. | | `Cleanup could not be proven safe` | Native-to-compatibility handoff | Run the printed sandbox deletion command, verify both the gateway row and OpenShell-managed Docker containers labeled for that sandbox are absent, then rerun onboarding. | -| The patched container exits or the compatibility attempt fails | Compatibility recreation | Inspect the saved diagnostics and the rollback outcome, then repair the NVIDIA Container Toolkit/CDI configuration. Keep the sandbox when the pre-patch container was restored; delete it only after inspection confirms restoration failed. Then rerun onboarding. | +| The patched container exits or the compatibility attempt fails | Compatibility recreation | Inspect the saved diagnostics and the rollback outcome, then repair the NVIDIA Container Toolkit/CDI configuration. Keep the sandbox when the pre-patch container was restored. Use only an exact-container cleanup command printed after rollback. If no command was printed, inspect the sandbox and its labeled containers before removing anything. Then rerun onboarding. | | A recreated container inherits only a loopback DNS stub and no usable upstream | Compatibility DNS fallback | Repair the host's `systemd-resolved` upstream configuration, then rerun onboarding. | For bridge-networked compatibility recreation without an explicit container DNS setting, NemoClaw selects a usable IPv4 upstream from `systemd-resolved` and probes that exact `--dns` path before it stops the original container. @@ -2716,9 +2735,11 @@ Use `NEMOCLAW_DOCKER_GPU_PATCH=0` only for troubleshooting because it bypasses t After compatibility recreation starts, onboarding keeps the pre-patch container as a rollback backup until the replacement passes the Ready, direct GPU, and applicable local-inference checks. If a later check fails, onboarding prints failure diagnostics and attempts to restore the pre-patch container before it exits. When rollback succeeds, the pre-patch sandbox remains available. -When rollback fails, onboarding reports that the pre-patch container was not restored and prints container-cleanup guidance. -GPU-proof diagnostics are captured before rollback and can print that guidance before the final container state is known. -Inspect the sandbox and its labeled Docker containers before running a deletion command. +If the failed replacement may remain and NemoClaw retains its validated exact container ID, it prints only an exact-container `docker rm -f` command. +If replacement cleanup cannot be confirmed without a validated exact ID, onboarding reports cleanup as unknown and prints no deletion command. +When rollback fails, onboarding reports that the sandbox and container state is uncertain and prints no deletion command. +A diagnostic bundle captured before rollback records cleanup as pending and contains no deletion command. +Inspect the diagnostics, the sandbox, and its labeled Docker containers before removing anything. Starting with NemoClaw v0.0.43, the standard installer handles the `/proc//task//comm` permission case during this patch path. diff --git a/src/lib/onboard/docker-gpu-patch-clone.ts b/src/lib/onboard/docker-gpu-patch-clone.ts index 828ce0c540b..ce5a4c323c6 100644 --- a/src/lib/onboard/docker-gpu-patch-clone.ts +++ b/src/lib/onboard/docker-gpu-patch-clone.ts @@ -299,6 +299,14 @@ export function sameContainerId( return left.startsWith(right) || right.startsWith(left); } +/** Return only a complete Docker container ID that is safe for exact-ID cleanup. */ +export function fullDockerContainerId(value: string | null | undefined): string | null { + const normalized = String(value ?? "") + .trim() + .toLowerCase(); + return /^[0-9a-f]{64}$/u.test(normalized) ? normalized : null; +} + function dockerNetworkAliases( inspect: DockerContainerInspect, networkMode: string | null | undefined, diff --git a/src/lib/onboard/docker-gpu-patch-diagnostics-classification.test.ts b/src/lib/onboard/docker-gpu-patch-diagnostics-classification.test.ts index d11fd247254..d60f60e8e26 100644 --- a/src/lib/onboard/docker-gpu-patch-diagnostics-classification.test.ts +++ b/src/lib/onboard/docker-gpu-patch-diagnostics-classification.test.ts @@ -147,6 +147,41 @@ describe("Docker GPU patch diagnostics", () => { expect(flat).toContain("patched_create_option=--gpus all"); }); + it("explains exit code 127 when container logs prove the managed startup command is missing (#7996)", () => { + const result = classifyDockerGpuPatchFailure( + failureSnapshot("Error", { Status: "restarting", ExitCode: 127 }, "alpha Error 1m ago"), + GPU_MODE, + { managedStartupCommandMissing: true }, + ); + + expect(result.kind).toBe("patched_container_failed"); + const hints = (result.hints ?? []).join("\n"); + expect(hints).toContain("does not provide the NemoClaw-managed `nemoclaw-start` command"); + expect(hints).toContain("selected agent and NemoClaw release"); + expect(hints).not.toMatch(/OpenClaw|sandbox-base/); + // The prose stays out of the machine-readable on-disk summary. + expect(result.summaryLines.join("\n")).not.toContain("selected agent"); + }); + + it("does not infer a missing startup command when its child process returns 127 (#7996)", () => { + const result = classify( + failureSnapshot("Error", { Status: "exited", ExitCode: 127 }, "alpha Error 1m ago"), + ); + + expect(result.kind).toBe("patched_container_failed"); + expect(result.headline).toContain("exited with code 127"); + expect(result.hints ?? []).toEqual([]); + }); + + it("does not attach the missing-startup-command hints to other non-zero exits (#7996)", () => { + const result = classify( + failureSnapshot("Error", { Status: "exited", ExitCode: 125 }, "alpha Error 1m ago"), + ); + + expect(result.kind).toBe("patched_container_failed"); + expect(result.hints ?? []).toEqual([]); + }); + it("classifies an Error-phase sandbox with unknown container state as sandbox_error_phase", () => { const result = classify(failureSnapshot("Error", null, null)); diff --git a/src/lib/onboard/docker-gpu-patch-diagnostics-collection.test.ts b/src/lib/onboard/docker-gpu-patch-diagnostics-collection.test.ts index 3ba96e6ede0..e22fd565c20 100644 --- a/src/lib/onboard/docker-gpu-patch-diagnostics-collection.test.ts +++ b/src/lib/onboard/docker-gpu-patch-diagnostics-collection.test.ts @@ -113,4 +113,145 @@ describe("Docker GPU patch diagnostics", () => { fs.rmSync(tmpDir, { recursive: true, force: true }); } }); + + it("omits cleanup after rollback confirms that the replacement is absent (#7996)", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-docker-gpu-rollback-")); + try { + const diagnostics = collectDockerGpuPatchDiagnostics( + "alpha", + { + context: { + sandboxName: "alpha", + newContainerId: "removed-container-id", + rolledBack: true, + replacementStopConfirmed: true, + replacementRemovalConfirmed: true, + replacementPresence: "absent", + }, + }, + { + dockerCapture: vi.fn(() => ""), + dockerLogs: vi.fn(() => ""), + homedir: () => tmpDir, + now: () => new Date("2026-05-12T00:00:00Z"), + }, + ); + + const summary = fs.readFileSync(path.join(diagnostics?.dir || "", "summary.txt"), "utf-8"); + expect(diagnostics?.cleanupCommands).toEqual([]); + expect(diagnostics?.cleanupDisposition).toBe("not_required"); + expect(summary).toContain("cleanup_required=no"); + expect(summary).not.toContain("openshell sandbox delete"); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it("keeps cleanup unknown when rollback cannot confirm replacement absence (#7996)", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-docker-gpu-unknown-")); + try { + const diagnostics = collectDockerGpuPatchDiagnostics( + "alpha", + { + context: { + sandboxName: "alpha", + newContainerId: "unconfirmed-container-id", + rolledBack: true, + replacementStopConfirmed: false, + replacementRemovalConfirmed: false, + replacementPresence: "unknown", + }, + }, + { + dockerCapture: vi.fn(() => ""), + dockerLogs: vi.fn(() => ""), + homedir: () => tmpDir, + now: () => new Date("2026-05-12T00:00:01Z"), + }, + ); + + const summary = fs.readFileSync(path.join(diagnostics?.dir || "", "summary.txt"), "utf-8"); + expect(diagnostics?.cleanupCommands).toEqual([]); + expect(diagnostics?.cleanupDisposition).toBe("unknown"); + expect(summary).toContain("replacement_presence=unknown"); + expect(summary).toContain("cleanup_required=unknown"); + expect(summary).not.toContain("openshell sandbox delete"); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it("keeps cleanup unknown when a present replacement lacks an exact ID (#7996)", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-docker-gpu-invalid-id-")); + try { + const diagnostics = collectDockerGpuPatchDiagnostics( + "alpha", + { + context: { + sandboxName: "alpha", + newContainerId: "short-container-id", + rolledBack: true, + replacementStopConfirmed: false, + replacementRemovalConfirmed: false, + replacementPresence: "present", + }, + }, + { + dockerCapture: vi.fn(() => ""), + dockerLogs: vi.fn(() => ""), + homedir: () => tmpDir, + now: () => new Date("2026-05-12T00:00:02Z"), + }, + ); + + const summary = fs.readFileSync(path.join(diagnostics?.dir || "", "summary.txt"), "utf-8"); + expect(diagnostics?.cleanupCommands).toEqual([]); + expect(diagnostics?.cleanupDisposition).toBe("unknown"); + expect(summary).toContain("replacement_presence=present"); + expect(summary).toContain("cleanup_required=unknown"); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it("uses exact-ID cleanup when post-rollback inspection finds the replacement (#7996)", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-docker-gpu-inspect-")); + const replacementId = "b".repeat(64); + try { + const diagnostics = collectDockerGpuPatchDiagnostics( + "alpha", + { + context: { + sandboxName: "alpha", + newContainerId: replacementId, + rolledBack: true, + replacementStopConfirmed: false, + replacementRemovalConfirmed: false, + replacementPresence: "unknown", + }, + }, + { + dockerCapture: vi.fn((args: readonly string[]) => + args[0] === "inspect" && args[1] === replacementId + ? JSON.stringify([{ Id: replacementId }]) + : "", + ), + dockerLogs: vi.fn(() => ""), + homedir: () => tmpDir, + now: () => new Date("2026-05-12T00:00:02Z"), + }, + ); + + const summary = fs.readFileSync(path.join(diagnostics?.dir || "", "summary.txt"), "utf-8"); + expect(diagnostics?.cleanupCommands).toEqual([ + `docker rm -f ${JSON.stringify(replacementId)}`, + ]); + expect(diagnostics?.cleanupDisposition).toBe("manual"); + expect(summary).toContain("replacement_presence=present"); + expect(summary).toContain("cleanup_required=yes"); + expect(summary).not.toContain("openshell sandbox delete"); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); }); diff --git a/src/lib/onboard/docker-gpu-patch-diagnostics.ts b/src/lib/onboard/docker-gpu-patch-diagnostics.ts index 5cd0f4b4671..399b05373dd 100644 --- a/src/lib/onboard/docker-gpu-patch-diagnostics.ts +++ b/src/lib/onboard/docker-gpu-patch-diagnostics.ts @@ -10,6 +10,7 @@ import { GATEWAY_PORT } from "../core/ports"; import { rejectSymlinksOnPath } from "../state/config-io"; import { nemoclawStateRoot } from "../state/state-root"; import { createDockerGpuDiagnosticRedactor } from "./docker-gpu-diagnostic-redaction"; +import { fullDockerContainerId } from "./docker-gpu-patch-clone"; import { DOCKER_GPU_PATCH_TIMEOUT_MS } from "./docker-gpu-patch-constants"; import { getDockerGpuPatchFailureContext } from "./docker-gpu-patch-recreate"; import type { @@ -129,6 +130,14 @@ export function dockerGpuPatchCleanupCommands(sandboxName: string): string[] { return [`openshell sandbox delete ${JSON.stringify(sandboxName)}`]; } +function dockerGpuReplacementCleanupCommands(containerId: string): string[] { + return [`docker rm -f ${JSON.stringify(containerId)}`]; +} + +function confirmationValue(value: boolean | undefined): string { + return value === true ? "yes" : value === false ? "no" : "unknown"; +} + export function collectDockerGpuPatchDiagnostics( sandboxName: string, options: { @@ -140,6 +149,11 @@ export function collectDockerGpuPatchDiagnostics( additionalSummaryLines?: readonly string[]; additionalSensitiveValues?: readonly string[]; dockerTopOutput?: string | null; + /** + * The caller captured evidence before rollback and cannot yet determine + * whether manual cleanup will be required. + */ + cleanupDisposition?: "pending-rollback"; } = {}, deps: DockerGpuPatchDeps = {}, ): DockerGpuPatchDiagnostics | null { @@ -162,6 +176,9 @@ export function collectDockerGpuPatchDiagnostics( } const context = options.context || getDockerGpuPatchFailureContext(options.error) || null; + const selectedMode = options.selectedMode || context?.selectedMode || null; + const snapshot = options.snapshot ?? null; + const classification = options.classification ?? null; const redactor = createDockerGpuDiagnosticRedactor(options.additionalSensitiveValues); let discoveredContainerIds: string[] = []; try { @@ -198,7 +215,34 @@ export function collectDockerGpuPatchDiagnostics( writeTextFile(dir, name, JSON.stringify(redactor.redactValue(value), null, 2)); }; - const cleanupCommands = dockerGpuPatchCleanupCommands(sandboxName).map(redactor.redactText); + const cleanupPendingRollback = options.cleanupDisposition === "pending-rollback"; + const prePatchRestored = context?.rolledBack === true; + const replacementId = fullDockerContainerId(context?.newContainerId); + const inspectConfirmsReplacementPresent = + replacementId !== null && + inspectedTargets.some(({ entries }) => + entries.some((entry) => fullDockerContainerId(entry.Id) === replacementId), + ); + const snapshotConfirmsReplacementPresent = + replacementId !== null && snapshot?.patchedContainerState != null; + const replacementPresence = + snapshotConfirmsReplacementPresent || inspectConfirmsReplacementPresent + ? "present" + : (context?.replacementPresence ?? "unknown"); + let cleanupDisposition: DockerGpuPatchDiagnostics["cleanupDisposition"]; + let cleanupCommands: string[] = []; + if (cleanupPendingRollback) { + cleanupDisposition = "pending_rollback"; + } else if (!prePatchRestored) { + cleanupDisposition = "unknown"; + } else if (replacementPresence === "absent") { + cleanupDisposition = "not_required"; + } else if (replacementId) { + cleanupDisposition = "manual"; + cleanupCommands = dockerGpuReplacementCleanupCommands(replacementId).map(redactor.redactText); + } else { + cleanupDisposition = "unknown"; + } const errorText = redactor.redactText( options.error instanceof Error ? options.error.message @@ -206,9 +250,6 @@ 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=${redactor.redactText(sandboxName)}`, @@ -218,9 +259,19 @@ export function collectDockerGpuPatchDiagnostics( `old_container_id=${redactor.redactText(context?.oldContainerId ?? "unknown")}`, `new_container_id=${redactor.redactText(context?.newContainerId ?? "unknown")}`, `backup_container_name=${redactor.redactText(context?.backupContainerName ?? "none")}`, - `rolled_back=${context?.rolledBack === true ? "yes" : context?.rolledBack === false ? "failed" : "no"}`, - "cleanup_commands:", - ...cleanupCommands.map((command) => ` ${command}`), + `rolled_back=${cleanupPendingRollback ? "pending" : context?.rolledBack === true ? "yes" : context?.rolledBack === false ? "failed" : "no"}`, + ...(context?.replacementStopConfirmed !== undefined + ? [`replacement_stop_confirmed=${confirmationValue(context.replacementStopConfirmed)}`] + : []), + ...(context?.replacementRemovalConfirmed !== undefined + ? [`replacement_removal_confirmed=${confirmationValue(context.replacementRemovalConfirmed)}`] + : []), + ...(prePatchRestored ? [`replacement_presence=${replacementPresence}`] : []), + `cleanup_disposition=${cleanupDisposition}`, + `cleanup_required=${cleanupDisposition === "manual" ? "yes" : cleanupDisposition === "not_required" ? "no" : "unknown"}`, + ...(cleanupCommands.length > 0 + ? ["cleanup_commands:", ...cleanupCommands.map((command) => ` ${command}`)] + : []), ]; if (context?.modeAttempts?.length) { summaryLines.push("gpu_mode_attempts:"); @@ -327,5 +378,5 @@ export function collectDockerGpuPatchDiagnostics( } } - return { dir, cleanupCommands, summaryLines }; + return { dir, cleanupCommands, cleanupDisposition, summaryLines }; } diff --git a/src/lib/onboard/docker-gpu-patch-failure-print.test.ts b/src/lib/onboard/docker-gpu-patch-failure-print.test.ts new file mode 100644 index 00000000000..765295671fe --- /dev/null +++ b/src/lib/onboard/docker-gpu-patch-failure-print.test.ts @@ -0,0 +1,214 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import fs from "node:fs"; + +import { describe, expect, it, vi } from "vitest"; + +import { + buildDockerGpuMode, + type DockerGpuPatchFailureClassification, + printDockerGpuPatchFailureAndExit, +} from "./docker-gpu-patch"; + +const PRE_ROLLBACK: DockerGpuPatchFailureClassification = { + kind: "patched_container_failed", + headline: "Patched GPU container exited with code 127 (--gpus all).", + selectedModeKind: "gpus", + summaryLines: ["patched_container_exit_code=127", "patched_create_option=--gpus all"], + hints: [ + "Container logs show that the sandbox image does not provide the NemoClaw-managed `nemoclaw-start` command.", + ], +}; + +/** + * Drive the printer and return everything it wrote to stderr. Diagnostics + * persistence is disabled so the assertions only observe console output. + */ +function printAndCapture(deps: Parameters[2]): string { + const output: string[] = []; + const errorSpy = vi.spyOn(console, "error").mockImplementation((...args: unknown[]) => { + output.push(args.map(String).join(" ")); + }); + const mkdirSpy = vi.spyOn(fs, "mkdirSync").mockImplementation(() => { + throw new Error("diagnostics disabled for test"); + }); + const exitSpy = vi.spyOn(process, "exit").mockImplementation(((_code?: number) => { + throw new Error("__test_exit__"); + }) as never); + try { + expect(() => + printDockerGpuPatchFailureAndExit("alpha", new Error("supervisor did not reconnect"), deps), + ).toThrow(/__test_exit__/); + return output.join("\n"); + } finally { + exitSpy.mockRestore(); + mkdirSpy.mockRestore(); + errorSpy.mockRestore(); + } +} + +describe("Docker GPU patch failure reporting (#7996)", () => { + it("prefers the pre-rollback verdict when fresh inspection cannot find the replacement", () => { + // Fresh inspection returns nothing after rollback, and the sandbox only + // shows a generic Error phase. + const stderr = printAndCapture({ + runCaptureOpenshell: vi.fn(() => "alpha Error 1m ago\n"), + dockerCapture: vi.fn(() => ""), + context: { + sandboxName: "alpha", + newContainerId: "new-container-id", + selectedMode: buildDockerGpuMode("gpus"), + rolledBack: true, + }, + preRollbackClassification: PRE_ROLLBACK, + }); + + expect(stderr).toContain("Patched GPU container exited with code 127"); + expect(stderr).toContain("patched_container_exit_code=127"); + expect(stderr).toContain("does not provide the NemoClaw-managed `nemoclaw-start` command"); + expect(stderr).not.toContain("entered Error phase"); + expect(stderr).toContain("The pre-patch sandbox container was restored and started"); + expect(stderr).not.toContain("replacement was removed"); + expect(stderr).not.toContain("left in place for inspection"); + expect(stderr).not.toContain("openshell sandbox delete"); + }); + + it("keeps the fresh verdict when an inspectable replacement container is running", () => { + // The live snapshot has first-hand evidence, so a stale pre-rollback + // verdict must not overwrite it. + const stderr = printAndCapture({ + runCaptureOpenshell: vi.fn(() => "alpha Error 1m ago\n"), + dockerCapture: vi.fn(() => JSON.stringify({ Status: "running", Running: true, ExitCode: 0 })), + context: { + sandboxName: "alpha", + newContainerId: "new-container-id", + selectedMode: buildDockerGpuMode("gpus"), + }, + preRollbackClassification: PRE_ROLLBACK, + }); + + expect(stderr).toContain("OpenShell sandbox entered Error phase"); + expect(stderr).not.toContain("code 127"); + }); + + it("keeps the fresh verdict when the pre-rollback create option does not match (#7996)", () => { + const stderr = printAndCapture({ + runCaptureOpenshell: vi.fn(() => "alpha Error 1m ago\n"), + dockerCapture: vi.fn(() => ""), + context: { + sandboxName: "alpha", + newContainerId: "new-container-id", + selectedMode: buildDockerGpuMode("cdi"), + rolledBack: true, + }, + preRollbackClassification: PRE_ROLLBACK, + }); + + expect(stderr).toContain("OpenShell sandbox entered Error phase"); + expect(stderr).not.toContain("code 127"); + expect(stderr).not.toContain("patched_container_exit_code=127"); + }); + + it("matches a saved verdict by mode kind when its display label changes (#7996)", () => { + const selectedMode = { ...buildDockerGpuMode("gpus"), label: "--gpus=all" }; + const stderr = printAndCapture({ + runCaptureOpenshell: vi.fn(() => "alpha Error 1m ago\n"), + dockerCapture: vi.fn(() => ""), + context: { + sandboxName: "alpha", + newContainerId: "new-container-id", + selectedMode, + rolledBack: true, + }, + preRollbackClassification: PRE_ROLLBACK, + }); + + expect(stderr).toContain("Patched GPU container exited with code 127"); + expect(stderr).toContain("patched_container_exit_code=127"); + expect(stderr).not.toContain("entered Error phase"); + }); + + it("falls back to the observed verdict when no pre-rollback verdict was captured", () => { + const stderr = printAndCapture({ + runCaptureOpenshell: vi.fn(() => "alpha Error 1m ago\n"), + dockerCapture: vi.fn(() => ""), + context: { + sandboxName: "alpha", + newContainerId: "new-container-id", + selectedMode: buildDockerGpuMode("gpus"), + }, + preRollbackClassification: null, + }); + + expect(stderr).toContain("entered Error phase"); + }); + + it.each([ + "sandbox_error_phase", + "supervisor_unreachable", + "proof_failure", + ] as const)("ignores a saved %s verdict after rollback", (kind) => { + const stderr = printAndCapture({ + runCaptureOpenshell: vi.fn(() => "alpha Error 1m ago\n"), + dockerCapture: vi.fn(() => ""), + context: { + sandboxName: "alpha", + newContainerId: "new-container-id", + selectedMode: buildDockerGpuMode("gpus"), + rolledBack: true, + replacementPresence: "absent", + }, + preRollbackClassification: { + ...PRE_ROLLBACK, + kind, + headline: `Stale ${kind} verdict.`, + }, + }); + + expect(stderr).toContain("OpenShell sandbox entered Error phase"); + expect(stderr).not.toContain(`Stale ${kind} verdict`); + expect(stderr).not.toContain("patched_container_exit_code=127"); + }); + + it("does not suggest deletion when replacement cleanup remains unknown after rollback", () => { + const stderr = printAndCapture({ + runCaptureOpenshell: vi.fn(() => "alpha Error 1m ago\n"), + dockerCapture: vi.fn(() => ""), + context: { + sandboxName: "alpha", + newContainerId: "new-container-id", + selectedMode: buildDockerGpuMode("gpus"), + rolledBack: true, + replacementPresence: "unknown", + }, + preRollbackClassification: PRE_ROLLBACK, + }); + + expect(stderr).toContain("Replacement container cleanup could not be confirmed"); + expect(stderr).toContain("before removing any container"); + expect(stderr).not.toContain("Manual cleanup"); + expect(stderr).not.toContain("openshell sandbox delete"); + expect(stderr).not.toContain("docker rm -f"); + }); + + it("does not suggest deleting the sandbox when rollback fails (#7996)", () => { + const stderr = printAndCapture({ + runCaptureOpenshell: vi.fn(() => ""), + dockerCapture: vi.fn(() => ""), + context: { + sandboxName: "alpha", + newContainerId: "a".repeat(64), + selectedMode: buildDockerGpuMode("gpus"), + rolledBack: false, + }, + preRollbackClassification: null, + }); + + expect(stderr).toContain("container state is uncertain"); + expect(stderr).toContain("before removing any container"); + expect(stderr).not.toContain("Manual cleanup"); + expect(stderr).not.toContain("openshell sandbox delete"); + expect(stderr).not.toContain("docker rm -f"); + }); +}); diff --git a/src/lib/onboard/docker-gpu-patch-finalize.test.ts b/src/lib/onboard/docker-gpu-patch-finalize.test.ts index 5a691435ce4..e220949bf8e 100644 --- a/src/lib/onboard/docker-gpu-patch-finalize.test.ts +++ b/src/lib/onboard/docker-gpu-patch-finalize.test.ts @@ -1,10 +1,17 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + import { describe, expect, it, vi } from "vitest"; -import type { DockerGpuPatchResult } from "./docker-gpu-patch"; -import { finalizeDockerGpuPatchBackup } from "./docker-gpu-patch-finalize"; +import { collectDockerGpuPatchDiagnostics, type DockerGpuPatchResult } from "./docker-gpu-patch"; +import { + type DockerGpuPatchFinalizeOutcome, + finalizeDockerGpuPatchBackup, +} from "./docker-gpu-patch-finalize"; function deferredCreateResult(): DockerGpuPatchResult { return { @@ -23,6 +30,38 @@ function deferredCreateResult(): DockerGpuPatchResult { }; } +function collectRollbackDiagnostics( + newContainerId: string, + outcome: DockerGpuPatchFinalizeOutcome, +): { cleanupCommands: string[]; cleanupDisposition: string; summary: string } { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-docker-gpu-finalize-")); + try { + const diagnostics = collectDockerGpuPatchDiagnostics( + "alpha", + { + context: { + sandboxName: "alpha", + newContainerId, + ...outcome, + }, + }, + { + dockerCapture: vi.fn(() => ""), + dockerLogs: vi.fn(() => ""), + homedir: () => tmpDir, + now: () => new Date("2026-08-04T00:00:00Z"), + }, + ); + return { + cleanupCommands: diagnostics?.cleanupCommands ?? [], + cleanupDisposition: diagnostics?.cleanupDisposition ?? "missing", + summary: fs.readFileSync(path.join(diagnostics?.dir ?? "", "summary.txt"), "utf-8"), + }; + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } +} + describe("finalizeDockerGpuPatchBackup", () => { it("removes the backup container when supervisor reconnect succeeded", () => { const dockerRm = vi.fn((_name: string) => ({ status: 0 })); @@ -46,7 +85,13 @@ describe("finalizeDockerGpuPatchBackup", () => { { result: deferredCreateResult(), supervisorReady: false }, { dockerStop, dockerRm, dockerRename, dockerStart }, ); - expect(outcome).toEqual({ backupRemoved: false, rolledBack: true }); + expect(outcome).toEqual({ + backupRemoved: false, + rolledBack: true, + replacementStopConfirmed: true, + replacementRemovalConfirmed: true, + replacementPresence: "absent", + }); expect(dockerStop).toHaveBeenCalledWith( "new-container-id", expect.objectContaining({ ignoreError: true }), @@ -66,6 +111,7 @@ describe("finalizeDockerGpuPatchBackup", () => { }); it("reports rolledBack=false when restoring the backup fails", () => { + const newContainerId = "e".repeat(64); const dockerStop = vi.fn(() => ({ status: 0 })); const dockerRm = vi.fn((_name: string) => ({ status: 0 })); const dockerRename = vi.fn((_old: string, _next: string) => ({ @@ -74,11 +120,26 @@ describe("finalizeDockerGpuPatchBackup", () => { })); const dockerStart = vi.fn(() => ({ status: 0 })); const outcome = finalizeDockerGpuPatchBackup( - { result: deferredCreateResult(), supervisorReady: false }, + { + result: { ...deferredCreateResult(), newContainerId }, + supervisorReady: false, + }, { dockerStop, dockerRm, dockerRename, dockerStart }, ); - expect(outcome).toEqual({ backupRemoved: false, rolledBack: false }); + expect(outcome).toEqual({ + backupRemoved: false, + rolledBack: false, + replacementStopConfirmed: true, + replacementRemovalConfirmed: true, + replacementPresence: "absent", + }); expect(dockerStart).not.toHaveBeenCalled(); + const diagnostics = collectRollbackDiagnostics(newContainerId, outcome); + expect(diagnostics.cleanupDisposition).toBe("unknown"); + expect(diagnostics.cleanupCommands).toEqual([]); + expect(diagnostics.summary).toContain("rolled_back=failed"); + expect(diagnostics.summary).not.toContain("openshell sandbox delete"); + expect(diagnostics.summary).not.toContain("docker rm -f"); }); it("does not report rollback success when restarting the backup has no exit status", () => { @@ -92,7 +153,13 @@ describe("finalizeDockerGpuPatchBackup", () => { }, ); - expect(outcome).toEqual({ backupRemoved: false, rolledBack: false }); + expect(outcome).toEqual({ + backupRemoved: false, + rolledBack: false, + replacementStopConfirmed: true, + replacementRemovalConfirmed: true, + replacementPresence: "absent", + }); }); it("is a no-op when the backup was already removed by the patch helper", () => { @@ -128,6 +195,116 @@ describe("finalizeDockerGpuPatchBackup", () => { expect(outcome).toEqual({ backupRemoved: false, rolledBack: false }); }); + it("records a remaining exact-ID replacement when removal fails (#7996)", () => { + const newContainerId = "a".repeat(64); + const outcome = finalizeDockerGpuPatchBackup( + { + result: { ...deferredCreateResult(), newContainerId }, + supervisorReady: false, + }, + { + dockerStop: vi.fn(() => ({ status: 0 })), + dockerRm: vi.fn(() => ({ status: 1 })), + dockerRun: vi.fn(() => ({ status: 0, stdout: `${newContainerId}\n` })), + dockerRename: vi.fn(() => ({ status: 0 })), + dockerStart: vi.fn(() => ({ status: 0 })), + }, + ); + + expect(outcome).toEqual({ + backupRemoved: false, + rolledBack: true, + replacementStopConfirmed: true, + replacementRemovalConfirmed: false, + replacementPresence: "present", + }); + }); + + it("records confirmed absence when exact-ID removal reports failure but listing is empty (#7996)", () => { + const newContainerId = "b".repeat(64); + const outcome = finalizeDockerGpuPatchBackup( + { + result: { ...deferredCreateResult(), newContainerId }, + supervisorReady: false, + }, + { + dockerStop: vi.fn(() => ({ status: 0 })), + dockerRm: vi.fn(() => ({ status: 1 })), + dockerRun: vi.fn(() => ({ status: 0, stdout: "" })), + dockerRename: vi.fn(() => ({ status: 0 })), + dockerStart: vi.fn(() => ({ status: 0 })), + }, + ); + + expect(outcome).toEqual({ + backupRemoved: false, + rolledBack: true, + replacementStopConfirmed: true, + replacementRemovalConfirmed: false, + replacementPresence: "absent", + }); + }); + + it("retries a failed replacement observation before confirming absence (#7996)", () => { + const newContainerId = "c".repeat(64); + const dockerRun = vi + .fn() + .mockReturnValueOnce({ status: 1, stderr: "daemon unavailable" }) + .mockReturnValueOnce({ status: 0, stdout: "" }); + const sleep = vi.fn(); + const outcome = finalizeDockerGpuPatchBackup( + { + result: { ...deferredCreateResult(), newContainerId }, + supervisorReady: false, + }, + { + dockerStop: vi.fn(() => ({ status: 0 })), + dockerRm: vi.fn(() => ({ status: 1 })), + dockerRun, + dockerRename: vi.fn(() => ({ status: 0 })), + dockerStart: vi.fn(() => ({ status: 0 })), + sleep, + }, + ); + + expect(outcome.replacementPresence).toBe("absent"); + expect(dockerRun).toHaveBeenCalledTimes(2); + expect(sleep).toHaveBeenCalledOnce(); + expect(sleep).toHaveBeenCalledWith(0.5); + }); + + it("keeps replacement presence unknown after repeated daemon errors (#7996)", () => { + const newContainerId = "d".repeat(64); + const dockerRun = vi.fn(() => ({ status: 1, stderr: "daemon unavailable" })); + const sleep = vi.fn(); + const outcome = finalizeDockerGpuPatchBackup( + { + result: { ...deferredCreateResult(), newContainerId }, + supervisorReady: false, + }, + { + dockerStop: vi.fn(() => ({ status: 0 })), + dockerRm: vi.fn(() => ({ status: 1 })), + dockerRun, + dockerRename: vi.fn(() => ({ status: 0 })), + dockerStart: vi.fn(() => ({ status: 0 })), + sleep, + }, + ); + + expect(outcome.replacementPresence).toBe("unknown"); + expect(dockerRun).toHaveBeenCalledTimes(3); + expect(sleep).toHaveBeenCalledTimes(2); + expect(sleep).toHaveBeenNthCalledWith(1, 0.5); + expect(sleep).toHaveBeenNthCalledWith(2, 0.5); + const diagnostics = collectRollbackDiagnostics(newContainerId, outcome); + expect(diagnostics.cleanupDisposition).toBe("manual"); + expect(diagnostics.cleanupCommands).toEqual([`docker rm -f ${JSON.stringify(newContainerId)}`]); + expect(diagnostics.summary).toContain("replacement_presence=unknown"); + expect(diagnostics.summary).toContain("cleanup_required=yes"); + expect(diagnostics.summary).not.toContain("openshell sandbox delete"); + }); + it("stops rollback before start when rename has no exit status", () => { const dockerStart = vi.fn(() => ({ status: 0 })); const outcome = finalizeDockerGpuPatchBackup( @@ -139,7 +316,13 @@ describe("finalizeDockerGpuPatchBackup", () => { dockerStart, }, ); - expect(outcome).toEqual({ backupRemoved: false, rolledBack: false }); + expect(outcome).toEqual({ + backupRemoved: false, + rolledBack: false, + replacementStopConfirmed: true, + replacementRemovalConfirmed: true, + replacementPresence: "absent", + }); expect(dockerStart).not.toHaveBeenCalled(); }); @@ -153,6 +336,12 @@ describe("finalizeDockerGpuPatchBackup", () => { dockerStart: vi.fn(() => ({ status: null })), }, ); - expect(outcome).toEqual({ backupRemoved: false, rolledBack: false }); + expect(outcome).toEqual({ + backupRemoved: false, + rolledBack: false, + replacementStopConfirmed: true, + replacementRemovalConfirmed: true, + replacementPresence: "absent", + }); }); }); diff --git a/src/lib/onboard/docker-gpu-patch-finalize.ts b/src/lib/onboard/docker-gpu-patch-finalize.ts index 4eeeedf399b..24b5b01aeb6 100644 --- a/src/lib/onboard/docker-gpu-patch-finalize.ts +++ b/src/lib/onboard/docker-gpu-patch-finalize.ts @@ -44,6 +44,9 @@ export type DockerGpuPatchFinalizeOptions = { export type DockerGpuPatchFinalizeOutcome = { backupRemoved: boolean; rolledBack: boolean; + replacementStopConfirmed?: boolean; + replacementRemovalConfirmed?: boolean; + replacementPresence?: "absent" | "present" | "unknown"; }; export function finalizeDockerGpuPatchBackup( @@ -68,7 +71,7 @@ export function finalizeDockerGpuPatchBackup( const rmResult = resolved.dockerRm(options.result.backupContainerName, containerOpts); return { backupRemoved: hasZeroDockerExitStatus(rmResult), rolledBack: false }; } - const rolledBack = rollbackToBackupContainer( + const rollback = rollbackToBackupContainer( { newContainerId: options.result.newContainerId, backupContainerName: options.result.backupContainerName, @@ -76,12 +79,12 @@ export function finalizeDockerGpuPatchBackup( }, resolved, ); - return { backupRemoved: false, rolledBack }; + return { backupRemoved: false, ...rollback }; } export type SupervisorReconnectOutcome = | { execReady: true; backupRemoved: boolean } - | { execReady: false; rolledBack: boolean; error: Error }; + | ({ execReady: false; error: Error } & Omit); export function reconcileSupervisorReconnect( execReady: boolean, @@ -103,12 +106,12 @@ export function reconcileSupervisorReconnect( const rmResult = resolved.dockerRm(refs.backupContainerName, containerOpts); return { execReady: true, backupRemoved: hasZeroDockerExitStatus(rmResult) }; } - const rolledBack = rollbackToBackupContainer(refs, resolved); + const rollback = rollbackToBackupContainer(refs, resolved); return { execReady: false, - rolledBack, + ...rollback, error: new Error( - rolledBack + rollback.rolledBack ? "OpenShell supervisor did not reconnect to the GPU-enabled container; pre-patch sandbox restored." : "OpenShell supervisor did not reconnect to the GPU-enabled container and rollback failed; pre-patch sandbox was NOT restored.", ), diff --git a/src/lib/onboard/docker-gpu-patch-recreate.ts b/src/lib/onboard/docker-gpu-patch-recreate.ts index b205c144af8..4eb1c0ad8d4 100644 --- a/src/lib/onboard/docker-gpu-patch-recreate.ts +++ b/src/lib/onboard/docker-gpu-patch-recreate.ts @@ -400,6 +400,9 @@ export function recreateOpenShellDockerSandboxContainer( ); if (!reconcile.execReady) { context.rolledBack = reconcile.rolledBack; + context.replacementStopConfirmed = reconcile.replacementStopConfirmed; + context.replacementRemovalConfirmed = reconcile.replacementRemovalConfirmed; + context.replacementPresence = reconcile.replacementPresence; throw reconcile.error; } return result(reconcile.backupRemoved); diff --git a/src/lib/onboard/docker-gpu-patch-rollback.test.ts b/src/lib/onboard/docker-gpu-patch-rollback.test.ts index 1c3a6b3c554..094e4ae8bf1 100644 --- a/src/lib/onboard/docker-gpu-patch-rollback.test.ts +++ b/src/lib/onboard/docker-gpu-patch-rollback.test.ts @@ -1,13 +1,19 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + import { describe, expect, it, vi } from "vitest"; import { + collectDockerGpuPatchDiagnostics, type DockerContainerInspect, getDockerGpuPatchFailureContext, recreateOpenShellDockerSandboxWithGpu, } from "./docker-gpu-patch"; +import { finalizeDockerGpuPatchBackup } from "./docker-gpu-patch-finalize"; // The recreate path probes sandbox DNS through a real `docker run` when these // stay unstubbed, which makes the rollback assertions depend on a live Docker @@ -115,6 +121,245 @@ describe("recreateOpenShellDockerSandboxWithGpu rollback path", () => { ).toBe(false); }); + it("retries from the restored sandbox after exact-ID replacement cleanup (#7996)", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-gpu-rollback-retry-")); + const restoredId = "b".repeat(64); + const replacementId = "a".repeat(64); + const retryId = "d".repeat(64); + const unrelatedId = "e".repeat(64); + const unrelatedName = "openshell-beta"; + const originalName = "openshell-alpha"; + const initialBackupName = "backup-container"; + let restoredName = initialBackupName; + let restoredPresent = true; + let restoredRunning = false; + let replacementPresent = true; + let retryPresent = false; + let unrelatedPresent = true; + const stopTargets: string[] = []; + const removeTargets: string[] = []; + const renameTargets: string[] = []; + const startTargets: string[] = []; + + const alphaContainerIds = () => + [ + restoredPresent ? restoredId : null, + replacementPresent ? replacementId : null, + retryPresent ? retryId : null, + ].filter((value): value is string => value !== null); + const inspectByTarget: Record DockerContainerInspect[]> = { + [restoredId]: () => + restoredPresent ? [{ ...inspectFixture(), Id: restoredId, Name: `/${restoredName}` }] : [], + [replacementId]: () => + replacementPresent + ? [ + { + Id: replacementId, + Name: "/failed-replacement", + Config: { Image: "openshell/sandbox:abc", Env: [], Labels: {} }, + State: { Running: false }, + HostConfig: {}, + NetworkSettings: { Networks: {} }, + }, + ] + : [], + [retryId]: () => + retryPresent ? [{ ...inspectFixture(), Id: retryId, Name: `/${originalName}` }] : [], + }; + const captureByCommand: Record string> = { + ps: () => `${alphaContainerIds().join("\n")}\n`, + info: () => "", + inspect: (args) => JSON.stringify(inspectByTarget[String(args.at(-1))]?.() ?? []), + }; + const dockerCapture = vi.fn( + (args: readonly string[]) => captureByCommand[String(args[0])]?.(args) ?? "", + ); + const runByCommand: Record { status: number; stdout: string }> = { + ps: () => ({ + status: 0, + stdout: replacementPresent ? `${replacementId}\n` : "", + }), + }; + const dockerRun = vi.fn( + (args: readonly string[]) => + runByCommand[String(args[0])]?.() ?? { status: 0, stdout: "probe-id\n" }, + ); + const stopHandlers = new Map void>([ + [ + restoredId, + () => { + restoredRunning = false; + }, + ], + ]); + const dockerStop = vi.fn((target: string) => { + stopTargets.push(target); + stopHandlers.get(target)?.(); + return { status: 0 }; + }); + const successfulRemoval = () => ({ status: 0 }); + const replacementRemovalHandlers = [ + () => ({ status: 1, stderr: "daemon timeout" }), + () => { + replacementPresent = false; + return successfulRemoval(); + }, + ]; + const removalHandlers = new Map { status: number; stderr?: string }>([ + [replacementId, () => (replacementRemovalHandlers.shift() ?? successfulRemoval)()], + [ + unrelatedId, + () => { + unrelatedPresent = false; + return successfulRemoval(); + }, + ], + [ + unrelatedName, + () => { + unrelatedPresent = false; + return successfulRemoval(); + }, + ], + ]); + const removeRestored = () => { + restoredPresent = false; + return successfulRemoval(); + }; + removalHandlers.set(restoredName, removeRestored); + const dockerRm = vi.fn((target: string) => { + removeTargets.push(target); + return removalHandlers.get(target)?.() ?? successfulRemoval(); + }); + const updateRestoredName = (next: string) => { + removalHandlers.delete(restoredName); + restoredName = next; + removalHandlers.set(restoredName, removeRestored); + }; + const renameHandlers = new Map void>([ + [initialBackupName, updateRestoredName], + [restoredId, updateRestoredName], + ]); + const dockerRename = vi.fn((from: string, to: string) => { + renameTargets.push(from, to); + renameHandlers.get(from)?.(to); + return { status: 0 }; + }); + const startHandlers = new Map void>([ + [ + originalName, + () => { + restoredRunning = true; + }, + ], + ]); + const dockerStart = vi.fn((target: string) => { + startTargets.push(target); + startHandlers.get(target)?.(); + return { status: 0 }; + }); + const dockerRunDetached = vi.fn(() => { + retryPresent = true; + return { status: 0, stdout: `${retryId}\n` }; + }); + const deps = { + dockerCapture, + dockerRun, + dockerRunDetached, + dockerStop, + dockerRm, + dockerRename, + dockerStart, + dockerLogs: vi.fn(() => ""), + runOpenshell: vi.fn(() => ({ status: 0 })), + runCaptureOpenshell: vi.fn(() => "alpha Ready\n"), + sleep: vi.fn(), + homedir: () => tmpDir, + now: () => new Date("2026-07-03T00:00:00Z"), + readDir: vi.fn(() => null), + readFile: vi.fn(() => null), + ...offlineDnsDeps, + }; + const failedResult = { + applied: true as const, + oldContainerId: restoredId, + newContainerId: replacementId, + originalName, + backupContainerName: initialBackupName, + mode: { + kind: "gpus" as const, + label: "--gpus all", + device: "all", + args: ["--gpus", "all"], + }, + backupRemoved: false, + }; + + try { + const rollback = finalizeDockerGpuPatchBackup( + { result: failedResult, supervisorReady: false }, + deps, + ); + + expect(rollback).toEqual({ + backupRemoved: false, + rolledBack: true, + replacementStopConfirmed: true, + replacementRemovalConfirmed: false, + replacementPresence: "present", + }); + expect(restoredName).toBe(originalName); + expect(restoredRunning).toBe(true); + expect(replacementPresent).toBe(true); + + const diagnostics = collectDockerGpuPatchDiagnostics( + "alpha", + { + context: { + sandboxName: "alpha", + oldContainerId: restoredId, + newContainerId: replacementId, + backupContainerName: initialBackupName, + selectedMode: failedResult.mode, + ...rollback, + }, + }, + deps, + ); + expect(diagnostics?.cleanupCommands).toEqual([ + `docker rm -f ${JSON.stringify(replacementId)}`, + ]); + + // Model the operator applying the sole exact-ID cleanup command before + // rerunning onboarding. The restored sandbox and unrelated beta sandbox + // remain outside that cleanup boundary. + const cleanupCommand = diagnostics?.cleanupCommands[0]; + expect(cleanupCommand).toBeDefined(); + const cleanupTarget = JSON.parse(String(cleanupCommand).slice("docker rm -f ".length)); + expect(cleanupTarget).toBe(replacementId); + expect(dockerRm(cleanupTarget).status).toBe(0); + expect(alphaContainerIds()).toEqual([restoredId]); + expect(unrelatedPresent).toBe(true); + + const retried = recreateOpenShellDockerSandboxWithGpu( + { sandboxName: "alpha", timeoutSecs: 1 }, + deps, + ); + + expect(retried.oldContainerId).toBe(restoredId); + expect(retried.oldContainerId).not.toBe(replacementId); + expect(retried.newContainerId).toBe(retryId); + expect(retried.backupRemoved).toBe(true); + expect(alphaContainerIds()).toEqual([retryId]); + expect(unrelatedPresent).toBe(true); + const touchedTargets = [...stopTargets, ...removeTargets, ...renameTargets, ...startTargets]; + expect(touchedTargets).not.toContain(unrelatedId); + expect(touchedTargets).not.toContain(unrelatedName); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + it.each([ 1, null, diff --git a/src/lib/onboard/docker-gpu-patch-rollback.ts b/src/lib/onboard/docker-gpu-patch-rollback.ts index 81532529503..79be9a39fe0 100644 --- a/src/lib/onboard/docker-gpu-patch-rollback.ts +++ b/src/lib/onboard/docker-gpu-patch-rollback.ts @@ -4,10 +4,12 @@ import { dockerRename as defaultDockerRename, dockerRm as defaultDockerRm, + dockerRun as defaultDockerRun, dockerStart as defaultDockerStart, dockerStop as defaultDockerStop, } from "../adapters/docker"; import { hasZeroDockerExitStatus } from "./docker-command-result"; +import { fullDockerContainerId } from "./docker-gpu-patch-clone"; import { DOCKER_GPU_PATCH_TIMEOUT_MS } from "./docker-gpu-patch-constants"; import type { DockerGpuPatchDeps } from "./docker-gpu-patch-types"; @@ -24,40 +26,104 @@ type DockerRenameFn = ( newContainerName: string, opts?: DockerRunOptions, ) => DockerRunResult; +type DockerRunFn = (args: readonly string[], opts?: DockerRunOptions) => DockerRunResult; + +const REPLACEMENT_PRESENCE_ATTEMPTS = 3; +const REPLACEMENT_PRESENCE_RETRY_SECONDS = 0.5; + +function sleepBeforeReplacementPresenceRetry(seconds: number): void { + if (seconds <= 0 || !Number.isFinite(seconds)) return; + const buffer = new Int32Array(new SharedArrayBuffer(4)); + Atomics.wait(buffer, 0, 0, seconds * 1000); +} + +export type DockerGpuPatchRollbackOutcome = { + rolledBack: boolean; + replacementStopConfirmed: boolean; + replacementRemovalConfirmed: boolean; + replacementPresence: "absent" | "present" | "unknown"; +}; export type ResolvedDockerGpuPatchRollbackDeps = { + dockerRun: DockerRunFn; dockerStop: DockerContainerFn; dockerRm: DockerContainerFn; dockerRename: DockerRenameFn; dockerStart: DockerContainerFn; + sleep: (seconds: number) => void; }; export function resolveDockerGpuPatchRollbackDeps( deps: DockerGpuPatchDeps, ): ResolvedDockerGpuPatchRollbackDeps { return { + dockerRun: deps.dockerRun ?? defaultDockerRun, dockerStop: deps.dockerStop ?? defaultDockerStop, dockerRm: deps.dockerRm ?? defaultDockerRm, dockerRename: deps.dockerRename ?? defaultDockerRename, dockerStart: deps.dockerStart ?? defaultDockerStart, + sleep: deps.sleep ?? sleepBeforeReplacementPresenceRetry, }; } +function outputText(value: string | Buffer | null | undefined): string { + return value ? value.toString() : ""; +} + +function observeReplacementPresence( + containerId: string, + deps: ResolvedDockerGpuPatchRollbackDeps, + options: DockerRunOptions, +): DockerGpuPatchRollbackOutcome["replacementPresence"] { + const exactId = fullDockerContainerId(containerId); + if (!exactId) return "unknown"; + for (let attempt = 0; attempt < REPLACEMENT_PRESENCE_ATTEMPTS; attempt += 1) { + const result = deps.dockerRun( + ["ps", "-a", "--no-trunc", "--filter", `id=${exactId}`, "--format", "{{.ID}}"], + options, + ); + if (hasZeroDockerExitStatus(result)) { + const ids = outputText(result.stdout) + .split(/\r?\n/u) + .map((value) => value.trim()) + .filter(Boolean); + return ids.some((id) => fullDockerContainerId(id) === exactId) ? "present" : "absent"; + } + if (attempt + 1 < REPLACEMENT_PRESENCE_ATTEMPTS) { + deps.sleep(REPLACEMENT_PRESENCE_RETRY_SECONDS); + } + } + return "unknown"; +} + export function rollbackToBackupContainer( refs: { newContainerId: string; backupContainerName: string; originalName: string }, deps: ResolvedDockerGpuPatchRollbackDeps, -): boolean { +): DockerGpuPatchRollbackOutcome { const containerOpts = { ignoreError: true, suppressOutput: true, timeout: DOCKER_GPU_PATCH_TIMEOUT_MS, }; - deps.dockerStop(refs.newContainerId, containerOpts); - deps.dockerRm(refs.newContainerId, containerOpts); + const replacementStopConfirmed = hasZeroDockerExitStatus( + deps.dockerStop(refs.newContainerId, containerOpts), + ); + const replacementRemovalConfirmed = hasZeroDockerExitStatus( + deps.dockerRm(refs.newContainerId, containerOpts), + ); + const replacementPresence = replacementRemovalConfirmed + ? "absent" + : observeReplacementPresence(refs.newContainerId, deps, containerOpts); const restored = deps.dockerRename(refs.backupContainerName, refs.originalName, containerOpts); - if (!hasZeroDockerExitStatus(restored)) return false; - const started = deps.dockerStart(refs.originalName, containerOpts); - return hasZeroDockerExitStatus(started); + const rolledBack = hasZeroDockerExitStatus(restored) + ? hasZeroDockerExitStatus(deps.dockerStart(refs.originalName, containerOpts)) + : false; + return { + rolledBack, + replacementStopConfirmed, + replacementRemovalConfirmed, + replacementPresence, + }; } /** Restore the original sandbox after `docker run` fails during GPU recreation. */ @@ -65,5 +131,5 @@ export function restoreDockerGpuPatchBackupAfterRecreateFailure( refs: { newContainerId: string; backupContainerName: string; originalName: string }, deps: DockerGpuPatchDeps = {}, ): boolean { - return rollbackToBackupContainer(refs, resolveDockerGpuPatchRollbackDeps(deps)); + return rollbackToBackupContainer(refs, resolveDockerGpuPatchRollbackDeps(deps)).rolledBack; } diff --git a/src/lib/onboard/docker-gpu-patch-types.ts b/src/lib/onboard/docker-gpu-patch-types.ts index d72046bd320..0a5e77ee78b 100644 --- a/src/lib/onboard/docker-gpu-patch-types.ts +++ b/src/lib/onboard/docker-gpu-patch-types.ts @@ -82,6 +82,9 @@ export type DockerGpuPatchFailureContext = { selectedMode?: DockerGpuPatchMode | null; modeAttempts?: DockerGpuPatchModeAttempt[]; rolledBack?: boolean; + replacementStopConfirmed?: boolean; + replacementRemovalConfirmed?: boolean; + replacementPresence?: "absent" | "present" | "unknown"; }; export type DockerGpuPatchResult = { @@ -133,6 +136,7 @@ export type DockerGpuCloneRunOptions = { export type DockerGpuPatchDiagnostics = { dir: string; cleanupCommands: string[]; + cleanupDisposition: "manual" | "not_required" | "pending_rollback" | "unknown"; summaryLines: string[]; }; @@ -180,7 +184,15 @@ export type DockerGpuPatchFailureKind = export type DockerGpuPatchFailureClassification = { kind: DockerGpuPatchFailureKind; headline: string; + /** Stable create-mode identity used when a saved verdict crosses rollback. */ + selectedModeKind?: DockerGpuPatchModeKind | null; summaryLines: string[]; + /** + * Prose guidance for failure signatures whose cause is supported by an + * exact runtime signal. Kept separate from `summaryLines` so the on-disk + * summary stays machine-readable `key=value` (#7996). + */ + hints?: string[]; }; export type DockerContainerInspect = { diff --git a/src/lib/onboard/docker-gpu-patch.ts b/src/lib/onboard/docker-gpu-patch.ts index d54847a7af6..4331d2f56f0 100644 --- a/src/lib/onboard/docker-gpu-patch.ts +++ b/src/lib/onboard/docker-gpu-patch.ts @@ -28,10 +28,7 @@ export { parseDockerInspectJson, } from "./docker-gpu-patch-clone"; -import { - collectDockerGpuPatchDiagnostics, - dockerGpuPatchCleanupCommands, -} from "./docker-gpu-patch-diagnostics"; +import { collectDockerGpuPatchDiagnostics } from "./docker-gpu-patch-diagnostics"; import { getDockerGpuPatchFailureContext, recreateOpenShellDockerSandboxContainer, @@ -101,12 +98,30 @@ export { waitForOpenShellSupervisorReconnect, }; -function printDockerGpuPatchCleanup(sandboxName: string): void { - console.error(" The failed sandbox/container has been left in place for inspection."); - console.error(" Manual cleanup:"); - for (const command of dockerGpuPatchCleanupCommands(sandboxName)) { - console.error(` ${command}`); +function printDockerGpuPatchCleanup( + context: DockerGpuPatchFailureContext | null, + diagnostics: ReturnType, +): void { + if (context?.rolledBack === true) { + console.error(" The pre-patch sandbox container was restored and started."); + if (diagnostics?.cleanupDisposition === "manual") { + console.error(" The failed replacement container may still be present."); + console.error(" Manual cleanup:"); + for (const command of diagnostics.cleanupCommands) console.error(` ${command}`); + } else if ( + !diagnostics || + diagnostics.cleanupDisposition === "unknown" || + diagnostics.cleanupDisposition === "pending_rollback" + ) { + console.error( + " Replacement container cleanup could not be confirmed. Inspect the diagnostics before removing any container.", + ); + } + return; } + console.error( + " The failed sandbox and container state is uncertain. Inspect the diagnostics before removing any container.", + ); } export function applyDockerGpuPatchOrExit( @@ -143,6 +158,7 @@ function printDockerGpuPatchClassificationLines( if (!classification) return; if (classification.headline) console.error(` ${classification.headline}`); for (const line of classification.summaryLines) console.error(` ${line}`); + for (const hint of classification.hints ?? []) console.error(` ${hint}`); } function patchedContainerIdFromContext( @@ -170,6 +186,14 @@ function snapshotInspectDeps( return inner; } +function classificationMatchesSelectedMode( + classification: DockerGpuPatchFailureClassification, + selectedMode: DockerGpuPatchMode | null, +): boolean { + if (!selectedMode) return false; + return classification.selectedModeKind === selectedMode.kind; +} + export function printDockerGpuPatchFailureAndExit( sandboxName: string, error: unknown, @@ -177,7 +201,13 @@ export function printDockerGpuPatchFailureAndExit( context?: DockerGpuPatchFailureContext | null; selectedMode?: DockerGpuPatchMode | null; additionalSummaryLines?: readonly string[]; - }, + /** + * Classification captured while the replacement container still existed. + * The failure path cannot rely on that replacement remaining inspectable + * after rollback, so a fresh snapshot can lose its exit state (#7996). + */ + preRollbackClassification?: DockerGpuPatchFailureClassification | null; + } & Pick, ): never { const context = deps.context || getDockerGpuPatchFailureContext(error) || null; const selectedMode = deps.selectedMode || context?.selectedMode || null; @@ -187,7 +217,16 @@ export function printDockerGpuPatchFailureAndExit( { patchedContainerId: patchedContainerIdFromContext(context) }, inspectDeps, ); - const classification = classifyDockerGpuPatchFailure(snapshot, selectedMode); + // Prefer the earlier verdict only when the fresh snapshot cannot inspect the + // replacement container after rollback and the verdict describes this + // failure's exact create option (#7996). + const observedClassification = classifyDockerGpuPatchFailure(snapshot, selectedMode); + const classification = + deps.preRollbackClassification?.kind === "patched_container_failed" && + snapshot.patchedContainerState === null && + classificationMatchesSelectedMode(deps.preRollbackClassification, selectedMode) + ? deps.preRollbackClassification + : observedClassification; const diagnostics = collectDockerGpuPatchDiagnostics( sandboxName, { @@ -198,7 +237,7 @@ export function printDockerGpuPatchFailureAndExit( classification, additionalSummaryLines: deps.additionalSummaryLines, }, - inspectDeps, + deps, ); const errorMessage = error instanceof Error && error.message @@ -221,7 +260,7 @@ export function printDockerGpuPatchFailureAndExit( console.error( " NEMOCLAW_SANDBOX_GPU=0 skip GPU passthrough entirely (or rerun with --no-gpu).", ); - printDockerGpuPatchCleanup(sandboxName); + printDockerGpuPatchCleanup(context, diagnostics); process.exit(1); } @@ -256,7 +295,7 @@ export function printDockerGpuReadinessFailure( if (diagnostics) { console.error(` Docker GPU diagnostics saved: ${diagnostics.dir}`); } - printDockerGpuPatchCleanup(sandboxName); + printDockerGpuPatchCleanup(context, diagnostics); } export function printDockerGpuProofFailure( @@ -294,7 +333,7 @@ export function printDockerGpuProofFailure( if (diagnostics) { console.error(` Diagnostics saved: ${diagnostics.dir}`); } - printDockerGpuPatchCleanup(sandboxName); + printDockerGpuPatchCleanup(context, diagnostics); } const SANDBOX_FAILURE_PHASE_TOKENS = new Set(["Error", "Failed", "CrashLoopBackOff"]); @@ -419,6 +458,15 @@ export function captureDockerGpuPatchSandboxSnapshot( return { sandboxPhase, sandboxListLine, patchedContainerState }; } +// Exit code 127 alone is ambiguous because `env` propagates a child process's +// status. Missing-startup guidance therefore requires a separate exact signal +// from the replacement container's captured logs (#7996). +const SANDBOX_STARTUP_COMMAND_NOT_FOUND_EXIT_CODE = 127; +const SANDBOX_STARTUP_COMMAND_NOT_FOUND_HINTS: readonly string[] = [ + "Container logs show that the sandbox image does not provide the NemoClaw-managed `nemoclaw-start` command.", + "Rebuild the sandbox image from the complete Dockerfile and source context for the selected agent and NemoClaw release.", +]; + function describePatchedContainerState(state: DockerContainerState | null): string[] { if (!state) return []; const lines: string[] = []; @@ -461,7 +509,7 @@ function patchedContainerLooksFailed(state: DockerContainerState | null): boolea export function classifyDockerGpuPatchFailure( snapshot: DockerGpuPatchSandboxSnapshot, selectedMode: DockerGpuPatchMode | null, - options: { proofError?: unknown } = {}, + options: { proofError?: unknown; managedStartupCommandMissing?: boolean } = {}, ): DockerGpuPatchFailureClassification { const lines: string[] = []; if (snapshot.sandboxPhase) lines.push(`sandbox_phase=${snapshot.sandboxPhase}`); @@ -474,6 +522,7 @@ export function classifyDockerGpuPatchFailure( const sandboxNotLive = !!snapshot.sandboxPhase && !SANDBOX_LIVE_PHASE_TOKENS.has(snapshot.sandboxPhase); + const hints: string[] = []; let kind: DockerGpuPatchFailureKind = "unknown"; let headline: string; if (containerFailed) { @@ -484,6 +533,12 @@ export function classifyDockerGpuPatchFailure( typeof exit === "number" && exit !== 0 ? `Patched GPU container exited with code ${exit}${opt}.` : `Patched GPU container is not running${opt}.`; + if ( + exit === SANDBOX_STARTUP_COMMAND_NOT_FOUND_EXIT_CODE && + options.managedStartupCommandMissing === true + ) { + hints.push(...SANDBOX_STARTUP_COMMAND_NOT_FOUND_HINTS); + } } else if (sandboxInErrorPhase) { kind = "sandbox_error_phase"; headline = `OpenShell sandbox entered ${snapshot.sandboxPhase} phase before the GPU proof could run.`; @@ -514,5 +569,11 @@ export function classifyDockerGpuPatchFailure( options.proofError instanceof Error ? options.proofError.message : String(options.proofError); if (proofText) lines.push(`proof_error=${proofText}`); } - return { kind, headline, summaryLines: lines }; + const classification = { + kind, + headline, + selectedModeKind: selectedMode?.kind ?? null, + summaryLines: lines, + }; + return hints.length > 0 ? { ...classification, hints } : classification; } diff --git a/src/lib/onboard/docker-gpu-pre-rollback-diagnostics.test.ts b/src/lib/onboard/docker-gpu-pre-rollback-diagnostics.test.ts index 5f58c9c4f02..9b2649226ba 100644 --- a/src/lib/onboard/docker-gpu-pre-rollback-diagnostics.test.ts +++ b/src/lib/onboard/docker-gpu-pre-rollback-diagnostics.test.ts @@ -99,15 +99,22 @@ describe("Docker GPU pre-rollback diagnostics (#6110)", () => { ); try { - const diagnostics = captureDockerGpuPreRollbackDiagnostics("alpha", patchResult(), { + const captured = captureDockerGpuPreRollbackDiagnostics("alpha", patchResult(), { dockerCapture, dockerLogs, homedir: () => tmpDir, now: () => new Date("2026-07-01T23:00:00Z"), runCaptureOpenshell, }); + const diagnostics = captured?.diagnostics; expect(diagnostics?.dir).toBeTruthy(); + const summary = fs.readFileSync(path.join(diagnostics?.dir ?? "", "summary.txt"), "utf-8"); + expect(diagnostics?.cleanupCommands).toEqual([]); + expect(summary).toContain("rolled_back=pending"); + expect(summary).toContain("cleanup_disposition=pending_rollback"); + expect(summary).toContain("cleanup_required=unknown"); + expect(summary).not.toContain("openshell sandbox delete"); expect( fs.readFileSync(path.join(diagnostics?.dir ?? "", "docker-top.txt"), "utf-8"), ).toContain("nemoclaw-start"); @@ -133,6 +140,10 @@ describe("Docker GPU pre-rollback diagnostics (#6110)", () => { expect(diagnosticContents).not.toContain(secretCanary); expect(diagnosticContents).not.toContain(discoveredSecretCanary); expect(diagnosticContents).not.toContain("untrusted.secret"); + const returnedClassification = JSON.stringify(captured?.classification); + expect(returnedClassification).not.toContain(secretCanary); + expect(returnedClassification).not.toContain(discoveredSecretCanary); + expect(returnedClassification).toContain(""); const fullInspectCalls = dockerCapture.mock.calls .map(([args], index) => ({ args, order: dockerCapture.mock.invocationCallOrder[index] })) .filter(({ args }) => args[0] === "inspect" && args[1] !== "--format"); @@ -168,6 +179,103 @@ describe("Docker GPU pre-rollback diagnostics (#6110)", () => { } }); + it("retains exit code 127 without assigning a cause when the bundle cannot be created (#7996)", () => { + const dockerResponses = new Map([ + [ + "inspect --format {{json .State}} new-container-id", + JSON.stringify({ Status: "exited", Running: false, ExitCode: 127 }), + ], + ]); + const dockerCapture = vi.fn( + (args: readonly string[]) => dockerResponses.get(args.join(" ")) ?? "", + ); + const openshellResponses = new Map([ + ["sandbox get", "Phase: Error\n"], + ["sandbox list", "alpha Error\n"], + ]); + const runCaptureOpenshell = vi.fn( + (args: string[]) => openshellResponses.get(`${args[0] ?? ""} ${args[1] ?? ""}`.trim()) ?? "", + ); + + const captured = captureDockerGpuPreRollbackDiagnostics("alpha", patchResult(), { + dockerCapture, + dockerLogs: vi.fn(() => "nemoclaw-start: child process returned status 127\n"), + homedir: () => "relative-home", + runCaptureOpenshell, + }); + + expect(captured?.diagnostics).toBeNull(); + expect(captured?.classification).toMatchObject({ + kind: "patched_container_failed", + headline: expect.stringContaining("exited with code 127"), + summaryLines: expect.arrayContaining(["patched_container_exit_code=127"]), + }); + expect(captured?.classification.hints ?? []).toEqual([]); + }); + + it("captures replacement state before optional enrichment exhausts the shared budget (#7996)", () => { + const clock = [0, 0, 0, 0]; + vi.spyOn(Date, "now").mockImplementation(() => clock.shift() ?? 10_001); + const dockerCapture = vi.fn((args: readonly string[]) => + args.join(" ") === "inspect --format {{json .State}} new-container-id" + ? JSON.stringify({ Status: "exited", Running: false, ExitCode: 127 }) + : "", + ); + const runCaptureOpenshell = vi.fn((args: string[]) => + args.join(" ") === "sandbox get alpha" ? "Phase: Error\n" : "alpha Error\n", + ); + + const captured = captureDockerGpuPreRollbackDiagnostics("alpha", patchResult(), { + dockerCapture, + dockerLogs: vi.fn(() => ""), + homedir: () => "relative-home", + runCaptureOpenshell, + }); + + expect(captured?.classification).toMatchObject({ + kind: "patched_container_failed", + summaryLines: expect.arrayContaining(["patched_container_exit_code=127"]), + }); + expect(dockerCapture).toHaveBeenCalledTimes(1); + expect(dockerCapture).toHaveBeenCalledWith( + ["inspect", "--format", "{{json .State}}", "new-container-id"], + expect.objectContaining({ timeout: 2_000 }), + ); + }); + + it.each([ + ["GNU", "/usr/bin/env: \u2018nemoclaw-start\u2019: No such file or directory\n"], + ["BusyBox", "env: can't execute 'nemoclaw-start': No such file or directory\n"], + ])("adds missing-startup guidance for the %s env error (#7996)", (_env, dockerLog) => { + const dockerResponses = new Map([ + [ + "inspect --format {{json .State}} new-container-id", + JSON.stringify({ Status: "exited", Running: false, ExitCode: 127 }), + ], + ]); + const dockerCapture = vi.fn( + (args: readonly string[]) => dockerResponses.get(args.join(" ")) ?? "", + ); + const openshellResponses = new Map([ + ["sandbox get", "Phase: Error\n"], + ["sandbox list", "alpha Error\n"], + ]); + const runCaptureOpenshell = vi.fn( + (args: string[]) => openshellResponses.get(`${args[0] ?? ""} ${args[1] ?? ""}`.trim()) ?? "", + ); + + const captured = captureDockerGpuPreRollbackDiagnostics("alpha", patchResult(), { + dockerCapture, + dockerLogs: vi.fn(() => dockerLog), + homedir: () => "relative-home", + runCaptureOpenshell, + }); + + expect(captured?.classification.hints).toEqual( + expect.arrayContaining([expect.stringContaining("NemoClaw-managed `nemoclaw-start`")]), + ); + }); + it("redacts snapshot values when the shared capture budget expires before collector inspect", () => { vi.spyOn(console, "error").mockImplementation(() => {}); const clock = [0, 0, 0, 0, 0, 0, 0, 0]; @@ -203,7 +311,7 @@ describe("Docker GPU pre-rollback diagnostics (#6110)", () => { ]); try { - const diagnostics = captureDockerGpuPreRollbackDiagnostics("alpha", patchResult(), { + const captured = captureDockerGpuPreRollbackDiagnostics("alpha", patchResult(), { dockerCapture: vi.fn( (args: readonly string[]) => dockerResponses.get(args.join(" ")) ?? "", ), @@ -214,6 +322,7 @@ describe("Docker GPU pre-rollback diagnostics (#6110)", () => { (args: string[]) => openshellResponses.get(`${args[0] ?? ""} ${args[1] ?? ""}`) ?? "", ), }); + const diagnostics = captured?.diagnostics; const summary = fs.readFileSync(path.join(diagnostics?.dir ?? "", "summary.txt"), "utf8"); const state = fs.readFileSync( diff --git a/src/lib/onboard/docker-gpu-pre-rollback-diagnostics.ts b/src/lib/onboard/docker-gpu-pre-rollback-diagnostics.ts index 0d1ab1dbb61..f69e69ec9c8 100644 --- a/src/lib/onboard/docker-gpu-pre-rollback-diagnostics.ts +++ b/src/lib/onboard/docker-gpu-pre-rollback-diagnostics.ts @@ -5,7 +5,10 @@ import { dockerCapture as defaultDockerCapture, dockerLogs as defaultDockerLogs, } from "../adapters/docker"; -import { discoverDockerGpuDiagnosticSensitiveValues } from "./docker-gpu-diagnostic-redaction"; +import { + createDockerGpuDiagnosticRedactor, + discoverDockerGpuDiagnosticSensitiveValues, +} from "./docker-gpu-diagnostic-redaction"; import { captureDockerGpuPatchSandboxSnapshot, classifyDockerGpuPatchFailure, @@ -17,12 +20,15 @@ import type { DockerContainerInspect, DockerGpuPatchDeps, DockerGpuPatchDiagnostics, + DockerGpuPatchFailureClassification, DockerGpuPatchFailureContext, DockerGpuPatchResult, } from "./docker-gpu-patch-types"; const PRE_ROLLBACK_DIAGNOSTICS_TOTAL_BUDGET_MS = 10_000; const PRE_ROLLBACK_DIAGNOSTICS_CALL_TIMEOUT_MS = 2_000; +const MISSING_MANAGED_STARTUP_COMMAND_LOG = + /(?:^|\n)(?:\/usr\/bin\/)?env: (?:[\u0027\u2018]?nemoclaw-start[\u0027\u2019]?|can't execute 'nemoclaw-start'): No such file or directory(?:\r?\n|$)/u; type PreRollbackDiagnosticsDeps = Pick< DockerGpuPatchDeps, @@ -118,11 +124,26 @@ function primeSensitiveDiagnosticValues( return [...sensitiveValues]; } +/** + * Diagnostics bundle plus the verdict computed while the replacement container + * was still inspectable. The failure path cannot rely on that replacement + * remaining inspectable after rollback, so `classification` preserves the + * exit-state evidence for the caller's user-facing failure message (#7996). + */ +export type DockerGpuPreRollbackDiagnostics = { + classification: DockerGpuPatchFailureClassification; + diagnostics: DockerGpuPatchDiagnostics | null; +}; + +function logsShowMissingManagedStartupCommand(logs: string): boolean { + return MISSING_MANAGED_STARTUP_COMMAND_LOG.test(logs); +} + export function captureDockerGpuPreRollbackDiagnostics( sandboxName: string, result: DockerGpuPatchResult, deps: PreRollbackDiagnosticsDeps = {}, -): DockerGpuPatchDiagnostics | null { +): DockerGpuPreRollbackDiagnostics | null { const context: DockerGpuPatchFailureContext = { sandboxName, oldContainerId: result.oldContainerId, @@ -131,17 +152,35 @@ export function captureDockerGpuPreRollbackDiagnostics( selectedMode: result.mode, }; const diagnosticDeps = boundedDiagnosticsDeps(deps); - const additionalSensitiveValues = primeSensitiveDiagnosticValues( + // Preserve the short-lived failure verdict before optional inspect + // enrichment can consume the shared capture budget. The replacement State + // is the only source of its exit code once rollback removes the container. + const snapshot = captureDockerGpuPatchSandboxSnapshot( sandboxName, - result, + { patchedContainerId: result.newContainerId }, diagnosticDeps, ); - const snapshot = captureDockerGpuPatchSandboxSnapshot( + const additionalSensitiveValues = primeSensitiveDiagnosticValues( sandboxName, - { patchedContainerId: result.newContainerId }, + result, diagnosticDeps, ); - const classification = classifyDockerGpuPatchFailure(snapshot, result.mode); + let patchedContainerLogs = ""; + try { + const logs = diagnosticDeps.dockerLogs ?? defaultDockerLogs; + patchedContainerLogs = logs(result.newContainerId, { + tail: 120, + timeout: DOCKER_GPU_PATCH_TIMEOUT_MS, + }); + } catch { + // An unavailable log does not establish a missing startup command. + } + const classification = classifyDockerGpuPatchFailure(snapshot, result.mode, { + managedStartupCommandMissing: logsShowMissingManagedStartupCommand(patchedContainerLogs), + }); + const redactedClassification = createDockerGpuDiagnosticRedactor( + additionalSensitiveValues, + ).redactValue(classification) as DockerGpuPatchFailureClassification; let dockerTopOutput: string | null = null; try { const dockerCapture = diagnosticDeps.dockerCapture ?? defaultDockerCapture; @@ -161,11 +200,10 @@ export function captureDockerGpuPreRollbackDiagnostics( classification, additionalSensitiveValues, dockerTopOutput, + cleanupDisposition: "pending-rollback", }, diagnosticDeps, ); - if (!diagnostics) return null; - - console.error(` Pre-rollback diagnostics saved: ${diagnostics.dir}`); - return diagnostics; + if (diagnostics) console.error(` Pre-rollback diagnostics saved: ${diagnostics.dir}`); + return { classification: redactedClassification, diagnostics }; } diff --git a/src/lib/onboard/docker-gpu-sandbox-create-diagnostics.test.ts b/src/lib/onboard/docker-gpu-sandbox-create-diagnostics.test.ts index e527ddbad7e..05bf88d5e16 100644 --- a/src/lib/onboard/docker-gpu-sandbox-create-diagnostics.test.ts +++ b/src/lib/onboard/docker-gpu-sandbox-create-diagnostics.test.ts @@ -1,9 +1,17 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + import { afterEach, describe, expect, it, vi } from "vitest"; import type { DockerGpuPatchResult } from "./docker-gpu-patch"; +import { + captureDockerGpuPreRollbackDiagnostics, + type DockerGpuPreRollbackDiagnostics, +} from "./docker-gpu-pre-rollback-diagnostics"; import { createDockerGpuSandboxCreatePatch } from "./docker-gpu-sandbox-create"; const RESULT: DockerGpuPatchResult = { @@ -111,4 +119,220 @@ describe("Docker GPU create diagnostics fail-safety (#6110)", () => { expect(finalizeBackup).toHaveBeenCalledWith({ result: RESULT, supervisorReady: false }, deps); expect(onPatchFailureExit).toHaveBeenCalledTimes(1); }); + + it("forwards the pre-rollback classification when bundle collection fails (#7996)", () => { + vi.spyOn(console, "log").mockImplementation(() => {}); + const deps = { + runOpenshell: vi.fn(() => ({ status: 0 })), + runCaptureOpenshell: vi.fn(() => ""), + sleep: vi.fn(), + dockerCapture: vi.fn(() => ""), + }; + const classification = { + kind: "patched_container_failed" as const, + headline: "Patched GPU container exited with code 127 (--gpus all).", + summaryLines: ["patched_container_exit_code=127"], + hints: ["Container logs show that `nemoclaw-start` is missing."], + }; + const onPatchFailureExit = vi.fn(); + const patch = createDockerGpuSandboxCreatePatch({ + route: "compatibility", + sandboxName: "alpha", + timeoutSecs: 60, + deps, + overrides: { + recreatePatch: vi.fn(() => RESULT), + waitForSupervisor: vi.fn(() => false), + capturePreRollbackDiagnostics: vi.fn(() => ({ + classification, + diagnostics: null, + })), + finalizeBackup: vi.fn(() => ({ backupRemoved: false, rolledBack: true })), + onPatchFailureExit, + }, + }); + + patch.ensureApplied(); + patch.waitForSupervisorReconnectIfNeeded(); + + // The printer cannot rely on the replacement remaining inspectable after + // rollback, so this hand-off preserves the exit-code evidence. + expect(onPatchFailureExit).toHaveBeenCalledWith( + "alpha", + expect.any(Error), + expect.objectContaining({ preRollbackClassification: classification }), + ); + }); + + it("passes a null pre-rollback classification when capture returns nothing (#7996)", () => { + vi.spyOn(console, "log").mockImplementation(() => {}); + const deps = { + runOpenshell: vi.fn(() => ({ status: 0 })), + runCaptureOpenshell: vi.fn(() => ""), + sleep: vi.fn(), + dockerCapture: vi.fn(() => ""), + }; + const onPatchFailureExit = vi.fn(); + const patch = createDockerGpuSandboxCreatePatch({ + route: "compatibility", + sandboxName: "alpha", + timeoutSecs: 60, + deps, + overrides: { + recreatePatch: vi.fn(() => RESULT), + waitForSupervisor: vi.fn(() => false), + capturePreRollbackDiagnostics: vi.fn(() => null), + finalizeBackup: vi.fn(() => ({ backupRemoved: false, rolledBack: true })), + onPatchFailureExit, + }, + }); + + patch.ensureApplied(); + patch.waitForSupervisorReconnectIfNeeded(); + + expect(onPatchFailureExit).toHaveBeenCalledWith( + "alpha", + expect.any(Error), + expect.objectContaining({ preRollbackClassification: null }), + ); + }); + + it("uses exact-ID cleanup when a restored sandbox retains the failed replacement (#7996)", () => { + vi.spyOn(console, "log").mockImplementation(() => {}); + const stderr: string[] = []; + vi.spyOn(console, "error").mockImplementation((...args: unknown[]) => { + stderr.push(args.map(String).join(" ")); + }); + vi.spyOn(process, "exit").mockImplementation(((_code?: number) => { + throw new Error("__test_exit__"); + }) as never); + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-gpu-composed-rollback-")); + const replacementId = "a".repeat(64); + let captured: DockerGpuPreRollbackDiagnostics | null = null; + const dockerResponses = new Map([ + [ + "ps -a --no-trunc --filter label=openshell.ai/managed-by=openshell --filter label=openshell.ai/sandbox-name=alpha --format {{.ID}}", + `${replacementId}\n`, + ], + [ + `inspect --format {{json .State}} ${replacementId}`, + JSON.stringify({ Status: "exited", Running: false, ExitCode: 127 }), + ], + [ + `inspect ${replacementId}`, + JSON.stringify([ + { + Id: replacementId, + Name: "/failed-replacement", + Config: { Env: [] }, + HostConfig: {}, + NetworkSettings: { Networks: {} }, + }, + ]), + ], + ["inspect old-container-id", "[]"], + ["inspect backup-container", "[]"], + ]); + const dockerCapture = vi.fn( + (args: readonly string[]) => dockerResponses.get(args.join(" ")) ?? "", + ); + const openshellResponses = new Map([ + ["sandbox get", "Phase: Error\n"], + ["sandbox list", "alpha Error\n"], + ]); + const runCaptureOpenshell = vi.fn( + (args: readonly string[]) => + openshellResponses.get(`${args[0] ?? ""} ${args[1] ?? ""}`.trim()) ?? "", + ); + const dockerRm = vi.fn(() => ({ status: 1, stderr: "daemon timeout" })); + const dockerRun = vi.fn(() => ({ status: 0, stdout: `${replacementId}\n` })); + const dockerRename = vi.fn(() => ({ status: 0 })); + const dockerStart = vi.fn(() => ({ status: 0 })); + const deps = { + runOpenshell: vi.fn(() => ({ status: 0 })), + runCaptureOpenshell, + sleep: vi.fn(), + dockerCapture, + dockerRun, + dockerLogs: vi.fn(() => "/usr/bin/env: 'nemoclaw-start': No such file or directory\n"), + homedir: () => tmpDir, + now: vi + .fn() + .mockReturnValueOnce(new Date("2026-07-03T00:00:00Z")) + .mockReturnValue(new Date("2026-07-03T00:00:01Z")), + dockerStop: vi.fn(() => ({ status: 0 })), + dockerRm, + dockerRename, + dockerStart, + }; + const result = { ...RESULT, newContainerId: replacementId }; + + try { + const patch = createDockerGpuSandboxCreatePatch({ + route: "compatibility", + sandboxName: "alpha", + timeoutSecs: 60, + deps, + overrides: { + recreatePatch: vi.fn(() => result), + waitForSupervisor: vi.fn(() => false), + capturePreRollbackDiagnostics: (...args) => { + captured = captureDockerGpuPreRollbackDiagnostics(...args); + return captured; + }, + }, + }); + + patch.ensureApplied(); + expect(() => patch.waitForSupervisorReconnectIfNeeded()).toThrow(/__test_exit__/); + + const preRollback = (captured as DockerGpuPreRollbackDiagnostics | null)?.diagnostics; + const preRollbackSummary = fs.readFileSync( + path.join(preRollback?.dir ?? "", "summary.txt"), + "utf-8", + ); + expect(preRollback?.cleanupCommands).toEqual([]); + expect(preRollbackSummary).toContain("cleanup_disposition=pending_rollback"); + expect(preRollbackSummary).toContain("cleanup_required=unknown"); + expect(preRollbackSummary).not.toContain("openshell sandbox delete"); + const postRollbackDir = path.join( + tmpDir, + ".nemoclaw", + "onboard-failures", + "2026-07-03T00-00-01-000Z-alpha-docker-gpu-patch", + ); + const postRollbackSummary = fs.readFileSync( + path.join(postRollbackDir, "summary.txt"), + "utf-8", + ); + expect(postRollbackSummary).toContain("rolled_back=yes"); + expect(postRollbackSummary).toContain("replacement_stop_confirmed=yes"); + expect(postRollbackSummary).toContain("replacement_removal_confirmed=no"); + expect(postRollbackSummary).toContain("replacement_presence=present"); + expect(postRollbackSummary).toContain("cleanup_disposition=manual"); + expect(postRollbackSummary).toContain("cleanup_required=yes"); + expect(postRollbackSummary).toContain(`docker rm -f ${JSON.stringify(replacementId)}`); + expect(postRollbackSummary).not.toContain("openshell sandbox delete"); + expect(dockerRm).toHaveBeenCalledWith( + replacementId, + expect.objectContaining({ ignoreError: true }), + ); + expect(dockerRename).toHaveBeenCalledWith( + "backup-container", + "openshell-alpha", + expect.objectContaining({ ignoreError: true }), + ); + expect(dockerStart).toHaveBeenCalledWith( + "openshell-alpha", + expect.objectContaining({ ignoreError: true }), + ); + const output = stderr.join("\n"); + expect(output).toContain("pre-patch sandbox container was restored and started"); + expect(output).toContain("failed replacement container may still be present"); + expect(output).toContain(`docker rm -f ${JSON.stringify(replacementId)}`); + expect(output).not.toContain("openshell sandbox delete"); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); }); diff --git a/src/lib/onboard/docker-gpu-sandbox-create.ts b/src/lib/onboard/docker-gpu-sandbox-create.ts index 52c15d0b5be..23f3b39e2da 100644 --- a/src/lib/onboard/docker-gpu-sandbox-create.ts +++ b/src/lib/onboard/docker-gpu-sandbox-create.ts @@ -15,6 +15,7 @@ import { finalizeDockerGpuPatchBackup } from "./docker-gpu-patch-finalize"; import type { DockerGpuPatchBackend, DockerGpuPatchDeps, + DockerGpuPatchFailureClassification, DockerGpuPatchFailureContext, DockerGpuPatchMode, DockerGpuPatchResult, @@ -42,7 +43,18 @@ export { type DockerGpuSandboxCreateDeps = Pick< DockerGpuPatchDeps, - "runOpenshell" | "runCaptureOpenshell" | "sleep" | "dockerCapture" | "dockerRun" | "dockerStop" + | "runOpenshell" + | "runCaptureOpenshell" + | "sleep" + | "dockerCapture" + | "dockerRun" + | "dockerStop" + | "dockerRm" + | "dockerRename" + | "dockerStart" + | "dockerLogs" + | "homedir" + | "now" >; type WaitSupervisorFn = typeof waitForOpenShellSupervisorReconnect; @@ -159,6 +171,13 @@ export function createDockerGpuSandboxCreatePatch( options.overrides?.capturePreRollbackDiagnostics ?? captureDockerGpuPreRollbackDiagnostics; const onPatchFailureExit = options.overrides?.onPatchFailureExit ?? printDockerGpuPatchFailureAndExit; + const failureDiagnosticDeps = { + runCaptureOpenshell: options.deps.runCaptureOpenshell, + dockerCapture: options.deps.dockerCapture, + dockerLogs: options.deps.dockerLogs, + homedir: options.deps.homedir, + now: options.deps.now, + }; const applyOptions = { sandboxName: options.sandboxName, @@ -296,8 +315,7 @@ export function createDockerGpuSandboxCreatePatch( const rollbackError = await rollbackAfterFailure(); if (!rollbackError) return; onPatchFailureExit(options.sandboxName, rollbackError, { - runCaptureOpenshell: options.deps.runCaptureOpenshell, - dockerCapture: options.deps.dockerCapture, + ...failureDiagnosticDeps, additionalSummaryLines: routeAdapter.additionalSummaryLines, context: { ...failureContext(), @@ -341,9 +359,15 @@ export function createDockerGpuSandboxCreatePatch( needsSupervisorWait = false; return; } - if (!supervisorReady && result) { + // Keep the pre-rollback verdict: it is computed while the replacement + // container is still inspectable, so it is the only classification that + // can name the exit code. The failure printer cannot rely on that + // replacement remaining inspectable after rollback (#7996). + let preRollbackClassification: DockerGpuPatchFailureClassification | null = null; + if (result) { try { - captureFailedClone(options.sandboxName, result, options.deps); + preRollbackClassification = + captureFailedClone(options.sandboxName, result, options.deps)?.classification ?? null; } catch (error) { console.warn( ` ⚠ Could not capture the failed GPU container before rollback: ${error instanceof Error ? error.message : String(error)}`, @@ -361,7 +385,11 @@ export function createDockerGpuSandboxCreatePatch( onPatchFailureExit(options.sandboxName, new Error(failureMessage), { runCaptureOpenshell: options.deps.runCaptureOpenshell, dockerCapture: options.deps.dockerCapture, + dockerLogs: options.deps.dockerLogs, + homedir: options.deps.homedir, + now: options.deps.now, additionalSummaryLines: routeAdapter.additionalSummaryLines, + preRollbackClassification, context: { sandboxName: options.sandboxName, oldContainerId: result?.oldContainerId, @@ -369,6 +397,9 @@ export function createDockerGpuSandboxCreatePatch( backupContainerName: result?.backupContainerName, selectedMode: result?.mode ?? null, rolledBack: finalizeOutcome?.rolledBack ?? false, + replacementStopConfirmed: finalizeOutcome?.replacementStopConfirmed, + replacementRemovalConfirmed: finalizeOutcome?.replacementRemovalConfirmed, + replacementPresence: finalizeOutcome?.replacementPresence, }, }); },