diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 96a549f2657..33178216098 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -177,7 +177,6 @@ const { dockerInspect, dockerRemoveVolumesByPrefix, dockerRm, - dockerRmi, dockerStop, } = docker; const gatewayDrift: typeof import("./adapters/openshell/gateway-drift") = require("./adapters/openshell/gateway-drift"); @@ -2530,20 +2529,6 @@ async function createSandboxWithBaseImageResolution( // biome-ignore format: keep src/lib/onboard.ts net-neutral for growth guardrail. if (recreateRuntime.beginDelete() === "source") { runSandboxProviderPreDeleteCleanup(sandboxName, { runOpenshell, redact }); runOpenshell(["sandbox", "delete", "-g", recreateRuntime.journaledGatewayName ?? GATEWAY_NAME, sandboxName], { ignoreError: true }); if (!waitForSandboxRecreateDeleteAbsence(sandboxName, recreateRuntime.journaledGatewayName ?? GATEWAY_NAME, note)) throw new Error(`Cannot continue sandbox '${sandboxName}' recreation: OpenShell did not confirm explicit source absence after delete.`); } recreateRuntime.confirmDeleted(); - const replacementReusesPreviousImage = - replacementWorkload.source.kind === "managed-image" && - replacementWorkload.source.reference === previousEntry?.imageTag; - if ( - previousEntry?.imageTag && - previousEntry.workload?.shared !== true && - !replacementReusesPreviousImage - ) { - // biome-ignore format: keep src/lib/onboard.ts net-neutral for growth guardrail. - const rmiResult = dockerRmi(previousEntry.imageTag, { ignoreError: true, suppressOutput: true }); - if (rmiResult.status !== 0) { - console.warn(` Warning: failed to remove old sandbox image '${previousEntry.imageTag}'.`); - } - } sandboxLifecycle.removeSandboxUnlessSessionReservation(previousEntry, sandboxName); } const preparedSandboxWorkload = await managedWorkloadRuntime.ensurePreparedWorkload(); @@ -2595,7 +2580,7 @@ async function createSandboxWithBaseImageResolution( createArgv, sandboxEnv, sandboxStartupCommand, - lifecycleRegistrationFields: recreateRuntime.registrationFields, + lifecycleGeneration: recreateRuntime.targetGeneration, prebuild, restoreBackupPath, terminalAgent: agentDefs.isTerminalAgent(agent), @@ -2725,6 +2710,7 @@ async function createSandboxWithBaseImageResolution( hermesDashboardState: finalHermesDashboardState, dashboardPort: actualDashboardPort, ...lifecycleRegistrationFields, + ...recreateRuntime.registrationFields, gatewayName: GATEWAY_NAME, gatewayPort: GATEWAY_PORT, }), diff --git a/src/lib/onboard/machine/handlers/sandbox-recreate-journal.test.ts b/src/lib/onboard/machine/handlers/sandbox-recreate-journal.test.ts index f2a8f4d1572..e74784361b2 100644 --- a/src/lib/onboard/machine/handlers/sandbox-recreate-journal.test.ts +++ b/src/lib/onboard/machine/handlers/sandbox-recreate-journal.test.ts @@ -161,6 +161,61 @@ it("journals not-ready repair on the selected non-default gateway (#6492)", asyn expect(session.checkpoint?.sandboxRecreate).toBeNull(); }); +it.each([ + "replacement-unproven", + "shared-image", + "authority-unproven", + "no-owned-image", + "image-reused", +] as const)("reports the bounded %s image-retirement skip after journaled recreation", async (reason) => { + const session = createSession({ sandboxName: "saved", agent: "openclaw" }); + const journal = bindJournaledRecreate(session); + const sourceEntry: SandboxEntry = { + name: "saved", + provider: "provider", + model: "model", + endpointUrl: null, + preferredInferenceApi: "openai-completions", + webSearchEnabled: false, + toolDisclosure: "progressive", + fromDockerfile: null, + hermesAuthMethod: null, + imageTag: "openshell/sandbox-from:old", + workload: { + schemaVersion: 1, + kind: "legacy-dockerfile", + reference: "openshell/sandbox-from:old", + shared: false, + }, + }; + const retireReplacedSandboxWorkload = vi.fn(() => ({ + status: "skipped" as const, + reason, + })); + const { deps, calls } = createDeps( + { + getSandboxReuseState: () => "not_ready", + getSandboxRecreateObservation: journal.observe, + getSandboxRegistryEntry: () => sourceEntry, + createSandbox: journal.completeCreate, + retireReplacedSandboxWorkload, + }, + session, + ); + + await handleSandboxState({ + ...baseOptions(deps, session), + resume: true, + sandboxName: "saved", + }); + + const diagnostics = calls.note.mock.calls + .map(([message]) => message) + .filter((message) => message.startsWith(" Obsolete sandbox image retirement skipped:")); + expect(diagnostics).toEqual([` Obsolete sandbox image retirement skipped: ${reason}`]); + expect(retireReplacedSandboxWorkload).toHaveBeenCalledOnce(); +}); + it("continues an outer rebuild journal after the outer rebuild deletes the source sandbox", async () => { const session = createSession({ sandboxName: "saved", agent: "openclaw" }); session.steps.sandbox.status = "complete"; diff --git a/src/lib/onboard/machine/handlers/sandbox.ts b/src/lib/onboard/machine/handlers/sandbox.ts index f318a1aa5c5..7e4f8cec14e 100644 --- a/src/lib/onboard/machine/handlers/sandbox.ts +++ b/src/lib/onboard/machine/handlers/sandbox.ts @@ -119,6 +119,19 @@ import { type SandboxResumeDecision, } from "./sandbox-resume"; +type SandboxRecreateWorkloadSkipReason = Extract< + ReplacedSandboxWorkloadCleanupResult, + { readonly status: "skipped" } +>["reason"]; + +const SANDBOX_RECREATE_WORKLOAD_SKIP_DIAGNOSTIC = { + "replacement-unproven": " Obsolete sandbox image retirement skipped: replacement-unproven", + "shared-image": " Obsolete sandbox image retirement skipped: shared-image", + "authority-unproven": " Obsolete sandbox image retirement skipped: authority-unproven", + "no-owned-image": " Obsolete sandbox image retirement skipped: no-owned-image", + "image-reused": " Obsolete sandbox image retirement skipped: image-reused", +} as const satisfies Record; + function isAdvisoryPeerRouteDifference( result: Exclude, sandboxName: string, @@ -1623,6 +1636,8 @@ class SandboxStateFlow< this.deps.note( ` Warning: failed to remove obsolete ${retired.engineDisplayName} image ${retired.reference}; run '${this.deps.cliName()} gc' to clean up.`, ); + } else if (retired.status === "skipped") { + this.deps.note(SANDBOX_RECREATE_WORKLOAD_SKIP_DIAGNOSTIC[retired.reason]); } } diff --git a/src/lib/onboard/runtime-provider/replaced-workload.test.ts b/src/lib/onboard/runtime-provider/replaced-workload.test.ts index b067c444343..37bff4cf6b1 100644 --- a/src/lib/onboard/runtime-provider/replaced-workload.test.ts +++ b/src/lib/onboard/runtime-provider/replaced-workload.test.ts @@ -15,6 +15,7 @@ const TARGET_IDENTITY = "target-identity"; function entry(imageTag: string, generation: string): SandboxEntry { return { name: "alpha", + openshellDriver: "docker", imageTag, workload: { schemaVersion: 1, @@ -152,6 +153,37 @@ describe("same-name replacement workload cleanup", () => { expect(removeImage).not.toHaveBeenCalled(); }); + it.each([ + ["provider identity", ({ openshellDriver: _provider, ...source }: SandboxEntry) => source], + ["workload receipt", ({ workload: _workload, ...source }: SandboxEntry) => source], + [ + "matching workload receipt", + (source: SandboxEntry) => ({ + ...source, + workload: { + schemaVersion: 1 as const, + kind: "legacy-dockerfile" as const, + reference: "openshell/sandbox-from:foreign", + shared: false as const, + }, + }), + ], + ] as const)("does not remove the source image without its durable %s", (_field, mutate) => { + const removeImage = vi.fn(() => ({ status: 0 })); + + expect( + retireReplacedSandboxWorkload( + "alpha", + "target", + TARGET_IDENTITY, + mutate(entry(SOURCE_IMAGE, "source")), + entry(REPLACEMENT_IMAGE, "target"), + { runtimeProviders: providers(removeImage) }, + ), + ).toEqual({ status: "skipped", reason: "authority-unproven" }); + expect(removeImage).not.toHaveBeenCalled(); + }); + it("skips image cleanup only for expected provider-selection failures", () => { const removeImage = vi.fn(() => ({ status: 0 })); diff --git a/src/lib/onboard/sandbox-gpu-create-flow.test.ts b/src/lib/onboard/sandbox-gpu-create-flow.test.ts index d7048656f18..217880717a1 100644 --- a/src/lib/onboard/sandbox-gpu-create-flow.test.ts +++ b/src/lib/onboard/sandbox-gpu-create-flow.test.ts @@ -680,21 +680,17 @@ describe("runSandboxGpuCreateFlow native failure and readiness", () => { it("configures the portable lifecycle after sandbox creation succeeds (#8441)", async () => { const input = createInput(); - input.lifecycleRegistrationFields = { - lifecycleGeneration: "current-generation", - lifecycleLiveIdentityFingerprint: "current-fingerprint", - }; + input.lifecycleGeneration = "current-generation"; const deps = createDeps(); deps.installPortableDemoLifecycle = vi.fn( - () => input.lifecycleRegistrationFields?.lifecycleGeneration ?? null, + (_sandboxName, _startupCommand, _env, options) => options.registryGeneration ?? null, ); - await expect(runSandboxGpuCreateFlow(input, deps)).resolves.toMatchObject({ - lifecycleRegistrationFields: { - lifecycleGeneration: "current-generation", - lifecycleLiveIdentityFingerprint: "current-fingerprint", - }, - route: "native", + const result = await runSandboxGpuCreateFlow(input, deps); + + expect(result.route).toBe("native"); + expect(result.lifecycleRegistrationFields).toEqual({ + lifecycleGeneration: "current-generation", }); expect(deps.installPortableDemoLifecycle).toHaveBeenCalledWith( diff --git a/src/lib/onboard/sandbox-gpu-create-flow.ts b/src/lib/onboard/sandbox-gpu-create-flow.ts index d63cda96d82..6f52aefdbdf 100644 --- a/src/lib/onboard/sandbox-gpu-create-flow.ts +++ b/src/lib/onboard/sandbox-gpu-create-flow.ts @@ -66,10 +66,7 @@ function exitForManagedBootstrapRecovery(error: ManagedBootstrapRecoveryBlockedE type RunOpenshell = NonNullable; type RunCaptureOpenshell = NonNullable; type Sleep = NonNullable; -type LifecycleRegistrationFields = Pick< - SandboxEntry, - "lifecycleGeneration" | "lifecycleLiveIdentityFingerprint" ->; +type LifecycleRegistrationFields = Pick; export interface SandboxGpuCreateFlowInput { sandboxName: string; @@ -84,7 +81,7 @@ export interface SandboxGpuCreateFlowInput { createArgv: string[]; sandboxEnv: NodeJS.ProcessEnv; sandboxStartupCommand: string[]; - lifecycleRegistrationFields?: LifecycleRegistrationFields; + lifecycleGeneration?: SandboxEntry["lifecycleGeneration"]; prebuild: SandboxPrebuildResult; restoreBackupPath: string | null; terminalAgent: boolean; @@ -264,9 +261,7 @@ export async function runSandboxGpuCreateFlow( input.sandboxStartupCommand, process.env, { - ...(input.lifecycleRegistrationFields?.lifecycleGeneration - ? { registryGeneration: input.lifecycleRegistrationFields.lifecycleGeneration } - : {}), + ...(input.lifecycleGeneration ? { registryGeneration: input.lifecycleGeneration } : {}), }, ) ?? null; } catch (error) { @@ -281,7 +276,7 @@ export async function runSandboxGpuCreateFlow( registryImageRef, lifecycleRegistrationFields: { ...(portableLifecycleGeneration ? { lifecycleGeneration: portableLifecycleGeneration } : {}), - ...input.lifecycleRegistrationFields, + ...(input.lifecycleGeneration ? { lifecycleGeneration: input.lifecycleGeneration } : {}), }, }; } diff --git a/src/lib/onboard/sandbox-recreate-transaction.ts b/src/lib/onboard/sandbox-recreate-transaction.ts index baea631917f..1a5fa26e1f3 100644 --- a/src/lib/onboard/sandbox-recreate-transaction.ts +++ b/src/lib/onboard/sandbox-recreate-transaction.ts @@ -82,6 +82,19 @@ export function retireReplacedSandboxWorkload( if (source.workload?.shared === true) { return { status: "skipped", reason: "shared-image" }; } + if (!source.imageTag) { + return { status: "skipped", reason: "no-owned-image" }; + } + if ( + typeof source.openshellDriver !== "string" || + source.openshellDriver.trim().length === 0 || + source.workload?.schemaVersion !== 1 || + source.workload.kind !== "legacy-dockerfile" || + source.workload.shared !== false || + source.workload.reference !== source.imageTag + ) { + return { status: "skipped", reason: "authority-unproven" }; + } const cleanupSource = providerCleanupSource(source); const providers = deps.runtimeProviders ?? CURRENT_RUNTIME_PROVIDER_BUNDLES; diff --git a/test/e2e/live/rebuild-hermes-image-state.ts b/test/e2e/live/rebuild-hermes-image-state.ts index 1c6f8f21957..3efa68c0a4c 100644 --- a/test/e2e/live/rebuild-hermes-image-state.ts +++ b/test/e2e/live/rebuild-hermes-image-state.ts @@ -8,8 +8,20 @@ import { } from "../../../src/lib/domain/sandbox/image-tag"; export interface RebuildHermesRegistryImageState { + openshellDriver: "docker"; imageTag: string; fromDockerfile: null; + workload: { + schemaVersion: 1; + kind: "legacy-dockerfile"; + reference: string; + shared: false; + }; +} + +export interface RebuildHermesReplacementLifecycleReceipt { + lifecycleGeneration: string; + lifecycleLiveIdentityFingerprint: string; } export async function cleanupTrackedRebuildHermesImage( @@ -31,6 +43,28 @@ export function requireRebuildHermesInitialImageTag(value: unknown, sandboxName: return imageTag; } +export function requireRebuildHermesReplacementLifecycleReceipt( + value: Record, +): RebuildHermesReplacementLifecycleReceipt { + const lifecycleGeneration = value.lifecycleGeneration; + const lifecycleLiveIdentityFingerprint = value.lifecycleLiveIdentityFingerprint; + if ( + typeof lifecycleGeneration !== "string" || + !/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/iu.test( + lifecycleGeneration, + ) + ) { + throw new Error("rebuilt Hermes registry is missing its journaled lifecycle generation"); + } + if ( + typeof lifecycleLiveIdentityFingerprint !== "string" || + !/^[0-9a-f]{64}$/u.test(lifecycleLiveIdentityFingerprint) + ) { + throw new Error("rebuilt Hermes registry is missing its live lifecycle identity fingerprint"); + } + return { lifecycleGeneration, lifecycleLiveIdentityFingerprint }; +} + export function rebuildHermesRegistryImageState( createOutput: string, ): RebuildHermesRegistryImageState { @@ -42,5 +76,15 @@ export function rebuildHermesRegistryImageState( `old Hermes sandbox create must report an exact ${prefix} image tag; got ${imageTag ?? ""}`, ); } - return { imageTag, fromDockerfile: null }; + return { + openshellDriver: "docker", + imageTag, + fromDockerfile: null, + workload: { + schemaVersion: 1, + kind: "legacy-dockerfile", + reference: imageTag, + shared: false, + }, + }; } diff --git a/test/e2e/live/rebuild-hermes.test.ts b/test/e2e/live/rebuild-hermes.test.ts index 64636f66910..8976095e791 100644 --- a/test/e2e/live/rebuild-hermes.test.ts +++ b/test/e2e/live/rebuild-hermes.test.ts @@ -55,6 +55,7 @@ import { type RebuildHermesRegistryImageState, rebuildHermesRegistryImageState, requireRebuildHermesInitialImageTag, + requireRebuildHermesReplacementLifecycleReceipt, } from "./rebuild-hermes-image-state.ts"; import { REBUILD_HERMES_OLD_BASE_FIXTURE, @@ -1268,6 +1269,10 @@ test(STALE_BASE_REBUILD resultText(oldImageInspect), ).toBe(true); expect(resultText(oldImageInspect)).toMatch(/No such (?:image|object)(?::|\s)/iu); + await artifacts.writeJson( + "phase-6-replacement-registry-lifecycle-receipt.json", + requireRebuildHermesReplacementLifecycleReceipt(rebuiltRegistry), + ); progress.phase("validate upgraded state inference and backup hygiene"); const restoredMarker = await host.command( diff --git a/test/e2e/support/rebuild-hermes-image-state.test.ts b/test/e2e/support/rebuild-hermes-image-state.test.ts index 85ff9194a8a..86e38e5ed85 100644 --- a/test/e2e/support/rebuild-hermes-image-state.test.ts +++ b/test/e2e/support/rebuild-hermes-image-state.test.ts @@ -6,6 +6,7 @@ import { cleanupTrackedRebuildHermesImage, rebuildHermesRegistryImageState, requireRebuildHermesInitialImageTag, + requireRebuildHermesReplacementLifecycleReceipt, } from "../live/rebuild-hermes-image-state.ts"; describe("Hermes rebuild fixture image ownership", () => { @@ -39,6 +40,30 @@ describe("Hermes rebuild fixture image ownership", () => { ).toThrow("owned"); }); + it("requires the replacement registry row to carry its journaled live identity", () => { + const receipt = { + lifecycleGeneration: "5f63a0a3-e0f0-4e41-847b-8bc7c1f135ad", + lifecycleLiveIdentityFingerprint: "a".repeat(64), + }; + + expect(requireRebuildHermesReplacementLifecycleReceipt(receipt)).toEqual(receipt); + expect(() => requireRebuildHermesReplacementLifecycleReceipt({})).toThrow( + "lifecycle generation", + ); + expect(() => + requireRebuildHermesReplacementLifecycleReceipt({ + ...receipt, + lifecycleGeneration: "5f63a0a3-e0f0-1e41-847b-8bc7c1f135ad", + }), + ).toThrow("lifecycle generation"); + expect(() => + requireRebuildHermesReplacementLifecycleReceipt({ + ...receipt, + lifecycleLiveIdentityFingerprint: "unproven", + }), + ).toThrow("live lifecycle identity"); + }); + it("retains the exact OpenShell-derived tag in managed rebuild state", () => { expect( rebuildHermesRegistryImageState( @@ -48,8 +73,15 @@ describe("Hermes rebuild fixture image ownership", () => { ].join("\n"), ), ).toEqual({ + openshellDriver: "docker", imageTag: "openshell/sandbox-from:1784010200", fromDockerfile: null, + workload: { + schemaVersion: 1, + kind: "legacy-dockerfile", + reference: "openshell/sandbox-from:1784010200", + shared: false, + }, }); }); diff --git a/test/onboard-sandbox-recreation.test.ts b/test/onboard-sandbox-recreation.test.ts index 16b05615715..01fb41a82b4 100644 --- a/test/onboard-sandbox-recreation.test.ts +++ b/test/onboard-sandbox-recreation.test.ts @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import assert from "node:assert/strict"; +import { createHash } from "node:crypto"; import { spawnSync } from "node:child_process"; import fs from "node:fs"; import os from "node:os"; @@ -100,7 +101,7 @@ const { createSandbox } = require(${onboardPath}); it.each([ "balanced", "restricted", - ])("recreate-sandbox materializes and records the %s policy tier", { + ])("recreate-sandbox records the %s policy tier and late replacement identity", { timeout: 60_000, }, async (policyTier) => { const repoRoot = path.join(import.meta.dirname, ".."); @@ -118,19 +119,31 @@ const { createSandbox } = require(${onboardPath}); const runner = require(${runnerPath}); require(${onboardScriptMocksPath}).mockStandaloneGatewayTeardownAuthority(); const _n = (c) => (Array.isArray(c) ? c.join(" ") : String(c)).replace(/'/g, ""); -let _deleted = false; +let _deleted = false; let _sandboxId = "sbx-4f2a91c0d7"; const registry = require(${registryPath}); const childProcess = require("node:child_process"); const { EventEmitter } = require("node:events"); const commands = []; let registeredSandbox = null; +const sourceSandbox = { + name: "my-assistant", + gpuEnabled: false, + openshellDriver: "docker", + imageTag: "openshell/sandbox-from:source", + workload: { + schemaVersion: 1, + kind: "legacy-dockerfile", + reference: "openshell/sandbox-from:source", + shared: false, + }, +}; runner.run = (command, opts = {}) => { _deleted = _deleted || _n(command).includes("sandbox delete"); commands.push({ command: _n(command), env: opts.env || null }); return { status: 0 }; }; runner.runCapture = (command) => { - if (_n(command).includes("sandbox get") && _n(command).includes("my-assistant")) return _deleted ? "" : ["my-assistant", "Id: sbx-4f2a91c0d7"].join(String.fromCharCode(10)); + if (_n(command).includes("sandbox get") && _n(command).includes("my-assistant")) return _deleted ? "" : ["my-assistant", "Id: " + _sandboxId].join(String.fromCharCode(10)); if (_n(command).includes("sandbox list")) return _deleted ? "" : "my-assistant Ready"; if (_n(command).includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running"; { @@ -141,7 +154,7 @@ runner.runCapture = (command) => { } return ""; }; -registry.getSandbox = () => ({ name: "my-assistant", gpuEnabled: false }); +registry.getSandbox = () => registeredSandbox || sourceSandbox; registry.registerSandbox = (entry) => { registeredSandbox = entry; return true; }; registry.updateSandbox = () => true; registry.setDefault = () => true; @@ -152,6 +165,7 @@ preflight.checkPortAvailable = async () => ({ ok: true }); childProcess.spawn = (...args) => { _deleted = false; + _sandboxId = "sbx-8e6b10fd33"; const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); @@ -210,6 +224,21 @@ const { createSandbox } = require(${onboardPath}); payload.registeredSandbox?.policyTier === policyTier, "should create a sandbox and persist its tier before policy finalization", ); + assert.ok( + !payload.commands.some((entry: CommandEntry) => + entry.command.includes("docker rmi openshell/sandbox-from:source"), + ), + "must defer source image retirement until replacement registration is proven", + ); + const sourceFingerprint = createHash("sha256").update("sbx-4f2a91c0d7").digest("hex"); + const replacementFingerprint = createHash("sha256").update("sbx-8e6b10fd33").digest("hex"); + assert.match(payload.registeredSandbox?.lifecycleGeneration ?? "", /^[0-9a-f-]{36}$/); + assert.equal( + payload.registeredSandbox?.lifecycleLiveIdentityFingerprint, + replacementFingerprint, + "replacement registration must read the live identity after creation", + ); + assert.notEqual(payload.registeredSandbox?.lifecycleLiveIdentityFingerprint, sourceFingerprint); }); it("recreate-sandbox flag backs up and restores workspace state", { timeout: 60_000,