diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index dec04499970..60c37efa307 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -3493,7 +3493,7 @@ jobs: needs: generate-matrix if: ${{ (github.event_name != 'workflow_dispatch' || (inputs.jobs == '' && inputs.targets == '')) || contains(format(',{0},', inputs.jobs), ',upgrade-stale-sandbox,') || contains(format(',{0},', inputs.targets), ',upgrade-stale-sandbox,') }} runs-on: ubuntu-latest - timeout-minutes: 55 + timeout-minutes: 85 env: E2E_JOB: "1" E2E_TARGET_ID: "upgrade-stale-sandbox" diff --git a/docs/inference/use-shared-gateway-routes.mdx b/docs/inference/use-shared-gateway-routes.mdx index 96faa4c2b78..421c5c27378 100644 --- a/docs/inference/use-shared-gateway-routes.mdx +++ b/docs/inference/use-shared-gateway-routes.mdx @@ -38,6 +38,17 @@ Before changing the route, `connect` verifies the same provider-global identity When the identity is compatible, `connect` warns and re-points the route to the sandbox's recorded provider and model. When the identity differs or required metadata is incomplete, `connect` stops because a provider-and-model-only route change cannot safely reconstruct that configuration. +## Rebuild a Legacy Shared Route + +During rebuild, NemoClaw may find same-gateway legacy sandbox records that use the selected supported provider but omit its credential environment-variable name. +Before deleting the target sandbox, NemoClaw fills only those missing names from the provider's canonical configuration and saves the target and peer metadata together. +The peer migration does not replace an explicit credential environment-variable name. + +Credential environment-variable name, custom endpoint, or API-family conflicts still stop the rebuild. +Incomplete routes and invalid gateway bindings also stop the rebuild. +NemoClaw reads a fresh registry snapshot immediately before deletion. +If that snapshot contains a target route change or peer provider-identity conflict, rebuild stops and leaves the original sandbox intact. + ## Inspect Recorded and Live Routes Run sandbox status to compare the sandbox's recorded route with the gateway's live route. diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index 674ccb61304..fc7a16bc883 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -2725,6 +2725,11 @@ Credentials are stripped from backups before storage. Policy presets applied to the old sandbox are reapplied to the new one so your egress rules survive the rebuild. Before creating the replacement sandbox, NemoClaw prints the finalized create-time policy scope whenever presets are included. The replacement uses the recorded compatible-endpoint reasoning mode, reasoning effort, and web search selection instead of ambient shell values. +When same-gateway legacy sandbox records use the selected supported provider but omit its credential environment-variable name, rebuild fills only those missing names from the provider's canonical configuration. +The target update and peer metadata migration use one registry update. +Conflicting credential environment-variable names, custom endpoints, or API families still stop the rebuild. +Incomplete routes and invalid gateway bindings also stop the rebuild. +NemoClaw checks the shared route again immediately before deleting the original sandbox. Rebuild preserves the recorded sandbox GPU enablement mode and, for an explicitly enabled sandbox, its recorded device selector. It re-resolves the Docker-driver GPU route from the current host and current `NEMOCLAW_DOCKER_GPU_PATCH` value, so native-only, explicitly authorized native-with-fallback, and compatibility-only routing may differ from the original onboarding run. A rebuild preserves the recorded tool-disclosure mode unless `--tool-disclosure` explicitly changes it; it ignores an ambient `NEMOCLAW_TOOL_DISCLOSURE` value while recreating the sandbox. diff --git a/src/lib/actions/sandbox/rebuild-destroy-phase.test.ts b/src/lib/actions/sandbox/rebuild-destroy-phase.test.ts index 993d3a6a92a..911ddc4077d 100644 --- a/src/lib/actions/sandbox/rebuild-destroy-phase.test.ts +++ b/src/lib/actions/sandbox/rebuild-destroy-phase.test.ts @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { expectNoSandboxDelete } from "../../../../test/helpers/rebuild-delete-assertions"; const mocks = vi.hoisted(() => ({ captureOpenshell: vi.fn(), @@ -182,6 +183,32 @@ describe("rebuild destroy phase", () => { expect(relockShieldsIfNeeded).toHaveBeenCalledWith(true); }); + it("blocks the exact delete edge when the shared inference route drifts (#7798)", async () => { + const bail = vi.fn((message: string): never => { + throw new Error(message); + }); + + await expect( + runRebuildDestroyPhase({ + sandboxName: "alpha", + sandboxEntry: { name: "alpha", agent: "openclaw" }, + staleRecovery: false, + backupManifest: null, + log: vi.fn(), + bail, + relockShieldsIfNeeded: vi.fn(() => true), + validateAtDeleteEdge: () => ({ + ok: false, + message: "Shared inference route changed before sandbox deletion.", + }), + onDeleted: vi.fn(), + }), + ).rejects.toThrow("Shared inference route changed before sandbox deletion."); + + expect(mocks.reattachMcpAfterDeleteFailure).toHaveBeenCalledOnce(); + expectNoSandboxDelete(mocks.runOpenshell); + }); + it("passes force=true to prepareMcpForRebuild when input.force is set (#7062)", async () => { const log = vi.fn(); const bail = vi.fn((message: string): never => { diff --git a/src/lib/actions/sandbox/rebuild-destroy-phase.ts b/src/lib/actions/sandbox/rebuild-destroy-phase.ts index 9226de56d55..ba5eca3049b 100644 --- a/src/lib/actions/sandbox/rebuild-destroy-phase.ts +++ b/src/lib/actions/sandbox/rebuild-destroy-phase.ts @@ -39,6 +39,7 @@ export interface RebuildDestroyPhaseInput { relockShieldsIfNeeded: (sandboxStillExists: boolean) => boolean; force?: boolean; validateAfterMcpPreparation?: () => Promise; + validateAtDeleteEdge?: () => RebuildDeleteValidationResult; onDeleted: () => void; onDeleteStateAmbiguous?: () => void; } @@ -229,6 +230,7 @@ export async function runRebuildDestroyPhase( bail, relockShieldsIfNeeded, validateAfterMcpPreparation, + validateAtDeleteEdge, onDeleted, } = input; const deleteTarget = resolveRebuildDeleteTarget(sandboxName, input.sandboxEntry); @@ -355,6 +357,35 @@ export async function runRebuildDestroyPhase( return null; } + if (validateAtDeleteEdge) { + let validation: RebuildDeleteValidationResult; + try { + validation = validateAtDeleteEdge(); + } catch (error) { + const detail = error instanceof Error ? error.message : String(error); + log(`Unexpected delete-edge validation failure: ${redactFull(detail)}`); + validation = { + ok: false, + message: "Replacement validation failed before sandbox deletion.", + }; + } + if (!validation.ok) { + const mcpRecoveryFailure = await reattachMcpAfterDeleteFailure( + sandboxName, + rebuildDetachedMcpProviderEntries, + rebuildScrubbedMcpAdapterEntries, + ); + relockShieldsIfNeeded(true); + bail( + mcpRecoveryFailure + ? `${validation.message} MCP provider recovery also failed: ${mcpRecoveryFailure}` + : validation.message, + validation.code, + ); + return null; + } + } + log(`Running: openshell sandbox delete -g ${gatewayName} ${sandboxName}`); const deleteResult = runOpenshell(["sandbox", "delete", "-g", gatewayName, sandboxName], { ignoreError: true, diff --git a/src/lib/actions/sandbox/rebuild-pipeline.ts b/src/lib/actions/sandbox/rebuild-pipeline.ts index 1f973340629..55998649437 100644 --- a/src/lib/actions/sandbox/rebuild-pipeline.ts +++ b/src/lib/actions/sandbox/rebuild-pipeline.ts @@ -20,7 +20,10 @@ import { disposeRebuildAgentBaseImagePreflight } from "./rebuild-flow-helpers"; import { stageMessagingManifestPlanForRebuild } from "./rebuild-messaging-phase"; import { runRebuildPostRestorePhase } from "./rebuild-post-restore-phase"; import { printRebuildPreflightFailure } from "./rebuild-preflight-error"; -import { blockRebuildOnPendingBaselineTransition } from "./rebuild-preflight-guards"; +import { + blockRebuildOnPendingBaselineTransition, + revalidateRebuildRouteBeforeDelete, +} from "./rebuild-preflight-guards"; import { runRebuildPreflightPhase } from "./rebuild-preflight-phase"; import { disposePreparedBuildContext, @@ -102,6 +105,7 @@ async function rebuildSandboxUnlocked( recoveryManifest: validatedRecoveryManifest, dcodePreflight, preparedImage, + routePreflightReceipt, releaseOnboardLock, log, bail, @@ -251,6 +255,7 @@ async function rebuildSandboxUnlocked( recreateOptions.targetGatewayPort, ); }, + validateAtDeleteEdge: () => revalidateRebuildRouteBeforeDelete(routePreflightReceipt), onDeleted: () => { sandboxStillExists = false; }, diff --git a/src/lib/actions/sandbox/rebuild-preflight-guards.ts b/src/lib/actions/sandbox/rebuild-preflight-guards.ts index 9e086abfc4b..01bfe655c7e 100644 --- a/src/lib/actions/sandbox/rebuild-preflight-guards.ts +++ b/src/lib/actions/sandbox/rebuild-preflight-guards.ts @@ -6,13 +6,229 @@ import { printOpenShellStateRpcIssue, } from "../../adapters/openshell/gateway-drift"; import { CLI_NAME } from "../../cli/branding"; +import { + checkGatewayRouteCompatibility, + formatGatewayRouteConflict, + type GatewayInferenceRoute, + isAdvisoryGatewayRouteConflict, +} from "../../inference/gateway-route-compatibility"; +import { normalizeInferenceSelection } from "../../inference/selection"; import { resolveSandboxGatewayName } from "../../onboard/gateway-binding"; import * as onboardSession from "../../state/onboard-session"; import * as registry from "../../state/registry"; +import { withLock } from "../../state/registry/lock"; +import { load, save } from "../../state/registry/persistence"; +import type { SandboxEntry, SandboxRegistry } from "../../state/registry/types"; import type { RebuildBail } from "./rebuild-credential-preflight"; import type { RebuildSandboxEntry } from "./rebuild-flow-helpers"; import type { RebuildVersionCheck } from "./rebuild-preflight-confirmation"; import { printRebuildPreflightFailure } from "./rebuild-preflight-error"; +import { getRebuildCredentialEnvFromRegistry } from "./rebuild-resume-preflight"; + +export interface RebuildRoutePreflightReceipt { + readonly sandboxName: string; + readonly gatewayName: string; + readonly route: GatewayInferenceRoute; + readonly migratedSandboxNames: readonly string[]; +} + +export type RebuildRoutePreflightResult = + | { ok: true; receipt: RebuildRoutePreflightReceipt } + | { ok: false; message: string }; + +interface RebuildRouteRegistryDependencies { + withLock(fn: () => T): T; + load(): SandboxRegistry; + save(data: SandboxRegistry): void; +} + +const defaultRouteDependencies: RebuildRouteRegistryDependencies = { + withLock, + load, + save, +}; + +function normalizedRoute(entry: Partial): GatewayInferenceRoute { + const route = normalizeInferenceSelection(entry); + return { + provider: route.provider, + model: route.model, + endpointUrl: route.endpointUrl, + preferredInferenceApi: route.preferredInferenceApi, + credentialEnv: route.credentialEnv, + }; +} + +function missingCredentialIdentity(value: unknown): boolean { + return typeof value !== "string" || value.trim().length === 0; +} + +function sameRoute(left: GatewayInferenceRoute, right: GatewayInferenceRoute): boolean { + return ( + left.provider === right.provider && + left.model === right.model && + left.endpointUrl === right.endpointUrl && + left.preferredInferenceApi === right.preferredInferenceApi && + left.credentialEnv === right.credentialEnv + ); +} + +function hardRouteConflict( + gatewayName: string, + sandboxName: string, + route: GatewayInferenceRoute, + sandboxes: readonly SandboxEntry[], +): string | null { + if (!route.provider || !route.model) { + return "Prepared rebuild inference route is missing its provider or model."; + } + const compatibility = checkGatewayRouteCompatibility({ + gatewayName, + sandboxName, + route, + sandboxes, + }); + if (compatibility.ok || isAdvisoryGatewayRouteConflict(compatibility)) return null; + return formatGatewayRouteConflict(compatibility); +} + +/** + * Persist the target route and canonical credential identities for compatible + * legacy peers in one registry transaction. The ordinary compatibility guard + * remains literal and fail-closed; only this rebuild migration may fill a + * missing identity from the provider's canonical configuration. + */ +export function commitRebuildRoutePreflight( + input: { + sandboxName: string; + gatewayName: string; + targetUpdate: Partial>; + }, + dependencies: RebuildRouteRegistryDependencies = defaultRouteDependencies, +): RebuildRoutePreflightResult { + return dependencies.withLock(() => { + const sandboxRegistry = dependencies.load(); + const currentTarget = sandboxRegistry.sandboxes[input.sandboxName]; + if (!currentTarget) { + return { + ok: false, + message: "Sandbox registry entry disappeared during rebuild route preflight.", + }; + } + let targetGatewayName: string; + try { + targetGatewayName = resolveSandboxGatewayName(currentTarget); + } catch { + return { + ok: false, + message: "Sandbox gateway binding changed during rebuild route preflight.", + }; + } + if (targetGatewayName !== input.gatewayName) { + return { + ok: false, + message: "Sandbox gateway binding changed during rebuild route preflight.", + }; + } + + const target = { ...currentTarget, ...input.targetUpdate }; + const targetRoute = normalizedRoute(target); + const migratedSandboxNames: string[] = []; + for (const peer of Object.values(sandboxRegistry.sandboxes)) { + if ( + peer.name === input.sandboxName || + peer.provider !== targetRoute.provider || + !missingCredentialIdentity(peer.credentialEnv) + ) { + continue; + } + let peerGatewayName: string; + try { + peerGatewayName = resolveSandboxGatewayName(peer); + } catch { + continue; + } + if (peerGatewayName !== input.gatewayName) continue; + const credentialEnv = getRebuildCredentialEnvFromRegistry(peer.provider, peer.credentialEnv); + if (!credentialEnv) continue; + peer.credentialEnv = credentialEnv; + migratedSandboxNames.push(peer.name); + } + + const projectedSandboxes = Object.values(sandboxRegistry.sandboxes).map((entry) => + entry.name === input.sandboxName ? target : entry, + ); + const conflict = hardRouteConflict( + input.gatewayName, + input.sandboxName, + targetRoute, + projectedSandboxes, + ); + if (conflict) return { ok: false, message: conflict }; + + Object.assign(currentTarget, input.targetUpdate); + dependencies.save(sandboxRegistry); + return { + ok: true, + receipt: { + sandboxName: input.sandboxName, + gatewayName: input.gatewayName, + route: targetRoute, + migratedSandboxNames: migratedSandboxNames.sort(), + }, + }; + }); +} + +/** + * Re-read the complete shared-gateway route at the synchronous delete edge. + * A target-route change or peer hard conflict in that snapshot invalidates the + * earlier preflight receipt. + */ +export function revalidateRebuildRouteBeforeDelete( + receipt: RebuildRoutePreflightReceipt, + dependencies: Pick = defaultRouteDependencies, +): RebuildRoutePreflightResult { + // Registry writes install complete files atomically. A read lock would end before + // the external delete, so this guard uses a fresh fail-closed snapshot. + const sandboxRegistry = dependencies.load(); + const target = sandboxRegistry.sandboxes[receipt.sandboxName]; + if (!target) { + return { + ok: false, + message: "Sandbox registry entry disappeared before sandbox deletion.", + }; + } + let gatewayName: string; + try { + gatewayName = resolveSandboxGatewayName(target); + } catch { + return { + ok: false, + message: "Sandbox gateway binding changed before sandbox deletion.", + }; + } + if (gatewayName !== receipt.gatewayName) { + return { + ok: false, + message: "Sandbox gateway binding changed before sandbox deletion.", + }; + } + const currentRoute = normalizedRoute(target); + if (!sameRoute(currentRoute, receipt.route)) { + return { + ok: false, + message: "Sandbox inference route changed before sandbox deletion.", + }; + } + const conflict = hardRouteConflict( + receipt.gatewayName, + receipt.sandboxName, + currentRoute, + Object.values(sandboxRegistry.sandboxes), + ); + return conflict ? { ok: false, message: conflict } : { ok: true, receipt }; +} export function checkRebuildGatewaySchemaPreflight( sandboxName: string, diff --git a/src/lib/actions/sandbox/rebuild-preflight-phase.ts b/src/lib/actions/sandbox/rebuild-preflight-phase.ts index 03a9c3b1296..689ba3c4591 100644 --- a/src/lib/actions/sandbox/rebuild-preflight-phase.ts +++ b/src/lib/actions/sandbox/rebuild-preflight-phase.ts @@ -43,6 +43,7 @@ import { expectedRebuildEntryAfterVersionCheck, getRebuildSandboxEntryOrBail, isSingleAgentRebuildSupported, + type RebuildRoutePreflightReceipt, runRebuildGatewayIntentPreflight, } from "./rebuild-preflight-guards"; import { prepareRebuildTargetPreflights } from "./rebuild-preflight-target-phase"; @@ -66,6 +67,7 @@ export interface RebuildPreflightPhaseResult { recoveryManifest: RebuildManifest | null; dcodePreflight: DcodeRebuildOrchestrator; preparedImage: PreparedRebuildImage | null; + routePreflightReceipt: RebuildRoutePreflightReceipt; releaseOnboardLock: () => void; log: RebuildLog; bail: RebuildBail; diff --git a/src/lib/actions/sandbox/rebuild-preflight-target-phase.ts b/src/lib/actions/sandbox/rebuild-preflight-target-phase.ts index 3d396ddea13..6e92a68aec2 100644 --- a/src/lib/actions/sandbox/rebuild-preflight-target-phase.ts +++ b/src/lib/actions/sandbox/rebuild-preflight-target-phase.ts @@ -32,7 +32,11 @@ import { import type { RebuildRecreateOnboardOpts } from "./rebuild-gpu-opt-out"; import { preflightRebuildMessagingConflicts } from "./rebuild-messaging-conflict-preflight"; import { stageRebuildMessagingPlanOrBail } from "./rebuild-messaging-phase"; -import { checkRebuildGatewaySchemaPreflight } from "./rebuild-preflight-guards"; +import { + checkRebuildGatewaySchemaPreflight, + commitRebuildRoutePreflight, + type RebuildRoutePreflightReceipt, +} from "./rebuild-preflight-guards"; import { disposePreparedBuildContext } from "./rebuild-prepared-image-context"; import { hydrateMessagingConfigForRebuild, @@ -75,6 +79,7 @@ export interface RebuildPreparedTarget { messagingPlan: SandboxMessagingPlan | null; baseImagePreflight: RebuildAgentBaseImagePreflight; preparedImage: PreparedRebuildImage | null; + routePreflightReceipt: RebuildRoutePreflightReceipt; } /** Carry the outer resolver's verified provenance into the inner onboard build. */ @@ -279,11 +284,21 @@ export async function prepareRebuildTargetPreflights(args: { fromDockerfile, credentialEnv, ); - if (!registry.updateSandbox(sandboxName, validatedRegistryUpdate)) { - bail("Sandbox registry entry disappeared during rebuild preflight"); + const routePreflight = commitRebuildRoutePreflight({ + sandboxName, + gatewayName: recreateOptions.targetGatewayName, + targetUpdate: validatedRegistryUpdate, + }); + if (!routePreflight.ok) { + bail(routePreflight.message); return null; } Object.assign(sandboxEntry, validatedRegistryUpdate); + if (routePreflight.receipt.migratedSandboxNames.length > 0) { + console.log( + `Migrated legacy shared-gateway credential metadata for: ${routePreflight.receipt.migratedSandboxNames.join(", ")}`, + ); + } if (preparedImage) { recreateOptions.preparedImageRebuild = { buildContext: preparedImage, @@ -299,6 +314,7 @@ export async function prepareRebuildTargetPreflights(args: { messagingPlan, baseImagePreflight, preparedImage, + routePreflightReceipt: routePreflight.receipt, }; } finally { if (!retainPreparedImage && preparedImage) disposePreparedBuildContext(preparedImage); diff --git a/src/lib/actions/sandbox/rebuild-resume-snapshot.test.ts b/src/lib/actions/sandbox/rebuild-resume-snapshot.test.ts index 36e0aae14ff..83472557089 100644 --- a/src/lib/actions/sandbox/rebuild-resume-snapshot.test.ts +++ b/src/lib/actions/sandbox/rebuild-resume-snapshot.test.ts @@ -22,6 +22,7 @@ import * as destroy from "./destroy"; import { rebuildSandbox } from "./rebuild"; import * as rebuildImagePreflight from "./rebuild-custom-image-preflight"; import { rebuildOnboardDependencies } from "./rebuild-onboard-dependencies"; +import * as rebuildRoutePreflight from "./rebuild-preflight-guards"; import * as rebuildShields from "./rebuild-shields"; import * as rebuildUsageNotice from "./rebuild-usage-notice"; @@ -130,6 +131,24 @@ describe("rebuild resume snapshot repair", () => { } as never), vi.spyOn(registry, "updateSandbox").mockReturnValue(true), vi.spyOn(registry, "listSandboxes").mockReturnValue({ sandboxes: [] } as never), + vi.spyOn(rebuildRoutePreflight, "commitRebuildRoutePreflight").mockReturnValue({ + ok: true, + receipt: { + sandboxName: "alpha", + gatewayName: "nemoclaw", + route: { + provider: "ollama-local", + model: "nvidia/nemotron", + endpointUrl: null, + preferredInferenceApi: null, + credentialEnv: null, + }, + migratedSandboxNames: [], + }, + }), + vi + .spyOn(rebuildRoutePreflight, "revalidateRebuildRouteBeforeDelete") + .mockImplementation((receipt) => ({ ok: true, receipt })), vi.spyOn(sandboxSession, "getActiveSandboxSessions").mockReturnValue({ detected: false, sessions: [], diff --git a/src/lib/actions/sandbox/rebuild-route-preflight.test.ts b/src/lib/actions/sandbox/rebuild-route-preflight.test.ts new file mode 100644 index 00000000000..1226c19da28 --- /dev/null +++ b/src/lib/actions/sandbox/rebuild-route-preflight.test.ts @@ -0,0 +1,324 @@ +// 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 type { SandboxEntry, SandboxRegistry } from "../../state/registry/types"; +import { + commitRebuildRoutePreflight, + type RebuildRoutePreflightReceipt, + revalidateRebuildRouteBeforeDelete, +} from "./rebuild-preflight-guards"; + +const { LOCAL_INFERENCE_PROVIDERS, REMOTE_PROVIDER_CONFIG } = + require("../../onboard/providers") as { + LOCAL_INFERENCE_PROVIDERS: string[]; + REMOTE_PROVIDER_CONFIG: Record< + string, + { + providerName: string; + credentialEnv: string | null; + endpointUrl?: string | null; + } + >; + }; + +function sandbox( + name: string, + provider: string, + overrides: Partial = {}, +): SandboxEntry { + const custom = provider.includes("compatible"); + return { + name, + provider, + model: "model-a", + endpointUrl: custom ? "https://inference.example.test/v1" : null, + preferredInferenceApi: + provider === "compatible-anthropic-endpoint" + ? "anthropic-messages" + : custom + ? "openai-completions" + : null, + credentialEnv: null, + gatewayName: "nemoclaw", + ...overrides, + }; +} + +function registry(...entries: SandboxEntry[]): SandboxRegistry { + return { + sandboxes: Object.fromEntries(entries.map((entry) => [entry.name, entry])), + defaultSandbox: entries[0]?.name ?? null, + }; +} + +function transactionDependencies(initial: SandboxRegistry) { + let persisted = structuredClone(initial); + const save = vi.fn((next: SandboxRegistry) => { + persisted = structuredClone(next); + }); + return { + dependencies: { + withLock: (fn: () => T): T => fn(), + load: () => structuredClone(persisted), + save, + }, + persisted: () => persisted, + save, + }; +} + +function targetUpdate(entry: SandboxEntry): Partial> { + return { + provider: entry.provider, + model: entry.model, + endpointUrl: entry.endpointUrl, + preferredInferenceApi: entry.preferredInferenceApi, + credentialEnv: entry.credentialEnv, + }; +} + +const remoteProviders = [ + ...Object.values(REMOTE_PROVIDER_CONFIG), + { + providerName: "nvidia-nim", + credentialEnv: REMOTE_PROVIDER_CONFIG.build?.credentialEnv ?? null, + }, +].filter( + (provider): provider is typeof provider & { credentialEnv: string } => + typeof provider.credentialEnv === "string" && provider.credentialEnv.length > 0, +); + +describe("commitRebuildRoutePreflight", () => { + it("includes a credential-bearing provider in the migration matrix (#7798)", () => { + expect(remoteProviders.length).toBeGreaterThan(0); + }); + + it.each( + remoteProviders, + )("migrates missing shared-gateway credential identity for $providerName (#7798)", (providerConfig) => { + const target = sandbox("target", providerConfig.providerName, { + credentialEnv: providerConfig.credentialEnv, + }); + const peer = sandbox("peer", providerConfig.providerName); + const state = transactionDependencies(registry(target, peer)); + + const result = commitRebuildRoutePreflight( + { + sandboxName: target.name, + gatewayName: "nemoclaw", + targetUpdate: targetUpdate(target), + }, + state.dependencies, + ); + + expect(result).toMatchObject({ + ok: true, + receipt: { + migratedSandboxNames: ["peer"], + }, + }); + expect(state.persisted().sandboxes.target?.credentialEnv).toBe(providerConfig.credentialEnv); + expect(state.persisted().sandboxes.peer?.credentialEnv).toBe(providerConfig.credentialEnv); + expect(state.save).toHaveBeenCalledOnce(); + }); + + it.each( + LOCAL_INFERENCE_PROVIDERS, + )("keeps credential-free local provider %s compatible (#7798)", (provider) => { + const target = sandbox("target", provider); + const peer = sandbox("peer", provider); + const state = transactionDependencies(registry(target, peer)); + + const result = commitRebuildRoutePreflight( + { + sandboxName: target.name, + gatewayName: "nemoclaw", + targetUpdate: targetUpdate(target), + }, + state.dependencies, + ); + + expect(result).toMatchObject({ + ok: true, + receipt: { migratedSandboxNames: [] }, + }); + expect(state.persisted().sandboxes.peer?.credentialEnv).toBeNull(); + }); + + it("keeps credential-free routed inference compatible (#7798)", () => { + const target = sandbox("target", "nvidia-router"); + const peer = sandbox("peer", "nvidia-router"); + const state = transactionDependencies(registry(target, peer)); + + const result = commitRebuildRoutePreflight( + { + sandboxName: target.name, + gatewayName: "nemoclaw", + targetUpdate: targetUpdate(target), + }, + state.dependencies, + ); + + expect(result).toMatchObject({ + ok: true, + receipt: { migratedSandboxNames: [] }, + }); + }); + + it("does not replace an explicit conflicting credential identity (#7798)", () => { + const target = sandbox("target", "nvidia-prod", { + credentialEnv: "NVIDIA_INFERENCE_API_KEY", + }); + const peer = sandbox("peer", "nvidia-prod", { credentialEnv: "OPENAI_API_KEY" }); + const state = transactionDependencies(registry(target, peer)); + + const result = commitRebuildRoutePreflight( + { + sandboxName: target.name, + gatewayName: "nemoclaw", + targetUpdate: targetUpdate(target), + }, + state.dependencies, + ); + + expect(result).toMatchObject({ ok: false }); + expect(result.ok ? "" : result.message).toContain("credential identity"); + expect(state.save).not.toHaveBeenCalled(); + }); + + it("does not migrate peers when the target gateway binding changed (#7798)", () => { + const target = sandbox("target", "nvidia-prod", { + credentialEnv: "NVIDIA_INFERENCE_API_KEY", + gatewayName: "nemoclaw-19080", + gatewayPort: 19080, + }); + const peer = sandbox("peer", "nvidia-prod"); + const state = transactionDependencies(registry(target, peer)); + + const result = commitRebuildRoutePreflight( + { + sandboxName: target.name, + gatewayName: "nemoclaw", + targetUpdate: targetUpdate(target), + }, + state.dependencies, + ); + + expect(result).toEqual({ + ok: false, + message: "Sandbox gateway binding changed during rebuild route preflight.", + }); + expect(state.save).not.toHaveBeenCalled(); + }); + + it("does not hide a custom endpoint conflict while migrating credentials (#7798)", () => { + const target = sandbox("target", "compatible-endpoint", { + credentialEnv: "COMPATIBLE_API_KEY", + }); + const peer = sandbox("peer", "compatible-endpoint", { + endpointUrl: "https://other.example.test/v1", + }); + const state = transactionDependencies(registry(target, peer)); + + const result = commitRebuildRoutePreflight( + { + sandboxName: target.name, + gatewayName: "nemoclaw", + targetUpdate: targetUpdate(target), + }, + state.dependencies, + ); + + expect(result).toMatchObject({ ok: false }); + expect(result.ok ? "" : result.message).toContain("endpoint"); + expect(state.save).not.toHaveBeenCalled(); + }); + + it("migrates stopped peers and leaves another gateway untouched (#7798)", () => { + const target = sandbox("target", "nvidia-prod", { + credentialEnv: "NVIDIA_INFERENCE_API_KEY", + }); + const stoppedPeer = sandbox("stopped-peer", "nvidia-prod"); + const otherGateway = sandbox("other-gateway", "nvidia-prod", { + gatewayName: "nemoclaw-19080", + gatewayPort: 19080, + }); + const state = transactionDependencies(registry(target, stoppedPeer, otherGateway)); + + const result = commitRebuildRoutePreflight( + { + sandboxName: target.name, + gatewayName: "nemoclaw", + targetUpdate: targetUpdate(target), + }, + state.dependencies, + ); + + expect(result).toMatchObject({ + ok: true, + receipt: { migratedSandboxNames: ["stopped-peer"] }, + }); + expect(state.persisted().sandboxes["stopped-peer"]?.credentialEnv).toBe( + "NVIDIA_INFERENCE_API_KEY", + ); + expect(state.persisted().sandboxes["other-gateway"]?.credentialEnv).toBeNull(); + }); +}); + +describe("revalidateRebuildRouteBeforeDelete", () => { + function receipt(route: SandboxEntry): RebuildRoutePreflightReceipt { + return { + sandboxName: route.name, + gatewayName: "nemoclaw", + route: targetUpdate(route), + migratedSandboxNames: ["peer"], + }; + } + + it("accepts the unchanged migrated shared route (#7798)", () => { + const target = sandbox("target", "nvidia-prod", { + credentialEnv: "NVIDIA_INFERENCE_API_KEY", + }); + const peer = sandbox("peer", "nvidia-prod", { + credentialEnv: "NVIDIA_INFERENCE_API_KEY", + }); + + expect( + revalidateRebuildRouteBeforeDelete(receipt(target), { + load: () => registry(target, peer), + }), + ).toMatchObject({ ok: true }); + }); + + it("blocks deletion when a peer credential identity drifts after preflight (#7798)", () => { + const target = sandbox("target", "nvidia-prod", { + credentialEnv: "NVIDIA_INFERENCE_API_KEY", + }); + const peer = sandbox("peer", "nvidia-prod", { credentialEnv: "OPENAI_API_KEY" }); + + const result = revalidateRebuildRouteBeforeDelete(receipt(target), { + load: () => registry(target, peer), + }); + + expect(result).toMatchObject({ ok: false }); + expect(result.ok ? "" : result.message).toContain("credential identity"); + }); + + it("blocks deletion when the target route drifts after preflight (#7798)", () => { + const target = sandbox("target", "nvidia-prod", { + credentialEnv: "NVIDIA_INFERENCE_API_KEY", + }); + const drifted = { ...target, model: "changed-model" }; + + expect( + revalidateRebuildRouteBeforeDelete(receipt(target), { + load: () => registry(drifted), + }), + ).toEqual({ + ok: false, + message: "Sandbox inference route changed before sandbox deletion.", + }); + }); +}); diff --git a/test/e2e/live/upgrade-stale-sandbox-helpers.ts b/test/e2e/live/upgrade-stale-sandbox-helpers.ts index e6981ec7aaf..588ba3bc8ce 100644 --- a/test/e2e/live/upgrade-stale-sandbox-helpers.ts +++ b/test/e2e/live/upgrade-stale-sandbox-helpers.ts @@ -4,7 +4,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; - +import { findAvailableDashboardPort } from "../../../src/lib/onboard/dashboard-port.ts"; import { buildAvailabilityProbeEnv } from "../fixtures/availability-env.ts"; import type { CleanupRegistry } from "../fixtures/cleanup.ts"; import type { HostCliClient } from "../fixtures/clients/host.ts"; @@ -30,7 +30,10 @@ export const SANDBOX_NAME = [TEST_SANDBOX_PREFIX, process.env.GITHUB_RUN_ID, process.env.GITHUB_RUN_ATTEMPT, process.pid] .filter(Boolean) .join("-"); +export const SIBLING_SANDBOX_NAME = `${SANDBOX_NAME}-peer`; +export const SANDBOX_NAMES = [SANDBOX_NAME, SIBLING_SANDBOX_NAME] as const; validateSandboxName(SANDBOX_NAME); +validateSandboxName(SIBLING_SANDBOX_NAME); assertSafeSandboxName(); export const OLD_OPENCLAW_VERSION = "2026.3.11"; export const OLD_BASE_TAG = `nemoclaw-old-base:${SANDBOX_NAME.toLowerCase().replace(/[^a-z0-9_.-]+/g, "-")}`; @@ -39,10 +42,12 @@ const SESSION_FILE = path.join(os.homedir(), ".nemoclaw", "onboard-session.json" const INSTALL_ATTEMPTS = process.env.CI === "true" || process.env.GITHUB_ACTIONS === "true" ? 3 : 1; function assertSafeSandboxName(): void { - if (!SANDBOX_NAME.startsWith(TEST_SANDBOX_PREFIX)) { - throw new Error( - `upgrade-stale-sandbox live test is destructive and only accepts sandbox names with prefix ${TEST_SANDBOX_PREFIX}; got ${SANDBOX_NAME}`, - ); + for (const sandboxName of SANDBOX_NAMES) { + if (!sandboxName.startsWith(TEST_SANDBOX_PREFIX)) { + throw new Error( + `upgrade-stale-sandbox live test is destructive and only accepts sandbox names with prefix ${TEST_SANDBOX_PREFIX}; got ${sandboxName}`, + ); + } } } @@ -67,7 +72,29 @@ async function bestEffortPreclean(run: () => Promise): Promise { } } -export function writeStaleRegistryEntry(): void { +export function allocateSiblingDashboardPort(forwardListOutput: string | null): number { + const registry = readJsonFileOrFallback<{ + sandboxes?: Record>; + }>(REGISTRY_FILE, {}); + const primaryDashboardPort = registry.sandboxes?.[SANDBOX_NAME]?.dashboardPort; + expect( + typeof primaryDashboardPort === "number" && + Number.isInteger(primaryDashboardPort) && + primaryDashboardPort > 0 && + primaryDashboardPort <= 65535, + "initial onboard must persist the dashboard port used by authoritative rebuild", + ).toBe(true); + const occupied = new Map([[String(primaryDashboardPort), SANDBOX_NAME]]); + return findAvailableDashboardPort( + SIBLING_SANDBOX_NAME, + primaryDashboardPort === 18790 ? 18791 : 18790, + forwardListOutput, + undefined, + occupied, + ); +} + +export function writeStaleRegistryEntries(siblingDashboardPort: number): void { const session = readJsonFileOrFallback>(SESSION_FILE, {}); const envProvider = process.env.NEMOCLAW_PROVIDER === "custom" @@ -86,7 +113,8 @@ export function writeStaleRegistryEntry(): void { sandboxes?: Record>; defaultSandbox?: string; }>(REGISTRY_FILE, {}); - const dashboardPort = registry.sandboxes?.[SANDBOX_NAME]?.dashboardPort; + const currentEntry = registry.sandboxes?.[SANDBOX_NAME] ?? {}; + const dashboardPort = currentEntry.dashboardPort; expect( typeof dashboardPort === "number" && Number.isInteger(dashboardPort) && @@ -94,20 +122,48 @@ export function writeStaleRegistryEntry(): void { dashboardPort <= 65535, "initial onboard must persist the dashboard port used by authoritative rebuild", ).toBe(true); + const endpointUrl = + (typeof currentEntry.endpointUrl === "string" && currentEntry.endpointUrl) || + (typeof session.endpointUrl === "string" && session.endpointUrl) || + null; + const preferredInferenceApi = + (typeof currentEntry.preferredInferenceApi === "string" && + currentEntry.preferredInferenceApi) || + (typeof session.preferredInferenceApi === "string" && session.preferredInferenceApi) || + null; + if (provider === "compatible-endpoint" || provider === "compatible-anthropic-endpoint") { + expect(endpointUrl, "custom stale route must retain its durable endpoint").toBeTruthy(); + expect( + preferredInferenceApi, + "custom stale route must retain its durable inference API family", + ).toBeTruthy(); + } registry.sandboxes = registry.sandboxes ?? {}; - registry.sandboxes[SANDBOX_NAME] = { - name: SANDBOX_NAME, - createdAt: new Date().toISOString(), - model, - provider, - gpuEnabled: false, - policies: [], - policyTier: null, - fromDockerfile: null, - dashboardPort, - agent: null, - agentVersion: OLD_OPENCLAW_VERSION, - }; + for (const [sandboxName, assignedDashboardPort] of [ + [SANDBOX_NAME, dashboardPort], + [SIBLING_SANDBOX_NAME, siblingDashboardPort], + ] as const) { + registry.sandboxes[sandboxName] = { + name: sandboxName, + createdAt: new Date().toISOString(), + model, + provider, + endpointUrl, + preferredInferenceApi, + gpuEnabled: false, + policies: [], + policyTier: null, + fromDockerfile: null, + dashboardPort: assignedDashboardPort, + gatewayName: "nemoclaw", + openshellVersion: "0.0.71", + nemoclawVersion: "0.0.71", + agent: null, + agentVersion: OLD_OPENCLAW_VERSION, + // Deliberately omit credentialEnv on both legacy rows. Rebuild must + // migrate the shared provider identity before deleting either sandbox. + }; + } registry.defaultSandbox = SANDBOX_NAME; writeJsonFile(REGISTRY_FILE, registry); writeJsonFile(SESSION_FILE, { ...session, sandboxName: SANDBOX_NAME, status: "complete" }); @@ -140,20 +196,22 @@ export async function precleanStaleSandbox( host: HostCliClient, sandbox: SandboxClient, ): Promise { - await bestEffortPreclean(() => - host.nemoclaw([SANDBOX_NAME, "destroy", "--yes"], { - artifactName: "cleanup-nemoclaw-destroy-upgrade-stale", - env: commandEnv(), - timeoutMs: 120_000, - }), - ); - await bestEffortPreclean(() => - sandbox.openshell(["sandbox", "delete", SANDBOX_NAME], { - artifactName: "cleanup-openshell-delete-upgrade-stale", - env: commandEnv(), - timeoutMs: 60_000, - }), - ); + for (const sandboxName of SANDBOX_NAMES) { + await bestEffortPreclean(() => + host.nemoclaw([sandboxName, "destroy", "--yes"], { + artifactName: `cleanup-nemoclaw-destroy-${sandboxName}`, + env: commandEnv(), + timeoutMs: 120_000, + }), + ); + await bestEffortPreclean(() => + sandbox.openshell(["sandbox", "delete", sandboxName], { + artifactName: `cleanup-openshell-delete-${sandboxName}`, + env: commandEnv(), + timeoutMs: 60_000, + }), + ); + } } export async function cleanupOldImage(host: HostCliClient): Promise { @@ -251,13 +309,14 @@ export function createFixtureDockerfile(cleanup: Pick { return await host.command( "bash", [ "-lc", - `for _i in $(seq 1 30); do openshell sandbox list 2>/dev/null | grep -q '${SANDBOX_NAME}.*Ready' && exit 0; sleep 5; done; openshell sandbox list >&2; exit 1`, + `for _i in $(seq 1 30); do openshell sandbox list 2>/dev/null | grep -q '${sandboxName}.*Ready' && exit 0; sleep 5; done; openshell sandbox list >&2; exit 1`, ], { artifactName, env: commandEnv(), timeoutMs: 180_000 }, ); diff --git a/test/e2e/live/upgrade-stale-sandbox.test.ts b/test/e2e/live/upgrade-stale-sandbox.test.ts index 64392c7c5dd..d328556e9f6 100644 --- a/test/e2e/live/upgrade-stale-sandbox.test.ts +++ b/test/e2e/live/upgrade-stale-sandbox.test.ts @@ -4,8 +4,9 @@ /** * Preserves the #1904 contract with real Docker/OpenShell/NemoClaw * boundaries: onboard current NemoClaw, create an old OpenClaw sandbox from a - * real image, register stale sandbox metadata, prove upgrade-sandboxes detects - * the stale sandbox, rebuild it, and prove the stale version is gone. + * real image, register two legacy sandboxes on one gateway, prove + * upgrade-sandboxes detects both, rebuild them as one batch, and prove the + * stale version and missing shared-route credential metadata are gone. */ import { buildAvailabilityProbeEnv } from "../fixtures/availability-env.ts"; @@ -13,6 +14,7 @@ import { resultText } from "../fixtures/clients/index.ts"; import { expect, test } from "../fixtures/e2e-test.ts"; import { requireHostedInferenceConfig } from "../fixtures/hosted-inference.ts"; import { + allocateSiblingDashboardPort, assertDeleteInstalledSandboxAllowed, assertDockerAvailable, buildOldOpenClawBase, @@ -25,21 +27,23 @@ import { registeredStaleSandboxJson, registerStateRestore, SANDBOX_NAME, + SANDBOX_NAMES, + SIBLING_SANDBOX_NAME, waitSandboxReady, - writeStaleRegistryEntry, + writeStaleRegistryEntries, } from "./upgrade-stale-sandbox-helpers.ts"; -const LIVE_TIMEOUT_MS = 45 * 60_000; +const LIVE_TIMEOUT_MS = 75 * 60_000; -test("upgrade-sandboxes detects and rebuilds stale OpenClaw sandboxes (#1904)", { +test("upgrade-sandboxes rebuilds two legacy sandboxes on one shared route (#1904, #7798)", { timeout: LIVE_TIMEOUT_MS, meta: { e2ePhases: [ "confirm Docker and install current NemoClaw", - "construct an old OpenClaw sandbox", - "register stale sandbox metadata", - "detect the stale sandbox", - "rebuild to the current OpenClaw runtime", + "construct two old OpenClaw sandboxes on one gateway", + "register stale shared-route sandbox metadata", + "detect both stale sandboxes", + "rebuild both to the current OpenClaw runtime", "confirm the upgrade check is clean", ], }, @@ -50,12 +54,15 @@ test("upgrade-sandboxes detects and rebuilds stale OpenClaw sandboxes (#1904)", id: "upgrade-stale-sandbox", boundary: "install.sh + Docker old base image + OpenShell sandbox create + NemoClaw rebuild", sandboxName: SANDBOX_NAME, + sandboxNames: [...SANDBOX_NAMES], oldOpenClawVersion: OLD_OPENCLAW_VERSION, contracts: [ "current NemoClaw install/onboard succeeds before stale fixture creation", "an old OpenClaw base image can be created with the legacy version", - "a sandbox registered with old agentVersion is reported stale by upgrade-sandboxes --check", - "nemoclaw rebuild --yes upgrades the sandbox away from the old OpenClaw version", + "two legacy sandboxes with missing credentialEnv share one complete inference route", + "both sandboxes are reported stale by upgrade-sandboxes --check", + "upgrade-sandboxes --auto upgrades both without orphaning the first sandbox", + "both registry rows carry the canonical shared credential identity after rebuild", "upgrade-sandboxes --check reports up-to-date after rebuild", ], }); @@ -69,73 +76,96 @@ test("upgrade-sandboxes detects and rebuilds stale OpenClaw sandboxes (#1904)", registerStateRestore(cleanup); cleanup.trackDisposable("remove stale OpenClaw test image", () => cleanupOldImage(host)); - cleanup.trackDisposable(`delete OpenShell sandbox ${SANDBOX_NAME}`, () => - sandbox.cleanupSandbox(SANDBOX_NAME, { - artifactName: "cleanup-openshell-delete-upgrade-stale", + for (const sandboxName of SANDBOX_NAMES) { + cleanup.trackDisposable(`delete OpenShell sandbox ${sandboxName}`, () => + sandbox.cleanupSandbox(sandboxName, { + artifactName: `cleanup-openshell-delete-${sandboxName}`, + env: commandEnv(), + timeoutMs: 60_000, + }), + ); + cleanup.trackSandbox(host, sandboxName, { + artifactName: `cleanup-nemoclaw-destroy-${sandboxName}`, env: commandEnv(), - timeoutMs: 60_000, - }), - ); - cleanup.trackSandbox(host, SANDBOX_NAME, { - artifactName: "cleanup-nemoclaw-destroy-upgrade-stale", - env: commandEnv(), - timeoutMs: 120_000, - }); + timeoutMs: 120_000, + }); + } await precleanStaleSandbox(host, sandbox); const install = await installCurrentNemoclaw(host, hosted); expect(install.exitCode, resultText(install)).toBe(0); - progress.phase("construct an old OpenClaw sandbox"); + progress.phase("construct two old OpenClaw sandboxes on one gateway"); const deleteInstalledSandbox = await sandbox.openshell(["sandbox", "delete", SANDBOX_NAME], { artifactName: "phase-2-delete-installed-sandbox", env: commandEnv(), timeoutMs: 120_000, }); assertDeleteInstalledSandboxAllowed(deleteInstalledSandbox); + const forwardList = await sandbox.openshell(["forward", "list"], { + artifactName: "phase-2-forward-list-before-stale-fixture", + env: commandEnv(), + timeoutMs: 60_000, + }); + expect(forwardList.exitCode, resultText(forwardList)).toBe(0); + const siblingDashboardPort = allocateSiblingDashboardPort(resultText(forwardList)); const buildOldBase = await buildOldOpenClawBase(host); expect(buildOldBase.exitCode, resultText(buildOldBase)).toBe(0); const fixtureDockerfile = createFixtureDockerfile(cleanup); - const createOldSandbox = await sandbox.openshell( - [ - "sandbox", - "create", - "--name", - SANDBOX_NAME, - "--from", - fixtureDockerfile, - "--gateway", - "nemoclaw", - "--no-tty", - "--", - "true", - ], - { - artifactName: "phase-3-create-old-openclaw-sandbox", + for (const sandboxName of SANDBOX_NAMES) { + const createOldSandbox = await sandbox.openshell( + [ + "sandbox", + "create", + "--name", + sandboxName, + "--from", + fixtureDockerfile, + "--gateway", + "nemoclaw", + "--no-tty", + "--", + "true", + ], + { + artifactName: `phase-3-create-old-openclaw-${sandboxName}`, + env: commandEnv(), + timeoutMs: 15 * 60_000, + }, + ); + expect(createOldSandbox.exitCode, resultText(createOldSandbox)).toBe(0); + + const waitReady = await waitSandboxReady( + host, + sandboxName, + `phase-3-wait-old-${sandboxName}-ready`, + ); + expect(waitReady.exitCode, resultText(waitReady)).toBe(0); + + const oldVersion = await sandbox.exec(sandboxName, ["openclaw", "--version"], { + artifactName: `phase-3-old-openclaw-version-${sandboxName}`, env: commandEnv(), - timeoutMs: 15 * 60_000, - }, - ); - expect(createOldSandbox.exitCode, resultText(createOldSandbox)).toBe(0); - - const waitReady = await waitSandboxReady(host, "phase-3-wait-old-sandbox-ready"); - expect(waitReady.exitCode, resultText(waitReady)).toBe(0); - - const oldVersion = await sandbox.exec(SANDBOX_NAME, ["openclaw", "--version"], { - artifactName: "phase-3-old-openclaw-version", - env: commandEnv(), - timeoutMs: 60_000, - }); - expect(oldVersion.exitCode, resultText(oldVersion)).toBe(0); - expect(resultText(oldVersion)).toContain(OLD_OPENCLAW_VERSION); - - progress.phase("register stale sandbox metadata"); - writeStaleRegistryEntry(); - await artifacts.writeText("registered-stale-sandbox.json", registeredStaleSandboxJson()); - - progress.phase("detect the stale sandbox"); + timeoutMs: 60_000, + }); + expect(oldVersion.exitCode, resultText(oldVersion)).toBe(0); + expect(resultText(oldVersion)).toContain(OLD_OPENCLAW_VERSION); + } + + progress.phase("register stale shared-route sandbox metadata"); + writeStaleRegistryEntries(siblingDashboardPort); + const staleRegistryJson = registeredStaleSandboxJson(); + await artifacts.writeText("registered-stale-sandboxes.json", staleRegistryJson); + const staleRegistry = JSON.parse(staleRegistryJson) as { + sandboxes: Record>; + }; + for (const sandboxName of SANDBOX_NAMES) { + expect(staleRegistry.sandboxes[sandboxName]).toBeDefined(); + expect(Object.hasOwn(staleRegistry.sandboxes[sandboxName]!, "credentialEnv")).toBe(false); + } + + progress.phase("detect both stale sandboxes"); const staleCheck = await host.nemoclaw(["upgrade-sandboxes", "--check"], { artifactName: "phase-5-upgrade-sandboxes-check-stale", env: commandEnv(hosted.env), @@ -145,26 +175,41 @@ test("upgrade-sandboxes detects and rebuilds stale OpenClaw sandboxes (#1904)", expect(staleCheck.exitCode, resultText(staleCheck)).toBe(0); expect(resultText(staleCheck)).toMatch(/stale|need upgrading/i); expect(resultText(staleCheck)).not.toMatch(/up to date/i); + expect(resultText(staleCheck)).toContain(SANDBOX_NAME); + expect(resultText(staleCheck)).toContain(SIBLING_SANDBOX_NAME); - progress.phase("rebuild to the current OpenClaw runtime"); - const rebuild = await host.nemoclaw([SANDBOX_NAME, "rebuild", "--yes"], { - artifactName: "phase-6-rebuild-stale-sandbox", + progress.phase("rebuild both to the current OpenClaw runtime"); + const rebuild = await host.nemoclaw(["upgrade-sandboxes", "--auto"], { + artifactName: "phase-6-upgrade-both-stale-sandboxes", env: commandEnv(hosted.env), redactionValues: [hosted.apiKey], - timeoutMs: 25 * 60_000, + timeoutMs: 45 * 60_000, }); expect(rebuild.exitCode, resultText(rebuild)).toBe(0); - - const waitRebuiltReady = await waitSandboxReady(host, "phase-6-wait-rebuilt-sandbox-ready"); - expect(waitRebuiltReady.exitCode, resultText(waitRebuiltReady)).toBe(0); - - const newVersion = await sandbox.exec(SANDBOX_NAME, ["openclaw", "--version"], { - artifactName: "phase-6-new-openclaw-version", - env: commandEnv(), - timeoutMs: 60_000, - }); - expect(newVersion.exitCode, resultText(newVersion)).toBe(0); - expect(resultText(newVersion)).not.toContain(OLD_OPENCLAW_VERSION); + expect(resultText(rebuild)).toMatch(/2 sandbox\(es\) rebuilt/i); + + for (const sandboxName of SANDBOX_NAMES) { + const waitRebuiltReady = await waitSandboxReady( + host, + sandboxName, + `phase-6-wait-rebuilt-${sandboxName}-ready`, + ); + expect(waitRebuiltReady.exitCode, resultText(waitRebuiltReady)).toBe(0); + + const newVersion = await sandbox.exec(sandboxName, ["openclaw", "--version"], { + artifactName: `phase-6-new-openclaw-version-${sandboxName}`, + env: commandEnv(), + timeoutMs: 60_000, + }); + expect(newVersion.exitCode, resultText(newVersion)).toBe(0); + expect(resultText(newVersion)).not.toContain(OLD_OPENCLAW_VERSION); + } + const rebuiltRegistry = JSON.parse(registeredStaleSandboxJson()) as { + sandboxes: Record>; + }; + for (const sandboxName of SANDBOX_NAMES) { + expect(rebuiltRegistry.sandboxes[sandboxName]?.credentialEnv).toBe(hosted.credentialEnv); + } progress.phase("confirm the upgrade check is clean"); const cleanCheck = await host.nemoclaw(["upgrade-sandboxes", "--check"], { diff --git a/test/helpers/rebuild-flow-harness.ts b/test/helpers/rebuild-flow-harness.ts index ef2846b26c6..eabf2c1fb90 100644 --- a/test/helpers/rebuild-flow-harness.ts +++ b/test/helpers/rebuild-flow-harness.ts @@ -52,6 +52,7 @@ const rebuildInference = requireDist("./rebuild-inference-preflight.js"); const rebuildFlowHelpers = requireDist("./rebuild-flow-helpers.js"); const rebuildManagedImage = requireDist("./rebuild-managed-image-preflight.js"); const rebuildMessagingConflict = requireDist("./rebuild-messaging-conflict-preflight.js"); +const rebuildRoutePreflight = requireDist("./rebuild-preflight-guards.js"); const shields = requireDist("../../shields/index.js"); type RebuildFlowStep = { @@ -137,6 +138,9 @@ export type RebuildFlowOverrides = { openShieldsWindow?: () => { relocked: boolean; wasLocked: boolean } | null; preflightMessagingConflicts?: () => Promise | void; preflightAuthoritativeRebuildTarget?: (options: Record) => Promise | void; + revalidateRebuildRouteBeforeDelete?: ( + receipt: Record, + ) => { ok: true; receipt: Record } | { ok: false; message: string }; mcpPreparation?: { entries: Array>; detachedProviderEntries: Array>; @@ -442,6 +446,42 @@ export function createRebuildFlowHarness(overrides: RebuildFlowOverrides = {}): }); vi.spyOn(registry, "listSandboxes").mockReturnValue({ sandboxes: [] }); const registryUpdateSpy = vi.spyOn(registry, "updateSandbox").mockReturnValue(true); + vi.spyOn(rebuildRoutePreflight, "commitRebuildRoutePreflight").mockImplementation( + (...args: unknown[]) => { + const input = args[0] as { + sandboxName: string; + gatewayName: string; + targetUpdate: Record; + }; + if (!registry.updateSandbox(input.sandboxName, input.targetUpdate)) { + return { + ok: false, + message: "Sandbox registry entry disappeared during rebuild route preflight.", + }; + } + return { + ok: true, + receipt: { + sandboxName: input.sandboxName, + gatewayName: input.gatewayName, + route: { + provider: input.targetUpdate.provider ?? null, + model: input.targetUpdate.model ?? null, + endpointUrl: input.targetUpdate.endpointUrl ?? null, + preferredInferenceApi: input.targetUpdate.preferredInferenceApi ?? null, + credentialEnv: input.targetUpdate.credentialEnv ?? null, + }, + migratedSandboxNames: [], + }, + }; + }, + ); + vi.spyOn(rebuildRoutePreflight, "revalidateRebuildRouteBeforeDelete").mockImplementation( + (...args: unknown[]) => { + const receipt = args[0] as Record; + return overrides.revalidateRebuildRouteBeforeDelete?.(receipt) ?? { ok: true, receipt }; + }, + ); const restoreSandboxEntrySpy = vi .spyOn(registry, "restoreSandboxEntry") .mockImplementation(() => undefined); diff --git a/test/helpers/rebuild-flow-lifecycle-cases.ts b/test/helpers/rebuild-flow-lifecycle-cases.ts index af32d4ade64..5ee296c6ed2 100644 --- a/test/helpers/rebuild-flow-lifecycle-cases.ts +++ b/test/helpers/rebuild-flow-lifecycle-cases.ts @@ -135,6 +135,25 @@ export function registerRebuildFlowLifecycleTests(): void { ); }); + it("keeps the original sandbox when the shared route drifts at the delete edge (#7798)", async () => { + const harness = createRebuildFlowHarness({ + revalidateRebuildRouteBeforeDelete: () => ({ + ok: false, + message: "Shared inference route changed before sandbox deletion.", + }), + }); + + await expect( + harness.rebuildSandbox("alpha", ["--yes"], { throwOnError: true }), + ).rejects.toThrow("Shared inference route changed before sandbox deletion."); + + expect(harness.backupSandboxStateSpy).toHaveBeenCalledOnce(); + expect(harness.prepareMcpBridgesForRebuildSpy).toHaveBeenCalledOnce(); + expect(harness.reattachMcpProvidersAfterRebuildAbortSpy).toHaveBeenCalledOnce(); + expect(harness.onboardSpy).not.toHaveBeenCalled(); + expectNoSandboxDelete(harness.runOpenshellSpy); + }); + it("keeps baseline exclusions durable through successful replacement onboarding (#7194)", async () => { const harness = createRebuildFlowHarness({ sandboxEntry: { diff --git a/test/helpers/rebuild-flow-test-harness.ts b/test/helpers/rebuild-flow-test-harness.ts index 20aab708d39..b7adc2391f2 100644 --- a/test/helpers/rebuild-flow-test-harness.ts +++ b/test/helpers/rebuild-flow-test-harness.ts @@ -49,6 +49,7 @@ const gatewayState = requireDist("./gateway-state.js"); const rebuildFlowHelpers = requireDist("./rebuild-flow-helpers.js"); const rebuildCustomImagePreflight = requireDist("./rebuild-custom-image-preflight.js"); const rebuildPreparedImageContext = requireDist("./rebuild-prepared-image-context.js"); +const rebuildRoutePreflight = requireDist("./rebuild-preflight-guards.js"); const buildContextFingerprint = requireDist("../../adapters/fs/build-context-fingerprint.js"); const rebuildUsageNotice = requireDist("./rebuild-usage-notice.js"); const rebuildShields = requireDist("./rebuild-shields.js"); @@ -270,6 +271,42 @@ export function createRebuildFlowHarness(overrides: RebuildFlowOverrides = {}): Object.assign(currentSandboxEntry, updates); return true; }); + vi.spyOn(rebuildRoutePreflight, "commitRebuildRoutePreflight").mockImplementation( + (...args: unknown[]) => { + const input = args[0] as { + sandboxName: string; + gatewayName: string; + targetUpdate: Record; + }; + if (!registry.updateSandbox(input.sandboxName, input.targetUpdate)) { + return { + ok: false, + message: "Sandbox registry entry disappeared during rebuild route preflight.", + }; + } + return { + ok: true, + receipt: { + sandboxName: input.sandboxName, + gatewayName: input.gatewayName, + route: { + provider: input.targetUpdate.provider ?? null, + model: input.targetUpdate.model ?? null, + endpointUrl: input.targetUpdate.endpointUrl ?? null, + preferredInferenceApi: input.targetUpdate.preferredInferenceApi ?? null, + credentialEnv: input.targetUpdate.credentialEnv ?? null, + }, + migratedSandboxNames: [], + }, + }; + }, + ); + vi.spyOn(rebuildRoutePreflight, "revalidateRebuildRouteBeforeDelete").mockImplementation( + (...args: unknown[]) => { + const receipt = args[0] as Record; + return overrides.revalidateRebuildRouteBeforeDelete?.(receipt) ?? { ok: true, receipt }; + }, + ); const restoreSandboxEntrySpy = vi .spyOn(registry, "restoreSandboxEntry") .mockImplementation((...args: unknown[]) => { diff --git a/test/helpers/rebuild-flow-test-support.ts b/test/helpers/rebuild-flow-test-support.ts index 195f3a5e8f3..4082a0adf21 100644 --- a/test/helpers/rebuild-flow-test-support.ts +++ b/test/helpers/rebuild-flow-test-support.ts @@ -67,6 +67,9 @@ export type RebuildFlowOverrides = { agentPolicyAdditionsContent?: string; preflightWithProductionBaselineResolver?: boolean; preflightAuthoritativeRebuildTarget?: (options: Record) => Promise | void; + revalidateRebuildRouteBeforeDelete?: ( + receipt: Record, + ) => { ok: true; receipt: Record } | { ok: false; message: string }; sandboxEntry?: Record; sandboxBaseImageLabelsOutput?: string; sessionSandboxName?: string; diff --git a/tools/e2e/workflow-boundary.mts b/tools/e2e/workflow-boundary.mts index e2035913b52..ee529e38fe3 100644 --- a/tools/e2e/workflow-boundary.mts +++ b/tools/e2e/workflow-boundary.mts @@ -2159,8 +2159,8 @@ function validateUpgradeStaleSandboxJob(errors: string[], jobs: WorkflowRecord): errors.push("upgrade-stale-sandbox job must run on ubuntu-latest"); } validateFreeStandingJobSelector(errors, jobs, jobName, targetName); - if (job["timeout-minutes"] !== 55) { - errors.push("upgrade-stale-sandbox job must keep the legacy 55 minute timeout"); + if (job["timeout-minutes"] !== 85) { + errors.push("upgrade-stale-sandbox job must keep the two-sandbox 85 minute timeout"); } const jobEnv = asRecord(job.env);