From 5868b6c395be0dca6faf45c706222913538a4fff Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Sun, 16 Aug 2026 14:57:39 -0700 Subject: [PATCH 1/4] fix(cli): restore full E2E lifecycle convergence Preserve locally proven platform provenance during DCode finalization. Make the Docker GPU replacement start the authoritative final lifecycle event. Retry only route-family cutover responses that can come from the previous route. Validate exact quiescent owner cleanup before the protected harness deletes it. Signed-off-by: Prekshi Vyas --- docs/inference/switch-providers.mdx | 5 +- docs/reference/commands.mdx | 9 +- .../checks/run-managed-image-openshell-e2e.ts | 70 ++++++++++--- .../inference-set-compatible-provider.test.ts | 99 +++++++++++++++++++ src/lib/actions/inference-set-provider.ts | 32 ++++++ src/lib/actions/inference-set.test-support.ts | 3 + src/lib/actions/inference-set.ts | 34 +++++-- .../onboard/docker-gpu-patch-finalize.test.ts | 86 ++++++++++++++-- src/lib/onboard/docker-gpu-patch-finalize.ts | 30 ++++-- ...ocker-gpu-sandbox-create-lifecycle.test.ts | 8 +- src/lib/onboard/docker-gpu-sandbox-create.ts | 21 +++- ...sandbox-base-image-platform-digest.test.ts | 49 +++++++++ .../sandbox-base-image/resolution-metadata.ts | 22 ++++- ...managed-image-protected-runtime-helpers.ts | 4 +- ...d-image-protected-runtime-contract.test.ts | 84 ++++++++++------ 15 files changed, 483 insertions(+), 73 deletions(-) diff --git a/docs/inference/switch-providers.mdx b/docs/inference/switch-providers.mdx index 2f894e51994..afadf5ebec2 100644 --- a/docs/inference/switch-providers.mdx +++ b/docs/inference/switch-providers.mdx @@ -74,7 +74,10 @@ Run the rebuild before relying on the running agent. Use `--no-verify` only when OpenShell cannot verify the target provider at switch time and you have already confirmed its provider and credential. This flag does not bypass shared-gateway compatibility checks. When you explicitly supply a direct compatible endpoint at `http://host.openshell.internal:`, NemoClaw skips OpenShell's host-side provider probe because that hostname resolves only inside the sandbox network. -It then sends one validation request from the target sandbox before persisting the route in NemoClaw state; the request allows up to 16 output tokens. +It then sends a validation request from the target sandbox before persisting the route in NemoClaw state; the request allows up to 16 output tokens. +When the switch changes the API family and that request returns HTTP `400` or `404`, NemoClaw retries up to two times after delays of one and two seconds. +Each retry allows another 16 output tokens. +Other failures are not retried. If that request fails, NemoClaw attempts to restore the previous OpenShell selection and remove a provider that this switch created. If the error reports that rollback could not complete, rerun onboarding before using the route or retrying the switch. Endpoint-shape and shared-gateway compatibility checks still apply. diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index 317e17cee7e..3f3ff7d0036 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -1045,7 +1045,9 @@ On Docker Desktop WSL, the compatibility path skips CDI and tries Docker `--gpus On Jetson/Tegra hosts, the compatibility path uses the NVIDIA runtime and adds eligible host group IDs for the supported GPU device nodes. These include selected `/dev/nvmap`, `/dev/nvhost-*`, and `/dev/nvgpu/igpu0/*` nodes plus real `/dev/dri/renderD*` character devices. After compatibility recreation starts, onboarding keeps the pre-patch container as a rollback backup until the replacement passes the Ready, GPU, and applicable local-inference checks. -If a later check fails, onboarding prints failure diagnostics and attempts to restore the pre-patch container. +If one of those checks fails before backup removal, onboarding prints failure diagnostics and attempts to restore the pre-patch container. +To commit the replacement, NemoClaw stops it, removes the rollback backup, starts the replacement as the final container lifecycle event, and verifies OpenShell supervisor readiness again. +If that final handoff cannot be confirmed, onboarding exits with the container diagnostics and cleanup guidance instead of reporting success. If 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, so inspect the sandbox and its labeled Docker containers before running a deletion command. @@ -4126,7 +4128,10 @@ If the in-sandbox config write or integrity hash update fails, the OpenShell rou Supported provider names are `nvidia-prod`, `nvidia-nim`, `nvidia-router`, `openai-api`, `anthropic-prod`, `compatible-anthropic-endpoint`, `gemini-api`, `compatible-endpoint`, `hermes-provider`, `ollama-local`, and `vllm-local`. Use `--no-verify` only when OpenShell cannot verify the provider at switch time but you have already confirmed the provider and credential. When you explicitly supply a direct compatible endpoint at `http://host.openshell.internal:`, NemoClaw skips OpenShell's host-side provider probe because that hostname resolves only inside the sandbox network. -Before it persists the route in the NemoClaw registry or agent config, the command sends one validation request from the target sandbox with a 16-token output limit. +Before it persists the route in the NemoClaw registry or agent config, the command sends a validation request from the target sandbox with a 16-token output limit. +When the switch changes the API family and that request returns HTTP `400` or `404`, NemoClaw retries up to two times after delays of one and two seconds. +Each retry has the same 16-token output limit. +Other failures are not retried. If that request fails, the command attempts to restore the previous OpenShell selection and remove a provider that this switch created. If the error reports that rollback could not complete, rerun onboarding before using the route or retrying the switch. Endpoint-shape and shared-gateway compatibility checks still apply. diff --git a/scripts/checks/run-managed-image-openshell-e2e.ts b/scripts/checks/run-managed-image-openshell-e2e.ts index 68ce2aab7a4..9b1dfa76958 100644 --- a/scripts/checks/run-managed-image-openshell-e2e.ts +++ b/scripts/checks/run-managed-image-openshell-e2e.ts @@ -8,6 +8,7 @@ import os from "node:os"; import path from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; import { resolveAgent } from "../../src/lib/agent/onboard.ts"; +import { parseOpenShellSandboxId } from "../../src/lib/adapters/openshell/sandbox-identity.ts"; import { isValidName, NAME_ALLOWED_FORMAT } from "../../src/lib/name-validation.ts"; import { type StopHostGatewayResult, @@ -21,6 +22,7 @@ import { MANAGED_BOOTSTRAP_SCHEMA_VERSION, type ManagedBootstrapAdapter, type ManagedBootstrapAuthorityStore, + ManagedBootstrapOwnerCleanupRequiredError, } from "../../src/lib/onboard/managed-bootstrap/adapter.ts"; import { createDockerManagedBootstrapAdapter } from "../../src/lib/onboard/managed-bootstrap/docker.ts"; import { createDockerManagedBootstrapSurface } from "../../src/lib/onboard/managed-bootstrap/docker-runtime.ts"; @@ -786,23 +788,50 @@ export function assertExactSandboxImage( return resolved.exactIds[0] ?? ""; } -export function assertFailedBootstrapContainerCleanup( +export function assertFailedBootstrapOwnerCleanupRetention( input: Inputs, networkName: string, + expectedRuntimeId: string, env: NodeJS.ProcessEnv, runCommand: ManagedImageCommandRunner = commandResult, ): void { const resolved = exactHarnessContainerIds(input, networkName, env, true, runCommand); - if (resolved.candidateCount !== 0 || resolved.exactIds.length !== 0) { + if ( + resolved.candidateCount !== 1 || + resolved.exactIds.length !== 1 || + resolved.exactIds[0] !== expectedRuntimeId + ) { + throw new Error( + `managed-bootstrap rollback did not retain its one exact owner-cleanup runtime: found ${resolved.candidateCount} labeled and ${resolved.exactIds.length} exact containers`, + ); + } + const inspect = runCommand(["docker", "inspect", expectedRuntimeId], env); + if (inspect.status !== 0) { throw new Error( - `managed-bootstrap rollback retained a failed held sandbox: found ${resolved.candidateCount} labeled and ${resolved.exactIds.length} exact containers`, + `managed-bootstrap rollback could not inspect its retained owner-cleanup runtime: ${commandDetail(inspect)}`, + ); + } + try { + const records = JSON.parse(String(inspect.stdout ?? "")) as Array<{ + State?: { Paused?: boolean; Restarting?: boolean; Running?: boolean }; + }>; + const state = records.length === 1 ? records[0]?.State : undefined; + if (state?.Running !== false || state.Paused !== false || state.Restarting !== false) { + throw new Error("retained runtime is not explicitly quiescent"); + } + } catch (error) { + throw new Error( + `managed-bootstrap rollback did not prove a quiescent owner-cleanup runtime: ${ + error instanceof Error ? error.message : String(error) + }`, ); } } -function assertFailedSandboxAbsent( +function assertFailedSandboxOwnerCleanupRetention( onboard: OnboardModule, input: Inputs, + expectedSandboxId: string, env: NodeJS.ProcessEnv, ): void { const get = onboard.runOpenshell(["sandbox", "get", input.sandbox], { @@ -816,12 +845,13 @@ function assertFailedSandboxAbsent( stdio: ["ignore", "pipe", "pipe"], }); if ( - get.status === 0 || + get.status !== 0 || + parseOpenShellSandboxId(String(get.stdout ?? "")) !== expectedSandboxId || list.status !== 0 || - `${list.stdout ?? ""}\n${list.stderr ?? ""}`.includes(input.sandbox) + !`${list.stdout ?? ""}\n${list.stderr ?? ""}`.includes(input.sandbox) ) { throw new Error( - `managed-bootstrap rollback retained failed OpenShell sandbox state: get=${commandDetail(get)} list=${commandDetail(list)}`, + `managed-bootstrap rollback did not retain its exact OpenShell owner-cleanup state: get=${commandDetail(get)} list=${commandDetail(list)}`, ); } } @@ -1021,11 +1051,29 @@ async function run { ); }); + it("waits for a changed API family to replace the previous sandbox route", async () => { + const captureOpenshell = createCompatibleProviderCapture({ + name: "compatible-anthropic-endpoint", + type: "anthropic", + credentialEnv: "COMPATIBLE_ANTHROPIC_API_KEY", + configKey: "ANTHROPIC_BASE_URL", + initiallyPresent: false, + }); + const probeSandboxRoute = vi + .fn() + .mockReturnValueOnce({ + ok: false, + detail: "sandbox inference invocation probe returned HTTP 400", + httpStatus: 400, + }) + .mockReturnValueOnce({ ok: true }); + const deps = createDeps({ + config: { + agents: { defaults: { model: { primary: "inference/old-model" } } }, + models: { providers: { inference: { api: "openai-completions", models: [] } } }, + }, + entry: { + name: "alpha", + agent: "openclaw", + provider: "compatible-endpoint", + model: "old-model", + }, + session: baseSession({ + provider: "compatible-endpoint", + model: "old-model", + preferredInferenceApi: "openai-completions", + }), + captureOpenshell, + probeSandboxRoute, + }); + + await runInferenceSet( + { + provider: "compatible-anthropic-endpoint", + model: "mock-anthropic-model", + endpointUrl: "http://host.openshell.internal:18767/", + credentialEnv: "COMPATIBLE_ANTHROPIC_API_KEY", + inferenceApi: "anthropic-messages", + }, + deps, + ); + + expect(probeSandboxRoute).toHaveBeenCalledTimes(2); + expect(deps.calls.sleep).toHaveBeenCalledWith(1_000); + expect(deps.calls.updateSandbox).toHaveBeenCalled(); + }); + + it("does not retry a target rejection when the API family did not change", async () => { + const captureOpenshell = createCompatibleProviderCapture({ + name: "compatible-endpoint", + type: "openai", + credentialEnv: "COMPATIBLE_API_KEY", + configKey: "OPENAI_BASE_URL", + initiallyPresent: false, + }); + const probeSandboxRoute = vi.fn(() => ({ + ok: false as const, + detail: "sandbox inference invocation probe returned HTTP 400", + httpStatus: 400, + })); + const deps = createDeps({ + config: { + agents: { defaults: { model: { primary: "inference/old-model" } } }, + models: { providers: { inference: { api: "openai-completions", models: [] } } }, + }, + entry: { + name: "alpha", + agent: "openclaw", + provider: "nvidia-prod", + model: "old-model", + }, + session: baseSession({ provider: "nvidia-prod", model: "old-model" }), + captureOpenshell, + probeSandboxRoute, + }); + + await expect( + runInferenceSet( + { + provider: "compatible-endpoint", + model: "mock-model", + endpointUrl: "http://host.openshell.internal:18767/", + credentialEnv: "COMPATIBLE_API_KEY", + inferenceApi: "openai-completions", + }, + deps, + ), + ).rejects.toThrow(/Sandbox-side verification rejected/); + + expect(probeSandboxRoute).toHaveBeenCalledOnce(); + expect(deps.calls.sleep).not.toHaveBeenCalled(); + expect(deps.calls.updateSandbox).not.toHaveBeenCalled(); + }); + it.each([ [ "returns a rejection", diff --git a/src/lib/actions/inference-set-provider.ts b/src/lib/actions/inference-set-provider.ts index daf4cef5cc0..1adb71f7f34 100644 --- a/src/lib/actions/inference-set-provider.ts +++ b/src/lib/actions/inference-set-provider.ts @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { type CaptureOpenshellResult, stripAnsi } from "../adapters/openshell/client"; +import { retryUntilAsync } from "../core/retry"; import { matchesGatewayProviderBinding, parseGatewayProviderMetadata, @@ -32,6 +33,12 @@ export type InferenceSetSandboxRouteProbe = ( input: SandboxInferenceInvocationInput, ) => SandboxInferenceInvocationResult; +const ROUTE_FAMILY_CONVERGENCE_RETRY_DELAYS_MS = [1_000, 2_000] as const; + +export function sleepInferenceSetRouteConvergence(milliseconds: number): Promise { + return new Promise((resolve) => setTimeout(resolve, milliseconds)); +} + export function probeInferenceSetSandboxRoute( input: SandboxInferenceInvocationInput, ): SandboxInferenceInvocationResult { @@ -45,6 +52,31 @@ export function probeInferenceSetSandboxRoute( ); } +export async function probeInferenceSetSandboxRouteUntilConverged( + options: { + input: SandboxInferenceInvocationInput; + previousInferenceApi: string | null; + targetInferenceApi: string | null; + }, + deps: { + probe: InferenceSetSandboxRouteProbe; + sleep: (milliseconds: number) => Promise; + } = { + probe: probeInferenceSetSandboxRoute, + sleep: sleepInferenceSetRouteConvergence, + }, +): Promise { + const inferenceApiChanged = options.previousInferenceApi !== options.targetInferenceApi; + return await retryUntilAsync(() => deps.probe(options.input), { + accept: (result) => + result.ok || + !inferenceApiChanged || + (result.httpStatus !== 400 && result.httpStatus !== 404), + retryDelaysMs: ROUTE_FAMILY_CONVERGENCE_RETRY_DELAYS_MS, + sleep: deps.sleep, + }); +} + export function requireInferenceSetRuntimeAuthority( entry: SandboxEntry, providers: RuntimeProviderBundleRegistry = CURRENT_RUNTIME_PROVIDER_BUNDLES, diff --git a/src/lib/actions/inference-set.test-support.ts b/src/lib/actions/inference-set.test-support.ts index 94cb2b679e3..59cf82a96f0 100644 --- a/src/lib/actions/inference-set.test-support.ts +++ b/src/lib/actions/inference-set.test-support.ts @@ -160,6 +160,7 @@ export function createDeps(options: { ensureHttpsPinRuntimeAdapter: ReturnType; revokeHttpsPinRuntimeAdapterRoute: ReturnType; probeSandboxRoute: ReturnType; + sleep: ReturnType; restartSandboxGateway: ReturnType; withGatewayRouteMutationLock: ReturnType; }; @@ -221,6 +222,7 @@ export function createDeps(options: { options.revokeHttpsPinRuntimeAdapterRoute ?? (async () => true), ), probeSandboxRoute: vi.fn(options.probeSandboxRoute ?? (() => ({ ok: true }) as const)), + sleep: vi.fn(async () => {}), restartSandboxGateway: vi.fn( options.restartSandboxGateway ?? ((): ReturnType => ({ @@ -266,6 +268,7 @@ export function createDeps(options: { revokeHttpsPinRuntimeAdapterRoute: calls.revokeHttpsPinRuntimeAdapterRoute as InferenceSetDeps["revokeHttpsPinRuntimeAdapterRoute"], probeSandboxRoute: calls.probeSandboxRoute as InferenceSetDeps["probeSandboxRoute"], + sleep: calls.sleep, withGatewayRouteMutationLock: calls.withGatewayRouteMutationLock as InferenceSetDeps["withGatewayRouteMutationLock"], restartSandboxGateway: calls.restartSandboxGateway, diff --git a/src/lib/actions/inference-set.ts b/src/lib/actions/inference-set.ts index 9ae0367b7e9..7086bf06a0c 100644 --- a/src/lib/actions/inference-set.ts +++ b/src/lib/actions/inference-set.ts @@ -74,9 +74,11 @@ import { type InferenceSetSandboxRouteProbe, prepareInferenceSetProviderBinding, probeInferenceSetSandboxRoute, + probeInferenceSetSandboxRouteUntilConverged, type RuntimeProviderBundleRegistry, RuntimeProviderSelectionError, requireInferenceSetRuntimeAuthority, + sleepInferenceSetRouteConvergence, } from "./inference-set-provider"; import { buildInferenceSetFailure } from "./inference-set-provider-diagnostics"; import { @@ -167,6 +169,7 @@ export interface InferenceSetDeps extends InferenceGatewayRestartDeps { ensureHttpsPinRuntimeAdapter: EnsureHttpsPinRuntimeAdapterFn; revokeHttpsPinRuntimeAdapterRoute: (routeId: string) => Promise; probeSandboxRoute: InferenceSetSandboxRouteProbe; + sleep: (milliseconds: number) => Promise; withGatewayRouteMutationLock: typeof withGatewayRouteMutationLock; } @@ -273,6 +276,7 @@ function defaultDeps(): InferenceSetDeps { ensureHttpsPinRuntimeAdapter, revokeHttpsPinRuntimeAdapterRoute, probeSandboxRoute: probeInferenceSetSandboxRoute, + sleep: sleepInferenceSetRouteConvergence, withGatewayRouteMutationLock, restartSandboxGateway: defaultInferenceGatewayRestart, isSandboxConfigMutable: (sandboxName) => { @@ -1003,6 +1007,14 @@ async function runInferenceSetWithoutHostLock( sandboxName, session, }); + const previousInferenceApi = resolveRuntimeInferenceApi({ + agentName, + config, + currentProvider: entry.provider, + provider: entry.provider ?? "", + sandboxName, + session, + }); assertReasoningEffortRoute(reasoningEffortRequest, provider, preMutationInferenceApi); const previousProvider = typeof entry.provider === "string" ? entry.provider.trim() : ""; const previousModel = typeof entry.model === "string" ? entry.model.trim() : ""; @@ -1145,12 +1157,22 @@ async function runInferenceSetWithoutHostLock( if (probeDirectSandboxBridge) { let probe: ReturnType; try { - probe = deps.probeSandboxRoute({ - sandboxName, - provider, - model, - preferredInferenceApi: preMutationInferenceApi, - }); + probe = await probeInferenceSetSandboxRouteUntilConverged( + { + input: { + sandboxName, + provider, + model, + preferredInferenceApi: preMutationInferenceApi, + }, + previousInferenceApi, + targetInferenceApi: preMutationInferenceApi, + }, + { + probe: deps.probeSandboxRoute, + sleep: deps.sleep, + }, + ); } catch (probeError) { const probeFailureDetail = probeError instanceof Error && probeError.message diff --git a/src/lib/onboard/docker-gpu-patch-finalize.test.ts b/src/lib/onboard/docker-gpu-patch-finalize.test.ts index e220949bf8e..428611aaf38 100644 --- a/src/lib/onboard/docker-gpu-patch-finalize.test.ts +++ b/src/lib/onboard/docker-gpu-patch-finalize.test.ts @@ -63,17 +63,38 @@ function collectRollbackDiagnostics( } describe("finalizeDockerGpuPatchBackup", () => { - it("removes the backup container when supervisor reconnect succeeded", () => { + it("makes the replacement restart the final lifecycle event after removing the backup", () => { + const dockerStop = vi.fn(() => ({ status: 0 })); const dockerRm = vi.fn((_name: string) => ({ status: 0 })); + const dockerStart = vi.fn(() => ({ status: 0 })); const outcome = finalizeDockerGpuPatchBackup( { result: deferredCreateResult(), supervisorReady: true }, - { dockerRm }, + { dockerStop, dockerRm, dockerStart }, + ); + expect(outcome).toEqual({ + backupRemoved: true, + rolledBack: false, + replacementStoppedForCommit: true, + replacementRestarted: true, + }); + expect(dockerStop).toHaveBeenCalledWith( + "new-container-id", + expect.objectContaining({ ignoreError: true }), ); - expect(outcome).toEqual({ backupRemoved: true, rolledBack: false }); expect(dockerRm).toHaveBeenCalledWith( "openshell-alpha-nemoclaw-gpu-backup-1780491860342", expect.objectContaining({ ignoreError: true }), ); + expect(dockerStart).toHaveBeenCalledWith( + "new-container-id", + expect.objectContaining({ ignoreError: true }), + ); + expect(dockerStop.mock.invocationCallOrder[0]).toBeLessThan( + dockerRm.mock.invocationCallOrder[0], + ); + expect(dockerRm.mock.invocationCallOrder[0]).toBeLessThan( + dockerStart.mock.invocationCallOrder[0], + ); }); it("rolls back to the backup container when supervisor reconnect failed", () => { @@ -171,15 +192,22 @@ describe("finalizeDockerGpuPatchBackup", () => { }); it("reports backupRemoved=false when supervisor reconnect succeeded but docker rm of the backup failed", () => { + const dockerStop = vi.fn(() => ({ status: 0 })); const dockerRm = vi.fn((_name: string) => ({ status: 1, stderr: "Error response from daemon: container is in use", })); + const dockerStart = vi.fn(() => ({ status: 0 })); const outcome = finalizeDockerGpuPatchBackup( { result: deferredCreateResult(), supervisorReady: true }, - { dockerRm }, + { dockerStop, dockerRm, dockerStart }, ); - expect(outcome).toEqual({ backupRemoved: false, rolledBack: false }); + expect(outcome).toEqual({ + backupRemoved: false, + rolledBack: false, + replacementStoppedForCommit: true, + replacementRestarted: true, + }); expect(dockerRm).toHaveBeenCalledWith( "openshell-alpha-nemoclaw-gpu-backup-1780491860342", expect.objectContaining({ ignoreError: true }), @@ -187,12 +215,56 @@ describe("finalizeDockerGpuPatchBackup", () => { }); it("fails closed when backup removal has no exit status", () => { + const dockerStop = vi.fn(() => ({ status: 0 })); const dockerRm = vi.fn((_name: string) => ({ status: null, stderr: "timed out" })); + const dockerStart = vi.fn(() => ({ status: 0 })); const outcome = finalizeDockerGpuPatchBackup( { result: deferredCreateResult(), supervisorReady: true }, - { dockerRm }, + { dockerStop, dockerRm, dockerStart }, ); - expect(outcome).toEqual({ backupRemoved: false, rolledBack: false }); + expect(outcome).toEqual({ + backupRemoved: false, + rolledBack: false, + replacementStoppedForCommit: true, + replacementRestarted: true, + }); + }); + + it("retains the backup when the replacement cannot be stopped for the final handoff", () => { + const dockerStop = vi.fn(() => ({ status: 1 })); + const dockerRm = vi.fn(() => ({ status: 0 })); + const dockerStart = vi.fn(() => ({ status: 0 })); + + const outcome = finalizeDockerGpuPatchBackup( + { result: deferredCreateResult(), supervisorReady: true }, + { dockerStop, dockerRm, dockerStart }, + ); + + expect(outcome).toEqual({ + backupRemoved: false, + rolledBack: false, + replacementStoppedForCommit: false, + }); + expect(dockerRm).not.toHaveBeenCalled(); + expect(dockerStart).not.toHaveBeenCalled(); + }); + + it("reports a failed replacement restart after the backup is removed", () => { + const outcome = finalizeDockerGpuPatchBackup( + { result: deferredCreateResult(), supervisorReady: true }, + { + dockerStop: vi.fn(() => ({ status: 0 })), + dockerRm: vi.fn(() => ({ status: 0 })), + dockerStart: vi.fn(() => ({ status: 1 })), + }, + ); + + expect(outcome).toEqual({ + backupRemoved: true, + rolledBack: false, + replacementStoppedForCommit: true, + replacementRestarted: false, + }); }); it("records a remaining exact-ID replacement when removal fails (#7996)", () => { diff --git a/src/lib/onboard/docker-gpu-patch-finalize.ts b/src/lib/onboard/docker-gpu-patch-finalize.ts index 24b5b01aeb6..26b8b7c0ea1 100644 --- a/src/lib/onboard/docker-gpu-patch-finalize.ts +++ b/src/lib/onboard/docker-gpu-patch-finalize.ts @@ -44,6 +44,8 @@ export type DockerGpuPatchFinalizeOptions = { export type DockerGpuPatchFinalizeOutcome = { backupRemoved: boolean; rolledBack: boolean; + replacementStoppedForCommit?: boolean; + replacementRestarted?: boolean; replacementStopConfirmed?: boolean; replacementRemovalConfirmed?: boolean; replacementPresence?: "absent" | "present" | "unknown"; @@ -63,13 +65,29 @@ export function finalizeDockerGpuPatchBackup( return { backupRemoved: true, rolledBack: false }; } if (options.supervisorReady) { - // Backup removal is best-effort: the supervisor probe already confirmed - // the new GPU container is reachable, so the backup is no longer needed - // even if `docker rm` cannot delete it (e.g. concurrent admin action, - // daemon timeout). Reflect the actual rm status in the outcome so - // diagnostics can flag a leaked backup container. + // Stop the replacement before retiring the labelled backup, then start it + // afterward. OpenShell observes Docker lifecycle events for both containers; + // leaving the backup's removal as the final event can demote the already + // reconnected replacement back to not-ready. The final start makes the live + // replacement's registration authoritative while the rollback container is + // still retained until the destructive removal succeeds. + const stopResult = resolved.dockerStop(options.result.newContainerId, containerOpts); + if (!hasZeroDockerExitStatus(stopResult)) { + return { + backupRemoved: false, + rolledBack: false, + replacementStoppedForCommit: false, + }; + } const rmResult = resolved.dockerRm(options.result.backupContainerName, containerOpts); - return { backupRemoved: hasZeroDockerExitStatus(rmResult), rolledBack: false }; + const backupRemoved = hasZeroDockerExitStatus(rmResult); + const startResult = resolved.dockerStart(options.result.newContainerId, containerOpts); + return { + backupRemoved, + rolledBack: false, + replacementStoppedForCommit: true, + replacementRestarted: hasZeroDockerExitStatus(startResult), + }; } const rollback = rollbackToBackupContainer( { diff --git a/src/lib/onboard/docker-gpu-sandbox-create-lifecycle.test.ts b/src/lib/onboard/docker-gpu-sandbox-create-lifecycle.test.ts index 0206ce9a80d..23ed6edde0c 100644 --- a/src/lib/onboard/docker-gpu-sandbox-create-lifecycle.test.ts +++ b/src/lib/onboard/docker-gpu-sandbox-create-lifecycle.test.ts @@ -50,6 +50,7 @@ describe("createDockerGpuSandboxCreatePatch composed flow", () => { const finalizeBackup = vi.fn(() => ({ backupRemoved: true, rolledBack: false, + replacementRestarted: true, })); const capturePreRollbackDiagnostics = vi.fn(() => null); const onPatchFailureExit = vi.fn(); @@ -89,6 +90,7 @@ describe("createDockerGpuSandboxCreatePatch composed flow", () => { await patch.commitAfterReady(); expect(finalizeBackup).toHaveBeenCalledTimes(1); expect(finalizeBackup).toHaveBeenCalledWith({ result, supervisorReady: true }, deps); + expect(waitForSupervisor).toHaveBeenCalledTimes(2); expect(capturePreRollbackDiagnostics).not.toHaveBeenCalled(); expect(onPatchFailureExit).not.toHaveBeenCalled(); }); @@ -159,13 +161,13 @@ describe("createDockerGpuSandboxCreatePatch composed flow", () => { patch.waitForSupervisorReconnectIfNeeded(); expect(onPatchFailureExit).not.toHaveBeenCalled(); - await expect(patch.commitAfterReady()).rejects.toThrow("rollback backup"); - await expect(patch.commitAfterReady()).rejects.toThrow("rollback backup"); + await expect(patch.commitAfterReady()).rejects.toThrow("final runtime handoff"); + await expect(patch.commitAfterReady()).rejects.toThrow("final runtime handoff"); expect(onPatchFailureExit).toHaveBeenCalledOnce(); expect(onPatchFailureExit.mock.calls[0]?.[1]).toEqual( expect.objectContaining({ - message: expect.stringContaining("rollback backup"), + message: expect.stringContaining("final runtime handoff"), }), ); expect(onPatchFailureExit.mock.calls[0]?.[2]).toEqual( diff --git a/src/lib/onboard/docker-gpu-sandbox-create.ts b/src/lib/onboard/docker-gpu-sandbox-create.ts index 4bcda5765ca..fd1a0870277 100644 --- a/src/lib/onboard/docker-gpu-sandbox-create.ts +++ b/src/lib/onboard/docker-gpu-sandbox-create.ts @@ -469,9 +469,26 @@ export function createDockerGpuSandboxCreatePatch( ? finalizeBackup({ result, supervisorReady: true }, options.deps) : null; cutoverFinalized = true; - if (!finalizeOutcome || finalizeOutcome.backupRemoved) return; + if (!finalizeOutcome) return; + if (finalizeOutcome.backupRemoved && finalizeOutcome.replacementRestarted) { + const supervisorReconnectTimeoutSecs = getDockerGpuSupervisorReconnectTimeoutSecs( + options.timeoutSecs, + ); + console.log( + ` Waiting for OpenShell supervisor to confirm the final container handoff (up to ${supervisorReconnectTimeoutSecs}s)...`, + ); + if ( + waitForSupervisor(options.sandboxName, supervisorReconnectTimeoutSecs, { + runOpenshell: options.deps.runOpenshell, + runCaptureOpenshell: options.deps.runCaptureOpenshell, + sleep: options.deps.sleep, + }) + ) { + return; + } + } const failure = new Error( - "Managed startup passed Ready, but its rollback backup could not be removed.", + "Managed startup passed Ready, but its final runtime handoff did not converge.", ); cutoverFinalizationFailure = failure; onPatchFailureExit(options.sandboxName, failure, { diff --git a/src/lib/sandbox-base-image-platform-digest.test.ts b/src/lib/sandbox-base-image-platform-digest.test.ts index 926fabbe421..476ca15dfe4 100644 --- a/src/lib/sandbox-base-image-platform-digest.test.ts +++ b/src/lib/sandbox-base-image-platform-digest.test.ts @@ -148,6 +148,55 @@ describe("sandbox base-image pinned platform digest resolution", () => { expect(dockerMocks.build).not.toHaveBeenCalled(); }); + it("uses the one locally proven platform digest when the first inspect reports the index", () => { + dockerMocks.imageInspect.mockImplementation((ref: string) => ({ + status: ref === REF || ref === PLATFORM_REF ? 0 : 1, + })); + dockerMocks.imageInspectFormat.mockImplementation((format: string, ref: string) => + ( + new Map([ + [`{{json .RepoDigests}}\0${REF}`, JSON.stringify([REF])], + [ + `{{json .}}\0${REF}`, + JSON.stringify({ + Id: IMAGE_ID, + RepoDigests: [PLATFORM_REF], + Os: "linux", + Architecture: "amd64", + }), + ], + [ + `{{json .}}\0${PLATFORM_REF}`, + JSON.stringify({ + Id: IMAGE_ID, + RepoDigests: [PLATFORM_REF], + Os: "linux", + Architecture: "amd64", + }), + ], + ]).get(`${format}\0${ref}`) ?? "" + ).trim(), + ); + + const resolved = resolveSandboxBaseImage({ + ...resolutionOptions(), + pinnedRemoteRef: REF, + preferPinnedRemoteRef: true, + }); + + expect(resolved).toMatchObject({ + ref: PLATFORM_REF, + digest: PLATFORM_DIGEST, + source: "pinned", + pinnedRemoteRef: REF, + metadata: { + ref: PLATFORM_REF, + digest: PLATFORM_DIGEST, + imageId: IMAGE_ID, + }, + }); + }); + it("falls back to the Dockerfile-pinned digest when RepoDigests JSON is malformed", () => { dockerMocks.imageInspect.mockImplementation((ref: string) => ({ status: ref === REF ? 0 : 1, diff --git a/src/lib/sandbox-base-image/resolution-metadata.ts b/src/lib/sandbox-base-image/resolution-metadata.ts index f26b31d1a6a..7f8a48bb5ee 100644 --- a/src/lib/sandbox-base-image/resolution-metadata.ts +++ b/src/lib/sandbox-base-image/resolution-metadata.ts @@ -114,8 +114,26 @@ export function finalizeSandboxBaseImageResolution( key: string, resolution: SandboxBaseImageResolution, ): SandboxBaseImageResolution { - const metadata = createSandboxBaseImageResolutionMetadata(options, key, resolution); - return metadata ? { ...resolution, metadata } : resolution; + let locallyProvenResolution = resolution; + if (resolution.digest) { + const inspected = inspectLocalImageMetadata(resolution.ref); + const expectedRepoDigest = `${options.imageName}@${resolution.digest}`; + const matchingRepoDigests = Array.isArray(inspected?.RepoDigests) + ? inspected.RepoDigests.map(String).filter((entry) => + entry.startsWith(`${options.imageName}@sha256:`), + ) + : []; + if (!matchingRepoDigests.includes(expectedRepoDigest) && matchingRepoDigests.length === 1) { + const ref = matchingRepoDigests[0]; + locallyProvenResolution = { + ...resolution, + ref, + digest: ref.slice(ref.indexOf("@") + 1), + }; + } + } + const metadata = createSandboxBaseImageResolutionMetadata(options, key, locallyProvenResolution); + return metadata ? { ...locallyProvenResolution, metadata } : resolution; } export function reuseSandboxBaseImageResolutionHint( diff --git a/test/e2e/live/managed-image-protected-runtime-helpers.ts b/test/e2e/live/managed-image-protected-runtime-helpers.ts index f0728070db3..1ceede4756e 100644 --- a/test/e2e/live/managed-image-protected-runtime-helpers.ts +++ b/test/e2e/live/managed-image-protected-runtime-helpers.ts @@ -810,10 +810,10 @@ async function qualifyRollback( ); expect(result.exitCode, resultText(result)).toBe(0); expect(result.stdout).toContain( - `removed the failed exact ${contract.agent} sandbox before harness cleanup`, + `retained one exact quiescent ${contract.agent} sandbox for owner cleanup`, ); expect(result.stdout).toContain( - `left no sandbox, container, network, or harness state orphan for ${contract.agent}`, + `retained only its exact quiescent sandbox until harness owner cleanup and left no sandbox, container, network, or harness state orphan for ${contract.agent}`, ); } diff --git a/test/managed-image-protected-runtime-contract.test.ts b/test/managed-image-protected-runtime-contract.test.ts index c19be1b79ea..eaa09af68dd 100644 --- a/test/managed-image-protected-runtime-contract.test.ts +++ b/test/managed-image-protected-runtime-contract.test.ts @@ -19,7 +19,7 @@ import { } from "../scripts/checks/managed-image-protected-runtime-contract.ts"; import { assertExactSandboxImage, - assertFailedBootstrapContainerCleanup, + assertFailedBootstrapOwnerCleanupRetention, createProtectedManagedImageBootstrapInput, failureInjectingAdapter, MANAGED_IMAGE_OPENSHELL_SUPERVISOR_ARGV, @@ -44,7 +44,10 @@ const SUCCESS_WITHOUT_OUTPUT: ManagedImageCommandResult = { stderr: "", }; -function managedContainerInspectResult(contentId: string): ManagedImageCommandResult { +function managedContainerInspectResult( + contentId: string, + running: boolean, +): ManagedImageCommandResult { return { status: 0, stdout: `${JSON.stringify([ @@ -57,6 +60,7 @@ function managedContainerInspectResult(contentId: string): ManagedImageCommandRe }, Image: contentId, NetworkSettings: { Networks: { "managed-network": {} } }, + State: { Paused: false, Restarting: false, Running: running }, }, ])}\n`, stderr: "", @@ -69,11 +73,12 @@ function createManagedImageCommandRunner( listScope: "-q" | "-aq", listOutput: string, calls: string[][], + running = listScope === "-q", ): ManagedImageCommandRunner { const responses = new Map([ ["docker image inspect", { status: 0, stdout: `${contentId}\n`, stderr: "" }], [`docker ps ${listScope}`, { status: 0, stdout: listOutput, stderr: "" }], - [`docker inspect ${containerId}`, managedContainerInspectResult(contentId)], + [`docker inspect ${containerId}`, managedContainerInspectResult(contentId, running)], ]); return (argv) => { calls.push([...argv]); @@ -202,7 +207,7 @@ describe("protected managed-image runtime contract", () => { expect(fs.existsSync(stateDir)).toBe(false); }); - it("qualifies the running exact image before rollback cleanup (#7744)", () => { + it("distinguishes the running image from exact quiescent rollback retention (#7744)", () => { const calls: string[][] = []; const contentId = `sha256:${"b".repeat(64)}`; const containerId = "c".repeat(64); @@ -221,45 +226,62 @@ describe("protected managed-image runtime contract", () => { `${containerId}\n`, calls, ); - const cleanedCommand = createManagedImageCommandRunner( + const retainedCommand = createManagedImageCommandRunner( contentId, containerId, "-aq", - "", + `${containerId}\n`, calls, ); expect(assertExactSandboxImage(input, "managed-network", {}, runningCommand)).toBe(containerId); - assertFailedBootstrapContainerCleanup(input, "managed-network", {}, cleanedCommand); + assertFailedBootstrapOwnerCleanupRetention( + input, + "managed-network", + containerId, + {}, + retainedCommand, + ); expect(calls.filter((argv) => argv[1] === "ps").map((argv) => argv[2])).toEqual(["-q", "-aq"]); }); - it("rejects a stopped labeled container after failed bootstrap cleanup (#7744)", () => { - const contentId = `sha256:${"b".repeat(64)}`; - const containerId = "c".repeat(64); - const input = parseManagedImageOpenShellE2eInputs([ - "--agent", - "openclaw", - "--image", - IMAGE, - "--sandbox", - VALID_SANDBOX, - ]); - const runCommand = createManagedImageCommandRunner( - contentId, - containerId, - "-aq", - `${containerId}\n`, - [], - ); + it.each([ + ["missing", "", false, "one exact owner-cleanup runtime"], + ["running", `${"c".repeat(64)}\n`, true, "quiescent owner-cleanup runtime"], + ] as const)( + "rejects a %s owner-cleanup runtime after failed bootstrap", + (_case, list, running, message) => { + const contentId = `sha256:${"b".repeat(64)}`; + const containerId = "c".repeat(64); + const input = parseManagedImageOpenShellE2eInputs([ + "--agent", + "openclaw", + "--image", + IMAGE, + "--sandbox", + VALID_SANDBOX, + ]); + const runCommand = createManagedImageCommandRunner( + contentId, + containerId, + "-aq", + list, + [], + running, + ); - expect(() => - assertFailedBootstrapContainerCleanup(input, "managed-network", {}, runCommand), - ).toThrow( - "managed-bootstrap rollback retained a failed held sandbox: found 1 labeled and 1 exact containers", - ); - }); + expect(() => + assertFailedBootstrapOwnerCleanupRetention( + input, + "managed-network", + containerId, + {}, + runCommand, + ), + ).toThrow(message); + }, + ); it("assigns every protected agent and route a unique OpenShell-compatible sandbox name (#8497)", () => { const routeKinds = [...MANAGED_IMAGE_LOCAL_INFERENCE_KINDS, "rollback"] as const; From be63778351b052f92d43794aec5f54ba6f7017ae Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Sun, 16 Aug 2026 16:06:04 -0700 Subject: [PATCH 2/4] fix(ci): close lifecycle convergence proof gaps Signed-off-by: Prekshi Vyas --- .../checks/run-managed-image-openshell-e2e.ts | 5 +- .../inference-set-compatible-provider.test.ts | 95 +++++++++++++++++++ ...ocker-gpu-sandbox-create-lifecycle.test.ts | 60 ++++++++++++ src/lib/onboard/docker-gpu-sandbox-create.ts | 6 ++ .../credential-filter-failure.test.ts | 68 ------------- src/lib/security/credential-filter.test.ts | 48 +++++++++- ...d-image-protected-runtime-contract.test.ts | 41 +++++++- 7 files changed, 251 insertions(+), 72 deletions(-) delete mode 100644 src/lib/security/credential-filter-failure.test.ts diff --git a/scripts/checks/run-managed-image-openshell-e2e.ts b/scripts/checks/run-managed-image-openshell-e2e.ts index 9b1dfa76958..50b6945ece7 100644 --- a/scripts/checks/run-managed-image-openshell-e2e.ts +++ b/scripts/checks/run-managed-image-openshell-e2e.ts @@ -38,6 +38,7 @@ import type { RuntimeProviderBundle, } from "../../src/lib/onboard/runtime-provider/contract.ts"; import { createDockerRuntimeProviderBundle } from "../../src/lib/onboard/runtime-provider/docker.ts"; +import { parseLiveSandboxNames } from "../../src/lib/runtime-recovery.ts"; import { OPENSHELL_SANDBOX_SUPERVISOR_ARGV, prepareSandboxCreateLaunch, @@ -828,7 +829,7 @@ export function assertFailedBootstrapOwnerCleanupRetention( } } -function assertFailedSandboxOwnerCleanupRetention( +export function assertFailedSandboxOwnerCleanupRetention( onboard: OnboardModule, input: Inputs, expectedSandboxId: string, @@ -848,7 +849,7 @@ function assertFailedSandboxOwnerCleanupRetention( get.status !== 0 || parseOpenShellSandboxId(String(get.stdout ?? "")) !== expectedSandboxId || list.status !== 0 || - !`${list.stdout ?? ""}\n${list.stderr ?? ""}`.includes(input.sandbox) + !parseLiveSandboxNames(String(list.stdout ?? "")).has(input.sandbox) ) { throw new Error( `managed-bootstrap rollback did not retain its exact OpenShell owner-cleanup state: get=${commandDetail(get)} list=${commandDetail(list)}`, diff --git a/src/lib/actions/inference-set-compatible-provider.test.ts b/src/lib/actions/inference-set-compatible-provider.test.ts index 099c23bcf40..1ecdcff1e36 100644 --- a/src/lib/actions/inference-set-compatible-provider.test.ts +++ b/src/lib/actions/inference-set-compatible-provider.test.ts @@ -720,6 +720,101 @@ describe("runInferenceSet compatible providers", () => { expect(deps.calls.updateSandbox).toHaveBeenCalled(); }); + it("restores the prior route after changed-family convergence retries are exhausted", async () => { + const captureOpenshell = createCompatibleProviderCapture({ + name: "compatible-anthropic-endpoint", + type: "anthropic", + credentialEnv: "COMPATIBLE_ANTHROPIC_API_KEY", + configKey: "ANTHROPIC_BASE_URL", + initiallyPresent: false, + }); + const probeSandboxRoute = vi + .fn() + .mockReturnValueOnce({ + ok: false, + detail: "sandbox inference invocation probe returned HTTP 400", + httpStatus: 400, + }) + .mockReturnValueOnce({ + ok: false, + detail: "sandbox inference invocation probe returned HTTP 404", + httpStatus: 404, + }) + .mockReturnValueOnce({ + ok: false, + detail: "sandbox inference invocation probe returned HTTP 400", + httpStatus: 400, + }); + const deps = createDeps({ + config: { + agents: { defaults: { model: { primary: "inference/old-model" } } }, + models: { providers: { inference: { api: "openai-completions", models: [] } } }, + }, + entry: { + name: "alpha", + agent: "openclaw", + provider: "nvidia-prod", + model: "old-model", + }, + session: baseSession({ provider: "nvidia-prod", model: "old-model" }), + captureOpenshell, + probeSandboxRoute, + }); + + await expect( + runInferenceSet( + { + provider: "compatible-anthropic-endpoint", + model: "mock-anthropic-model", + endpointUrl: "http://host.openshell.internal:18767/", + credentialEnv: "COMPATIBLE_ANTHROPIC_API_KEY", + inferenceApi: "anthropic-messages", + }, + deps, + ), + ).rejects.toThrow( + /Sandbox-side verification rejected.*previous OpenShell inference selection was restored/s, + ); + + expect(probeSandboxRoute).toHaveBeenCalledTimes(3); + expect(deps.calls.sleep.mock.calls).toEqual([[1_000], [2_000]]); + expect( + captureOpenshell.mock.calls + .filter(([args]) => args[0] === "inference" && args[1] === "set") + .map(([args]) => args), + ).toEqual([ + [ + "inference", + "set", + "-g", + "nemoclaw", + "--provider", + "compatible-anthropic-endpoint", + "--model", + "mock-anthropic-model", + "--no-verify", + ], + [ + "inference", + "set", + "-g", + "nemoclaw", + "--provider", + "nvidia-prod", + "--model", + "old-model", + "--no-verify", + ], + ]); + expect( + captureOpenshell.mock.calls.some( + ([args]) => args[0] === "provider" && args[1] === "delete", + ), + ).toBe(true); + expect(deps.calls.updateSandbox).not.toHaveBeenCalled(); + expect(deps.calls.writeSandboxConfig).not.toHaveBeenCalled(); + }); + it("does not retry a target rejection when the API family did not change", async () => { const captureOpenshell = createCompatibleProviderCapture({ name: "compatible-endpoint", diff --git a/src/lib/onboard/docker-gpu-sandbox-create-lifecycle.test.ts b/src/lib/onboard/docker-gpu-sandbox-create-lifecycle.test.ts index 23ed6edde0c..75b8fcd4672 100644 --- a/src/lib/onboard/docker-gpu-sandbox-create-lifecycle.test.ts +++ b/src/lib/onboard/docker-gpu-sandbox-create-lifecycle.test.ts @@ -95,6 +95,66 @@ describe("createDockerGpuSandboxCreatePatch composed flow", () => { expect(onPatchFailureExit).not.toHaveBeenCalled(); }); + it("accepts a backup that the patch helper already finalized after reconnect", async () => { + const deps = makeDeps(); + const result = { ...deferredCreateResult(), backupRemoved: true }; + const waitForSupervisor = vi.fn(() => true); + const finalizeBackup = vi.fn(() => ({ + backupRemoved: true, + rolledBack: false, + })); + const onPatchFailureExit = vi.fn(); + const patch = createDockerGpuSandboxCreatePatch({ + route: "compatibility", + sandboxName: "alpha", + timeoutSecs: 60, + deps, + overrides: { + findContainerIds: vi.fn(() => ["existing-container"]), + recreatePatch: vi.fn(() => result), + waitForSupervisor, + finalizeBackup, + onPatchFailureExit, + }, + }); + + patch.maybeApplyDuringCreate(); + patch.waitForSupervisorReconnectIfNeeded(); + await expect(patch.commitAfterReady()).resolves.toBeUndefined(); + + expect(finalizeBackup).toHaveBeenCalledWith({ result, supervisorReady: true }, deps); + expect(waitForSupervisor).toHaveBeenCalledTimes(1); + expect(onPatchFailureExit).not.toHaveBeenCalled(); + }); + + it("rejects an explicit replacement restart failure after backup removal", async () => { + const deps = makeDeps(); + const result = deferredCreateResult(); + const onPatchFailureExit = vi.fn(); + const patch = createDockerGpuSandboxCreatePatch({ + route: "compatibility", + sandboxName: "alpha", + timeoutSecs: 60, + deps, + overrides: { + findContainerIds: vi.fn(() => ["existing-container"]), + recreatePatch: vi.fn(() => result), + waitForSupervisor: vi.fn(() => true), + finalizeBackup: vi.fn(() => ({ + backupRemoved: true, + rolledBack: false, + replacementRestarted: false, + })), + onPatchFailureExit, + }, + }); + + patch.maybeApplyDuringCreate(); + patch.waitForSupervisorReconnectIfNeeded(); + await expect(patch.commitAfterReady()).rejects.toThrow("final runtime handoff"); + expect(onPatchFailureExit).toHaveBeenCalledOnce(); + }); + it("reports a failed post-Ready rollback instead of treating it as restored", async () => { const deps = makeDeps(); const result = deferredCreateResult(); diff --git a/src/lib/onboard/docker-gpu-sandbox-create.ts b/src/lib/onboard/docker-gpu-sandbox-create.ts index fd1a0870277..409bdbc7c02 100644 --- a/src/lib/onboard/docker-gpu-sandbox-create.ts +++ b/src/lib/onboard/docker-gpu-sandbox-create.ts @@ -470,6 +470,12 @@ export function createDockerGpuSandboxCreatePatch( : null; cutoverFinalized = true; if (!finalizeOutcome) return; + if ( + finalizeOutcome.backupRemoved && + finalizeOutcome.replacementRestarted === undefined + ) { + return; + } if (finalizeOutcome.backupRemoved && finalizeOutcome.replacementRestarted) { const supervisorReconnectTimeoutSecs = getDockerGpuSupervisorReconnectTimeoutSecs( options.timeoutSecs, diff --git a/src/lib/security/credential-filter-failure.test.ts b/src/lib/security/credential-filter-failure.test.ts deleted file mode 100644 index 3849b138471..00000000000 --- a/src/lib/security/credential-filter-failure.test.ts +++ /dev/null @@ -1,68 +0,0 @@ -// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; -import { afterEach, describe, expect, it, vi } from "vitest"; - -const fsControl = vi.hoisted(() => ({ - noFollowUnavailable: false, -})); - -vi.mock("node:fs", async (importOriginal) => { - const original = await importOriginal(); - return { - ...original, - constants: { - ...original.constants, - get O_NOFOLLOW(): number | undefined { - return fsControl.noFollowUnavailable ? undefined : original.constants.O_NOFOLLOW; - }, - }, - }; -}); - -import { - sanitizeConfigFile, - sanitizeEnvFile, - sanitizeYamlConfigFile, -} from "./credential-filter.js"; - -const temporaryRoots: string[] = []; - -function makeRoot(): string { - const root = mkdtempSync(join(tmpdir(), "nemoclaw-credential-filter-failure-")); - temporaryRoots.push(root); - return root; -} - -afterEach(() => { - fsControl.noFollowUnavailable = false; - for (const root of temporaryRoots.splice(0)) { - rmSync(root, { recursive: true, force: true }); - } -}); - -describe("credential filter no-follow boundary", () => { - it("fails closed without atomic no-follow support", () => { - const root = makeRoot(); - const jsonPath = join(root, "openclaw.json"); - const yamlPath = join(root, "config.yaml"); - const envPath = join(root, ".env"); - const jsonSource = JSON.stringify({ apiKey: "sk-secret-value" }); - const yamlSource = "api_key: sk-secret-value\n"; - const envSource = "API_KEY=sk-secret-value\n"; - writeFileSync(jsonPath, jsonSource); - writeFileSync(yamlPath, yamlSource); - writeFileSync(envPath, envSource); - fsControl.noFollowUnavailable = true; - - expect(sanitizeConfigFile(jsonPath)).toBe(false); - expect(sanitizeYamlConfigFile(yamlPath)).toBe(false); - expect(sanitizeEnvFile(envPath)).toBe(false); - expect(readFileSync(jsonPath, "utf-8")).toBe(jsonSource); - expect(readFileSync(yamlPath, "utf-8")).toBe(yamlSource); - expect(readFileSync(envPath, "utf-8")).toBe(envSource); - }); -}); diff --git a/src/lib/security/credential-filter.test.ts b/src/lib/security/credential-filter.test.ts index 6ffb0809265..30190eb0f42 100644 --- a/src/lib/security/credential-filter.test.ts +++ b/src/lib/security/credential-filter.test.ts @@ -4,7 +4,24 @@ import { mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const fsControl = vi.hoisted(() => ({ + noFollowUnavailable: false, +})); + +vi.mock("node:fs", async (importOriginal) => { + const original = await importOriginal(); + return { + ...original, + constants: { + ...original.constants, + get O_NOFOLLOW(): number | undefined { + return fsControl.noFollowUnavailable ? undefined : original.constants.O_NOFOLLOW; + }, + }, + }; +}); import { isConfigValue, @@ -385,6 +402,35 @@ describe("sanitizeEnvFile", () => { }); }); +describe("credential filter no-follow boundary", () => { + it("fails closed without atomic no-follow support", () => { + const root = mkdtempSync(join(tmpdir(), "nemoclaw-credential-filter-failure-")); + const jsonPath = join(root, "openclaw.json"); + const yamlPath = join(root, "config.yaml"); + const envPath = join(root, ".env"); + const jsonSource = JSON.stringify({ apiKey: "sk-secret-value" }); + const yamlSource = "api_key: sk-secret-value\n"; + const envSource = "API_KEY=sk-secret-value\n"; + + try { + writeFileSync(jsonPath, jsonSource); + writeFileSync(yamlPath, yamlSource); + writeFileSync(envPath, envSource); + fsControl.noFollowUnavailable = true; + + expect(sanitizeConfigFile(jsonPath)).toBe(false); + expect(sanitizeYamlConfigFile(yamlPath)).toBe(false); + expect(sanitizeEnvFile(envPath)).toBe(false); + expect(readFileSync(jsonPath, "utf-8")).toBe(jsonSource); + expect(readFileSync(yamlPath, "utf-8")).toBe(yamlSource); + expect(readFileSync(envPath, "utf-8")).toBe(envSource); + } finally { + fsControl.noFollowUnavailable = false; + rmSync(root, { recursive: true, force: true }); + } + }); +}); + describe("isSensitiveFile", () => { it("detects credential-bearing auth state basenames", () => { expect(isSensitiveFile("auth-profiles.json")).toBe(true); diff --git a/test/managed-image-protected-runtime-contract.test.ts b/test/managed-image-protected-runtime-contract.test.ts index eaa09af68dd..5f1e4450feb 100644 --- a/test/managed-image-protected-runtime-contract.test.ts +++ b/test/managed-image-protected-runtime-contract.test.ts @@ -6,7 +6,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { managedStartupE2eProfile } from "../scripts/checks/generate-managed-startup-profile-fixture.mts"; import { @@ -20,6 +20,7 @@ import { import { assertExactSandboxImage, assertFailedBootstrapOwnerCleanupRetention, + assertFailedSandboxOwnerCleanupRetention, createProtectedManagedImageBootstrapInput, failureInjectingAdapter, MANAGED_IMAGE_OPENSHELL_SUPERVISOR_ARGV, @@ -283,6 +284,44 @@ describe("protected managed-image runtime contract", () => { }, ); + it.each([ + ["exact", `NAME STATUS\n${VALID_SANDBOX} Ready\n`, false], + ["containing", `NAME STATUS\n${VALID_SANDBOX}-other Ready\n`, true], + ] as const)("%s-match checks the retained OpenShell sandbox name exactly", (_case, output, rejects) => { + const expectedSandboxId = "sandbox-id-123"; + const input = parseManagedImageOpenShellE2eInputs([ + "--agent", + "openclaw", + "--image", + IMAGE, + "--sandbox", + VALID_SANDBOX, + ]); + const runOpenshell = vi.fn((argv: readonly string[]) => + argv[1] === "get" + ? { status: 0, stdout: `Id: ${expectedSandboxId}\n`, stderr: "" } + : { status: 0, stdout: output, stderr: `diagnostic mentions ${VALID_SANDBOX}` }, + ); + const assertion = () => + assertFailedSandboxOwnerCleanupRetention( + { runOpenshell } as never, + input, + expectedSandboxId, + {}, + ); + + if (rejects) { + expect(assertion).toThrow("exact OpenShell owner-cleanup state"); + } else { + expect(assertion).not.toThrow(); + } + expect(runOpenshell).toHaveBeenNthCalledWith( + 2, + ["sandbox", "list"], + expect.objectContaining({ ignoreError: true }), + ); + }); + it("assigns every protected agent and route a unique OpenShell-compatible sandbox name (#8497)", () => { const routeKinds = [...MANAGED_IMAGE_LOCAL_INFERENCE_KINDS, "rollback"] as const; const qualifications = PROTECTED_MANAGED_IMAGE_AGENTS.flatMap((agent) => From 56b2313a26718ef73a47d85628f2e93b85a6968b Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Sun, 16 Aug 2026 16:15:26 -0700 Subject: [PATCH 3/4] test(e2e): keep cleanup proof linear Signed-off-by: Prekshi Vyas --- ...d-image-protected-runtime-contract.test.ts | 58 ++++++++++++++----- 1 file changed, 45 insertions(+), 13 deletions(-) diff --git a/test/managed-image-protected-runtime-contract.test.ts b/test/managed-image-protected-runtime-contract.test.ts index 5f1e4450feb..f21c658a154 100644 --- a/test/managed-image-protected-runtime-contract.test.ts +++ b/test/managed-image-protected-runtime-contract.test.ts @@ -284,10 +284,7 @@ describe("protected managed-image runtime contract", () => { }, ); - it.each([ - ["exact", `NAME STATUS\n${VALID_SANDBOX} Ready\n`, false], - ["containing", `NAME STATUS\n${VALID_SANDBOX}-other Ready\n`, true], - ] as const)("%s-match checks the retained OpenShell sandbox name exactly", (_case, output, rejects) => { + it("accepts an exact retained OpenShell sandbox name", () => { const expectedSandboxId = "sandbox-id-123"; const input = parseManagedImageOpenShellE2eInputs([ "--agent", @@ -297,10 +294,49 @@ describe("protected managed-image runtime contract", () => { "--sandbox", VALID_SANDBOX, ]); - const runOpenshell = vi.fn((argv: readonly string[]) => - argv[1] === "get" - ? { status: 0, stdout: `Id: ${expectedSandboxId}\n`, stderr: "" } - : { status: 0, stdout: output, stderr: `diagnostic mentions ${VALID_SANDBOX}` }, + const responses = new Map([ + ["get", { status: 0, stdout: `Id: ${expectedSandboxId}\n`, stderr: "" }], + ["list", { status: 0, stdout: `NAME STATUS\n${VALID_SANDBOX} Ready\n`, stderr: "" }], + ]); + const runOpenshell = vi.fn( + (argv: readonly string[]) => + responses.get(argv[1] ?? "") ?? { status: 1, stdout: "", stderr: "unexpected command" }, + ); + + expect(() => + assertFailedSandboxOwnerCleanupRetention( + { runOpenshell } as never, + input, + expectedSandboxId, + {}, + ), + ).not.toThrow(); + }); + + it("rejects a containing sandbox name and an exact name mentioned only in stderr", () => { + const expectedSandboxId = "sandbox-id-123"; + const input = parseManagedImageOpenShellE2eInputs([ + "--agent", + "openclaw", + "--image", + IMAGE, + "--sandbox", + VALID_SANDBOX, + ]); + const responses = new Map([ + ["get", { status: 0, stdout: `Id: ${expectedSandboxId}\n`, stderr: "" }], + [ + "list", + { + status: 0, + stdout: `NAME STATUS\n${VALID_SANDBOX}-other Ready\n`, + stderr: `diagnostic mentions ${VALID_SANDBOX}`, + }, + ], + ]); + const runOpenshell = vi.fn( + (argv: readonly string[]) => + responses.get(argv[1] ?? "") ?? { status: 1, stdout: "", stderr: "unexpected command" }, ); const assertion = () => assertFailedSandboxOwnerCleanupRetention( @@ -310,11 +346,7 @@ describe("protected managed-image runtime contract", () => { {}, ); - if (rejects) { - expect(assertion).toThrow("exact OpenShell owner-cleanup state"); - } else { - expect(assertion).not.toThrow(); - } + expect(assertion).toThrow("exact OpenShell owner-cleanup state"); expect(runOpenshell).toHaveBeenNthCalledWith( 2, ["sandbox", "list"], From 07ae0011aca92750fafb2777e7c619529d60414e Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Sun, 16 Aug 2026 16:39:34 -0700 Subject: [PATCH 4/4] test(inference): assert exact rollback cleanup Signed-off-by: Prekshi Vyas --- .../inference-set-compatible-provider.test.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/lib/actions/inference-set-compatible-provider.test.ts b/src/lib/actions/inference-set-compatible-provider.test.ts index 1ecdcff1e36..d15693c750c 100644 --- a/src/lib/actions/inference-set-compatible-provider.test.ts +++ b/src/lib/actions/inference-set-compatible-provider.test.ts @@ -807,10 +807,18 @@ describe("runInferenceSet compatible providers", () => { ], ]); expect( - captureOpenshell.mock.calls.some( - ([args]) => args[0] === "provider" && args[1] === "delete", - ), - ).toBe(true); + captureOpenshell.mock.calls + .filter(([args]) => args[0] === "provider" && args[1] === "delete") + .map(([args]) => args), + ).toEqual([ + [ + "provider", + "delete", + "-g", + "nemoclaw", + "compatible-anthropic-endpoint", + ], + ]); expect(deps.calls.updateSandbox).not.toHaveBeenCalled(); expect(deps.calls.writeSandboxConfig).not.toHaveBeenCalled(); });