diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index d4d2fbed1c3..656052fa25c 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -452,18 +452,32 @@ For a new or fresh session, `--yes` and `NEMOCLAW_YES=1` accept supported confir If onboarding returns without reaching the final `complete` state, the command exits with status `1`. When that result is resumable, NemoClaw keeps the session `in_progress` at its last checkpoint instead of marking it failed, so correct the reported condition and run `$$nemoclaw onboard --resume`. -If onboarding cannot complete after sandbox creation, NemoClaw preserves the sandbox and records its create-attempt label. +#### Recover a retained sandbox + +If onboarding cannot complete after sandbox creation, NemoClaw preserves the sandbox. +When available, NemoClaw records and prints the create-attempt label as the exact `ai.nvidia.nemoclaw.create-attempt=` selector. When available, NemoClaw also records a durable identity fingerprint and verified policy evidence for recovery. Automatic and explicit resume, reuse, recreation, and fresh onboarding with that sandbox name remain blocked. -Do not destroy the retained sandbox by name. -Ask an OpenShell administrator to verify the exact live durable ID before using an identity-bound removal procedure. -NemoClaw does not provide an operation to clear the recovery record, even after external removal. -Start another onboarding run with a different name: +When the recovery record contains a durable identity fingerprint, run `$$nemoclaw destroy` to attempt identity-bound recovery. + +Use the result from `destroy` to choose the next action: + +- If OpenShell still reports the sandbox present, `destroy` preserves the record and removes no resources. Do not delete the sandbox manually by mutable name. Give the displayed create-attempt label to an OpenShell administrator when present and ask them to remove that exact sandbox through an identity-bound procedure. Without a label, preserve the terminal output and ask the administrator to identify the exact sandbox from gateway or controller evidence. +- After the administrator removes the exact sandbox, rerun `$$nemoclaw destroy`. +- If OpenShell confirms the sandbox is absent, `destroy` verifies one retained recovery record and its immutable runtime identity. For Docker-backed sandboxes, it also verifies the immutable Docker sandbox identity and removes only qualified residual containers. It clears the matching recovery record only after verified cleanup. +- If the recovery record is ambiguous or immutable runtime-identity verification fails, cleanup stops and the record remains. For Docker-backed sandboxes, a foreign container, changed Docker identity, or failed Docker probe has the same fail-closed result. + +If OpenShell did not return a durable identity fingerprint, `destroy` cannot complete recovery. A create-attempt label can help an OpenShell administrator identify and remove the exact sandbox, but it does not let NemoClaw retire the record without immutable identity authority. If neither a fingerprint nor a label is available, preserve the terminal output and ask the administrator to identify the exact sandbox from gateway or controller evidence. If NemoClaw reports that it could not save recovery evidence, preserve the terminal output and follow the same escalation. + +This fail-closed record keeps only the affected sandbox name unavailable. It is not retired from mutable-name absence alone, and this command does not accept administrator-supplied identity authority. +To onboard another sandbox while the record remains unresolved, supply a different explicit name: ```bash -$$nemoclaw onboard --fresh --name +$$nemoclaw onboard --name ``` +`--fresh` alone does not clear the recovery record or permit reuse of the retained sandbox name. + OpenClaw sessions also record the web search selection, messaging selection and non-secret settings, and resource profile. @@ -530,12 +544,9 @@ $$nemoclaw onboard --fresh --apf-interceptor --name my-apf-sandbox If post-create verification or native GPU fallback fails after OpenShell may have created the sandbox, NemoClaw preserves the incomplete sandbox because automatic deletion would use its mutable name. -Do not destroy that sandbox by name. -Retain the reported sandbox name, create-attempt label, and durable identity fingerprint for comparison only. -If OpenShell did not return the fingerprint, recovery remains blocked until an administrator resolves the create-attempt label to one exact sandbox. -Ask an OpenShell administrator to obtain the exact live durable ID, verify it against the fingerprint, and use an identity-bound removal procedure. +Follow the [retained-sandbox recovery procedure](#recover-a-retained-sandbox); its OpenShell-absence, immutable runtime-identity, and administrator requirements also apply to APF creation. For Docker-backed APF creation, its Docker container-identity requirement also applies. This onboarding mode does not support `--resume` or `--recreate-sandbox`, regardless of whether sandbox creation began. -After the administrator confirms identity-bound removal, repeat the original command with `--fresh` and a new name. +After destroy completes, repeat the original command with `--fresh`. @@ -938,18 +949,8 @@ Pairing and `TELEGRAM_ALLOWED_IDS` still govern direct messages. If you cancel a brand-new onboarding run at the policy-tier selector or either policy-preset selector after sandbox creation, NemoClaw preserves the incomplete sandbox, registry entry, and onboarding session for identity-bound recovery. NemoClaw reports the durable sandbox identity fingerprint when it is available. It does not run OpenShell's mutable-name deletion command because the name may now identify a replacement sandbox. -Do not delete the sandbox by mutable name. -Shared inference providers are gateway configuration, not sandbox cleanup targets. -Sandbox-scoped provider registrations or gateway-bound credentials may remain when the durable recovery record lists them. -Ask an OpenShell administrator to inspect the exact sandbox identity and remove only sandbox-scoped resources whose ownership is confirmed for the retained sandbox. -A credential environment name in the recovery record does not prove that its value was exposed. -Rotate a credential only when identity-bound inspection proves that it was exposed or attached to a retained sandbox-scoped resource. -NemoClaw stores the recovery record independently from the active onboarding session. -A fresh run with a different name can proceed without clearing that record, but automatic resume, explicit `--resume`, reuse, recreation, and fresh onboarding with the retained name remain blocked. -NemoClaw has no supported operation in this release to clear the recovery record, so the retained name remains unavailable even after external recovery or removal. -Preserve the record as evidence. -Start fresh onboarding with `$$nemoclaw onboard --fresh --name `. -Select the required provider, model, agent, policy, and environment inputs again because `--fresh` does not retain them. +Follow the [retained-sandbox recovery procedure](#recover-a-retained-sandbox) to reconcile this cancellation. +A fresh run with a different explicit name can continue while the cancelled sandbox name remains blocked. If you run onboarding again with the same sandbox name and choose a different inference provider or model, NemoClaw detects the drift and recreates the sandbox so the running agent config matches your selection. In interactive mode, the wizard asks for confirmation before delete and recreate. @@ -2659,7 +2660,8 @@ For one matching container, the command continues only when all these labels hav - A nonempty `openshell.ai/sandbox-workspace` - A nonempty `openshell.ai/sandbox-id` -If the initial inspection cannot complete, more than one container matches, a matching container has conflicting or incomplete labels, or Docker returns malformed identity data, `destroy` exits before changing sandbox resources. +In an ordinary destroy flow, if the initial inspection cannot complete, more than one container matches, a matching container has conflicting or incomplete labels, or Docker returns malformed identity data, `destroy` exits before changing sandbox resources. +Retained-sandbox recovery accepts multiple managed containers only when every immutable sandbox ID matches the retained recovery fingerprint. The identity checks still apply with `--force`, `--yes`, or `NEMOCLAW_NON_INTERACTIVE=1`; those controls authorize confirmation but do not authorize an unproven container identity. NemoClaw rechecks the identity after read-only preflight, before provider cleanup, and synchronously at the sandbox-deletion boundary. If a later recheck detects drift or fails, `destroy` refuses sandbox deletion, restores managed MCP preparation when possible, preserves local ownership state, and reports any earlier cleanup already performed. diff --git a/src/lib/actions/sandbox/destroy-container-identity.test.ts b/src/lib/actions/sandbox/destroy-container-identity.test.ts index a63f1f61fbf..a0d3b524557 100644 --- a/src/lib/actions/sandbox/destroy-container-identity.test.ts +++ b/src/lib/actions/sandbox/destroy-container-identity.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it, vi } from "vitest"; +import { fingerprintOpenShellSandboxId } from "../../adapters/openshell/sandbox-identity"; import { classifyDestroyContainerIdentity, type DestroyContainerIdentityVerdict, @@ -126,6 +127,35 @@ describe("classifyDestroyContainerIdentity", () => { expect(verdict.reason).toContain("2 managed containers"); }); + it("accepts every managed container bound to one retained sandbox identity (#10547)", () => { + const sandboxIdentityFingerprint = fingerprintOpenShellSandboxId(MANAGED.sandboxId)!; + const identities = [MANAGED, { ...MANAGED, id: "dddd000000000000" }]; + + expect( + classifyDestroyContainerIdentity( + "destroytest", + observeRows(identities), + sandboxIdentityFingerprint, + ), + ).toEqual({ status: "recovery", identities }); + }); + + it("refuses a retained recovery set that contains another sandbox identity (#10547)", () => { + const sandboxIdentityFingerprint = fingerprintOpenShellSandboxId(MANAGED.sandboxId)!; + const verdict = expectAmbiguous( + classifyDestroyContainerIdentity( + "destroytest", + observeRows([ + MANAGED, + { ...MANAGED, id: "dddd000000000000", sandboxId: "sb-replacement" }, + ]), + sandboxIdentityFingerprint, + ), + ); + + expect(verdict.reason).toContain("retained sandbox identity"); + }); + it.each([ ["workspace", { ...MANAGED, workspace: "" }], ["sandbox ID", { ...MANAGED, sandboxId: "" }], diff --git a/src/lib/actions/sandbox/destroy-execution.ts b/src/lib/actions/sandbox/destroy-execution.ts index a54be5f7bd6..70a4389f93b 100644 --- a/src/lib/actions/sandbox/destroy-execution.ts +++ b/src/lib/actions/sandbox/destroy-execution.ts @@ -33,11 +33,12 @@ import { readTimerMarker } from "../../shields/timer-control"; import type { SandboxEntry } from "../../state/registry"; import { classifyDestroyContainerIdentity, - isSameDestroyContainerIdentity, + isSameDestroyContainerIdentityProof, observeDestroyContainerIdentity, - removeExactDestroyContainerIdentity, + type DestroyContainerIdentityProof, type SandboxNameLabeledContainer, } from "./destroy-presence"; +import { removeExactOpenShellDockerSandboxContainers } from "../../onboard/openshell-docker-sandbox-containers"; import { type DestroyRunOpenshell, SANDBOX_DESTROY_TIMEOUT_MS } from "./destroy-gateway"; import { finalizeMcpBridgesAfterSandboxDelete, @@ -69,10 +70,11 @@ type SandboxDestroyExecutionInput = { sandbox: SandboxEntry | null; sandboxConfirmedAbsent: boolean; sandboxName: string; - // `undefined` delegates identity gating to the runtime provider. - // `null` records confirmed absence; an object records the one managed - // container observed by the pre-destroy guard. - expectedContainerIdentity?: SandboxNameLabeledContainer | null; + // `undefined` delegates identity gating to the runtime provider. An empty + // array records confirmed absence; other arrays contain the immutable + // Docker IDs qualified before destroy preparation. + expectedContainerIdentities?: readonly SandboxNameLabeledContainer[]; + expectedContainerIdentityFingerprint?: string; portableContainerAuthority?: PreparedPortableDemoSandboxDestroyAuthority; stopInferenceResources: () => void; runtimeProviders?: RuntimeProviderBundleRegistry; @@ -299,7 +301,8 @@ export async function executeSandboxDestroy({ sandbox, sandboxConfirmedAbsent, sandboxName, - expectedContainerIdentity, + expectedContainerIdentities, + expectedContainerIdentityFingerprint, portableContainerAuthority, stopInferenceResources, runtimeProviders = CURRENT_RUNTIME_PROVIDER_BUNDLES, @@ -312,6 +315,17 @@ export async function executeSandboxDestroy({ | { status: "ambiguous"; detail: string; subject?: string } | { status: "probe-failed"; detail: string; subject?: string }; const pendingPolicyVerification = sandbox?.pendingPolicyVerification; + const expectedContainerProof: DestroyContainerIdentityProof = + expectedContainerIdentities === undefined ? {} : { identities: expectedContainerIdentities }; + const proofFromVerdict = ( + verdict: ReturnType, + ): DestroyContainerIdentityProof | null => { + if (verdict.status === "clear") { + return { identities: verdict.identity === null ? [] : [verdict.identity] }; + } + if (verdict.status === "recovery") return { identities: verdict.identities }; + return null; + }; const inspectPendingPolicyVerificationContinuity = (): IdentityContinuity => { if (!pendingPolicyVerification) return { status: "match" }; if (!getSandbox) { @@ -326,6 +340,16 @@ export async function executeSandboxDestroy({ if (!isDeepStrictEqual(readCurrentCheckpoint(), pendingPolicyVerification)) { return { status: "changed", subject: "Pending policy verification authority" }; } + if ( + sandboxConfirmedAbsent && + expectedContainerIdentities !== undefined && + expectedContainerIdentityFingerprint === + pendingPolicyVerification.sandboxIdentityFingerprint + ) { + return isDeepStrictEqual(readCurrentCheckpoint(), pendingPolicyVerification) + ? { status: "match" } + : { status: "changed", subject: "Pending policy verification authority" }; + } const inspectIdentity = deps.inspectOpenShellSandboxIdentityFingerprint ?? inspectOpenShellSandboxIdentityFingerprint; @@ -362,12 +386,17 @@ export async function executeSandboxDestroy({ return { status: "probe-failed", detail: redactDestroyError(error) }; } } - if (expectedContainerIdentity === undefined) return { status: "match" }; + if (expectedContainerIdentities === undefined) return { status: "match" }; const verdict = classifyDestroyContainerIdentity( sandboxName, observeDestroyContainerIdentity(sandboxName), + expectedContainerIdentityFingerprint, ); - if (isSameDestroyContainerIdentity(expectedContainerIdentity, verdict)) { + const actualContainerProof = proofFromVerdict(verdict); + if ( + actualContainerProof && + isSameDestroyContainerIdentityProof(expectedContainerProof, actualContainerProof) + ) { return { status: "match" }; } if (verdict.status === "probe-failed") { @@ -510,14 +539,14 @@ export async function executeSandboxDestroy({ " Managed inference cleanup may already be partial; inspect or restart its resources before retrying.", ); } - // `expectedContainerIdentity === null` is a completed Docker identity + // An empty `expectedContainerIdentities` is a completed Docker identity // probe with zero matching containers. `undefined` means this runtime // does not use that probe (or Portable owns identity); skip hardening // only when OpenShell already proved absence. A live labeled Docker // identity still hardens even if the OpenShell list says absent. const sandboxRuntimeConfirmedAbsent = - expectedContainerIdentity === null || - (expectedContainerIdentity === undefined && sandboxConfirmedAbsent); + expectedContainerIdentities?.length === 0 || + (expectedContainerIdentities === undefined && sandboxConfirmedAbsent); let hardened: HardenedDeleteState; try { hardened = wipeAndHardenLiveSandbox( @@ -585,18 +614,24 @@ export async function executeSandboxDestroy({ const deleteArgs = pendingPolicyVerification ? ["sandbox", "delete", "-g", pendingPolicyVerification.gatewayName, sandboxName] : ["sandbox", "delete", sandboxName]; - const deleteResult = runOpenshell(deleteArgs, { - ignoreError: true, - killSignal: "SIGKILL", - stdio: ["ignore", "pipe", "pipe"], - timeout: SANDBOX_DESTROY_TIMEOUT_MS, - }); + // A successful preflight absence is already the required OpenShell + // lifecycle proof. Do not issue a later mutable-name delete that could + // target a same-name replacement created after that observation. + const deleteResult: ReturnType = sandboxConfirmedAbsent + ? { status: 0, stdout: "", stderr: "" } + : runOpenshell(deleteArgs, { + ignoreError: true, + killSignal: "SIGKILL", + stdio: ["ignore", "pipe", "pipe"], + timeout: SANDBOX_DESTROY_TIMEOUT_MS, + }); const { output: capturedDeleteOutput, - alreadyGone, + alreadyGone: deleteReportedAlreadyGone, gatewayUnreachable, timedOut, } = getSandboxDeleteOutcome(deleteResult); + const alreadyGone = sandboxConfirmedAbsent || deleteReportedAlreadyGone; const deleteOutput = timedOut ? `OpenShell sandbox delete timed out after ${String(SANDBOX_DESTROY_TIMEOUT_MS / 1000)} seconds. Deletion could not be confirmed.` : capturedDeleteOutput; @@ -638,12 +673,19 @@ export async function executeSandboxDestroy({ }; } - if (!forcedLocalCleanup && (portableContainerAuthority || expectedContainerIdentity)) { + if ( + !forcedLocalCleanup && + (portableContainerAuthority || expectedContainerIdentities !== undefined) + ) { try { if (portableContainerAuthority) { portableContainerAuthority.verifyAbsent(); - } else if (expectedContainerIdentity) { - removeExactDestroyContainerIdentity(sandboxName, expectedContainerIdentity, console.log); + } else if (expectedContainerIdentities !== undefined) { + removeExactOpenShellDockerSandboxContainers( + sandboxName, + expectedContainerIdentities.map(({ id }) => id), + console.log, + ); } } catch (error) { const detail = redactDestroyError(error); diff --git a/src/lib/actions/sandbox/destroy-flow.test.ts b/src/lib/actions/sandbox/destroy-flow.test.ts index 70698178f7a..fc94c1bb2d9 100644 --- a/src/lib/actions/sandbox/destroy-flow.test.ts +++ b/src/lib/actions/sandbox/destroy-flow.test.ts @@ -121,6 +121,7 @@ describe("destroySandbox flow", () => { switch (`${String(argv[0])}:${String(argv[1])}`) { case "sandbox:delete": trace.push("delete"); + harness.setSandboxPresent(false); return { status: 0, stdout: "", stderr: "" }; case "sandbox:list": trace.push("list"); @@ -177,6 +178,7 @@ describe("destroySandbox flow", () => { switch (`${String(argv[0])}:${String(argv[1])}`) { case "sandbox:delete": crossedDeleteBoundary = true; + harness.setSandboxPresent(false); return { status: 0, stdout: "", stderr: "" }; case "sandbox:list": return { @@ -992,7 +994,13 @@ describe("destroySandbox flow", () => { await expect(harness.destroySandbox("alpha", { yes: true })).resolves.toBeUndefined(); - expect(trace.slice(-2)).toEqual([`probe:${String(identityProbeCalls)}`, "delete"]); + const deleteIndex = trace.indexOf("delete"); + expect(deleteIndex).toBeGreaterThan(0); + expect(trace[deleteIndex - 1]).toMatch(/^probe:/u); + expect(trace.slice(deleteIndex + 1)).toEqual([ + `probe:${String(identityProbeCalls - 1)}`, + `probe:${String(identityProbeCalls)}`, + ]); }); it("preserves provider and registry ownership when runtime authority is unknown", async () => { diff --git a/src/lib/actions/sandbox/destroy-preflight.ts b/src/lib/actions/sandbox/destroy-preflight.ts index 496edb895b1..ce24ed460ba 100644 --- a/src/lib/actions/sandbox/destroy-preflight.ts +++ b/src/lib/actions/sandbox/destroy-preflight.ts @@ -23,7 +23,10 @@ import type { SandboxEntry } from "../../state/registry"; import * as registry from "../../state/registry"; import { type DestroyRunOpenshell, selectGatewayForSandboxDestroy } from "./destroy-gateway"; import { classifyDestroySandboxPresence } from "./destroy-presence"; -import { getSandboxTargetGatewayName } from "./gateway-target"; +import { + getPersistedSandboxTargetGatewayName, + getSandboxTargetGatewayName, +} from "./gateway-target"; import { assertMcpAdapterConfigMutationsAllowed } from "./mcp-bridge-runtime-capabilities"; export type SandboxDestroyPreflight = { @@ -260,7 +263,10 @@ export async function stopModelRouterForDestroyedSandbox( return true; } -export function prepareSandboxDestroy(sandboxName: string): SandboxDestroyPreflight { +export function prepareSandboxDestroy( + sandboxName: string, + retainedRecoveryGatewayName?: string, +): SandboxDestroyPreflight { const sandbox = registry.getSandbox(sandboxName); console.log(` Deleting sandbox '${sandboxName}'...`); const { runOpenshell } = require("../../adapters/openshell/runtime") as { @@ -268,8 +274,21 @@ export function prepareSandboxDestroy(sandboxName: string): SandboxDestroyPrefli }; // Capture the sandbox gateway before destructive work, then pin every - // following OpenShell subprocess against that same registry-owned gateway. - const cleanupGatewayName = getSandboxTargetGatewayName(sandboxName); + // following OpenShell subprocess against that same durable authority. A + // retained recovery record remains authoritative after a partial destroy + // has already retired the registry row. + const registeredGatewayName = sandbox ? getPersistedSandboxTargetGatewayName(sandbox) : null; + if ( + retainedRecoveryGatewayName && + registeredGatewayName && + retainedRecoveryGatewayName !== registeredGatewayName + ) { + throw new Error( + `Refusing to destroy sandbox '${sandboxName}': retained recovery gateway '${retainedRecoveryGatewayName}' does not match registered gateway '${registeredGatewayName}'.`, + ); + } + const cleanupGatewayName = + retainedRecoveryGatewayName ?? registeredGatewayName ?? getSandboxTargetGatewayName(); selectGatewayForSandboxDestroy(sandboxName, cleanupGatewayName, runOpenshell); process.env.OPENSHELL_GATEWAY = cleanupGatewayName; diff --git a/src/lib/actions/sandbox/destroy-presence.ts b/src/lib/actions/sandbox/destroy-presence.ts index 0077feb122d..78ef7b10175 100644 --- a/src/lib/actions/sandbox/destroy-presence.ts +++ b/src/lib/actions/sandbox/destroy-presence.ts @@ -6,21 +6,19 @@ import { OPENSHELL_MANAGED_BY_VALUE, OPENSHELL_SANDBOX_ID_LABEL, OPENSHELL_SANDBOX_NAME_LABEL, - removeExactOpenShellDockerSandboxContainer, + OPENSHELL_SANDBOX_WORKSPACE_LABEL, + inspectDockerSandboxNameLabeledContainers, } from "../../onboard/openshell-docker-sandbox-containers"; +import { fingerprintOpenShellSandboxId } from "../../adapters/openshell/sandbox-identity"; import { sanitizeReadinessText } from "../../readiness/sanitize"; import { type DockerSandboxIdentityObservation, - inspectDockerSandboxIdentities, } from "../../adapters/docker/inspect"; import { classifyOpenShellSandboxPresence, type OpenShellSandboxPresence, } from "../../adapters/openshell/sandbox-presence"; -/** Workspace label OpenShell stamps on every managed sandbox container. */ -export const OPENSHELL_SANDBOX_WORKSPACE_LABEL = "openshell.ai/sandbox-workspace"; - const IDENTITY_VALUE_MAX_LENGTH = 256; const IDENTITY_DIAGNOSTIC_MAX_LENGTH = 500; @@ -35,6 +33,7 @@ export type SandboxNameLabeledContainer = { /** Verdict for whether destroy resolved one complete managed container identity. */ export type DestroyContainerIdentityVerdict = | { status: "clear"; identity: SandboxNameLabeledContainer | null } + | { status: "recovery"; identities: SandboxNameLabeledContainer[] } | { status: "probe-failed"; detail: string } | { status: "ambiguous"; @@ -47,37 +46,27 @@ export type DestroyContainerIdentityVerdict = export type AssertUnambiguousDestroyIdentityDeps = { providerId: string; redact: (detail: string) => string; + retainedSandboxIdentityFingerprint?: string; cliName?: string; - classify?: (sandboxName: string) => DestroyContainerIdentityVerdict; + classify?: ( + sandboxName: string, + retainedSandboxIdentityFingerprint?: string, + ) => DestroyContainerIdentityVerdict; error?: (message: string) => void; }; export type DestroyContainerIdentityProof = { - identity: SandboxNameLabeledContainer | null | undefined; + // `undefined` delegates identity gating to the runtime provider. An empty + // array records confirmed Docker absence; other arrays contain the exact + // immutable Docker identities qualified for this destroy operation. + identities?: readonly SandboxNameLabeledContainer[]; }; -function observeDockerSandboxIdentities(sandboxName: string): DockerSandboxIdentityObservation { - return inspectDockerSandboxIdentities(`${OPENSHELL_SANDBOX_NAME_LABEL}=${sandboxName}`, { - managedBy: OPENSHELL_MANAGED_BY_LABEL, - workspace: OPENSHELL_SANDBOX_WORKSPACE_LABEL, - sandboxId: OPENSHELL_SANDBOX_ID_LABEL, - }); -} - /** Read the host observation consumed by the pure identity classifier. */ export function observeDestroyContainerIdentity( sandboxName: string, ): DockerSandboxIdentityObservation { - return observeDockerSandboxIdentities(sandboxName); -} - -/** Retire only the provider runtime bound to the pre-destroy identity proof. */ -export function removeExactDestroyContainerIdentity( - sandboxName: string, - expectedIdentity: SandboxNameLabeledContainer, - log: (message: string) => void, -): void { - removeExactOpenShellDockerSandboxContainer(sandboxName, expectedIdentity.id, log); + return inspectDockerSandboxNameLabeledContainers(sandboxName); } /** @@ -88,6 +77,7 @@ export function removeExactDestroyContainerIdentity( export function classifyDestroyContainerIdentity( sandboxName: string, observation: DockerSandboxIdentityObservation, + retainedSandboxIdentityFingerprint?: string, ): DestroyContainerIdentityVerdict { if (observation.status === "probe-failed") { return { @@ -124,6 +114,30 @@ export function classifyDestroyContainerIdentity( managed, }; } + if (managed.length > 0 && retainedSandboxIdentityFingerprint !== undefined) { + const identityMatches = managed.every( + (row) => + row.workspace.length > 0 && + row.sandboxId.length > 0 && + fingerprintOpenShellSandboxId(row.sandboxId) === retainedSandboxIdentityFingerprint, + ); + const oneWorkspace = new Set(managed.map((row) => row.workspace)).size === 1; + if (!identityMatches || !oneWorkspace) { + return { + status: "ambiguous", + sandboxName, + reason: "one or more managed containers do not match the retained sandbox identity", + foreign, + managed, + }; + } + if (managed.length > 1) { + return { + status: "recovery", + identities: [...managed].sort((left, right) => left.id.localeCompare(right.id)), + }; + } + } if (managed.length !== 1) { return { status: "ambiguous", @@ -153,21 +167,6 @@ export function classifyDestroyContainerIdentity( return { status: "clear", identity }; } -/** Require the same immutable container row, including the already-absent state. */ -export function isSameDestroyContainerIdentity( - expected: SandboxNameLabeledContainer | null, - verdict: DestroyContainerIdentityVerdict, -): boolean { - if (verdict.status !== "clear") return false; - if (expected === null || verdict.identity === null) return expected === verdict.identity; - return ( - expected.id === verdict.identity.id && - expected.managedBy === verdict.identity.managedBy && - expected.workspace === verdict.identity.workspace && - expected.sandboxId === verdict.identity.sandboxId - ); -} - /** Human-readable lines describing an ambiguous-identity refusal. */ export function formatAmbiguousDestroyIdentity( verdict: Extract, @@ -208,12 +207,18 @@ export function assertUnambiguousDestroyContainerIdentity( ): DestroyContainerIdentityProof | false { const classify = deps.classify ?? - ((name: string) => - classifyDestroyContainerIdentity(name, observeDestroyContainerIdentity(name))); + ((name: string, retainedSandboxIdentityFingerprint?: string) => + classifyDestroyContainerIdentity( + name, + observeDestroyContainerIdentity(name), + retainedSandboxIdentityFingerprint, + )); const error = deps.error ?? ((message: string) => console.error(` ${message}`)); - if (deps.providerId !== "docker") return { identity: undefined }; + if (deps.providerId !== "docker") return {}; - const verdict = classify(sandboxName); + const verdict = deps.retainedSandboxIdentityFingerprint + ? classify(sandboxName, deps.retainedSandboxIdentityFingerprint) + : classify(sandboxName); if (verdict.status === "ambiguous") { for (const line of formatAmbiguousDestroyIdentity(verdict, deps.cliName ?? "nemoclaw")) { error(line); @@ -228,7 +233,14 @@ export function assertUnambiguousDestroyContainerIdentity( ); return false; } - return { identity: verdict.identity }; + return { + identities: + verdict.status === "recovery" + ? verdict.identities + : verdict.identity === null + ? [] + : [verdict.identity], + }; } /** Compare provider-owned identity proofs across two destroy checkpoints. */ @@ -236,12 +248,21 @@ export function isSameDestroyContainerIdentityProof( expected: DestroyContainerIdentityProof, actual: DestroyContainerIdentityProof, ): boolean { - if (expected.identity === undefined || actual.identity === undefined) { - return expected.identity === actual.identity; + const expectedIdentities = expected.identities; + const actualIdentities = actual.identities; + if (expectedIdentities === undefined || actualIdentities === undefined) { + return expectedIdentities === actualIdentities; } - return isSameDestroyContainerIdentity(expected.identity, { - status: "clear", - identity: actual.identity, + if (expectedIdentities.length !== actualIdentities.length) return false; + return expectedIdentities.every((identity, index) => { + const candidate = actualIdentities[index]; + return ( + candidate !== undefined && + identity.id === candidate.id && + identity.managedBy === candidate.managedBy && + identity.workspace === candidate.workspace && + identity.sandboxId === candidate.sandboxId + ); }); } diff --git a/src/lib/actions/sandbox/destroy-retained-recovery-flow.test.ts b/src/lib/actions/sandbox/destroy-retained-recovery-flow.test.ts new file mode 100644 index 00000000000..ac6345c20fb --- /dev/null +++ b/src/lib/actions/sandbox/destroy-retained-recovery-flow.test.ts @@ -0,0 +1,411 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { createHash } from "node:crypto"; + +import { afterEach, beforeEach, describe, expect, it, type MockInstance, vi } from "vitest"; + +import { + createDestroyHarness, + resetDestroyModuleCache, +} from "../../../../test/helpers/destroy-flow-test-harness"; +import type { RetainedSandboxRecoveryRecord } from "../../state/onboard-session/retained-sandbox-recovery"; + +function retainedRecoveryRecord(sandboxId = "sb-alpha"): RetainedSandboxRecoveryRecord { + return { + schemaVersion: 1, + recordId: "f".repeat(64), + sandboxName: "alpha", + sandboxIdentityFingerprint: createHash("sha256").update(sandboxId).digest("hex"), + gatewayName: "nemoclaw-19080", + gatewayPort: 19080, + lifecycleGeneration: "generation-alpha", + verifiedEffectivePolicyIdentity: null, + createAttemptNonce: "c".repeat(62), + policyCreationReceipt: null, + reason: "retained_after_sandbox_creation_failure", + recordedAt: "2026-08-28T00:00:00.000Z", + }; +} + +describe("destroySandbox retained recovery flow", () => { + let exitSpy: MockInstance; + let originalGatewayEnv: string | undefined; + + beforeEach(() => { + originalGatewayEnv = process.env.OPENSHELL_GATEWAY; + exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number | string | null) => { + throw new Error(`process.exit(${code ?? 0})`); + }) as never); + }); + + afterEach(() => { + originalGatewayEnv === undefined + ? delete process.env.OPENSHELL_GATEWAY + : (process.env.OPENSHELL_GATEWAY = originalGatewayEnv); + vi.restoreAllMocks(); + vi.unstubAllEnvs(); + resetDestroyModuleCache(); + }); + + it( + "removes every container after OpenShell confirms the retained sandbox absent (#10547)", + { timeout: 30_000 }, + async () => { + const recovery = retainedRecoveryRecord(); + const sandboxContainerId = "a".repeat(64); + const bootstrapContainerId = "b".repeat(64); + const identityRows = [sandboxContainerId, bootstrapContainerId] + .map((id) => `${id}\topenshell\tdefault\tsb-alpha`) + .join("\n"); + const harness = createDestroyHarness({ + sandboxPresent: false, + dockerOrphanIds: [bootstrapContainerId], + dockerRunResult: { status: 0, stdout: identityRows }, + registryEntryOverrides: { + lifecycleGeneration: recovery.lifecycleGeneration!, + lifecycleLiveIdentityFingerprint: recovery.sandboxIdentityFingerprint!, + }, + retainedRecoveryRecords: [recovery], + }); + + await expect(harness.destroySandbox("alpha", { yes: true })).resolves.toBeUndefined(); + + expect(harness.runOpenshellSpy).not.toHaveBeenCalledWith( + ["sandbox", "delete", "alpha"], + expect.anything(), + ); + expect(harness.dockerRunSpy).toHaveBeenCalledWith( + ["rm", "-f", bootstrapContainerId], + expect.objectContaining({ ignoreError: true }), + ); + expect(harness.resolveRetainedSandboxRecoverySpy).toHaveBeenCalledWith(recovery); + expect(harness.removeSandboxSpy).toHaveBeenCalledWith("alpha"); + expect(harness.sessionState.sandboxName).toBeNull(); + expect(exitSpy).not.toHaveBeenCalled(); + + const exactRemovalCallsAfterCleanup = harness.dockerRunSpy.mock.calls.filter( + ([args]) => Array.isArray(args) && args[0] === "rm" && args[1] === "-f", + ).length; + harness.setSandboxPresent(false); + harness.setRegistryEntryPresent(false); + harness.setRetainedRecoveryRecords([]); + harness.setDockerIdentityResult({ status: 0, stdout: "" }); + + await expect(harness.destroySandbox("alpha", { yes: true })).resolves.toBeUndefined(); + + expect( + harness.dockerRunSpy.mock.calls.filter( + ([args]) => Array.isArray(args) && args[0] === "rm" && args[1] === "-f", + ), + ).toHaveLength(exactRemovalCallsAfterCleanup); + expect(harness.resolveRetainedSandboxRecoverySpy).toHaveBeenCalledOnce(); + }, + ); + + it( + "preserves recovery when a foreign name-labeled container appears after continuity checks (#10547)", + { timeout: 30_000 }, + async () => { + const recovery = retainedRecoveryRecord(); + const sandboxContainerId = "a".repeat(64); + const bootstrapContainerId = "b".repeat(64); + const foreignContainerId = "e".repeat(64); + const identityRows = [sandboxContainerId, bootstrapContainerId] + .map((id) => `${id}\topenshell\tdefault\tsb-alpha`) + .join("\n"); + const harness = createDestroyHarness({ + sandboxPresent: false, + dockerNameLabeledIds: [bootstrapContainerId, foreignContainerId], + dockerOrphanIds: [bootstrapContainerId], + dockerRunResult: { status: 0, stdout: identityRows }, + registryEntryOverrides: { + lifecycleGeneration: recovery.lifecycleGeneration!, + lifecycleLiveIdentityFingerprint: recovery.sandboxIdentityFingerprint!, + }, + retainedRecoveryRecords: [recovery], + }); + + await expect(harness.destroySandbox("alpha", { yes: true })).rejects.toThrow( + "process.exit(1)", + ); + + expect(harness.runOpenshellSpy).not.toHaveBeenCalledWith( + ["sandbox", "delete", "alpha"], + expect.anything(), + ); + expect(harness.dockerRunSpy).not.toHaveBeenCalledWith( + ["rm", "-f", expect.any(String)], + expect.anything(), + ); + expect(harness.errorSpy).toHaveBeenCalledWith( + expect.stringContaining("outside the retained identity set"), + ); + expect(harness.resolveRetainedSandboxRecoverySpy).not.toHaveBeenCalled(); + }, + ); + + it( + "does not delete a live retained sandbox when Docker identity is absent (#10547)", + { timeout: 30_000 }, + async () => { + const recovery = retainedRecoveryRecord(); + const harness = createDestroyHarness({ + dockerRunResult: { status: 0, stdout: "" }, + registryEntryOverrides: { + lifecycleGeneration: recovery.lifecycleGeneration!, + lifecycleLiveIdentityFingerprint: recovery.sandboxIdentityFingerprint!, + }, + retainedRecoveryRecords: [recovery], + }); + + await expect(harness.destroySandbox("alpha", { yes: true })).rejects.toThrow( + "process.exit(1)", + ); + + expect(harness.errorSpy).toHaveBeenCalledWith( + expect.stringContaining("delete command accepts only the mutable sandbox name"), + ); + expect(harness.runOpenshellSpy).not.toHaveBeenCalledWith( + ["sandbox", "delete", "alpha"], + expect.anything(), + ); + expect(harness.resolveRetainedSandboxRecoverySpy).not.toHaveBeenCalled(); + expect(harness.removeSandboxSpy).not.toHaveBeenCalled(); + }, + ); + + it( + "does not issue mutable-name deletion for a live retained sandbox with matching identity (#10547)", + { timeout: 30_000 }, + async () => { + const recovery = retainedRecoveryRecord(); + const containerId = "a".repeat(64); + const harness = createDestroyHarness({ + dockerRunResult: { + status: 0, + stdout: `${containerId}\topenshell\tdefault\tsb-alpha`, + }, + registryEntryOverrides: { + lifecycleGeneration: recovery.lifecycleGeneration!, + lifecycleLiveIdentityFingerprint: recovery.sandboxIdentityFingerprint!, + }, + retainedRecoveryRecords: [recovery], + }); + + await expect(harness.destroySandbox("alpha", { yes: true })).rejects.toThrow( + "process.exit(1)", + ); + + expect(harness.errorSpy).toHaveBeenCalledWith( + expect.stringContaining("cannot bind that deletion to the retained immutable identity"), + ); + expect(harness.runOpenshellSpy).not.toHaveBeenCalledWith( + ["sandbox", "delete", "alpha"], + expect.anything(), + ); + expect(harness.resolveRetainedSandboxRecoverySpy).not.toHaveBeenCalled(); + expect(harness.removeSandboxSpy).not.toHaveBeenCalled(); + }, + ); + + it( + "finishes retained cleanup after OpenShell already removed the sandbox (#10547)", + { timeout: 30_000 }, + async () => { + const recovery = retainedRecoveryRecord(); + const bootstrapContainerId = "b".repeat(64); + const pendingPolicyVerification = { + schemaVersion: 1 as const, + state: "verified-create" as const, + policyAuthority: "externally-managed" as const, + observedPolicyAuthority: "externally-managed" as const, + gatewayName: recovery.gatewayName, + gatewayPort: recovery.gatewayPort, + sandboxName: recovery.sandboxName, + lifecycleGeneration: recovery.lifecycleGeneration!, + sandboxIdentityFingerprint: recovery.sandboxIdentityFingerprint!, + createAttemptNonce: recovery.createAttemptNonce, + route: "none" as const, + policyHash: "policy-hash", + policyVersion: 1, + }; + const harness = createDestroyHarness({ + sandboxPresent: false, + dockerOrphanIds: [bootstrapContainerId], + dockerRunResult: { + status: 0, + stdout: `${bootstrapContainerId}\topenshell\tdefault\tsb-alpha`, + }, + registryEntryOverrides: { + lifecycleGeneration: recovery.lifecycleGeneration!, + lifecycleLiveIdentityFingerprint: recovery.sandboxIdentityFingerprint!, + pendingPolicyVerification, + }, + retainedRecoveryRecords: [recovery], + }); + + await expect(harness.destroySandbox("alpha", { yes: true })).resolves.toBeUndefined(); + + expect(harness.dockerRunSpy).toHaveBeenCalledWith( + ["rm", "-f", bootstrapContainerId], + expect.objectContaining({ ignoreError: true }), + ); + expect(harness.resolveRetainedSandboxRecoverySpy).toHaveBeenCalledWith(recovery); + expect(exitSpy).not.toHaveBeenCalled(); + }, + ); + + it( + "fails closed and then retires the lone record after recovery authority loss (#10547)", + { timeout: 30_000 }, + async () => { + const recovery = retainedRecoveryRecord(); + const bootstrapContainerId = "b".repeat(64); + const harness = createDestroyHarness({ + sandboxPresent: false, + dockerOrphanIds: [bootstrapContainerId], + dockerRunResult: { + status: 0, + stdout: `${bootstrapContainerId}\topenshell\tdefault\tsb-alpha`, + }, + registryEntryOverrides: { + lifecycleGeneration: recovery.lifecycleGeneration!, + lifecycleLiveIdentityFingerprint: recovery.sandboxIdentityFingerprint!, + }, + retainedRecoveryRecords: [recovery], + }); + harness.resolveRetainedSandboxRecoverySpy + .mockImplementation(() => { + harness.setRetainedRecoveryRecords([]); + return true; + }) + .mockReturnValueOnce(false); + + await expect(harness.destroySandbox("alpha", { yes: true })).rejects.toThrow( + "process.exit(1)", + ); + + expect(harness.resolveRetainedSandboxRecoverySpy).toHaveBeenCalledWith(recovery); + expect(harness.errorSpy).toHaveBeenCalledWith( + expect.stringContaining("local recovery cleanup was not confirmed"), + ); + expect(harness.errorSpy).toHaveBeenCalledWith( + expect.stringContaining("Resolve the recovery record conflict"), + ); + expect( + harness.logSpy.mock.calls.some(([message]) => + String(message).includes("Sandbox 'alpha' destroyed"), + ), + ).toBe(false); + expect(harness.removeSandboxSpy).toHaveBeenCalledWith("alpha"); + expect(exitSpy).toHaveBeenCalledWith(1); + + const exactRemovalCallsAfterFailure = harness.dockerRunSpy.mock.calls.filter( + ([args]) => Array.isArray(args) && args[0] === "rm" && args[1] === "-f", + ).length; + harness.setRegistryEntryPresent(false); + harness.setSandboxPresent(false); + harness.setDockerIdentityResult({ status: 0, stdout: "" }); + exitSpy.mockClear(); + harness.logSpy.mockClear(); + + await expect(harness.destroySandbox("alpha", { yes: true })).resolves.toBeUndefined(); + + expect(harness.resolveRetainedSandboxRecoverySpy).toHaveBeenCalledTimes(2); + expect(harness.resolveRetainedSandboxRecoverySpy).toHaveBeenLastCalledWith(recovery); + expect( + harness.dockerRunSpy.mock.calls.filter( + ([args]) => Array.isArray(args) && args[0] === "rm" && args[1] === "-f", + ), + ).toHaveLength(exactRemovalCallsAfterFailure); + expect( + harness.logSpy.mock.calls.some(([message]) => + String(message).includes("Sandbox 'alpha' destroyed"), + ), + ).toBe(true); + expect(exitSpy).not.toHaveBeenCalled(); + + harness.logSpy.mockClear(); + await expect(harness.destroySandbox("alpha", { yes: true })).resolves.toBeUndefined(); + expect(harness.resolveRetainedSandboxRecoverySpy).toHaveBeenCalledTimes(2); + }, + ); + + it( + "selects only the retained record matching observed Docker identity without a registry row (#10547)", + { timeout: 30_000 }, + async () => { + const matchingRecovery = retainedRecoveryRecord(); + const olderRecovery = { + ...retainedRecoveryRecord("sb-older"), + recordId: "e".repeat(64), + lifecycleGeneration: "generation-older", + }; + const sandboxContainerId = "a".repeat(64); + const bootstrapContainerId = "b".repeat(64); + const identityRows = [sandboxContainerId, bootstrapContainerId] + .map((id) => `${id}\topenshell\tdefault\tsb-alpha`) + .join("\n"); + const harness = createDestroyHarness({ + registryEntryPresent: false, + sandboxPresent: false, + dockerOrphanIds: [bootstrapContainerId], + dockerRunResult: { status: 0, stdout: identityRows }, + retainedRecoveryRecords: [olderRecovery, matchingRecovery], + }); + + await expect(harness.destroySandbox("alpha", { yes: true })).resolves.toBeUndefined(); + + expect(harness.dockerRunSpy).toHaveBeenCalledWith( + ["rm", "-f", bootstrapContainerId], + expect.objectContaining({ ignoreError: true }), + ); + expect(harness.resolveRetainedSandboxRecoverySpy).toHaveBeenCalledOnce(); + expect(harness.resolveRetainedSandboxRecoverySpy).toHaveBeenCalledWith(matchingRecovery); + expect(harness.resolveRetainedSandboxRecoverySpy).not.toHaveBeenCalledWith(olderRecovery); + expect(harness.selectGatewaySpy).toHaveBeenCalledWith( + "alpha", + matchingRecovery.gatewayName, + harness.runOpenshellSpy, + ); + expect(harness.gatewayPinsAtSandboxList).toEqual([matchingRecovery.gatewayName]); + expect(exitSpy).not.toHaveBeenCalled(); + }, + ); + + it( + "refuses destroy when observed Docker identity matches multiple retained records (#10547)", + { timeout: 30_000 }, + async () => { + const firstRecovery = retainedRecoveryRecord(); + const secondRecovery = { + ...firstRecovery, + recordId: "e".repeat(64), + lifecycleGeneration: "generation-second", + createAttemptNonce: "d".repeat(62), + }; + const harness = createDestroyHarness({ + registryEntryPresent: false, + dockerRunResult: { + status: 0, + stdout: `${"a".repeat(64)}\topenshell\tdefault\tsb-alpha`, + }, + retainedRecoveryRecords: [firstRecovery, secondRecovery], + }); + + await expect(harness.destroySandbox("alpha", { yes: true })).rejects.toThrow( + "process.exit(1)", + ); + + expect(harness.errorSpy).toHaveBeenCalledWith( + expect.stringContaining("could not select exactly one recovery record"), + ); + expect(harness.runOpenshellSpy).not.toHaveBeenCalledWith( + ["sandbox", "delete", "alpha"], + expect.anything(), + ); + expect(harness.resolveRetainedSandboxRecoverySpy).not.toHaveBeenCalled(); + }, + ); +}); diff --git a/src/lib/actions/sandbox/destroy.test.ts b/src/lib/actions/sandbox/destroy.test.ts index f621f220f87..9af10ead97a 100644 --- a/src/lib/actions/sandbox/destroy.test.ts +++ b/src/lib/actions/sandbox/destroy.test.ts @@ -110,7 +110,7 @@ describe("assertUnambiguousDestroyContainerIdentity (#8999)", () => { redact: String, classify: classify as never, }), - ).toEqual({ identity }); + ).toEqual({ identities: [identity] }); }); it("does not probe or block a non-Docker runtime provider", () => { @@ -120,7 +120,7 @@ describe("assertUnambiguousDestroyContainerIdentity (#8999)", () => { redact: String, classify: classify as never, }); - expect(proceed).toEqual({ identity: undefined }); + expect(proceed).toEqual({}); expect(classify).not.toHaveBeenCalled(); }); diff --git a/src/lib/actions/sandbox/destroy.ts b/src/lib/actions/sandbox/destroy.ts index 2f1b91d26c4..9aba963667b 100644 --- a/src/lib/actions/sandbox/destroy.ts +++ b/src/lib/actions/sandbox/destroy.ts @@ -38,6 +38,7 @@ import { validateName } from "../../runner"; import { killTimer as defaultKillShieldsTimer } from "../../shields/timer-control"; import { withMcpLifecycleLock } from "../../state/mcp-lifecycle-lock"; import * as onboardSession from "../../state/onboard-session"; +import type { RetainedSandboxRecoveryRecord } from "../../state/onboard-session/retained-sandbox-recovery"; import { resolveNemoclawStateDir } from "../../state/paths"; import * as registry from "../../state/registry"; import { @@ -54,8 +55,10 @@ import { cleanupGatewayAfterLastSandbox } from "./destroy-gateway"; import { shouldCleanupGatewayAfterConfirmedFinalDestroy } from "./destroy-gateway-cleanup"; import { assertUnambiguousDestroyContainerIdentity, + classifyDestroyContainerIdentity, classifyDestroySandboxPresence, isSameDestroyContainerIdentityProof, + observeDestroyContainerIdentity, } from "./destroy-presence"; import { prepareSandboxDestroy, @@ -83,6 +86,60 @@ type RemoveSandboxRegistryEntryWithReceiptDeps = { removeSandboxWithReceipt?: typeof registry.removeSandboxWithReceipt; }; +function selectRetainedSandboxRecoveryAuthority( + sandboxName: string, + sandbox: registry.SandboxEntry | null, + records: readonly RetainedSandboxRecoveryRecord[], +): RetainedSandboxRecoveryRecord | null { + const candidates = records.filter( + (record) => + record.sandboxName === sandboxName && record.sandboxIdentityFingerprint !== null, + ); + if (!sandbox) { + // Once resource cleanup has removed the registry row, a retry must still + // select the lone durable record so the later Docker proof can confirm + // absence and retire it. Multiple records continue to require immutable + // Docker evidence so the mutable name never chooses between authorities. + if (candidates.length === 1) return candidates[0]!; + if (candidates.length === 0) return null; + const observation = observeDestroyContainerIdentity(sandboxName); + const observedMatches = candidates.filter((record) => { + const verdict = classifyDestroyContainerIdentity( + sandboxName, + observation, + record.sandboxIdentityFingerprint!, + ); + return ( + verdict.status === "recovery" || + (verdict.status === "clear" && verdict.identity !== null) + ); + }); + return observedMatches.length === 1 ? observedMatches[0]! : null; + } + + const matchesRegistryAuthority = (record: RetainedSandboxRecoveryRecord): boolean => { + const pending = sandbox.pendingPolicyVerification; + if (pending) { + return ( + record.gatewayName === pending.gatewayName && + record.gatewayPort === pending.gatewayPort && + record.lifecycleGeneration === pending.lifecycleGeneration && + record.sandboxIdentityFingerprint === pending.sandboxIdentityFingerprint && + (pending.createAttemptNonce === undefined || + record.createAttemptNonce === pending.createAttemptNonce) + ); + } + return ( + record.gatewayName === sandbox.gatewayName && + record.gatewayPort === sandbox.gatewayPort && + record.lifecycleGeneration === sandbox.lifecycleGeneration && + record.sandboxIdentityFingerprint === sandbox.lifecycleLiveIdentityFingerprint + ); + }; + const matching = candidates.filter(matchesRegistryAuthority); + return matching.length === 1 ? matching[0]! : null; +} + export type RemoveSandboxRegistryEntryOutcome = | { readonly status: "complete"; @@ -508,6 +565,24 @@ async function destroySandboxUnlocked( const normalized = normalizeDestroySandboxOptions(options); if (!(await confirmSandboxDestroy(sandboxName, normalized))) return; const destroySession = onboardSession.loadSession(); + const registeredSandbox = registry.getSandbox(sandboxName); + const retainedRecoveryRecords = onboardSession.listRetainedSandboxRecoveryRecords(); + const retainedRecoveryAuthority = selectRetainedSandboxRecoveryAuthority( + sandboxName, + registeredSandbox, + retainedRecoveryRecords, + ); + if ( + !retainedRecoveryAuthority && + retainedRecoveryRecords.some((record) => record.sandboxName === sandboxName) + ) { + console.error( + ` Refusing to destroy retained sandbox '${sandboxName}': NemoClaw could not select exactly one recovery record from the current immutable registry and Docker identities. No sandbox resources were removed. Resolve the identity conflict, then rerun '${CLI_NAME} ${sandboxName} destroy'.`, + ); + requestSandboxDestroyExit(1); + } + const retainedSandboxIdentityFingerprint = + retainedRecoveryAuthority?.sandboxIdentityFingerprint ?? undefined; let portableContainerAuthority: ReturnType; try { portableContainerAuthority = preparePortableDemoSandboxDestroyAuthority(sandboxName, () => { @@ -533,13 +608,16 @@ async function destroySandboxUnlocked( registry.getSandbox(sandboxName)?.openshellDriver, ), redact: redactDestroyError, + ...(retainedSandboxIdentityFingerprint + ? { retainedSandboxIdentityFingerprint } + : {}), }); const initialIdentity = portableContainerAuthority ? null : inspectContainerIdentity(); if (initialIdentity === false) { requestSandboxDestroyExit(1); } + const initialContainerIdentities = initialIdentity?.identities; - const registeredSandbox = registry.getSandbox(sandboxName); let preparedManagedLlamaCppCleanup: ReturnType< typeof prepareManagedLlamaCppRuntimeCleanupForSandbox > = null; @@ -592,8 +670,17 @@ async function destroySandboxUnlocked( } }; let destroyPreflight: ReturnType; - destroyPreflight = abortPreparedCleanupOnError(() => prepareSandboxDestroy(sandboxName)); + destroyPreflight = abortPreparedCleanupOnError(() => + prepareSandboxDestroy(sandboxName, retainedRecoveryAuthority?.gatewayName), + ); const { cleanupGatewayName, runOpenshell, sandbox, sandboxConfirmedAbsent } = destroyPreflight; + if (retainedRecoveryAuthority && !sandboxConfirmedAbsent) { + console.error( + ` Refusing to automatically delete retained sandbox '${sandboxName}': OpenShell still reports it present, but its delete command accepts only the mutable sandbox name. NemoClaw cannot bind that deletion to the retained immutable identity. No sandbox resources were removed. Ask an OpenShell administrator to resolve create-attempt label '${retainedRecoveryAuthority.createAttemptNonce}' to the exact sandbox and use an identity-bound removal procedure. After OpenShell confirms the retained sandbox is absent, rerun '${CLI_NAME} ${sandboxName} destroy --yes' to reconcile its verified Docker containers and recovery record.`, + ); + preparedManagedLlamaCppCleanup?.abort(); + requestSandboxDestroyExit(1); + } // Recheck identity after pre-delete qualification and recoverable journal // publication reconciliation, before any sandbox runtime mutation. if (portableContainerAuthority) { @@ -635,7 +722,10 @@ async function destroySandboxUnlocked( sandbox, sandboxConfirmedAbsent, sandboxName, - expectedContainerIdentity: initialIdentity?.identity, + expectedContainerIdentities: initialContainerIdentities, + ...(retainedSandboxIdentityFingerprint + ? { expectedContainerIdentityFingerprint: retainedSandboxIdentityFingerprint } + : {}), ...(portableContainerAuthority ? { portableContainerAuthority } : {}), stopInferenceResources: () => stopSandboxInferenceResources(sandboxName, sandbox), }); @@ -898,7 +988,17 @@ async function destroySandboxUnlocked( ); } } - if (!routedSessionCleanupHandled && destroySession?.sandboxName === sandboxName) { + const retainedRecoveryOwnsDestroySession = retainedRecoveryAuthority + ? onboardSession.retainedSandboxRecoveryMatchesSession( + retainedRecoveryAuthority, + destroySession, + ) + : false; + if ( + !retainedRecoveryOwnsDestroySession && + !routedSessionCleanupHandled && + destroySession?.sandboxName === sandboxName + ) { const cleanupResult = onboardSession.compareAndSwapSession( (current) => current.sessionId === destroySession.sessionId && @@ -919,6 +1019,30 @@ async function destroySandboxUnlocked( ); } } + if ( + deleteSucceededOrAlreadyGone && + retainedRecoveryAuthority + ) { + let recoveryResolved: boolean; + try { + recoveryResolved = onboardSession.resolveRetainedSandboxRecovery(retainedRecoveryAuthority); + } catch (error) { + console.error( + ` Sandbox '${sandboxName}' resources are gone, but NemoClaw could not clear its retained recovery record: ${redactDestroyError(error)}`, + ); + console.error(` Re-run '${CLI_NAME} ${sandboxName} destroy --yes' to finish local cleanup.`); + requestSandboxDestroyExit(1); + } + if (!recoveryResolved) { + console.error( + ` Sandbox '${sandboxName}' resources are gone, but its retained recovery authority was no longer current, so local recovery cleanup was not confirmed.`, + ); + console.error( + ` NemoClaw preserved any current recovery state. Resolve the recovery record conflict, then re-run '${CLI_NAME} ${sandboxName} destroy --yes'.`, + ); + requestSandboxDestroyExit(1); + } + } if ( shouldCleanupGatewayAfterConfirmedFinalDestroy({ deleteSucceededOrAlreadyGone, diff --git a/src/lib/actions/sandbox/rebuild-baseline-transition-preflight.test.ts b/src/lib/actions/sandbox/rebuild-baseline-transition-preflight.test.ts index 9fd0e7511c4..f1fd945ffd6 100644 --- a/src/lib/actions/sandbox/rebuild-baseline-transition-preflight.test.ts +++ b/src/lib/actions/sandbox/rebuild-baseline-transition-preflight.test.ts @@ -9,6 +9,7 @@ const mocks = vi.hoisted(() => ({ confirmRebuildIntent: vi.fn(), countActiveSessions: vi.fn(), getSandbox: vi.fn(), + listRetainedRecovery: vi.fn(), prepareTargets: vi.fn(), })); @@ -17,6 +18,11 @@ vi.mock("../../state/registry", async (importOriginal) => ({ getSandbox: mocks.getSandbox, })); +vi.mock("../../state/onboard-session", async (importOriginal) => ({ + ...(await importOriginal()), + listRetainedSandboxRecoveryRecords: mocks.listRetainedRecovery, +})); + vi.mock("./mcp-bridge-state", async (importOriginal) => ({ ...(await importOriginal()), assertMcpDestroyNotPending: mocks.assertMcpDestroyNotPending, @@ -47,9 +53,39 @@ afterEach(() => { vi.restoreAllMocks(); }); +describe("rebuild retained sandbox recovery preflight (#10547)", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.getSandbox.mockReturnValue({ name: "alpha", openshellDriver: "docker" }); + mocks.listRetainedRecovery.mockReturnValue([ + { + recordId: "f".repeat(64), + sandboxName: "alpha", + }, + ]); + }); + + it("stops before session probes, confirmation, MCP checks, or target preparation", async () => { + const error = vi.spyOn(console, "error").mockImplementation(() => undefined); + + await expect(runRebuildPreflightPhase("alpha", ["--yes"])).resolves.toBeNull(); + + expect(mocks.bail).toHaveBeenCalledWith( + "Retained sandbox recovery blocks rebuild for 'alpha'.", + 1, + ); + expect(error.mock.calls.flat().join("\n")).toContain("alpha destroy --yes"); + expect(mocks.countActiveSessions).not.toHaveBeenCalled(); + expect(mocks.assertMcpDestroyNotPending).not.toHaveBeenCalled(); + expect(mocks.confirmRebuildIntent).not.toHaveBeenCalled(); + expect(mocks.prepareTargets).not.toHaveBeenCalled(); + }); +}); + describe("rebuild baseline transition preflight (#7194)", () => { beforeEach(() => { vi.clearAllMocks(); + mocks.listRetainedRecovery.mockReturnValue([]); mocks.getSandbox.mockReturnValue({ name: "alpha", baselineExclusionTransition: { @@ -84,6 +120,7 @@ describe("rebuild baseline transition preflight (#7194)", () => { describe("rebuild MCP destroy marker preflight (#7794)", () => { beforeEach(() => { vi.clearAllMocks(); + mocks.listRetainedRecovery.mockReturnValue([]); mocks.getSandbox.mockReturnValue({ name: "alpha", agent: "openclaw", diff --git a/src/lib/actions/sandbox/rebuild-preflight-guards.ts b/src/lib/actions/sandbox/rebuild-preflight-guards.ts index 0d8ca2ae3aa..c5c0c153ddc 100644 --- a/src/lib/actions/sandbox/rebuild-preflight-guards.ts +++ b/src/lib/actions/sandbox/rebuild-preflight-guards.ts @@ -314,6 +314,26 @@ export function getRebuildSandboxEntryOrBail( return sb; } +/** Block rebuild before any live-state probe or cleanup can bypass retained recovery. */ +export function blockRebuildOnRetainedSandboxRecovery( + sandboxName: string, + bail: RebuildBail, +): boolean { + const retainedRecovery = onboardSession + .listRetainedSandboxRecoveryRecords() + .find((record) => record.sandboxName === sandboxName); + if (!retainedRecovery) return false; + + console.error( + ` Rebuild cannot use retained sandbox '${sandboxName}' while recovery record '${retainedRecovery.recordId}' is unresolved. No sandbox or Docker resources were removed.`, + ); + console.error( + ` Run '${CLI_NAME} ${sandboxName} destroy --yes'. If OpenShell still reports the sandbox present, follow destroy's create-attempt label guidance for identity-bound administrator removal.`, + ); + bail(`Retained sandbox recovery blocks rebuild for '${sandboxName}'.`, 1); + return true; +} + /** Keep the pending baseline-policy transaction guard identical at every rebuild boundary. */ export function blockRebuildOnPendingBaselineTransition( sandboxEntry: RebuildSandboxEntry, diff --git a/src/lib/actions/sandbox/rebuild-preflight-phase.ts b/src/lib/actions/sandbox/rebuild-preflight-phase.ts index caa7b676ab3..ecf1391e17e 100644 --- a/src/lib/actions/sandbox/rebuild-preflight-phase.ts +++ b/src/lib/actions/sandbox/rebuild-preflight-phase.ts @@ -43,6 +43,7 @@ import { acquireRebuildOnboardLock, assertRebuildEntryUnchanged, blockRebuildOnPendingBaselineTransition, + blockRebuildOnRetainedSandboxRecovery, checkRebuildGatewaySchemaPreflight, expectedRebuildEntryAfterVersionCheck, getRebuildSandboxEntryOrBail, @@ -137,6 +138,7 @@ export async function runRebuildPreflightPhase( } = createRebuildCommandContext(options, opts); const sandboxEntry = getRebuildSandboxEntryOrBail(sandboxName, bail); if (!sandboxEntry) return null; + if (blockRebuildOnRetainedSandboxRecovery(sandboxName, bail)) return null; if (blockRebuildOnPendingBaselineTransition(sandboxEntry, sandboxName, bail)) return null; const activeSessionCount = countActiveSandboxSessionsForRebuild(sandboxName); // #6376: refuse a stuck MCP destroy transaction up front — before backup, diff --git a/src/lib/onboard/cancel-rollback.test.ts b/src/lib/onboard/cancel-rollback.test.ts index 6fd504321cb..dd9d11cf41c 100644 --- a/src/lib/onboard/cancel-rollback.test.ts +++ b/src/lib/onboard/cancel-rollback.test.ts @@ -11,6 +11,14 @@ import { } from "./cancel-rollback"; const SANDBOX_FINGERPRINT = "a".repeat(64); +const RECOVERY_CONTEXT = { + gatewayName: "nemoclaw", + gatewayPort: 8080, + lifecycleGeneration: "generation-alpha", + verifiedEffectivePolicyIdentity: null, + createAttemptNonce: "c".repeat(62), + policyCreationReceipt: null, +} as const; function createHarness() { const log = vi.fn(); @@ -21,23 +29,22 @@ describe("createSandboxCancelRollback", () => { it("preserves an armed cancelled sandbox and reports its captured identity (#9833)", () => { const { rollback, log } = createHarness(); - rollback.arm("new-sb", SANDBOX_FINGERPRINT); + rollback.arm("new-sb", SANDBOX_FINGERPRINT, RECOVERY_CONTEXT); rollback.markCancelled(); rollback.runIfArmed(); const guidance = log.mock.calls.flat().join("\n"); expect(guidance).toContain("preserved incomplete sandbox 'new-sb'"); expect(guidance).toContain(SANDBOX_FINGERPRINT); - expect(guidance).toContain("OpenShell administrator"); + expect(guidance).toContain( + `ai.nvidia.nemoclaw.create-attempt=${RECOVERY_CONTEXT.createAttemptNonce}`, + ); expect(guidance).toContain("did not run OpenShell's mutable-name deletion command"); expect(guidance).toContain("Do not delete the sandbox by mutable sandbox name"); expect(guidance).toContain("Shared inference providers are gateway configuration"); expect(guidance).toContain("not sandbox cleanup targets"); - expect(guidance).toContain("sandbox-scoped resources whose ownership is confirmed"); - expect(guidance).toContain("no supported operation to clear this recovery record"); - expect(guidance).toContain("credential environment name alone does not prove exposure"); - expect(guidance).toContain("rotate a credential only when identity-bound inspection proves"); - expect(guidance).not.toContain("rotate any credential"); + expect(guidance).toContain("nemoclaw new-sb destroy"); + expect(guidance).toContain("clear the matching recovery record"); }); it.each([ @@ -296,12 +303,24 @@ describe("makeOnboardCancelExit", () => { describe("buildCancelRollbackMessage", () => { it("preserves identity-bound recovery guidance", () => { - const message = buildCancelRollbackMessage("sb", SANDBOX_FINGERPRINT).join("\n"); + const message = buildCancelRollbackMessage( + "sb", + SANDBOX_FINGERPRINT, + RECOVERY_CONTEXT, + ).join("\n"); expect(message).toContain("preserved incomplete sandbox 'sb'"); expect(message).toContain(SANDBOX_FINGERPRINT); + expect(message).toContain(RECOVERY_CONTEXT.createAttemptNonce); expect(message).toContain("identity-bound inspection, recovery, or removal"); expect(message).not.toContain("openshell sandbox delete"); expect(message).not.toContain("cannot delete it by immutable identity"); }); + + it("does not refer to an undisplayed create-attempt label", () => { + const message = buildCancelRollbackMessage("sb", SANDBOX_FINGERPRINT).join("\n"); + + expect(message).toContain("preserve the displayed fingerprint"); + expect(message).not.toContain("displayed create-attempt label"); + }); }); diff --git a/src/lib/onboard/cancel-rollback.ts b/src/lib/onboard/cancel-rollback.ts index f2d72f81afb..90d7e9d2192 100644 --- a/src/lib/onboard/cancel-rollback.ts +++ b/src/lib/onboard/cancel-rollback.ts @@ -1,7 +1,9 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import { NEMOCLAW_CREATE_ATTEMPT_LABEL } from "../adapters/openshell/sandbox-identity"; import type { RetainedSandboxRecoveryContext } from "../state/onboard-session"; +import { cliName } from "./branding"; // Re-exported so the onboard entrypoint imports its sandbox default/cancel // lifecycle helpers from a single module. @@ -54,10 +56,16 @@ export interface SandboxCancelRollback { export function buildCancelRollbackMessage( sandboxName: string, sandboxIdentityFingerprint?: string, + recoveryContext?: Pick, ): string[] { return [ "", ` Onboarding cancelled — preserved incomplete sandbox '${sandboxName}'.`, + ...(recoveryContext + ? [ + ` Create-attempt label: ${NEMOCLAW_CREATE_ATTEMPT_LABEL}=${recoveryContext.createAttemptNonce}`, + ] + : []), ...(sandboxIdentityFingerprint ? [ ` Durable sandbox identity fingerprint: ${sandboxIdentityFingerprint}`, @@ -70,10 +78,16 @@ export function buildCancelRollbackMessage( " NemoClaw did not run OpenShell's mutable-name deletion command because the name may now identify a replacement sandbox.", " Do not delete the sandbox by mutable sandbox name.", " Shared inference providers are gateway configuration and are not sandbox cleanup targets.", - " Sandbox-scoped provider registrations or gateway-bound credentials may remain when the durable recovery record lists them.", - " Ask an OpenShell administrator to inspect the exact sandbox identity and remove only sandbox-scoped resources whose ownership is confirmed for this retained sandbox.", - " A recorded credential environment name alone does not prove exposure; rotate a credential only when identity-bound inspection proves that it was exposed or attached to a retained sandbox-scoped resource.", - " NemoClaw has no supported operation to clear this recovery record; use a different sandbox name for later onboarding.", + ...(sandboxIdentityFingerprint + ? [ + ` Run '${cliName()} ${sandboxName} destroy'. If OpenShell confirms the retained sandbox absent, destroy removes only verified residual containers and can clear the matching recovery record.`, + recoveryContext + ? " If it is still live, give the displayed create-attempt label to an OpenShell administrator for identity-bound removal." + : " If it is still live, preserve the displayed fingerprint and ask an OpenShell administrator for identity-bound removal.", + ] + : [ + " NemoClaw cannot clear this recovery record until an OpenShell administrator establishes the exact sandbox identity.", + ]), ]; } @@ -200,6 +214,7 @@ export function createSandboxCancelRollback( for (const line of buildCancelRollbackMessage( sandboxName, identityFingerprint ?? undefined, + armedSandbox.context, )) { deps.log(line); } diff --git a/src/lib/onboard/entry-options.test.ts b/src/lib/onboard/entry-options.test.ts index ae40e6f3bc9..fe8b7da9b27 100644 --- a/src/lib/onboard/entry-options.test.ts +++ b/src/lib/onboard/entry-options.test.ts @@ -290,6 +290,30 @@ describe("resolveOnboardEntryOptions", () => { expect(deps.error).not.toHaveBeenCalled(); }); + it("treats an explicit different name as fresh while recovery remains isolated (#10547)", () => { + const deps = createDeps(); + + const result = resolveOnboardEntryOptions( + { + opts: { sandboxName: "replacement-sb" }, + env: {}, + stdinIsTty: true, + stdoutIsTty: true, + persistedSessionStatus: "recovery_required", + persistedRecoverySandboxName: "retained-sb", + retainedRecoverySandboxNames: ["retained-sb"], + }, + deps, + ); + + expect(result).toMatchObject({ + fresh: true, + resume: false, + requestedSandboxName: "replacement-sb", + }); + expect(deps.error).not.toHaveBeenCalled(); + }); + it("rejects a different fresh name when recovery has no independent durable record", () => { const deps = createDeps(); diff --git a/src/lib/onboard/entry-options.ts b/src/lib/onboard/entry-options.ts index b8f707b0ca1..6d2c98a7965 100644 --- a/src/lib/onboard/entry-options.ts +++ b/src/lib/onboard/entry-options.ts @@ -282,7 +282,7 @@ export function resolveOnboardEntryOptions( deps: OnboardEntryOptionsDeps, ): ResolvedOnboardEntryOptions { const explicitResume = input.opts.resume === true; - const fresh = input.opts.fresh === true; + let fresh = input.opts.fresh === true; // The mutual-exclusion error applies only to the explicit flags — a leftover // in_progress session combined with an explicit `--fresh` is not a conflict // (fresh wins, see below), so it must not trip this guard. @@ -348,7 +348,7 @@ export function resolveOnboardEntryOptions( " Onboarding cannot continue while a retained sandbox recovery record is unresolved without an explicit different sandbox name.", ); deps.error( - " Use --fresh --name ; the retained sandbox recovery record stays unresolved.", + " Use --name ; the retained sandbox recovery record stays unresolved.", ); deps.exitProcess(1); } @@ -357,13 +357,20 @@ export function resolveOnboardEntryOptions( ` Onboarding cannot use retained sandbox '${recoveryEntryName}' while its identity-bound recovery record is unresolved.`, ); deps.error( - " Automatic and explicit resume, reuse, recreation, and same-name fresh onboarding remain disabled; NemoClaw has no supported operation to clear this recovery record.", + ` Run the destroy command for retained sandbox '${recoveryEntryName}' to remove the verified failed attempt; resume, reuse, recreation, and same-name fresh onboarding remain disabled until destroy completes.`, ); deps.exitProcess(1); } } if (input.persistedSessionStatus === "recovery_required") { const recoverySandboxName = input.persistedRecoverySandboxName?.trim() || null; + const canStartDifferentSandbox = + !explicitResume && + recoverySandboxName !== null && + requestedSandboxName !== null && + requestedSandboxName !== recoverySandboxName && + retainedRecoverySandboxNames.has(recoverySandboxName); + if (!fresh && canStartDifferentSandbox) fresh = true; if (!fresh) { deps.error( ` Onboarding cannot continue because cancellation preserved sandbox '${recoverySandboxName ?? "unknown"}' in recovery-only state.`, @@ -372,7 +379,7 @@ export function resolveOnboardEntryOptions( " Automatic and explicit resume, reuse, and recreation are disabled to protect the retained sandbox.", ); deps.error( - " Use --fresh --name ; the retained sandbox recovery record stays unresolved.", + " Use --name to start another sandbox. The retained sandbox recovery record stays unresolved.", ); deps.exitProcess(1); } diff --git a/src/lib/onboard/lifecycle-contracts.md b/src/lib/onboard/lifecycle-contracts.md index 13d45e423c8..9a232d8c484 100644 --- a/src/lib/onboard/lifecycle-contracts.md +++ b/src/lib/onboard/lifecycle-contracts.md @@ -70,7 +70,13 @@ Onboarding binds policy authority after gateway setup and before provider, crede The session records the decision before later effects. A live sandbox must agree with both the saved session and registry entry. Onboarding rechecks that agreement before each policy-dependent change and after the created sandbox reaches Ready. -NemoClaw-managed onboarding keeps the existing policy creation and attribution behavior. Externally managed onboarding verifies that the effective policy contains every requirement for the selected agent, provider, messaging channels, observability, GPU mode, and web search setup. It does not pass a policy file, export `OPENSHELL_SANDBOX_POLICY`, change policy, or record NemoClaw policy attribution. After a post-create authority failure, NemoClaw leaves the sandbox running because the supported delete command targets its mutable name. Operators must preserve the durable sandbox identity fingerprint from the failure output and provide it to the OpenShell administrator. They must not delete the sandbox by name, even after comparing its identity. Contact the administrator for an identity-bound recovery or removal procedure. +NemoClaw-managed onboarding keeps the existing policy creation and attribution behavior. Externally managed onboarding verifies that the effective policy contains every requirement for the selected agent, provider, messaging channels, observability, GPU mode, and web search setup. It does not pass a policy file, export `OPENSHELL_SANDBOX_POLICY`, change policy, or record NemoClaw policy attribution. + +After a post-create authority failure, NemoClaw retains the durable sandbox identity fingerprint when available and reports the exact create-attempt label when available. While OpenShell reports the sandbox present, recovery refuses automatic deletion because OpenShell deletion accepts only a mutable sandbox name. When a create-attempt label is available, the operator gives it to an administrator for identity-bound removal. Without a label, the operator preserves the terminal output and asks the administrator to identify the exact sandbox from gateway or controller evidence. + +After OpenShell confirms absence, `destroy` verifies the retained immutable runtime identity. For Docker-backed sandboxes, it accepts multiple managed containers only when every immutable sandbox ID has the retained fingerprint. It snapshots the matching container IDs, revalidates the set, removes only remaining members, and verifies their absence without issuing a mutable-name delete. A foreign container, changed identity, failed probe, ambiguous record, or changed recovery authority stops cleanup. + +Recovery completion releases the matching recovery-only session before retiring the exact independent record. Either write failure leaves the record available for retry. A record without an immutable fingerprint remains fail-closed for the affected sandbox name; mutable-name absence cannot retire it, and this flow accepts no administrator-supplied identity authority. Rebuild and same-name onboarding remain blocked, but `nemoclaw onboard --name ` starts an unrelated sandbox. `--fresh` alone does not clear the record or permit reuse of the retained name. The command reference owns the detailed operator procedure. ## Effect-order flows @@ -125,7 +131,7 @@ runtime mutation | Journey and entry | Desired state, planning, and assembly | Visible and destructive boundaries | Checkpoint and secret boundary | Compensation, coverage, and gaps | |---|---|---|---|---| -| **New interactive or non-interactive onboard** — `onboard()` and `resolveOnboardEntryOptions` | Current flags, environment, and prompts. `MessagingWorkflowPlanner.buildPlan`, `prepareSandboxMessagingPreflight`, resource-profile selection, `resolveSandboxCreateIntent`, and `materializeSandboxCreatePlan` assemble policy, provider, package, resource, host-forward, and runtime-setup contributions. Non-interactive mode replaces prompts with defaults or hard aborts. | Consent/session/lock setup and preflight can persist local state, install OpenShell, or clean stale gateway artifacts before the gateway handler. Gateway reuse/recovery/start is the first provider-routing effect; inference-provider upserts follow. For OpenClaw, messaging selection and plan reconciliation complete before web-search or messaging provider registration. Each validated provider group is then created or updated and checkpointed before resource selection. A name with no live sandbox has no sandbox-destructive boundary; an existing target enters the recreate contract below. | Whole-step session plus machine snapshot. OpenClaw adds narrow checkpoints after each completed secret-free sandbox prompt group; sandbox registry registration is deferred until readiness and live validation. The session stores credential environment names, redacted endpoint metadata, legacy-value digests, and non-secret names of web-search and messaging providers registered for resume; real values remain process- or gateway-bound. | Readiness, post-create policy verification, dashboard forwarding, and cancellation failures preserve the live sandbox because NemoClaw refuses the available mutable-name deletion command when it could target a replacement. Exact provider-owned GPU cleanup can proceed through its owner receipt. Temporary policy and build-context cleanup remains best effort. Cancellation before sandbox creation can leave the session resumable. Cancellation after creation preserves the incomplete session, registry row, and an independent identity-bound recovery record. Shared inference providers remain gateway configuration and are not sandbox cleanup targets. Recovery removes only confirmed sandbox-scoped resources and rotates credentials only when inspection proves exposure or attachment to a retained resource; a recorded environment-variable name alone is not exposure evidence. Coverage: `transition-traces.test.ts`, `sandbox-create-intent-boundary.test.ts`, `sandbox-create-plan.test.ts`, and the focused cancellation, readiness, GPU cleanup, dashboard, and policy-authority tests. Gap: gateway upserts can outlive a failed or interrupted create. | +| **New interactive or non-interactive onboard** — `onboard()` and `resolveOnboardEntryOptions` | Current flags, environment, and prompts. `MessagingWorkflowPlanner.buildPlan`, `prepareSandboxMessagingPreflight`, resource-profile selection, `resolveSandboxCreateIntent`, and `materializeSandboxCreatePlan` assemble policy, provider, package, resource, host-forward, and runtime-setup contributions. Non-interactive mode replaces prompts with defaults or hard aborts. | Consent/session/lock setup and preflight can persist local state, install OpenShell, or clean stale gateway artifacts before the gateway handler. Gateway reuse/recovery/start is the first provider-routing effect; inference-provider upserts follow. For OpenClaw, messaging selection and plan reconciliation complete before web-search or messaging provider registration. Each validated provider group is then created or updated and checkpointed before resource selection. A name with no live sandbox has no sandbox-destructive boundary; an existing target enters the recreate contract below. | Whole-step session plus machine snapshot. OpenClaw adds narrow checkpoints after each completed secret-free sandbox prompt group; sandbox registry registration is deferred until readiness and live validation. The session stores credential environment names, redacted endpoint metadata, legacy-value digests, and non-secret names of web-search and messaging providers registered for resume; real values remain process- or gateway-bound. | Readiness, post-create policy verification, dashboard forwarding, and cancellation failures preserve the live sandbox and an independent identity-bound recovery record. A later `destroy` refuses mutable-name deletion while that sandbox is live. After administrator identity-bound removal, destroy uses the record to qualify immutable runtime identity and, for Docker-backed sandboxes, exact container identities before residual cleanup and record retirement. A different explicit sandbox name starts a fresh session without changing the retained record. Exact provider-owned GPU cleanup can proceed through its owner receipt. Temporary policy and build-context cleanup remains best effort. Cancellation before sandbox creation can leave the session resumable. Shared inference providers remain gateway configuration and are not sandbox cleanup targets. Coverage: `transition-traces.test.ts`, `sandbox-create-intent-boundary.test.ts`, `sandbox-create-plan.test.ts`, and the focused cancellation, readiness, GPU cleanup, dashboard, policy-authority, destroy, and retained-recovery tests. Gap: gateway upserts can outlive a failed or interrupted create. | | **`--fresh` onboard** — `resolveOnboardEntryOptions`, `prepareFreshSession`, `createBaseImageResolutionContext` | Current flags/environment/prompts replace resumable intent. `--fresh` disables auto-resume and forces base-image resolution; it does not prove that the selected sandbox name is unused. | The first destructive effect is local: the prior onboard session is cleared before a new session is saved. A matching live sandbox can later reuse or recreate through the normal sandbox decision; `--fresh` does not itself delete it. | The new session and machine snapshot replace the old resume checkpoint. Credential and effect boundaries then match new onboard or live recreate. | The discarded resume checkpoint is not restored on later failure. Covered by `entry-options.test.ts`, `session-bootstrap.test.ts`, and base-image resolution tests. | | **Resume, re-onboard, or recreate** — `onboard()`, `prepareOnboardSession`, `decideSandboxResume`, live-sandbox handling in `createSandbox` | For `--resume`, the recorded session is authoritative and conflicting current name/provider/model/image/tool-disclosure hints are rejected. A new re-onboard run takes current flags, environment, and prompts as intent while registry/gateway state provides drift evidence. The machine resolves a complete secret-free create intent, including policy, messaging/provider, GPU, resource, disabled-channel, and agent inputs, before repair/removal or live recreation. | Ordinary live recreation conditionally backs up before provider cleanup, **delete**, and image removal. The recreate journal preserves the source registry row after deletion. Replacement registration commits the new row after readiness and validation. A selected pre-upgrade backup suppresses a new one; an explicit override permits recreation without backup. Resume registry removal and `repair-and-recreate` occur only after complete intent validation. Temporary policy/build artifacts remain materialization effects after the delete boundary. | Resume continues the recorded session/machine snapshot; non-resume re-onboard writes a new session first. OpenClaw records completed sandbox name, web search, messaging, and resource choices with explicit progress markers, including explicit `null` choices, while the complete create intent stays process-local and is not persisted or emitted. Raw credential values remain outside the session. A missing process value can be rebound only when the same OpenClaw session recorded successfully registering that provider and its live provider name, provider type, and credential key still match; otherwise interactive resume requests it again and non-interactive resume exits with environment-variable guidance. Credentials are checked before mutation and again immediately before materialization. | A failed replacement keeps the source registry row. Restore failures warn and can still publish the replacement; managed-DCode live-selection failure leaves a running, unregistered sandbox with manual-delete guidance. Checkpoint replay reuses an exact live sandbox after an interrupted create and backfills missing create/register receipts. Cancel rollback is not armed and there is no rebuild-style receipt rollback. Coverage: transition traces, create-intent characterization, checkpoint replay and resume guards, and sandbox-handler crash recovery. Gaps: early backup asymmetry and no rebuild-style cross-effect rollback. | | **Rebuild or installer-driven upgrade** — `rebuildSandbox` in `rebuild-pipeline.ts`; `upgradeSandboxes` | Registry state is authoritative. A matching session may fill guarded legacy gaps only when its selection agrees; an unrelated/global session is never used. Ambient provider/model selection is quarantined by `isolateAmbientRecreateEnv`, apart from narrowly scoped legacy recovery. Legacy and custom-image rebuilds retain and fingerprint a prepared build context. Managed-image rebuilds instead stage an immutable image and startup-profile handoff, skip Dockerfile image preflight, and revalidate provider-bound workload authority before each deletion boundary. | Consent persistence, target-gateway selection/recovery, and target-preflight registry updates can precede disposable image build/probes. Backup is the first durable recovery checkpoint when available. Shields unlock, MCP detach/scrub, and NIM stop are destructive in-place effects before the **sandbox delete** boundary. Legacy and custom-image paths recheck prepared context and mutation-edge conditions before delete. Managed-image paths revalidate the exact provider-bound handoff before delete. | Durable checkpoints are the backup/recovery manifest when one exists and the rewritten recreate session; stale recovery can reach deletion without a manifest, making that session its first new durable checkpoint. Rollback receipts/snapshots are process-local. Credential metadata comes from the target or guarded fallback; raw credentials/providers are checked against current process/gateway state, while prepared installer recovery may reconstruct a missing gateway provider from a validated host credential. | In-process rollback best-effort restores registry/MCP retry metadata, but process death after non-MCP delete can still lose it. The inner onboarding consumes the exact managed-workload handoff or selects the legacy resource profile after deletion. Covered by rebuild, managed-workload authority, image-preflight, DCode, and messaging tests. Gaps: health-before-delete and atomic swap. Closed issue #5801 records the original gap; #6835 fixed only the printed recovery path. | diff --git a/src/lib/onboard/openshell-docker-sandbox-containers.test.ts b/src/lib/onboard/openshell-docker-sandbox-containers.test.ts index df081ac8bee..4516c985c46 100644 --- a/src/lib/onboard/openshell-docker-sandbox-containers.test.ts +++ b/src/lib/onboard/openshell-docker-sandbox-containers.test.ts @@ -4,7 +4,7 @@ import { describe, expect, it, vi } from "vitest"; import { queryOpenShellDockerSandboxRuntimeSnapshot, - removeExactOpenShellDockerSandboxContainer, + removeExactOpenShellDockerSandboxContainers, } from "./openshell-docker-sandbox-containers"; const IMAGE_ID = `sha256:${"a".repeat(64)}`; @@ -13,24 +13,121 @@ const EMPTY_RUNTIME_FIELDS = [IMAGE_ID, BOOKKEEPING_IMAGE_REF, "", null, [], "ru const ACTIVATED_CONTAINER_ID = "b".repeat(64); const ROLLBACK_CONTAINER_ID = "c".repeat(64); -describe("removeExactOpenShellDockerSandboxContainer", () => { +function observeContainerIds(ids: readonly string[], malformedRows = 0) { + return { + status: "observed" as const, + rows: ids.map((id) => ({ + id, + managedBy: "openshell", + workspace: "default", + sandboxId: "sb-alpha", + })), + malformedRows, + }; +} + +describe("removeExactOpenShellDockerSandboxContainers", () => { it("fails when Docker cannot confirm the exact container is absent after removal (#9073)", () => { const expectedContainerId = "a".repeat(64); - const queryContainers = vi + const inspectContainers = vi .fn() - .mockReturnValueOnce({ ok: true, ids: [expectedContainerId] }) - .mockReturnValueOnce({ ok: true, ids: [expectedContainerId] }); + .mockReturnValueOnce(observeContainerIds([expectedContainerId])) + .mockReturnValueOnce(observeContainerIds([expectedContainerId])); const forceRemove = vi.fn(() => ({ status: 0 })); expect(() => - removeExactOpenShellDockerSandboxContainer("alpha", expectedContainerId, vi.fn(), { - queryContainers, - forceRemove, - }), + removeExactOpenShellDockerSandboxContainers( + "alpha", + [expectedContainerId], + vi.fn(), + { inspectContainers, forceRemove }, + ), ).toThrow("could not confirm exact Docker container removal"); expect(forceRemove).toHaveBeenCalledWith(expectedContainerId); }); + + it("removes every remaining container from one exact failed attempt (#10547)", () => { + const expectedContainerIds = ["a".repeat(64), "b".repeat(64)]; + let currentContainerIds = [...expectedContainerIds]; + const inspectContainers = vi.fn(() => observeContainerIds(currentContainerIds)); + const forceRemove = vi.fn((containerId: string) => { + currentContainerIds = currentContainerIds.filter((candidate) => candidate !== containerId); + return { status: 0 }; + }); + + removeExactOpenShellDockerSandboxContainers( + "alpha", + expectedContainerIds, + vi.fn(), + { inspectContainers, forceRemove }, + ); + + expect(forceRemove.mock.calls.map(([containerId]) => containerId)).toEqual( + expectedContainerIds, + ); + expect(currentContainerIds).toEqual([]); + }); + + it("continues cleanup when an earlier exact container is already absent (#10547)", () => { + const alreadyRemovedId = "a".repeat(64); + const remainingId = "b".repeat(64); + let currentContainerIds = [remainingId]; + const inspectContainers = vi.fn(() => observeContainerIds(currentContainerIds)); + const forceRemove = vi.fn((containerId: string) => { + currentContainerIds = currentContainerIds.filter((candidate) => candidate !== containerId); + return { status: 0 }; + }); + + removeExactOpenShellDockerSandboxContainers( + "alpha", + [alreadyRemovedId, remainingId], + vi.fn(), + { inspectContainers, forceRemove }, + ); + + expect(forceRemove).toHaveBeenCalledExactlyOnceWith(remainingId); + expect(currentContainerIds).toEqual([]); + }); + + it("does not remove a container outside the retained identity set (#10547)", () => { + const expectedContainerId = "a".repeat(64); + const replacementContainerId = "b".repeat(64); + const forceRemove = vi.fn(() => ({ status: 0 })); + + expect(() => + removeExactOpenShellDockerSandboxContainers( + "alpha", + [expectedContainerId], + vi.fn(), + { + inspectContainers: vi.fn(() => observeContainerIds([replacementContainerId])), + forceRemove, + }, + ), + ).toThrow("refusing replacement cleanup"); + + expect(forceRemove).not.toHaveBeenCalled(); + }); + + it("rejects malformed Docker identity output during exact cleanup (#10547)", () => { + const expectedContainerId = "a".repeat(64); + const forceRemove = vi.fn(() => ({ status: 0 })); + + expect(() => + removeExactOpenShellDockerSandboxContainers( + "alpha", + [expectedContainerId], + vi.fn(), + { + inspectContainers: vi.fn(() => observeContainerIds([], 1)), + forceRemove, + }, + ), + ).toThrow("malformed container identity row"); + + expect(forceRemove).not.toHaveBeenCalled(); + }); }); function querySnapshot(fields: unknown, nvidiaVisibleDevices?: string) { diff --git a/src/lib/onboard/openshell-docker-sandbox-containers.ts b/src/lib/onboard/openshell-docker-sandbox-containers.ts index b5d6cb38663..ef111b4a347 100644 --- a/src/lib/onboard/openshell-docker-sandbox-containers.ts +++ b/src/lib/onboard/openshell-docker-sandbox-containers.ts @@ -2,6 +2,10 @@ // SPDX-License-Identifier: Apache-2.0 import { dockerCapture, dockerRun } from "../adapters/docker"; +import { + inspectDockerSandboxIdentities, + type DockerSandboxIdentityObservation, +} from "../adapters/docker/inspect"; import type { DockerGpuPatchDeps } from "./docker-gpu-patch-types"; export const OPENSHELL_MANAGED_BY_LABEL = "openshell.ai/managed-by"; @@ -9,22 +13,23 @@ export const OPENSHELL_MANAGED_BY_VALUE = "openshell"; export const OPENSHELL_SANDBOX_NAME_LABEL = "openshell.ai/sandbox-name"; export const OPENSHELL_SANDBOX_ID_LABEL = "openshell.ai/sandbox-id"; export const OPENSHELL_SANDBOX_NAMESPACE_LABEL = "openshell.ai/sandbox-namespace"; +export const OPENSHELL_SANDBOX_WORKSPACE_LABEL = "openshell.ai/sandbox-workspace"; const DOCKER_SANDBOX_QUERY_TIMEOUT_MS = 30_000; const STALE_DOCKER_ORPHAN_TIMEOUT_MS = 30_000; type DockerSandboxContainerQueryDeps = Pick; -function sandboxContainerFilterArgs(sandboxName: string, sandboxNamespace?: string): string[] { - const args = [ - "ps", - "-a", - "--no-trunc", - "--filter", - `label=${OPENSHELL_MANAGED_BY_LABEL}=${OPENSHELL_MANAGED_BY_VALUE}`, - "--filter", - `label=${OPENSHELL_SANDBOX_NAME_LABEL}=${sandboxName}`, - ]; +function sandboxContainerFilterArgs( + sandboxName: string, + sandboxNamespace?: string, + requireManagedBy = true, +): string[] { + const args = ["ps", "-a", "--no-trunc"]; + if (requireManagedBy) { + args.push("--filter", `label=${OPENSHELL_MANAGED_BY_LABEL}=${OPENSHELL_MANAGED_BY_VALUE}`); + } + args.push("--filter", `label=${OPENSHELL_SANDBOX_NAME_LABEL}=${sandboxName}`); if (sandboxNamespace !== undefined) { args.push("--filter", `label=${OPENSHELL_SANDBOX_NAMESPACE_LABEL}=${sandboxNamespace}`); } @@ -58,34 +63,21 @@ export type OpenShellDockerSandboxContainerQuery = | { ok: true; ids: string[] } | { ok: false; ids: []; error: string }; -/** - * Status-bearing lookup used when an empty container list is a safety proof. - * Unlike the best-effort discovery helper, this distinguishes Docker failure - * from a successful query with zero labeled matches. - */ -export function queryOpenShellDockerSandboxContainers( - sandboxName: string, - deps: DockerSandboxContainerQueryDeps = {}, - timeoutMs: number = DOCKER_SANDBOX_QUERY_TIMEOUT_MS, - sandboxNamespace?: string, +function queryDockerSandboxContainerIds( + filterArgs: readonly string[], + deps: DockerSandboxContainerQueryDeps, + timeoutMs: number, ): OpenShellDockerSandboxContainerQuery { const run = deps.dockerRun ?? dockerRun; const requestedTimeoutMs = Number.isFinite(timeoutMs) && timeoutMs > 0 ? Math.floor(timeoutMs) : DOCKER_SANDBOX_QUERY_TIMEOUT_MS; - const boundedTimeoutMs = Math.max( - 1, - Math.min(DOCKER_SANDBOX_QUERY_TIMEOUT_MS, requestedTimeoutMs), - ); - const result = run( - [...sandboxContainerFilterArgs(sandboxName, sandboxNamespace), "--format", "{{.ID}}"], - { - ignoreError: true, - suppressOutput: true, - timeout: boundedTimeoutMs, - }, - ); + const result = run([...filterArgs, "--format", "{{.ID}}"], { + ignoreError: true, + suppressOutput: true, + timeout: Math.max(1, Math.min(DOCKER_SANDBOX_QUERY_TIMEOUT_MS, requestedTimeoutMs)), + }); if (Number(result.status ?? 1) !== 0) { return { ok: false, @@ -93,11 +85,31 @@ export function queryOpenShellDockerSandboxContainers( error: commandResultText(result) || "docker ps did not complete successfully", }; } - const ids = String(result.stdout ?? "") - .split(/\r?\n/) - .map((line) => line.trim()) - .filter(Boolean); - return { ok: true, ids }; + return { + ok: true, + ids: String(result.stdout ?? "") + .split(/\r?\n/) + .map((line) => line.trim()) + .filter(Boolean), + }; +} + +/** + * Status-bearing lookup used when an empty container list is a safety proof. + * Unlike the best-effort discovery helper, this distinguishes Docker failure + * from a successful query with zero labeled matches. + */ +export function queryOpenShellDockerSandboxContainers( + sandboxName: string, + deps: DockerSandboxContainerQueryDeps = {}, + timeoutMs: number = DOCKER_SANDBOX_QUERY_TIMEOUT_MS, + sandboxNamespace?: string, +): OpenShellDockerSandboxContainerQuery { + return queryDockerSandboxContainerIds( + sandboxContainerFilterArgs(sandboxName, sandboxNamespace), + deps, + timeoutMs, + ); } type StaleDockerOrphanCleanupDeps = { @@ -105,41 +117,87 @@ type StaleDockerOrphanCleanupDeps = { forceRemove?: (containerId: string) => { status?: number | null }; }; -/** Remove only the Docker container whose immutable ID passed the caller's authority check. */ -export function removeExactOpenShellDockerSandboxContainer( +type ExactDockerContainerCleanupDeps = { + inspectContainers?: (sandboxName: string) => DockerSandboxIdentityObservation; + forceRemove?: (containerId: string) => { status?: number | null }; +}; + +export function inspectDockerSandboxNameLabeledContainers( sandboxName: string, - expectedContainerId: string, +): DockerSandboxIdentityObservation { + return inspectDockerSandboxIdentities(`${OPENSHELL_SANDBOX_NAME_LABEL}=${sandboxName}`, { + managedBy: OPENSHELL_MANAGED_BY_LABEL, + workspace: OPENSHELL_SANDBOX_WORKSPACE_LABEL, + sandboxId: OPENSHELL_SANDBOX_ID_LABEL, + }); +} + +function exactCleanupContainerIds( + observation: DockerSandboxIdentityObservation, + phase: "inspect" | "confirm", +): string[] { + const failurePrefix = + phase === "inspect" + ? "could not inspect the exact Docker cleanup target" + : "could not confirm exact Docker container removal"; + if (observation.status === "probe-failed") { + throw new Error(`${failurePrefix}: ${observation.detail || "Docker identity probe failed"}`); + } + if (observation.malformedRows > 0) { + throw new Error( + `${failurePrefix}: Docker returned ${String(observation.malformedRows)} malformed container identity row(s)`, + ); + } + return observation.rows.map((row) => row.id); +} + +/** Remove the remaining members of one immutable, prequalified Docker container set. */ +export function removeExactOpenShellDockerSandboxContainers( + sandboxName: string, + expectedContainerIds: readonly string[], log: (message: string) => void, - deps: StaleDockerOrphanCleanupDeps = {}, + deps: ExactDockerContainerCleanupDeps = {}, ): void { - const queryContainers = deps.queryContainers ?? queryOpenShellDockerSandboxContainers; - const initial = queryContainers(sandboxName); - if (!initial.ok) { - throw new Error(`could not inspect the exact Docker cleanup target: ${initial.error}`); + const expected = new Set(expectedContainerIds); + if ( + expected.size !== expectedContainerIds.length || + expectedContainerIds.some((containerId) => !/^[0-9a-f]{12,64}$/iu.test(containerId)) + ) { + throw new Error("exact Docker cleanup contains an invalid or duplicate container identity"); } - if (initial.ids.length === 0) return; - if (initial.ids.length !== 1 || initial.ids[0] !== expectedContainerId) { + + // Recovery retirement requires the mutable name itself to be unambiguous. + // Inspect every name-labeled container, including foreign containers that do + // not carry OpenShell's managed-by marker, while removing only qualified IDs. + const inspectContainers = deps.inspectContainers ?? inspectDockerSandboxNameLabeledContainers; + const initialIds = exactCleanupContainerIds(inspectContainers(sandboxName), "inspect"); + const unexpected = initialIds.filter((containerId) => !expected.has(containerId)); + if (unexpected.length > 0) { throw new Error( - `expected exactly labeled Docker container '${expectedContainerId}', found ` + - `${initial.ids.length === 0 ? "none" : initial.ids.join(", ")}; refusing replacement cleanup`, + `found labeled Docker container(s) outside the retained identity set: ${unexpected.join(", ")}; refusing replacement cleanup`, ); } - const removal = deps.forceRemove - ? deps.forceRemove(expectedContainerId) - : dockerRun(["rm", "-f", expectedContainerId], { - ignoreError: true, - suppressOutput: true, - timeout: STALE_DOCKER_ORPHAN_TIMEOUT_MS, - }); - if (Number(removal.status ?? 1) !== 0) { - throw new Error(`could not remove exact Docker container '${expectedContainerId}'`); + const remaining = new Set(initialIds); + for (const containerId of expectedContainerIds) { + if (!remaining.has(containerId)) continue; + const removal = deps.forceRemove + ? deps.forceRemove(containerId) + : dockerRun(["rm", "-f", containerId], { + ignoreError: true, + suppressOutput: true, + timeout: STALE_DOCKER_ORPHAN_TIMEOUT_MS, + }); + if (Number(removal.status ?? 1) !== 0) { + throw new Error(`could not remove exact Docker container '${containerId}'`); + } + log(`Removed exact Docker container '${containerId}' after OpenShell sandbox deletion`); } - const confirmed = queryContainers(sandboxName); - if (!confirmed.ok || confirmed.ids.length !== 0) { + + const confirmedIds = exactCleanupContainerIds(inspectContainers(sandboxName), "confirm"); + if (confirmedIds.length !== 0) { throw new Error("could not confirm exact Docker container removal"); } - log(`Removed exact Docker container '${expectedContainerId}' after OpenShell sandbox deletion`); } /** diff --git a/src/lib/onboard/sandbox-create/orchestration.test.ts b/src/lib/onboard/sandbox-create/orchestration.test.ts index 2b3bf8d4aab..e87c98387ba 100644 --- a/src/lib/onboard/sandbox-create/orchestration.test.ts +++ b/src/lib/onboard/sandbox-create/orchestration.test.ts @@ -912,6 +912,7 @@ describe("sandbox create policy authority checks", () => { it("removes temporary sources but preserves the sandbox after final authority failure (#9833)", async () => { const events: string[] = []; + const createAttemptNonce = "c".repeat(62); const revalidate = vi.fn(() => events.push("create-check")); const error = await runSandboxCreateWithPolicyAuthorityChecks({ @@ -923,6 +924,7 @@ describe("sandbox create policy authority checks", () => { return "created"; }, ...exactIdentityBoundary(), + captureCreatedSandboxCreateAttemptNonce: () => createAttemptNonce, revalidateVerifiedPolicy: () => { events.push("ready-check"); throw new Error("external policy authority changed"); @@ -933,7 +935,7 @@ describe("sandbox create policy authority checks", () => { expect(error).toBeInstanceOf(AggregateError); expect((error as AggregateError).message).toMatch( new RegExp( - `left sandbox 'alpha' in place.*identity fingerprint: ${exactIdentity}.*did not run OpenShell's mutable-name deletion command.*Do not delete the sandbox by mutable sandbox name.*OpenShell administrator.*identity-bound recovery or removal procedure`, + `Create-attempt label: ai\\.nvidia\\.nemoclaw\\.create-attempt=${createAttemptNonce}.*left sandbox 'alpha' in place.*identity fingerprint: ${exactIdentity}.*did not run OpenShell's mutable-name deletion command.*Do not delete the sandbox by mutable sandbox name.*OpenShell administrator.*identity-bound recovery or removal procedure`, "u", ), ); @@ -942,7 +944,7 @@ describe("sandbox create policy authority checks", () => { expect.objectContaining({ message: expect.stringMatching( new RegExp( - `left sandbox 'alpha' in place.*identity fingerprint: ${exactIdentity}.*did not run OpenShell's mutable-name deletion command.*Do not delete the sandbox by mutable sandbox name.*OpenShell administrator.*identity-bound recovery or removal procedure`, + `Create-attempt label: ai\\.nvidia\\.nemoclaw\\.create-attempt=${createAttemptNonce}.*left sandbox 'alpha' in place.*identity fingerprint: ${exactIdentity}.*did not run OpenShell's mutable-name deletion command.*Do not delete the sandbox by mutable sandbox name.*OpenShell administrator.*identity-bound recovery or removal procedure`, "u", ), ), diff --git a/src/lib/onboard/sandbox-create/orchestration.ts b/src/lib/onboard/sandbox-create/orchestration.ts index f29693d3197..09b757cf655 100644 --- a/src/lib/onboard/sandbox-create/orchestration.ts +++ b/src/lib/onboard/sandbox-create/orchestration.ts @@ -12,6 +12,7 @@ import { } from "../../adapters/openshell/policy-authority"; import type { SandboxPolicyAuthority } from "../../adapters/openshell/policy-authority"; import { HERMES_PORTABLE_OPENSHELL_VERSION } from "../../adapters/openshell/resolve-shared"; +import { NEMOCLAW_CREATE_ATTEMPT_LABEL } from "../../adapters/openshell/sandbox-identity"; import type { AgentDefinition } from "../../agent/defs"; import type { WebSearchConfig } from "../../inference/web-search"; import type { SandboxMessagingPlan } from "../../messaging/manifest"; @@ -33,6 +34,7 @@ import type { } from "../managed-workload/hermes-state-volume"; import type { OwnedSandboxRecreateRuntime } from "../onboard-recreate-journal"; import type { SandboxGpuConfig } from "../sandbox-gpu-mode"; +import { cliName } from "../branding"; import type { CreatedSandboxLifecycle, CreatedSandboxLifecycleRegistration, @@ -215,9 +217,11 @@ export function persistPostCreateRecovery(input: { ) => unknown | null; }): void { const message = + `Create-attempt label: ${NEMOCLAW_CREATE_ATTEMPT_LABEL}=${input.recoveryContext.createAttemptNonce}. ` + `Sandbox '${input.sandboxName}' was retained after ${input.stage} failed. ` + `Gateway '${input.gatewayName}'. Lifecycle generation '${input.lifecycleGeneration}'. ` + "Do not delete the sandbox by mutable name; preserve it for identity-bound administrator recovery."; + console.error(` ${message}`); let persisted = false; try { persisted = persistRetainedSandboxRecoveryMessage( @@ -655,6 +659,7 @@ export async function runSandboxCreateWithPolicyAuthorityChecks< readonly revalidate: (sandboxIsLive: boolean, operation: string) => void; readonly create: (verifyCreatedSandbox: (created: Created) => Promise) => Promise; readonly captureCreatedSandboxIdentity: (created: Created) => string; + readonly captureCreatedSandboxCreateAttemptNonce?: (created: Created) => string; readonly persistCreatedSandboxIdentity: (created: Created, exactIdentity: string) => void; readonly revalidateCreatedSandboxIdentity: (expectedIdentity: string, operation: string) => void; readonly verifyCreatedPolicy: (created: Created, exactIdentity: string) => Evidence; @@ -685,6 +690,7 @@ export async function runSandboxCreateWithPolicyAuthorityChecks< }): Promise { input.revalidate(false, `creating sandbox '${input.sandboxName}'`); let exactIdentity: string | null = null; + let createAttemptNonce: string | null = null; let observedPolicyEvidence: Evidence | null = null; let observedCreatedSandbox: Created | null = null; let cleanupAttempted = false; @@ -708,10 +714,15 @@ export async function runSandboxCreateWithPolicyAuthorityChecks< const identityGuidance = exactIdentity ? `Durable sandbox identity fingerprint: ${exactIdentity}. Use it only to compare the surviving sandbox with the failed create.` : "OpenShell did not return a durable sandbox identity fingerprint for comparison."; + const createAttemptGuidance = createAttemptNonce + ? `Create-attempt label: ${NEMOCLAW_CREATE_ATTEMPT_LABEL}=${createAttemptNonce}. ` + : ""; const recoveryGuidance = + createAttemptGuidance + `NemoClaw left sandbox '${input.sandboxName}' in place after post-create verification or finalization failed. ` + `${identityGuidance} NemoClaw did not run OpenShell's mutable-name deletion command because the name may now identify a replacement sandbox. ` + - "Do not delete the sandbox by mutable sandbox name. Ask the OpenShell administrator to inspect the surviving sandbox and use an identity-bound recovery or removal procedure."; + `Do not delete the sandbox by mutable sandbox name. Run '${cliName()} ${input.sandboxName} destroy' to use the retained identity. ` + + "If destroy cannot prove that identity, stop. Ask the OpenShell administrator to inspect the surviving sandbox and use an identity-bound recovery or removal procedure."; const compensationErrors: unknown[] = []; if (input.persistRetainedSandboxRecovery) { try { @@ -737,6 +748,16 @@ export async function runSandboxCreateWithPolicyAuthorityChecks< const verifyCreatedSandbox = async (created: Created): Promise => { observedCreatedSandbox = created; try { + const capturedCreateAttemptNonce = input.captureCreatedSandboxCreateAttemptNonce?.(created); + if ( + capturedCreateAttemptNonce !== undefined && + !/^[0-9a-f]{62}$/u.test(capturedCreateAttemptNonce) + ) { + throw new Error( + `OpenShell did not return one exact create-attempt label for sandbox '${input.sandboxName}'.`, + ); + } + createAttemptNonce = capturedCreateAttemptNonce ?? null; const capturedIdentity = input.captureCreatedSandboxIdentity(created); if (!/^[0-9a-f]{64}$/u.test(capturedIdentity)) { throw new Error( @@ -2633,6 +2654,9 @@ export function createSandboxWithBaseImageResolution(runtime: SandboxCreateOrche captureCreatedSandboxIdentity: ( identity: import("../sandbox-gpu-create-flow").CreatedSandboxIdentity, ) => identity.liveIdentityFingerprint, + captureCreatedSandboxCreateAttemptNonce: ( + identity: import("../sandbox-gpu-create-flow").CreatedSandboxIdentity, + ) => identity.createAttemptNonce, persistCreatedSandboxIdentity: (_identity, exactIdentity) => persistCreatedSandboxIdentity(exactIdentity), revalidateCreatedSandboxIdentity, diff --git a/src/lib/state/legacy-port-migration.test.ts b/src/lib/state/legacy-port-migration.test.ts index 1036a34848d..fa20608c1f5 100644 --- a/src/lib/state/legacy-port-migration.test.ts +++ b/src/lib/state/legacy-port-migration.test.ts @@ -7,7 +7,15 @@ import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { + type OnboardEntryOptionsDeps, + resolveOnboardEntryOptions, +} from "../onboard/entry-options"; import { migrateLegacyPortState } from "./legacy-port-migration"; +import { + listRetainedSandboxRecoveryRecords, + recordRetainedSandboxRecovery, +} from "./onboard-session/retained-sandbox-recovery"; const homes: string[] = []; @@ -26,13 +34,62 @@ function readJson(filePath: string): Record { return JSON.parse(fs.readFileSync(filePath, "utf8")) as Record; } +function recordRecovery( + filePath: string, + sandboxName: string, + gatewayPort: number, + seed: string, +): void { + recordRetainedSandboxRecovery(filePath, { + sandboxName, + sandboxIdentityFingerprint: seed.repeat(64), + gatewayName: gatewayPort === 8080 ? "nemoclaw" : `nemoclaw-${String(gatewayPort)}`, + gatewayPort, + lifecycleGeneration: `generation-${seed}`, + verifiedEffectivePolicyIdentity: null, + createAttemptNonce: seed.repeat(62), + policyCreationReceipt: null, + reason: "retained_after_sandbox_creation_failure", + recordedAt: "2026-08-29T00:00:00.000Z", + }); +} + +function expectRetainedNameBlocked( + records: readonly { readonly sandboxName: string }[], + sandboxName: string, +): void { + const entryDeps: OnboardEntryOptionsDeps = { + isNonInteractive: () => false, + validateName: (name) => name, + reservedSandboxNames: new Set(), + cliDisplayName: () => "NemoClaw", + getNameValidationGuidance: () => [], + error: vi.fn(), + exitProcess: vi.fn(() => { + throw new Error("blocked retained recovery name"); + }), + }; + expect(() => + resolveOnboardEntryOptions( + { + opts: { fresh: true, sandboxName }, + env: {}, + stdinIsTty: true, + stdoutIsTty: true, + retainedRecoverySandboxNames: records.map((record) => record.sandboxName), + }, + entryDeps, + ), + ).toThrow("blocked retained recovery name"); +} + afterEach(() => { vi.restoreAllMocks(); for (const home of homes.splice(0)) fs.rmSync(home, { recursive: true, force: true }); }); describe("legacy non-default gateway state migration", () => { - it("partitions a selected registry and moves identity-bound session, credentials, and snapshots", () => { + it("partitions a recovery-only session and mixed-gateway recovery authority", () => { const home = makeHome(); const shared = path.join(home, ".nemoclaw"); const selected = path.join(shared, "gateways", "9123"); @@ -56,9 +113,12 @@ describe("legacy non-default gateway state migration", () => { }); writeJson(path.join(shared, "onboard-session.json"), { sandboxName: "port-box", - status: "in_progress", + status: "recovery_required", metadata: { gatewayName: "nemoclaw-9123" }, }); + const sharedRecovery = path.join(shared, "retained-sandbox-recovery.json"); + recordRecovery(sharedRecovery, "port-box", 9123, "a"); + recordRecovery(sharedRecovery, "default-box", 8080, "b"); writeJson(path.join(shared, "credentials.json"), { NVIDIA_API_KEY: "legacy-secret" }); writeJson(path.join(shared, "usage-notice.json"), { acceptedVersion: "1" }); writeJson(path.join(shared, "state", "default-forward.json"), { pid: 123 }); @@ -80,6 +140,14 @@ describe("legacy non-default gateway state migration", () => { ).toEqual(["port-box"]); expect(fs.existsSync(path.join(shared, "onboard-session.json"))).toBe(false); expect(fs.existsSync(path.join(selected, "onboard-session.json"))).toBe(true); + const selectedRecoveryRecords = listRetainedSandboxRecoveryRecords( + path.join(selected, "retained-sandbox-recovery.json"), + ); + const remainingRecoveryRecords = listRetainedSandboxRecoveryRecords(sharedRecovery); + expect(selectedRecoveryRecords.map((record) => record.sandboxName)).toEqual(["port-box"]); + expect(remainingRecoveryRecords.map((record) => record.sandboxName)).toEqual(["default-box"]); + expectRetainedNameBlocked(selectedRecoveryRecords, "port-box"); + expectRetainedNameBlocked(remainingRecoveryRecords, "default-box"); expect(fs.existsSync(path.join(shared, "credentials.json"))).toBe(false); expect(fs.existsSync(path.join(selected, "credentials.json"))).toBe(true); expect(fs.existsSync(path.join(shared, "usage-notice.json"))).toBe(true); @@ -122,6 +190,79 @@ describe("legacy non-default gateway state migration", () => { expect(fs.existsSync(path.join(shared, "gateways", "9123", "sandboxes.json"))).toBe(false); }); + it("refuses conflicting retained recovery identity without mutating state", () => { + const home = makeHome(); + const shared = path.join(home, ".nemoclaw"); + const recoveryFile = path.join(shared, "retained-sandbox-recovery.json"); + recordRetainedSandboxRecovery(recoveryFile, { + sandboxName: "port-box", + sandboxIdentityFingerprint: "c".repeat(64), + gatewayName: "nemoclaw-9124", + gatewayPort: 9123, + lifecycleGeneration: "generation-c", + verifiedEffectivePolicyIdentity: null, + createAttemptNonce: "c".repeat(62), + policyCreationReceipt: null, + reason: "retained_after_sandbox_creation_failure", + recordedAt: "2026-08-29T00:00:00.000Z", + }); + const before = fs.readFileSync(recoveryFile, "utf8"); + + expect(() => migrateLegacyPortState({ home, gatewayPort: 9123 })).toThrow( + /conflicting gateway identity/, + ); + expect(fs.readFileSync(recoveryFile, "utf8")).toBe(before); + expect(fs.existsSync(path.join(shared, "gateways", "9123"))).toBe(false); + }); + + it("refuses an older published intent that omitted retained recovery", () => { + const home = makeHome(); + const shared = path.join(home, ".nemoclaw"); + const migration = path.join(shared, ".gateway-state-migration"); + const legacyRegistry = path.join(shared, "sandboxes.json"); + const recoveryFile = path.join(shared, "retained-sandbox-recovery.json"); + writeJson(legacyRegistry, { + defaultSandbox: "default-box", + sandboxes: { + "default-box": { name: "default-box", gatewayName: "nemoclaw", gatewayPort: 8080 }, + "port-box": { name: "port-box", gatewayName: "nemoclaw-9123", gatewayPort: 9123 }, + }, + }); + writeJson(path.join(migration, "intent.json"), { + version: 1, + gatewayPort: 9123, + selectedSandboxNames: ["port-box"], + sandboxBackupNames: [], + moveSession: false, + bundleEntries: [], + warnAmbiguousSession: false, + rewriteLegacyRegistry: true, + }); + writeJson(path.join(migration, "selected-registry.json"), { + defaultSandbox: "port-box", + sandboxes: { + "port-box": { name: "port-box", gatewayName: "nemoclaw-9123", gatewayPort: 9123 }, + }, + }); + writeJson(path.join(migration, "remaining-registry.json"), { + defaultSandbox: "default-box", + sandboxes: { + "default-box": { name: "default-box", gatewayName: "nemoclaw", gatewayPort: 8080 }, + }, + }); + recordRecovery(recoveryFile, "port-box", 9123, "f"); + const registryBefore = fs.readFileSync(legacyRegistry, "utf8"); + const recoveryBefore = fs.readFileSync(recoveryFile, "utf8"); + + expect(() => migrateLegacyPortState({ home, gatewayPort: 9123 })).toThrow( + /intent predates retained recovery partitioning/, + ); + expect(fs.readFileSync(legacyRegistry, "utf8")).toBe(registryBefore); + expect(fs.readFileSync(recoveryFile, "utf8")).toBe(recoveryBefore); + expect(fs.existsSync(migration)).toBe(true); + expect(fs.existsSync(path.join(shared, "gateways", "9123"))).toBe(false); + }); + it("preflights backup collisions before publishing the selected registry", () => { const home = makeHome(); const shared = path.join(home, ".nemoclaw"); @@ -169,6 +310,7 @@ describe("legacy non-default gateway state migration", () => { "port-box": { name: "port-box", gatewayName: "nemoclaw-9123", gatewayPort: 9123 }, }, }); + recordRecovery(path.join(shared, "retained-sandbox-recovery.json"), "port-box", 9123, "e"); writeJson(path.join(backupSource, "snapshot", "manifest.json"), {}); const renameSync = fs.renameSync.bind(fs); @@ -188,6 +330,12 @@ describe("legacy non-default gateway state migration", () => { expect(Object.keys(readJson(legacyRegistry).sandboxes as object)).toEqual(["default-box"]); expect(fs.existsSync(selectedRegistry)).toBe(false); + expect(fs.existsSync(path.join(shared, "retained-sandbox-recovery.json"))).toBe(false); + expect( + listRetainedSandboxRecoveryRecords( + path.join(selected, "retained-sandbox-recovery.json"), + ).map((record) => record.sandboxName), + ).toEqual(["port-box"]); expect(fs.existsSync(path.join(shared, ".gateway-state-migration"))).toBe(true); expect(() => migrateLegacyPortState({ home, gatewayPort: 8080 })).toThrow( /recoverable migration for gateway port 9123 is pending/, @@ -214,9 +362,10 @@ describe("legacy non-default gateway state migration", () => { }, ); - it("partitions provable rows but leaves credentials whose gateway ownership is ambiguous", () => { + it("partitions provable rows and recovery without a session but leaves credentials", () => { const home = makeHome(); const shared = path.join(home, ".nemoclaw"); + const selected = path.join(shared, "gateways", "9123"); writeJson(path.join(shared, "sandboxes.json"), { defaultSandbox: "default-box", sandboxes: { @@ -224,6 +373,7 @@ describe("legacy non-default gateway state migration", () => { "port-box": { name: "port-box", gatewayName: "nemoclaw-9123", gatewayPort: 9123 }, }, }); + recordRecovery(path.join(shared, "retained-sandbox-recovery.json"), "port-box", 9123, "d"); writeJson(path.join(shared, "credentials.json"), { NVIDIA_API_KEY: "ambiguous-secret" }); const result = migrateLegacyPortState({ home, gatewayPort: 9123 }); @@ -231,7 +381,13 @@ describe("legacy non-default gateway state migration", () => { expect(result.migratedSandboxNames).toEqual(["port-box"]); expect(result.warnings.join("\n")).toContain("Left ambiguous"); expect(fs.existsSync(path.join(shared, "credentials.json"))).toBe(true); - expect(fs.existsSync(path.join(shared, "gateways", "9123", "credentials.json"))).toBe(false); + expect(fs.existsSync(path.join(selected, "credentials.json"))).toBe(false); + expect(fs.existsSync(path.join(shared, "retained-sandbox-recovery.json"))).toBe(false); + const recoveryRecords = listRetainedSandboxRecoveryRecords( + path.join(selected, "retained-sandbox-recovery.json"), + ); + expect(recoveryRecords.map((record) => record.sandboxName)).toEqual(["port-box"]); + expectRetainedNameBlocked(recoveryRecords, "port-box"); }); it("moves singleton state when every legacy registry row belongs to the selected gateway", () => { @@ -253,6 +409,26 @@ describe("legacy non-default gateway state migration", () => { expect(fs.existsSync(path.join(selected, "credentials.json"))).toBe(true); }); + it.each([ + ["shared", (shared: string, _selected: string) => shared], + ["selected", (_shared: string, selected: string) => selected], + ])("refuses recovery-only migration while the %s onboarding lock is present", (_scope, root) => { + const home = makeHome(); + const shared = path.join(home, ".nemoclaw"); + const selected = path.join(shared, "gateways", "9123"); + const recoveryFile = path.join(shared, "retained-sandbox-recovery.json"); + recordRecovery(recoveryFile, "port-box", 9123, "d"); + const before = fs.readFileSync(recoveryFile, "utf8"); + fs.mkdirSync(root(shared, selected), { recursive: true }); + fs.writeFileSync(path.join(root(shared, selected), "onboard.lock"), "active writer"); + + expect(() => migrateLegacyPortState({ home, gatewayPort: 9123 })).toThrow( + /onboarding lock .* is present/u, + ); + expect(fs.readFileSync(recoveryFile, "utf8")).toBe(before); + expect(fs.existsSync(path.join(selected, "retained-sandbox-recovery.json"))).toBe(false); + }); + it.each( ["ollama-proxy-token", "ollama-proxy-port", "ollama-auth-proxy.pid"], )("keeps host-shared Ollama proxy state out of a non-default gateway migration [%s]", (entry) => { diff --git a/src/lib/state/legacy-port-migration.ts b/src/lib/state/legacy-port-migration.ts index 8bd03a686cc..be3c0017999 100644 --- a/src/lib/state/legacy-port-migration.ts +++ b/src/lib/state/legacy-port-migration.ts @@ -15,12 +15,19 @@ import { readGatewayRegistryFile, registryEntryGatewayPort, } from "./gateway-registry"; +import { + listRetainedSandboxRecoveryRecords, + retainedSandboxRecoveryFile, + type RetainedSandboxRecoveryRecord, +} from "./onboard-session/retained-sandbox-recovery"; import { nemoclawStateRoot, resolveHome } from "./state-root"; const MIGRATION_LOCK = ".gateway-state-migration.lock"; const MIGRATION_INTENT = ".gateway-state-migration"; const MIGRATION_INTENT_METADATA = "intent.json"; +const MIGRATION_INTENT_REMAINING_RECOVERY = "remaining-retained-sandbox-recovery.json"; const MIGRATION_INTENT_SELECTED_REGISTRY = "selected-registry.json"; +const MIGRATION_INTENT_SELECTED_RECOVERY = "selected-retained-sandbox-recovery.json"; const MIGRATION_INTENT_REMAINING_REGISTRY = "remaining-registry.json"; const MIGRATION_INTENT_VERSION = 1; const MIGRATION_LOCK_STALE_MS = 10_000; @@ -76,6 +83,13 @@ interface LegacyPortMigrationIntent { metadata: LegacyPortMigrationIntentMetadata; selectedRegistry: GatewayRegistryDocument; remainingRegistry: GatewayRegistryDocument | null; + selectedRecovery: RetainedRecoveryDocument | null; + remainingRecovery: RetainedRecoveryDocument | null; +} + +interface RetainedRecoveryDocument { + schemaVersion: 1; + unresolved: readonly RetainedSandboxRecoveryRecord[]; } function migrationError(message: string): Error { @@ -164,6 +178,44 @@ function readJsonNoFollow(home: string, filePath: string): unknown | null { } } +function retainedRecoveryDocument( + records: readonly RetainedSandboxRecoveryRecord[], +): RetainedRecoveryDocument { + return { schemaVersion: 1, unresolved: records }; +} + +function readRetainedRecoveryDocument( + home: string, + filePath: string, +): RetainedRecoveryDocument | null { + if (!lstatNoFollow(home, filePath)) return null; + let records: readonly RetainedSandboxRecoveryRecord[]; + try { + records = listRetainedSandboxRecoveryRecords(filePath); + } catch (error) { + throw migrationError( + `${filePath} is not valid retained sandbox recovery state: ${error instanceof Error ? error.message : String(error)}`, + ); + } + for (const record of records) { + const gatewayPort = resolveGatewayPortFromName(record.gatewayName); + if (gatewayPort === null || gatewayPort !== record.gatewayPort) { + throw migrationError( + `${filePath} record ${record.recordId} has conflicting gateway identity`, + ); + } + } + return retainedRecoveryDocument(records); +} + +function removeRetainedRecoveryFile(home: string, filePath: string): void { + const stat = lstatNoFollow(home, filePath); + if (!stat) return; + if (!stat.isFile()) throw migrationError(`${filePath} is not a regular file`); + fs.rmSync(filePath); + fsyncDirectory(path.dirname(filePath)); +} + function firstSandboxName(sandboxes: Record): string | null { return Object.keys(sandboxes).sort()[0] ?? null; } @@ -370,10 +422,19 @@ function readMigrationIntent(home: string, sharedRoot: string): LegacyPortMigrat rawMetadata.bundleEntries, "migration intent bundleEntries", ); + const selectedRecovery = readRetainedRecoveryDocument( + home, + path.join(intentDir, MIGRATION_INTENT_SELECTED_RECOVERY), + ); + const remainingRecovery = readRetainedRecoveryDocument( + home, + path.join(intentDir, MIGRATION_INTENT_REMAINING_RECOVERY), + ); if ( rawMetadata.rewriteLegacyRegistry !== selectedSandboxNames.length > 0 || (rawMetadata.moveSession && rawMetadata.warnAmbiguousSession) || - (selectedSandboxNames.length === 0 && !rawMetadata.moveSession) + (selectedSandboxNames.length === 0 && !rawMetadata.moveSession && !selectedRecovery) || + (selectedRecovery === null) !== (remainingRecovery === null) ) { throw migrationError("migration intent has inconsistent ownership metadata"); } @@ -388,6 +449,13 @@ function readMigrationIntent(home: string, sharedRoot: string): LegacyPortMigrat throw migrationError(`migration intent backup ${sandboxName} is not a selected sandbox`); } } + if ( + selectedRecovery?.unresolved.length === 0 || + selectedRecovery?.unresolved.some((record) => record.gatewayPort !== gatewayPort) || + remainingRecovery?.unresolved.some((record) => record.gatewayPort === gatewayPort) + ) { + throw migrationError("migration intent has inconsistent retained recovery ownership"); + } const selectedRegistryFile = path.join(intentDir, MIGRATION_INTENT_SELECTED_REGISTRY); const selectedRegistry = readGatewayRegistryFile(home, selectedRegistryFile); @@ -421,6 +489,17 @@ function readMigrationIntent(home: string, sharedRoot: string): LegacyPortMigrat } } } + if ( + selectedRecovery === null && + readRetainedRecoveryDocument( + home, + retainedSandboxRecoveryFile(sharedRoot), + )?.unresolved.some((record) => record.gatewayPort === gatewayPort) + ) { + throw migrationError( + "published migration intent predates retained recovery partitioning; retained recovery remains safely in the shared root", + ); + } return { intentDir, @@ -436,6 +515,8 @@ function readMigrationIntent(home: string, sharedRoot: string): LegacyPortMigrat }, selectedRegistry, remainingRegistry, + selectedRecovery, + remainingRecovery, }; } @@ -445,6 +526,8 @@ function createMigrationIntent( metadata: LegacyPortMigrationIntentMetadata, selectedRegistry: GatewayRegistryDocument, remainingRegistry: GatewayRegistryDocument | null, + selectedRecovery: RetainedRecoveryDocument | null, + remainingRecovery: RetainedRecoveryDocument | null, ): LegacyPortMigrationIntent { const intentDir = path.join(sharedRoot, MIGRATION_INTENT); if (lstatNoFollow(home, intentDir)) { @@ -455,6 +538,9 @@ function createMigrationIntent( if (metadata.rewriteLegacyRegistry && !remainingRegistry) { throw migrationError("migration intent is missing its remaining legacy registry"); } + if ((selectedRecovery === null) !== (remainingRecovery === null)) { + throw migrationError("migration intent is missing its retained recovery partition"); + } ensureRealDirectory(home, sharedRoot); const preparingDir = `${intentDir}.preparing.${String(process.pid)}.${String(Date.now())}`; @@ -474,6 +560,18 @@ function createMigrationIntent( remainingRegistry, ); } + if (selectedRecovery && remainingRecovery) { + writeJsonAtomic( + home, + path.join(preparingDir, MIGRATION_INTENT_SELECTED_RECOVERY), + selectedRecovery, + ); + writeJsonAtomic( + home, + path.join(preparingDir, MIGRATION_INTENT_REMAINING_RECOVERY), + remainingRecovery, + ); + } fsyncDirectory(preparingDir); fs.renameSync(preparingDir, intentDir); fsyncDirectory(sharedRoot); @@ -539,6 +637,22 @@ function applyMigrationIntent( writeJsonAtomic(home, legacyRegistryFile, intent.remainingRegistry); } + if (intent.selectedRecovery && intent.remainingRecovery) { + writeJsonAtomic( + home, + retainedSandboxRecoveryFile(selectedRoot), + intent.selectedRecovery, + ); + if (intent.remainingRecovery.unresolved.length > 0) { + writeJsonAtomic( + home, + retainedSandboxRecoveryFile(sharedRoot), + intent.remainingRecovery, + ); + } else { + removeRetainedRecoveryFile(home, retainedSandboxRecoveryFile(sharedRoot)); + } + } migrateSandboxBackups(home, sharedRoot, selectedRoot, intent.metadata.sandboxBackupNames); if (intent.metadata.moveSession) { resumeMovePath( @@ -628,6 +742,17 @@ function acquireDirectoryLock(home: string, lock: string): string { throw migrationError(`could not acquire ${lock}`); } +function assertOnboardStateUnlocked(home: string, stateRoots: readonly string[]): void { + for (const stateRoot of stateRoots) { + const activeLock = path.join(stateRoot, "onboard.lock"); + if (lstatNoFollow(home, activeLock)) { + throw migrationError( + `onboarding lock ${activeLock} is present; finish or stop that run before migrating state`, + ); + } + } +} + /** * Partition pre-segregation state into the selected non-default gateway root. * Registry rows move only when their persisted canonical gateway identity is @@ -669,6 +794,7 @@ export function migrateLegacyPortState( if (staleIntentDirectoriesExist) { const lock = acquireDirectoryLock(home, migrationLock); try { + assertOnboardStateUnlocked(home, [sharedRoot]); removeStaleMigrationIntentDirectories(home, sharedRoot); } finally { fs.rmSync(lock, { recursive: true, force: true }); @@ -682,10 +808,13 @@ export function migrateLegacyPortState( const legacyRegistry = readGatewayRegistryFile(home, legacyRegistryFile); const legacySessionFile = path.join(sharedRoot, "onboard-session.json"); const legacySession = readJsonNoFollow(home, legacySessionFile); + const legacyRecoveryFile = retainedSandboxRecoveryFile(sharedRoot); + const legacyRecoveryExists = lstatNoFollow(home, legacyRecoveryFile) !== null; if ( !pendingBeforeLock && !legacyRegistry && legacySession === null && + !legacyRecoveryExists && !staleIntentDirectoriesExist ) { return result; @@ -694,6 +823,10 @@ export function migrateLegacyPortState( const lock = acquireDirectoryLock(home, migrationLock); const registryLocks: string[] = []; try { + // Onboard writers recheck the migration lock after claiming onboard.lock. + // Checking both roots while this lock is held closes the opposite side of + // the handshake and serializes session/recovery state with partitioning. + assertOnboardStateUnlocked(home, [sharedRoot, selectedRoot]); removeStaleMigrationIntentDirectories(home, sharedRoot); registryLocks.push(acquireDirectoryLock(home, `${legacyRegistryFile}.lock`)); const pendingIntent = readMigrationIntent(home, sharedRoot); @@ -727,6 +860,18 @@ export function migrateLegacyPortState( } const session = readJsonNoFollow(home, legacySessionFile); + const recovery = readRetainedRecoveryDocument(home, legacyRecoveryFile); + const selectedRecoveryRecords = + recovery?.unresolved.filter((record) => record.gatewayPort === gatewayPort) ?? []; + const remainingRecoveryRecords = + recovery?.unresolved.filter((record) => record.gatewayPort !== gatewayPort) ?? []; + const selectedRecovery = + selectedRecoveryRecords.length > 0 + ? retainedRecoveryDocument(selectedRecoveryRecords) + : null; + const remainingRecovery = selectedRecovery + ? retainedRecoveryDocument(remainingRecoveryRecords) + : null; const recordedSessionPort = session === null ? null : sessionGatewayPort(session, registryPortsByName); const selectedNames = Object.keys(selectedEntries).sort(); @@ -736,7 +881,7 @@ export function migrateLegacyPortState( Object.keys(remainingEntries).length === 0 && (session === null || sessionBelongsToSelected); - if (selectedNames.length === 0 && !sessionBelongsToSelected) return result; + if (selectedNames.length === 0 && !sessionBelongsToSelected && !selectedRecovery) return result; const entriesToMove: readonly LegacyBundleEntry[] = wholeLegacyBundleBelongsToSelected ? MIGRATABLE_BUNDLE_ENTRIES @@ -745,12 +890,6 @@ export function migrateLegacyPortState( : []; let moveSession = false; if (sessionBelongsToSelected) { - const activeLock = path.join(sharedRoot, "onboard.lock"); - if (lstatNoFollow(home, activeLock)) { - throw migrationError( - `legacy onboarding lock ${activeLock} is present; finish or stop that run first`, - ); - } moveSession = preflightMovePath( home, legacySessionFile, @@ -769,6 +908,13 @@ export function migrateLegacyPortState( bundleEntries.push(entry); } } + if (selectedRecovery) { + preflightMovePath( + home, + legacyRecoveryFile, + retainedSandboxRecoveryFile(selectedRoot), + ); + } registryLocks.push(acquireDirectoryLock(home, `${selectedRegistryFile}.lock`)); const existingSelected = readGatewayRegistryFile(home, selectedRegistryFile); @@ -798,6 +944,8 @@ export function migrateLegacyPortState( }, selectedRegistry, remainingRegistry, + selectedRecovery, + remainingRecovery, ); return applyMigrationIntent( home, diff --git a/src/lib/state/onboard-session-cross-process-lock.test.ts b/src/lib/state/onboard-session-cross-process-lock.test.ts index aa37b509ea0..8abb59ab1be 100644 --- a/src/lib/state/onboard-session-cross-process-lock.test.ts +++ b/src/lib/state/onboard-session-cross-process-lock.test.ts @@ -39,6 +39,14 @@ afterEach(() => { }); describe("cross-process onboard lock", () => { + it("releases its lock when legacy-state migration already owns the handshake", () => { + const migrationLock = path.join(tempHome, ".nemoclaw", ".gateway-state-migration.lock"); + fs.mkdirSync(migrationLock, { recursive: true }); + + expect(session.acquireOnboardLock("nemoclaw onboard").acquired).toBe(false); + expect(fs.existsSync(session.LOCK_FILE)).toBe(false); + }); + it("rejects caller-asserted onboarding lock ownership without a live descriptor (#9833)", async () => { const authority = await import("../onboard/portable-retirement-authority"); @@ -346,11 +354,6 @@ describe("cross-process onboard lock", () => { verifiedEffectivePolicyIdentity: null, createAttemptNonce: "c".repeat(62), policyCreationReceipt: null, - resources: { - sharedInferenceProviders: [], - sandboxScopedProviders: [], - credentialEnvironmentVariables: [], - }, reason: "retained_after_sandbox_creation_failure", }); process.stdout.write(JSON.stringify({ ok: true, recordId: recorded.recordId })); diff --git a/src/lib/state/onboard-session.ts b/src/lib/state/onboard-session.ts index 01ba28aace3..1f247e9bdb9 100644 --- a/src/lib/state/onboard-session.ts +++ b/src/lib/state/onboard-session.ts @@ -14,17 +14,12 @@ import path from "node:path"; import type { SandboxPolicyAuthority } from "../adapters/openshell/policy-authority"; import { isErrnoException } from "../core/errno"; import { isObjectRecord, type JsonObject, type JsonValue } from "../core/json-types"; -import { GATEWAY_PORT } from "../core/ports"; +import { DEFAULT_GATEWAY_PORT, GATEWAY_PORT } from "../core/ports"; import { parseServingProfileProvenance, type ServingProfileProvenance, } from "../inference/serving/profile-provenance"; -import { - normalizeWebSearchConfig, - webSearchEnvFor, - webSearchProviderForConfig, - type WebSearchConfig, -} from "../inference/web-search"; +import { normalizeWebSearchConfig, type WebSearchConfig } from "../inference/web-search"; import type { SandboxMessagingPlan } from "../messaging/manifest"; import { compactSandboxMessagingPlanForPersistence } from "../messaging/persistence"; import { parseSandboxMessagingPlan } from "../messaging/plan-validation"; @@ -65,7 +60,9 @@ import { listRetainedSandboxRecoveryRecords as readRetainedSandboxRecoveryRecords, parseNemoClawPolicyCreationReceipt, recordRetainedSandboxRecovery as writeRetainedSandboxRecovery, + retainedSandboxRecoveryAuthorityIsCurrent, retainedSandboxRecoveryFile, + resolveRetainedSandboxRecovery as retireRetainedSandboxRecovery, type RecordRetainedSandboxRecoveryInput, type RetainedSandboxRecoveryRecord, type RetainedSandboxRecoveryReason, @@ -85,6 +82,10 @@ export const SESSION_DIR = nemoclawStateRoot(process.env.HOME || "/tmp", GATEWAY export const SESSION_FILE = path.join(SESSION_DIR, "onboard-session.json"); export const LOCK_FILE = path.join(SESSION_DIR, "onboard.lock"); export const RETAINED_SANDBOX_RECOVERY_FILE = retainedSandboxRecoveryFile(SESSION_DIR); +const LEGACY_STATE_MIGRATION_LOCK = path.join( + nemoclawStateRoot(process.env.HOME || "/tmp", DEFAULT_GATEWAY_PORT), + ".gateway-state-migration.lock", +); const SAFE_VLLM_INSTALL_MODEL = /^[A-Za-z0-9._:/-]+$/; export class InvalidPersistedPolicyAuthorityError extends Error {} @@ -1663,6 +1664,13 @@ export function acquireOnboardLock(command: string | null = null): LockResult { try { heldLockDirectory = openPinnedSessionDirectory(); assertOnboardLockOwned(); + // Legacy-port migration holds its lock before checking every onboard + // writer lock. Recheck here after atomically claiming onboard.lock so + // either the writer or the migrator wins, never both. + if (fs.existsSync(LEGACY_STATE_MIGRATION_LOCK)) { + releaseOnboardLock(); + return { acquired: false, lockFile: LOCK_FILE, stale: false }; + } } catch (error) { heldLockFd = null; if (heldLockDirectory !== null) fs.closeSync(heldLockDirectory.descriptor); @@ -2003,22 +2011,6 @@ export interface RetainedSandboxRecoveryContext { readonly policyCreationReceipt: RetainedSandboxRecoveryRecord["policyCreationReceipt"]; } -function retainedSandboxResourceEvidence(session: Session) { - const messagingCredentialEnvironmentVariables = - session.messagingPlan?.credentialBindings.map((binding) => binding.providerEnvKey) ?? []; - return { - sharedInferenceProviders: session.provider ? [session.provider] : [], - sandboxScopedProviders: session.stagedCredentialProviders, - credentialEnvironmentVariables: [ - ...(session.credentialEnv ? [session.credentialEnv] : []), - ...(session.webSearchConfig - ? [webSearchEnvFor(webSearchProviderForConfig(session.webSearchConfig))] - : []), - ...messagingCredentialEnvironmentVariables, - ], - }; -} - function persistIndependentRetainedSandboxRecovery( session: Session, reason: RetainedSandboxRecoveryReason, @@ -2034,7 +2026,6 @@ function persistIndependentRetainedSandboxRecovery( verifiedEffectivePolicyIdentity: context.verifiedEffectivePolicyIdentity, createAttemptNonce: context.createAttemptNonce, policyCreationReceipt: context.policyCreationReceipt, - resources: retainedSandboxResourceEvidence(session), reason, }); } @@ -2064,7 +2055,6 @@ export function listRetainedSandboxRecoveryRecords(): readonly RetainedSandboxRe verifiedEffectivePolicyIdentity: recovery.verifiedEffectivePolicyIdentity, createAttemptNonce: recovery.createAttemptNonce, policyCreationReceipt: recovery.policyCreationReceipt, - resources: retainedSandboxResourceEvidence(current), reason: recovery.reason, recordedAt: recovery.recordedAt, }); @@ -2087,6 +2077,44 @@ export function recordRetainedSandboxRecovery( ); } +export function retainedSandboxRecoveryMatchesSession( + record: RetainedSandboxRecoveryRecord, + session: Pick | null | undefined, +): boolean { + const recovery = session?.cancellationRecovery; + if (!recovery) return false; + return ( + record.sandboxName === recovery.sandboxName && + record.sandboxIdentityFingerprint === recovery.sandboxIdentityFingerprint && + record.gatewayName === recovery.gatewayName && + record.gatewayPort === recovery.gatewayPort && + record.lifecycleGeneration === recovery.lifecycleGeneration && + record.createAttemptNonce === recovery.createAttemptNonce + ); +} + +/** Clear one recovery-only session after destroy verifies the retained resources absent. */ +export function resolveRetainedSandboxRecovery(record: RetainedSandboxRecoveryRecord): boolean { + return withOwnedOnboardLock("nemoclaw retained sandbox recovery completion", () => { + if (!retainedSandboxRecoveryAuthorityIsCurrent(RETAINED_SANDBOX_RECOVERY_FILE, record)) { + return false; + } + const current = loadSession(); + if (current && retainedSandboxRecoveryMatchesSession(record, current)) { + current.status = "failed"; + current.resumable = false; + current.sandboxName = null; + current.cancellationRecovery = null; + saveSession(current); + } + // Release the recovery-only session first. If this write fails, the exact + // independent record remains available for a later completion attempt. If + // record retirement then fails, that record still blocks only the retained + // name while a different explicitly named onboarding run can proceed. + return retireRetainedSandboxRecovery(RETAINED_SANDBOX_RECOVERY_FILE, record); + }); +} + export function markCancellationRecovery( sandboxName: string, sandboxIdentityFingerprint: string | undefined, diff --git a/src/lib/state/onboard-session/retained-sandbox-recovery.ts b/src/lib/state/onboard-session/retained-sandbox-recovery.ts index fced883d05b..902f53d0ddb 100644 --- a/src/lib/state/onboard-session/retained-sandbox-recovery.ts +++ b/src/lib/state/onboard-session/retained-sandbox-recovery.ts @@ -4,20 +4,20 @@ import { createHash, randomUUID } from "node:crypto"; import fs from "node:fs"; import path from "node:path"; +import { isDeepStrictEqual } from "node:util"; import { openRegularFileNoFollow } from "../../adapters/fs/regular-file"; import { parseNemoClawPolicyCreationReceipt, type NemoClawPolicyCreationReceipt, } from "../../policy/merge"; +import { NAME_MAX_LENGTH, NAME_VALID_PATTERN } from "../../sandbox-name-contract"; export { parseNemoClawPolicyCreationReceipt } from "../../policy/merge"; const SCHEMA_VERSION = 1; const FINGERPRINT_PATTERN = /^[0-9a-f]{64}$/u; const SAFE_EVIDENCE_PATTERN = /^[A-Za-z0-9._:@/-]{1,256}$/u; -const NAME_MAX_LENGTH = 63; -const NAME_VALID_PATTERN = /^[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$/u; export function retainedSandboxRecoveryFile(sessionDirectory: string): string { return path.join(sessionDirectory, "retained-sandbox-recovery.json"); @@ -27,12 +27,6 @@ export type RetainedSandboxRecoveryReason = | "cancelled_after_sandbox_creation" | "retained_after_sandbox_creation_failure"; -export interface RetainedSandboxResourceEvidence { - readonly sharedInferenceProviders: readonly string[]; - readonly sandboxScopedProviders: readonly string[]; - readonly credentialEnvironmentVariables: readonly string[]; -} - export interface RetainedSandboxVerifiedEffectivePolicyIdentity { readonly hash: string; readonly activeVersion: number; @@ -43,14 +37,12 @@ export interface RetainedSandboxRecoveryRecord { readonly recordId: string; readonly sandboxName: string; readonly sandboxIdentityFingerprint: string | null; - readonly identityWasUnavailable: boolean; readonly gatewayName: string; readonly gatewayPort: number; readonly lifecycleGeneration: string | null; readonly verifiedEffectivePolicyIdentity: RetainedSandboxVerifiedEffectivePolicyIdentity | null; readonly createAttemptNonce: string; readonly policyCreationReceipt: NemoClawPolicyCreationReceipt | null; - readonly resources: RetainedSandboxResourceEvidence; readonly reason: RetainedSandboxRecoveryReason; readonly recordedAt: string; } @@ -76,7 +68,6 @@ export interface RecordRetainedSandboxRecoveryInput { readonly verifiedEffectivePolicyIdentity: RetainedSandboxVerifiedEffectivePolicyIdentity | null; readonly createAttemptNonce: string; readonly policyCreationReceipt: NemoClawPolicyCreationReceipt | null; - readonly resources: RetainedSandboxResourceEvidence; readonly reason: RetainedSandboxRecoveryReason; readonly recordedAt?: string; } @@ -335,20 +326,6 @@ function validGatewayPort(value: unknown): value is number { return Number.isInteger(value) && Number(value) >= 1024 && Number(value) <= 65535; } -function parseEvidence(value: unknown): RetainedSandboxResourceEvidence | null { - if (!isObjectRecord(value)) return null; - const parse = (candidate: unknown): string[] | null => - Array.isArray(candidate) && candidate.every(validSafeEvidence) - ? [...new Set(candidate)].sort() - : null; - const sharedInferenceProviders = parse(value.sharedInferenceProviders); - const sandboxScopedProviders = parse(value.sandboxScopedProviders); - const credentialEnvironmentVariables = parse(value.credentialEnvironmentVariables); - return sharedInferenceProviders && sandboxScopedProviders && credentialEnvironmentVariables - ? { sharedInferenceProviders, sandboxScopedProviders, credentialEnvironmentVariables } - : null; -} - function parseVerifiedEffectivePolicyIdentity( value: unknown, ): RetainedSandboxVerifiedEffectivePolicyIdentity | null | undefined { @@ -366,7 +343,6 @@ function parseVerifiedEffectivePolicyIdentity( function parseRecord(value: unknown): RetainedSandboxRecoveryRecord | null { if (!isObjectRecord(value)) return null; - const resources = parseEvidence(value.resources); const fingerprint = value.sandboxIdentityFingerprint; const verifiedEffectivePolicyIdentity = parseVerifiedEffectivePolicyIdentity( value.verifiedEffectivePolicyIdentity, @@ -387,7 +363,6 @@ function parseRecord(value: unknown): RetainedSandboxRecoveryRecord | null { !validSandboxName(value.sandboxName) || (fingerprint !== null && (typeof fingerprint !== "string" || !FINGERPRINT_PATTERN.test(fingerprint))) || - value.identityWasUnavailable !== (fingerprint === null) || !validSafeEvidence(value.gatewayName) || !validGatewayPort(value.gatewayPort) || (value.lifecycleGeneration !== null && !validSafeEvidence(value.lifecycleGeneration)) || @@ -402,7 +377,6 @@ function parseRecord(value: unknown): RetainedSandboxRecoveryRecord | null { policyCreationReceipt.sandboxIdentityFingerprint !== fingerprint || policyCreationReceipt.policyHash !== verifiedEffectivePolicyIdentity?.hash || policyCreationReceipt.policyVersion !== verifiedEffectivePolicyIdentity?.activeVersion)) || - !resources || !["cancelled_after_sandbox_creation", "retained_after_sandbox_creation_failure"].includes( String(reason), ) || @@ -415,14 +389,12 @@ function parseRecord(value: unknown): RetainedSandboxRecoveryRecord | null { recordId: value.recordId, sandboxName: value.sandboxName, sandboxIdentityFingerprint: fingerprint, - identityWasUnavailable: fingerprint === null, gatewayName: value.gatewayName, gatewayPort: value.gatewayPort, lifecycleGeneration: value.lifecycleGeneration, verifiedEffectivePolicyIdentity, createAttemptNonce: value.createAttemptNonce, policyCreationReceipt, - resources, reason: reason as RetainedSandboxRecoveryReason, recordedAt: value.recordedAt, }; @@ -469,8 +441,7 @@ function assertRecordInput(input: RecordRetainedSandboxRecoveryInput): void { !validGatewayPort(input.gatewayPort) || (input.lifecycleGeneration !== null && !validSafeEvidence(input.lifecycleGeneration)) || parseVerifiedEffectivePolicyIdentity(input.verifiedEffectivePolicyIdentity) === undefined || - !/^[0-9a-f]{62}$/u.test(input.createAttemptNonce) || - !parseEvidence(input.resources) + !/^[0-9a-f]{62}$/u.test(input.createAttemptNonce) ) { throw new Error("Cannot persist invalid retained sandbox recovery evidence."); } @@ -511,7 +482,6 @@ export function recordRetainedSandboxRecovery( recordId: recoveryRecordId(input), sandboxName: input.sandboxName, sandboxIdentityFingerprint: input.sandboxIdentityFingerprint, - identityWasUnavailable: input.sandboxIdentityFingerprint === null, gatewayName: input.gatewayName, gatewayPort: input.gatewayPort, lifecycleGeneration: input.lifecycleGeneration, @@ -522,7 +492,6 @@ export function recordRetainedSandboxRecovery( policyCreationReceipt: input.policyCreationReceipt ? parseNemoClawPolicyCreationReceipt(input.policyCreationReceipt) : null, - resources: parseEvidence(input.resources)!, reason: input.reason, recordedAt: input.recordedAt ?? new Date().toISOString(), }; @@ -546,3 +515,42 @@ export function recordRetainedSandboxRecovery( } return reread; } + +function retainedSandboxRecoveryAuthorityMatchesState( + state: RetainedSandboxRecoveryState, + expected: RetainedSandboxRecoveryRecord, +): boolean { + const recorded = state.unresolved.find( + (candidate) => candidate.recordId === expected.recordId, + ); + if (!recorded) return false; + if (!isDeepStrictEqual(recorded, expected)) { + throw new Error("Retained sandbox recovery authority changed before cleanup completed."); + } + return true; +} + +/** Confirm that the exact cleanup authority is still present and unchanged. */ +export function retainedSandboxRecoveryAuthorityIsCurrent( + filePath: string, + expected: RetainedSandboxRecoveryRecord, +): boolean { + return retainedSandboxRecoveryAuthorityMatchesState(loadState(filePath), expected); +} + +/** Retire only the unchanged record whose external resources were verified absent. */ +export function resolveRetainedSandboxRecovery( + filePath: string, + expected: RetainedSandboxRecoveryRecord, +): boolean { + const current = loadState(filePath); + if (!retainedSandboxRecoveryAuthorityMatchesState(current, expected)) return false; + writeStateFile(filePath, { + ...current, + unresolved: current.unresolved.filter((candidate) => candidate.recordId !== expected.recordId), + }); + if (loadState(filePath).unresolved.some((candidate) => candidate.recordId === expected.recordId)) { + throw new Error("Retained sandbox recovery record remained after verified cleanup."); + } + return true; +} diff --git a/src/lib/state/retained-sandbox-recovery.test.ts b/src/lib/state/retained-sandbox-recovery.test.ts index 6fd70af1d27..1c4669b1c95 100644 --- a/src/lib/state/retained-sandbox-recovery.test.ts +++ b/src/lib/state/retained-sandbox-recovery.test.ts @@ -20,18 +20,13 @@ afterEach(() => { fs.rmSync(home, { recursive: true, force: true }); }); -const evidence = { - sharedInferenceProviders: ["nvidia"], - sandboxScopedProviders: ["sandbox-telegram"], - credentialEnvironmentVariables: ["NVIDIA_API_KEY", "TELEGRAM_BOT_TOKEN"], -} as const; const recoveryAuthority = { createAttemptNonce: "c".repeat(62), policyCreationReceipt: null, } as const; describe("retained sandbox recovery state", () => { - it("persists verified identity and secret-free resource evidence independently", async () => { + it("persists verified identity independently", async () => { const recovery = await import("./onboard-session"); const fingerprint = "a".repeat(64); const input = { @@ -42,7 +37,6 @@ describe("retained sandbox recovery state", () => { lifecycleGeneration: "00000000-0000-4000-8000-000000000001", verifiedEffectivePolicyIdentity: { hash: "sha256:policy-1", activeVersion: 1 }, ...recoveryAuthority, - resources: evidence, reason: "cancelled_after_sandbox_creation", recordedAt: "2026-08-27T00:00:00.000Z", } as const; @@ -53,9 +47,7 @@ describe("retained sandbox recovery state", () => { expect(recorded).toMatchObject({ sandboxName: "retained-sb", sandboxIdentityFingerprint: fingerprint, - identityWasUnavailable: false, verifiedEffectivePolicyIdentity: input.verifiedEffectivePolicyIdentity, - resources: evidence, }); expect(fs.readFileSync(recovery.RETAINED_SANDBOX_RECOVERY_FILE, "utf8")).not.toContain( "secret-value", @@ -73,21 +65,32 @@ describe("retained sandbox recovery state", () => { lifecycleGeneration: null, verifiedEffectivePolicyIdentity: null, ...recoveryAuthority, - resources: { - sharedInferenceProviders: [], - sandboxScopedProviders: [], - credentialEnvironmentVariables: [], - }, reason: "retained_after_sandbox_creation_failure", }); expect(recorded).toMatchObject({ sandboxIdentityFingerprint: null, - identityWasUnavailable: true, lifecycleGeneration: null, }); }); + it("rejects a recovery target outside the canonical sandbox-name contract", async () => { + const recovery = await import("./onboard-session"); + + expect(() => + recovery.recordRetainedSandboxRecovery({ + sandboxName: "1sandbox", + sandboxIdentityFingerprint: "a".repeat(64), + gatewayName: "nemoclaw", + gatewayPort: 8080, + lifecycleGeneration: "00000000-0000-4000-8000-000000000001", + verifiedEffectivePolicyIdentity: null, + ...recoveryAuthority, + reason: "retained_after_sandbox_creation_failure", + }), + ).toThrow("Cannot persist invalid retained sandbox recovery evidence"); + }); + it("preserves distinct unresolved lifecycle tuples for one sandbox name (#9833)", async () => { const recovery = await import("./onboard-session"); const first = recovery.recordRetainedSandboxRecovery({ @@ -98,7 +101,6 @@ describe("retained sandbox recovery state", () => { lifecycleGeneration: "00000000-0000-4000-8000-000000000001", verifiedEffectivePolicyIdentity: { hash: "sha256:policy-1", activeVersion: 1 }, ...recoveryAuthority, - resources: evidence, reason: "cancelled_after_sandbox_creation", }); const second = recovery.recordRetainedSandboxRecovery({ @@ -109,7 +111,6 @@ describe("retained sandbox recovery state", () => { lifecycleGeneration: "00000000-0000-4000-8000-000000000002", verifiedEffectivePolicyIdentity: { hash: "sha256:policy-2", activeVersion: 2 }, ...recoveryAuthority, - resources: evidence, reason: "retained_after_sandbox_creation_failure", }); @@ -156,7 +157,6 @@ describe("retained sandbox recovery state", () => { lifecycleGeneration: "generation-1", verifiedEffectivePolicyIdentity: null, ...recoveryAuthority, - resources: evidence, reason: "retained_after_sandbox_creation_failure", }), ).toThrow(/symbolic link|lock ownership changed/u); @@ -192,7 +192,6 @@ describe("retained sandbox recovery state", () => { lifecycleGeneration: "generation-1", verifiedEffectivePolicyIdentity: null, ...recoveryAuthority, - resources: evidence, reason: "retained_after_sandbox_creation_failure", }), ).toThrow(/state directory changed|lock ownership changed/u); @@ -245,9 +244,8 @@ describe("retained sandbox recovery state", () => { ).toEqual([]); }); - it("does not expose a caller-supplied recovery resolution path (#9833)", async () => { + it("retires only the exact retained recovery record after verified cleanup (#10547)", async () => { const recovery = await import("./onboard-session"); - const recoveryStore = await import("./onboard-session/retained-sandbox-recovery"); const fingerprint = "b".repeat(64); const recorded = recovery.recordRetainedSandboxRecovery({ sandboxName: "retained-sb", @@ -257,27 +255,127 @@ describe("retained sandbox recovery state", () => { lifecycleGeneration: "generation-1", verifiedEffectivePolicyIdentity: null, ...recoveryAuthority, - resources: evidence, reason: "cancelled_after_sandbox_creation", }); - const unsupportedClear = (recovery as unknown as Record)[ - "resolveRetainedSandboxRecovery" - ]; - (unsupportedClear as undefined | ((input: Record) => unknown))?.({ - recordId: recorded.recordId, - receiptId: "c".repeat(64), - sandboxName: recorded.sandboxName, - sandboxIdentityFingerprint: fingerprint, - gatewayName: recorded.gatewayName, - gatewayPort: recorded.gatewayPort, - outcome: "removed_verified_identity", + + expect(() => + recovery.resolveRetainedSandboxRecovery({ + ...recorded, + sandboxIdentityFingerprint: "d".repeat(64), + }), + ).toThrow(/changed before cleanup completed/u); + expect(recovery.listRetainedSandboxRecoveryRecords()).toEqual([recorded]); + + expect(recovery.resolveRetainedSandboxRecovery(recorded)).toBe(true); + expect(recovery.resolveRetainedSandboxRecovery(recorded)).toBe(false); + expect(recovery.listRetainedSandboxRecoveryRecords()).toEqual([]); + }); + + it("releases the matching recovery-only onboarding session after cleanup (#10547)", async () => { + const recovery = await import("./onboard-session"); + recovery.markRetainedSandboxRecovery( + "retained-sb", + "Sandbox creation failed after identity verification.", + "b".repeat(64), + { + gatewayName: "nemoclaw", + gatewayPort: 8080, + lifecycleGeneration: "generation-1", + verifiedEffectivePolicyIdentity: null, + ...recoveryAuthority, + }, + ); + const [recorded] = recovery.listRetainedSandboxRecoveryRecords(); + + expect(() => + recovery.resolveRetainedSandboxRecovery({ + ...recorded!, + reason: "cancelled_after_sandbox_creation", + }), + ).toThrow(/changed before cleanup completed/u); + expect(recovery.loadSession()).toMatchObject({ + status: "recovery_required", + sandboxName: "retained-sb", }); - expect(unsupportedClear).toBeUndefined(); - expect( - (recoveryStore as unknown as Record)["resolveRetainedSandboxRecovery"], - ).toBeUndefined(); + expect(recovery.resolveRetainedSandboxRecovery(recorded!)).toBe(true); + + expect(recovery.loadSession()).toMatchObject({ + status: "failed", + resumable: false, + sandboxName: null, + cancellationRecovery: null, + }); + expect(recovery.listRetainedSandboxRecoveryRecords()).toEqual([]); + }); + + it("keeps the exact record when retirement fails after session release (#10547)", async () => { + const recovery = await import("./onboard-session"); + recovery.markRetainedSandboxRecovery( + "retained-sb", + "Sandbox creation failed after identity verification.", + "b".repeat(64), + { + gatewayName: "nemoclaw", + gatewayPort: 8080, + lifecycleGeneration: "generation-1", + verifiedEffectivePolicyIdentity: null, + ...recoveryAuthority, + }, + ); + const [recorded] = recovery.listRetainedSandboxRecoveryRecords(); + const renameSync = fs.renameSync.bind(fs); + vi.spyOn(fs, "renameSync").mockImplementation((source, destination) => + String(destination) === recovery.RETAINED_SANDBOX_RECOVERY_FILE + ? (() => { + throw new Error("simulated recovery retirement write failure"); + })() + : renameSync(source, destination), + ); + + expect(() => recovery.resolveRetainedSandboxRecovery(recorded!)).toThrow( + /simulated recovery retirement write failure/u, + ); + expect(recovery.loadSession()).toMatchObject({ + status: "failed", + resumable: false, + sandboxName: null, + cancellationRecovery: null, + }); expect(recovery.listRetainedSandboxRecoveryRecords()).toEqual([recorded]); }); + it("preserves the exact record when recovery-only session release cannot be written (#10547)", async () => { + const recovery = await import("./onboard-session"); + recovery.markRetainedSandboxRecovery( + "retained-sb", + "Sandbox creation failed after identity verification.", + "b".repeat(64), + { + gatewayName: "nemoclaw", + gatewayPort: 8080, + lifecycleGeneration: "generation-1", + verifiedEffectivePolicyIdentity: null, + ...recoveryAuthority, + }, + ); + const [recorded] = recovery.listRetainedSandboxRecoveryRecords(); + const renameSync = fs.renameSync.bind(fs); + vi.spyOn(fs, "renameSync").mockImplementation((source, destination) => + String(destination) === recovery.SESSION_FILE + ? (() => { + throw new Error("simulated recovery session release write failure"); + })() + : renameSync(source, destination), + ); + + expect(() => recovery.resolveRetainedSandboxRecovery(recorded!)).toThrow( + /simulated recovery session release write failure/u, + ); + expect(recovery.loadSession()).toMatchObject({ + status: "recovery_required", + sandboxName: "retained-sb", + }); + expect(recovery.listRetainedSandboxRecoveryRecords()).toEqual([recorded]); + }); }); diff --git a/test/cli/destroy-gateway-cleanup.test.ts b/test/cli/destroy-gateway-cleanup.test.ts index da17fe7e8c2..3921b00a88e 100644 --- a/test/cli/destroy-gateway-cleanup.test.ts +++ b/test/cli/destroy-gateway-cleanup.test.ts @@ -9,7 +9,19 @@ import { describe, expect, it } from "vitest"; import { runWithEnv, testTimeoutOptions } from "./helpers"; const LIVE_DOCKER_IDENTITY = `#!/bin/sh -case "$*" in *'.Label '*) printf 'aaaaaaaaaaaa\topenshell\tdefault\tsb-alpha\n' ;; esac +removed_marker="$0.removed" +case "$1" in + ps) + if [ ! -e "$removed_marker" ]; then + printf 'aaaaaaaaaaaa\topenshell\tdefault\tsb-alpha\n' + fi + ;; + rm) + if [ "$2" = "-f" ] && [ "$3" = "aaaaaaaaaaaa" ]; then + : > "$removed_marker" + fi + ;; +esac exit 0 `; @@ -750,6 +762,7 @@ describe("CLI dispatch", () => { ].join("\n"), { mode: 0o755 }, ); + fs.writeFileSync(path.join(localBin, "docker"), LIVE_DOCKER_IDENTITY, { mode: 0o755 }); const r = runWithEnv("alpha destroy --yes", { HOME: home, diff --git a/test/e2e/live/mcp-bridge-cleanup.ts b/test/e2e/live/mcp-bridge-cleanup.ts index 82f8b36471a..4e3adc87d0a 100644 --- a/test/e2e/live/mcp-bridge-cleanup.ts +++ b/test/e2e/live/mcp-bridge-cleanup.ts @@ -2,9 +2,11 @@ // SPDX-License-Identifier: Apache-2.0 import { buildAvailabilityProbeEnv } from "../fixtures/availability-env.ts"; +import type { CleanupRegistry } from "../fixtures/cleanup.ts"; import { assertCleanupSucceededOrAbsent } from "../fixtures/cleanup-resources.ts"; import { resultText } from "../fixtures/clients/command.ts"; import type { HostCliClient } from "../fixtures/clients/host.ts"; +import type { SandboxClient } from "../fixtures/clients/sandbox.ts"; export type McpAdapter = "mcporter" | "hermes-config" | "deepagents-config"; @@ -17,6 +19,57 @@ export const MCP_MUTATION_TIMEOUT_MS: Record = { const MCP_BRIDGE_ALREADY_ABSENT = /No MCP servers are registered|No MCP server '.+' is registered|MCP server '.+' not found/iu; +function buildOwnedSandboxCleanupEnv(): NodeJS.ProcessEnv { + return { + ...buildAvailabilityProbeEnv(), + // Bind trusted administrator cleanup to the gateway NemoClaw initialized. + // ShellProbe otherwise forwards only PATH, which hides gateway metadata. + OPENSHELL_GATEWAY: process.env.OPENSHELL_GATEWAY?.trim() || "nemoclaw", + }; +} + +/** Prepare a sandbox name exclusively owned by this isolated qualification job. */ +export async function prepareOwnedSandboxForOnboard( + host: Pick, + sandbox: Pick, + cleanup: CleanupRegistry, + sandboxName: string, +): Promise { + const openshellCleanupEnv = buildOwnedSandboxCleanupEnv(); + cleanup.trackSandbox(host, sandboxName, { + artifactName: "cleanup-destroy-sandbox", + timeoutMs: 15 * 60_000, + }); + // A failed onboard may leave a live sandbox that the production CLI safely + // refuses to delete by mutable name. Register the trusted administrator + // deletion last so LIFO cleanup removes OpenShell state before `destroy` + // reconciles the durable recovery record and identity-verified containers. + cleanup.trackDisposable(`delete owned OpenShell sandbox ${sandboxName}`, () => + sandbox.cleanupSandbox(sandboxName, { + artifactName: "cleanup-delete-openshell-sandbox", + env: openshellCleanupEnv, + timeoutMs: 15 * 60_000, + }), + ); + // A fresh qualification runner has no active OpenShell gateway yet. Let the + // production CLI initialize it and perform any cleanup it can prove safe. + // Retained-state refusal remains non-fatal here because the identity-bound + // administrator deletion below is the isolated E2E fallback. + await host.bestEffortCleanupSandbox(sandboxName, { + artifactName: "precleanup-initialize-gateway", + timeoutMs: 15 * 60_000, + }); + await sandbox.cleanupSandbox(sandboxName, { + artifactName: "precleanup-delete-openshell-sandbox", + env: openshellCleanupEnv, + timeoutMs: 15 * 60_000, + }); + await host.cleanupSandbox(sandboxName, { + artifactName: "precleanup-destroy-sandbox", + timeoutMs: 15 * 60_000, + }); +} + export async function cleanupMcpBridge( host: HostCliClient, sandboxName: string, diff --git a/test/e2e/live/mcp-bridge.test.ts b/test/e2e/live/mcp-bridge.test.ts index 50b26fe2865..08f11f57ce4 100644 --- a/test/e2e/live/mcp-bridge.test.ts +++ b/test/e2e/live/mcp-bridge.test.ts @@ -30,6 +30,7 @@ import { cleanupMcpBridge, MCP_MUTATION_TIMEOUT_MS, type McpAdapter, + prepareOwnedSandboxForOnboard, removeMcpBridgeWithOneConcurrencyRetry, } from "./mcp-bridge-cleanup.ts"; import { @@ -121,6 +122,7 @@ function expectManagedImageQualificationReceipt(sandboxName: string, agent: McpA async function onboardAgent( host: HostCliClient, + sandbox: SandboxClient, cleanup: CleanupRegistry, endpointUrl: string, options: { @@ -131,14 +133,7 @@ async function onboardAgent( }, ): Promise { const corporateCaBundle = requireMcpBridgeTlsCaCert(); - cleanup.trackSandbox(host, options.sandboxName, { - artifactName: "cleanup-destroy-sandbox", - timeoutMs: 15 * 60_000, - }); - await host.cleanupSandbox(options.sandboxName, { - artifactName: "precleanup-destroy-sandbox", - timeoutMs: 15 * 60_000, - }); + await prepareOwnedSandboxForOnboard(host, sandbox, cleanup, options.sandboxName); const args = buildMcpBridgeOnboardArgs(); const commandOptions = { artifactName: options.artifactName, @@ -739,7 +734,7 @@ test("mcp-bridge", { const mcpUrl = fakeMcpTunnel.url; const decoyMcpUrl = decoyMcpTunnel.url; progress.phase("onboard OpenClaw and prove base policy"); - await onboardAgent(host, cleanup, endpointUrl, { + await onboardAgent(host, sandbox, cleanup, endpointUrl, { agent: "openclaw", sandboxName: OPENCLAW_SANDBOX_NAME, artifactName: "onboard-openclaw-mcp-bridge", @@ -1117,7 +1112,7 @@ mcpBridgeShardTest("hermes")( const endpointUrl = `http://${hostAddress}:${compatibleMock.port}/v1`; const mcpUrl = fakeMcpTunnel.url; progress.phase("onboard the Hermes MCP sandbox"); - await onboardAgent(host, cleanup, endpointUrl, { + await onboardAgent(host, sandbox, cleanup, endpointUrl, { agent: "hermes", sandboxName: HERMES_SANDBOX_NAME, artifactName: "onboard-hermes-mcp-bridge", @@ -1370,7 +1365,7 @@ mcpBridgeShardTest("deepagents")( mcpUrl, ); progress.phase("onboard the Deep Agents MCP sandbox"); - await onboardAgent(host, cleanup, endpointUrl, { + await onboardAgent(host, sandbox, cleanup, endpointUrl, { agent: "langchain-deepagents-code", sandboxName: DEEPAGENTS_SANDBOX_NAME, artifactName: "onboard-deepagents-mcp-bridge", diff --git a/test/e2e/mock-parity.json b/test/e2e/mock-parity.json index 92a7bdae3d3..df8ddc39001 100644 --- a/test/e2e/mock-parity.json +++ b/test/e2e/mock-parity.json @@ -487,6 +487,7 @@ }, { "live": "test/e2e/live/mcp-bridge.test.ts", + "liveSources": ["test/e2e/live/mcp-bridge-cleanup.ts"], "fast": [ "src/lib/actions/sandbox/mcp-bridge-adapter-registration.test.ts", "src/lib/actions/sandbox/mcp-bridge-adapter-teardown.test.ts", @@ -494,6 +495,7 @@ "src/lib/actions/sandbox/mcp-bridge-provider.test.ts", "test/e2e/support/e2e-cleanup-resources.test.ts", "test/e2e/support/e2e-clients.test.ts", + "test/e2e/support/mcp-bridge-cleanup.test.ts", "test/e2e/support/mcp-bridge-hermes-lifecycle.test.ts", "test/e2e/support/mcp-bridge-onboard-env.test.ts", "test/e2e/support/mcp-bridge-reliability.test.ts", diff --git a/test/e2e/support/mcp-bridge-cleanup.test.ts b/test/e2e/support/mcp-bridge-cleanup.test.ts new file mode 100644 index 00000000000..1963bbdf30f --- /dev/null +++ b/test/e2e/support/mcp-bridge-cleanup.test.ts @@ -0,0 +1,105 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it, vi } from "vitest"; + +import { CleanupRegistry } from "../fixtures/cleanup.ts"; +import type { ShellProbeRunOptions } from "../fixtures/shell-probe.ts"; +import { prepareOwnedSandboxForOnboard } from "../live/mcp-bridge-cleanup.ts"; + +function cleanupClient(owner: string, calls: string[]) { + const cleanupSandbox = vi.fn(async (_name: string, options: ShellProbeRunOptions = {}) => { + calls.push(`${owner}:${options.artifactName}`); + }); + return { + cleanupSandbox, + bestEffortCleanupSandbox: vi.fn(async (name: string, options: ShellProbeRunOptions = {}) => { + try { + await cleanupSandbox(name, options); + } catch { + // Match HostCliClient: the administrator fallback must still run. + } + }), + }; +} + +describe("MCP bridge owned-sandbox cleanup", () => { + it("initializes the gateway before administrator deletion and final reconciliation", async () => { + const calls: string[] = []; + const host = cleanupClient("host", calls); + const sandbox = cleanupClient("openshell", calls); + const cleanup = new CleanupRegistry(); + + await prepareOwnedSandboxForOnboard(host, sandbox, cleanup, "e2e-mcp-bridge"); + expect(calls).toEqual([ + "host:precleanup-initialize-gateway", + "openshell:precleanup-delete-openshell-sandbox", + "host:precleanup-destroy-sandbox", + ]); + expect(sandbox.cleanupSandbox).toHaveBeenNthCalledWith( + 1, + "e2e-mcp-bridge", + expect.objectContaining({ + env: expect.objectContaining({ + HOME: expect.any(String), + OPENSHELL_GATEWAY: process.env.OPENSHELL_GATEWAY?.trim() || "nemoclaw", + }), + }), + ); + + const result = await cleanup.runAll(); + + expect(result.failures).toEqual([]); + expect(calls).toEqual([ + "host:precleanup-initialize-gateway", + "openshell:precleanup-delete-openshell-sandbox", + "host:precleanup-destroy-sandbox", + "openshell:cleanup-delete-openshell-sandbox", + "host:cleanup-destroy-sandbox", + ]); + expect(sandbox.cleanupSandbox).toHaveBeenNthCalledWith( + 2, + "e2e-mcp-bridge", + expect.objectContaining({ + env: expect.objectContaining({ + HOME: expect.any(String), + OPENSHELL_GATEWAY: process.env.OPENSHELL_GATEWAY?.trim() || "nemoclaw", + }), + }), + ); + }); + + it("still attempts NemoClaw reconciliation when administrator deletion fails", async () => { + const calls: string[] = []; + const host = cleanupClient("host", calls); + const sandbox = cleanupClient("openshell", calls); + const cleanup = new CleanupRegistry(); + + await prepareOwnedSandboxForOnboard(host, sandbox, cleanup, "e2e-mcp-bridge"); + sandbox.cleanupSandbox.mockRejectedValueOnce(new Error("openshell cleanup failed")); + const result = await cleanup.runAll(); + + expect(result.failures).toEqual([ + { + name: "delete owned OpenShell sandbox e2e-mcp-bridge", + message: "openshell cleanup failed", + }, + ]); + expect(calls.at(-1)).toBe("host:cleanup-destroy-sandbox"); + }); + + it("uses administrator deletion when safe gateway initialization refuses cleanup", async () => { + const calls: string[] = []; + const host = cleanupClient("host", calls); + const sandbox = cleanupClient("openshell", calls); + const cleanup = new CleanupRegistry(); + host.cleanupSandbox.mockRejectedValueOnce(new Error("retained identity requires recovery")); + + await prepareOwnedSandboxForOnboard(host, sandbox, cleanup, "e2e-mcp-bridge"); + + expect(calls).toEqual([ + "openshell:precleanup-delete-openshell-sandbox", + "host:precleanup-destroy-sandbox", + ]); + }); +}); diff --git a/test/helpers/destroy-flow-test-harness.ts b/test/helpers/destroy-flow-test-harness.ts index 2c2ce3e4e80..206183c9537 100644 --- a/test/helpers/destroy-flow-test-harness.ts +++ b/test/helpers/destroy-flow-test-harness.ts @@ -8,6 +8,7 @@ import type { SandboxDestroyExecutionResult } from "../../src/lib/actions/sandbo import type { PreparedManagedLlamaCppRuntimeCleanup } from "../../src/lib/inference/local-model-profile/cleanup"; import type { ManagedHermesStateVolumeCleanupResult } from "../../src/lib/onboard/managed-workload/hermes-state-volume"; import type { Session } from "../../src/lib/state/onboard-session"; +import type { RetainedSandboxRecoveryRecord } from "../../src/lib/state/onboard-session/retained-sandbox-recovery"; import type { SandboxEntry, SandboxWorkloadReceipt } from "../../src/lib/state/registry"; type DestroySandbox = (typeof import("../../src/lib/actions/sandbox/destroy"))["destroySandbox"]; @@ -44,6 +45,7 @@ export type DestroyHarness = { promptSpy: MockInstance; removeManagedHermesStateVolumeSpy: MockInstance; removeSandboxSpy: MockInstance; + resolveRetainedSandboxRecoverySpy: MockInstance; retirePortableLifecycleReceiptSpy: MockInstance; portableDestroyRevalidateSpy: MockInstance; portableDestroyVerifyAbsentSpy: MockInstance; @@ -52,6 +54,13 @@ export type DestroyHarness = { runOpenshellSpy: MockInstance; selectGatewaySpy: MockInstance; sessionState: Session; + setDockerIdentityResult: (result: { + status: number | null; + stdout?: string; + stderr?: string; + }) => void; + setRegistryEntryPresent: (present: boolean) => void; + setRetainedRecoveryRecords: (records: RetainedSandboxRecoveryRecord[]) => void; setSandboxPresent: (present: boolean) => void; shieldsDownSpy: MockInstance; stopAllSpy: MockInstance; @@ -70,6 +79,7 @@ type DestroyHarnessOptions = { deleteError?: Error; deleteOutput?: string; deleteStatus?: number | null; + dockerNameLabeledIds?: string[]; dockerPsOutput?: string; dockerOrphanIds?: string[]; dockerOrphanQueryStatus?: number | null; @@ -105,7 +115,9 @@ type DestroyHarnessOptions = { promptResponses?: string[]; provider?: string; registryEntryPresent?: boolean; + registryEntryOverrides?: Partial; registeredSandboxCount?: number; + retainedRecoveryRecords?: RetainedSandboxRecoveryRecord[]; replaceSessionAfterRegistryRemoval?: boolean; removeSandboxResult?: boolean; restoreMcpError?: string; @@ -149,7 +161,7 @@ export function resetDestroyModuleCache(): void { } export function traceDestroyBoundaryCalls( - harness: Pick, + harness: Pick, trace: string[], ): void { harness.runOpenshellSpy.mockImplementation((args: unknown) => { @@ -157,9 +169,10 @@ export function traceDestroyBoundaryCalls( switch (`${String(argv[0])}:${String(argv[1])}`) { case "sandbox:delete": trace.push("delete"); + harness.setSandboxPresent(false); return { status: 0, stdout: "", stderr: "" }; case "sandbox:list": - return { status: 0, stdout: "[]", stderr: "" }; + return { status: 0, stdout: sandboxListJson(["alpha"]), stderr: "" }; default: return { status: 0, stdout: "", stderr: "" }; } @@ -176,6 +189,7 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr const events: string[] = []; const lifecycleLockEvents: string[] = []; let sandboxPresent = options.sandboxPresent !== false; + let exactDockerCleanupPhase = false; let sessionLockBusy = false; const sessionState = { sessionId: "session-alpha", @@ -207,6 +221,18 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr "../../state/mcp-lifecycle-lock.js", ) as typeof import("../../src/lib/state/mcp-lifecycle-lock"); const registry = requireSource("../../state/registry.js"); + const openShellDockerContainers = requireSource( + "../../onboard/openshell-docker-sandbox-containers.js", + ); + const removeExactDockerContainers = + openShellDockerContainers.removeExactOpenShellDockerSandboxContainers; + vi.spyOn( + openShellDockerContainers, + "removeExactOpenShellDockerSandboxContainers", + ).mockImplementation((...args: Parameters) => { + exactDockerCleanupPhase = true; + return removeExactDockerContainers(...args); + }); const destroyExecution = requireSource("./destroy-execution.js"); const destroyCommand = requireSource("../../../commands/sandbox/destroy.js").default; const destroyPreflight = requireSource("./destroy-preflight.js"); @@ -287,39 +313,40 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr detected: true, sessions: [{ pid: 1 }], }); - vi.spyOn(registry, "getSandbox").mockReturnValue( - options.registryEntryPresent === false - ? null - : { - ...sandboxEntry, - imageTag: options.imageTag === undefined ? sandboxEntry.imageTag : options.imageTag, - agent: options.agent ?? sandboxEntry.agent, - ...(options.provider ? { provider: options.provider } : {}), - ...(options.openshellDriver ? { openshellDriver: options.openshellDriver } : {}), - ...(options.endpointUrl ? { endpointUrl: options.endpointUrl } : {}), - ...(options.hostLocalInferenceReceipt !== undefined - ? { hostLocalInferenceReceipt: options.hostLocalInferenceReceipt } - : {}), - ...(options.hostLocalInferenceProvenance - ? { hostLocalInferenceProvenance: options.hostLocalInferenceProvenance } - : {}), - ...(options.workload ? { workload: options.workload } : {}), - ...(options.mcpServers?.length - ? { - mcp: { - bridges: Object.fromEntries( - options.mcpServers.map((server) => [ - server, - { - server, - ...(options.mcpAddState ? { addState: options.mcpAddState } : {}), - }, - ]), - ), + const configuredRegistryEntry = { + ...sandboxEntry, + imageTag: options.imageTag === undefined ? sandboxEntry.imageTag : options.imageTag, + agent: options.agent ?? sandboxEntry.agent, + ...(options.provider ? { provider: options.provider } : {}), + ...(options.openshellDriver ? { openshellDriver: options.openshellDriver } : {}), + ...(options.endpointUrl ? { endpointUrl: options.endpointUrl } : {}), + ...(options.hostLocalInferenceReceipt !== undefined + ? { hostLocalInferenceReceipt: options.hostLocalInferenceReceipt } + : {}), + ...(options.hostLocalInferenceProvenance + ? { hostLocalInferenceProvenance: options.hostLocalInferenceProvenance } + : {}), + ...(options.workload ? { workload: options.workload } : {}), + ...(options.mcpServers?.length + ? { + mcp: { + bridges: Object.fromEntries( + options.mcpServers.map((server) => [ + server, + { + server, + ...(options.mcpAddState ? { addState: options.mcpAddState } : {}), }, - } - : {}), - }, + ]), + ), + }, + } + : {}), + ...options.registryEntryOverrides, + } as SandboxEntry; + let registryEntryPresent = options.registryEntryPresent !== false; + vi.spyOn(registry, "getSandbox").mockImplementation(() => + registryEntryPresent ? configuredRegistryEntry : null, ); let registeredSandboxCount = options.registeredSandboxCount ?? 0; vi.spyOn(registry, "listSandboxes").mockImplementation(() => ({ @@ -370,6 +397,13 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr }); vi.spyOn(modelRouterProcess, "isRouterHealthy").mockResolvedValue(false); vi.spyOn(onboardSession, "loadSession").mockImplementation(() => ({ ...sessionState })); + let retainedRecoveryRecords = [...(options.retainedRecoveryRecords ?? [])]; + vi.spyOn(onboardSession, "listRetainedSandboxRecoveryRecords").mockImplementation( + () => retainedRecoveryRecords, + ); + const resolveRetainedSandboxRecoverySpy = vi + .spyOn(onboardSession, "resolveRetainedSandboxRecovery") + .mockReturnValue(true); vi.spyOn(onboardSession, "acquireOnboardLock").mockImplementation(() => sessionLockBusy ? { @@ -402,6 +436,7 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr return session; }); const gatewayPinsAtSandboxList: Array = []; + let identityProbeCall = 0; const runOpenshellSpy = vi.spyOn(runtime, "runOpenshell").mockImplementation((args: unknown) => { const argv = Array.isArray(args) ? args : []; switch (`${String(argv[0])}:${String(argv[1])}`) { @@ -422,6 +457,7 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr }; case "sandbox:delete": events.push("delete"); + sandboxPresent = false; return { status: options.deleteStatus === undefined ? 0 : options.deleteStatus, stdout: options.deleteOutput ?? "", @@ -450,8 +486,11 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr : names; return matchedNames.length > 0 ? `${matchedNames.join("\n")}\n` : ""; }); - let identityProbeCall = 0; let dockerOrphanIds = [...(options.dockerOrphanIds ?? [])]; + let dockerNameLabeledIds = [ + ...(options.dockerNameLabeledIds ?? options.dockerOrphanIds ?? []), + ]; + let dockerIdentityResult = options.dockerRunResult; const dockerRunSpy = vi.spyOn(dockerRun, "dockerRun").mockImplementation((args: unknown) => { const argv = Array.isArray(args) ? args.map(String) : []; const isDockerOrphanQuery = @@ -466,10 +505,23 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr stderr: "", } as ReturnType; } + const isDockerNameLabeledQuery = + argv[0] === "ps" && + !argv.includes("label=openshell.ai/managed-by=openshell") && + argv.includes("label=openshell.ai/sandbox-name=alpha") && + argv.at(-1) === "{{.ID}}"; + if (isDockerNameLabeledQuery) { + return { + status: options.dockerOrphanQueryStatus ?? 0, + stdout: dockerNameLabeledIds.join("\n"), + stderr: "", + } as ReturnType; + } if (argv[0] === "rm" && argv[1] === "-f") { const status = options.dockerRemoveStatus ?? 0; if (status === 0) { dockerOrphanIds = dockerOrphanIds.filter((id) => id !== argv[2]); + dockerNameLabeledIds = dockerNameLabeledIds.filter((id) => id !== argv[2]); } return { status, stdout: "", stderr: "" } as ReturnType; } @@ -489,9 +541,18 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr status: 0, stdout: sandboxPresent ? "aaaaaaaaaaaa\topenshell\tdefault\tsb-alpha" : "", }; + const sequencedResult = options.dockerRunResultSequence?.[identityProbeCall - 1]; + const exactCleanupResult = { + status: options.dockerOrphanQueryStatus ?? 0, + stdout: dockerNameLabeledIds + .map((id) => `${id}\topenshell\tdefault\tsb-alpha`) + .join("\n"), + stderr: "", + }; const result = - options.dockerRunResultSequence?.[identityProbeCall - 1] ?? - options.dockerRunResult ?? + (exactDockerCleanupPhase ? exactCleanupResult : undefined) ?? + sequencedResult ?? + dockerIdentityResult ?? defaultIdentityResult; return result as ReturnType; }); @@ -628,12 +689,22 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr promptSpy, removeManagedHermesStateVolumeSpy, removeSandboxSpy, + resolveRetainedSandboxRecoverySpy, retirePortableLifecycleReceiptSpy, revokeHttpsPinRuntimeAdapterRouteSpy, restoreMcpBridgesAfterDestroyAbortSpy, runOpenshellSpy, selectGatewaySpy, sessionState, + setDockerIdentityResult: (result) => { + dockerIdentityResult = result; + }, + setRegistryEntryPresent: (present: boolean) => { + registryEntryPresent = present; + }, + setRetainedRecoveryRecords: (records) => { + retainedRecoveryRecords = [...records]; + }, setSandboxPresent: (present: boolean) => { sandboxPresent = present; }, diff --git a/test/onboarding/onboard-fresh-create-identity.test.ts b/test/onboarding/onboard-fresh-create-identity.test.ts index 42cefb93c78..b526635f735 100644 --- a/test/onboarding/onboard-fresh-create-identity.test.ts +++ b/test/onboarding/onboard-fresh-create-identity.test.ts @@ -851,12 +851,25 @@ if (${JSON.stringify( false, ); }; + const assertCreateAttemptLabelReported = () => { + const match = payload.createCommand?.match( + /--label (ai\.nvidia\.nemoclaw\.create-attempt=[0-9a-f]{62})/u, + ); + assert.ok(match?.[1], "expected the sandbox create-attempt label"); + assert.ok( + `${payload.creationError ?? ""}\n${result.stderr}`.includes( + `Create-attempt label: ${match[1]}`, + ), + "expected recovery output to report the exact create-attempt label", + ); + }; const assertPostCreateAuthorityRefusal = () => { assert.equal(payload.sandboxName, null); assert.equal(payload.sandboxCreated, true); assert.equal(payload.deleted, false); assert.match(payload.creationError, /left sandbox 'my-assistant' in place/u); assert.match(payload.creationError, new RegExp(identityFingerprint, "u")); + assertCreateAttemptLabelReported(); assert.match( payload.creationError, /did not run OpenShell's mutable-name deletion command because the name may now identify a replacement sandbox/u, @@ -880,7 +893,6 @@ if (${JSON.stringify( const record = payload.retainedRecoveryRecords[0]; assert.equal(record.sandboxName, "my-assistant"); assert.equal(record.sandboxIdentityFingerprint, identityFingerprint); - assert.equal(record.identityWasUnavailable, false); assert.equal(record.gatewayName, "nemoclaw-18080"); assert.equal(record.gatewayPort, 18080); assert.match(record.lifecycleGeneration, /^[0-9a-f-]{36}$/u); @@ -893,6 +905,7 @@ if (${JSON.stringify( assert.equal(payload.deleted, false); assert.equal(payload.registeredSandbox, null); assert.match(payload.creationError, /automatic sandbox cleanup was not safe/u); + assertCreateAttemptLabelReported(); assert.equal(payload.savedSession.status, "recovery_required"); assert.equal(payload.savedSession.resumable, false); assert.equal( @@ -915,6 +928,7 @@ if (${JSON.stringify( assert.equal(payload.deleted, false); assert.equal(payload.registeredSandbox, null); assert.match(payload.creationError, /registry publication failed/u); + assertCreateAttemptLabelReported(); assert.equal(payload.savedSession.status, "recovery_required"); assert.equal(payload.savedSession.resumable, false); assert.equal( @@ -924,7 +938,6 @@ if (${JSON.stringify( assert.equal(payload.retainedRecoveryRecords.length, 1); const record = payload.retainedRecoveryRecords[0]; assert.equal(record.sandboxName, "my-assistant"); - assert.equal(record.identityWasUnavailable, false); assert.equal(record.reason, "retained_after_sandbox_creation_failure"); assertRecoveryTuple(record); }; @@ -934,6 +947,7 @@ if (${JSON.stringify( assert.equal(payload.deleted, false); assert.equal(payload.registeredSandbox, null); assert.match(payload.creationError, /recovery record could not be persisted/u); + assertCreateAttemptLabelReported(); assert.equal(payload.savedSession.status, "recovery_required"); assert.equal(payload.savedSession.resumable, false); assert.equal( @@ -980,6 +994,7 @@ if (${JSON.stringify( assert.equal(payload.deleted, false); assert.equal(payload.registeredSandbox, null); assert.match(payload.creationError, /recovery record could not be persisted/u); + assertCreateAttemptLabelReported(); assert.equal(payload.savedSession.status, "recovery_required"); assert.equal(payload.savedSession.resumable, false); assert.equal(payload.retainedRecoveryRecords.length, 1); @@ -1000,6 +1015,7 @@ if (${JSON.stringify( payload.creationError, /OpenShell sandbox policy authority inspection failed/u, ); + assertCreateAttemptLabelReported(); assert.equal(payload.savedSession.status, "recovery_required"); assert.equal(payload.savedSession.resumable, false); assert.equal( @@ -1040,14 +1056,9 @@ if (${JSON.stringify( assert.match(result.stderr, /Do not delete the sandbox by mutable sandbox name/u); assert.match(result.stderr, /Shared inference providers are gateway configuration/u); assert.match(result.stderr, /not sandbox cleanup targets/u); - assert.match(result.stderr, /sandbox-scoped resources whose ownership is confirmed/u); - assert.match(result.stderr, /no supported operation to clear this recovery record/u); - assert.match(result.stderr, /credential environment name alone does not prove exposure/u); - assert.match( - result.stderr, - /rotate a credential only when identity-bound inspection proves/u, - ); - assert.doesNotMatch(result.stderr, /rotate any credential/u); + assert.match(result.stderr, /nemoclaw my-assistant destroy/u); + assert.match(result.stderr, /clear the matching recovery record/u); + assertCreateAttemptLabelReported(); const differentName = spawnSync(process.execPath, [scriptPath], { cwd: repoRoot, diff --git a/test/onboarding/onboard-fsm-live-slices.test.ts b/test/onboarding/onboard-fsm-live-slices.test.ts index 4a8ec615bc2..11ecc8a2600 100644 --- a/test/onboarding/onboard-fsm-live-slices.test.ts +++ b/test/onboarding/onboard-fsm-live-slices.test.ts @@ -478,11 +478,6 @@ if (scenario.mode === "stale-recovery-admission") { verifiedEffectivePolicyIdentity: null, createAttemptNonce: "c".repeat(62), policyCreationReceipt: null, - resources: { - sharedInferenceProviders: [], - sandboxScopedProviders: [], - credentialEnvironmentVariables: [], - }, reason: "retained_after_sandbox_creation_failure", }); return [];