From a225c1b41676b7cb97e0aecf105da551f34db5db Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Wed, 8 Jul 2026 15:42:04 -0400 Subject: [PATCH 1/4] refactor(rebuild): gate completion on post-restore verification --- .../sandbox/rebuild-finalization.test.ts | 187 --------------- .../actions/sandbox/rebuild-finalization.ts | 168 ------------- src/lib/actions/sandbox/rebuild-mcp-phase.ts | 18 -- src/lib/actions/sandbox/rebuild-pipeline.ts | 10 +- .../sandbox/rebuild-post-restore-phase.ts | 227 +++++++++++------- .../rebuild-post-restore-verification.test.ts | 96 ++++++++ .../rebuild-transaction-boundary.test.ts | 26 ++ .../rebuild-transaction-coordinator.ts | 25 +- ...rebuild-flow-credential-preflight-cases.ts | 1 + test/helpers/rebuild-flow-lifecycle-cases.ts | 1 + test/helpers/rebuild-flow-recovery-cases.ts | 24 +- .../rebuild-flow-target-image-cases.ts | 1 + test/helpers/rebuild-flow-test-harness.ts | 2 +- 13 files changed, 318 insertions(+), 468 deletions(-) delete mode 100644 src/lib/actions/sandbox/rebuild-finalization.test.ts delete mode 100644 src/lib/actions/sandbox/rebuild-finalization.ts create mode 100644 src/lib/actions/sandbox/rebuild-post-restore-verification.test.ts diff --git a/src/lib/actions/sandbox/rebuild-finalization.test.ts b/src/lib/actions/sandbox/rebuild-finalization.test.ts deleted file mode 100644 index 287eae87493..00000000000 --- a/src/lib/actions/sandbox/rebuild-finalization.test.ts +++ /dev/null @@ -1,187 +0,0 @@ -// 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 { - finalizeRebuildPostRestore, - type RebuildPostRestoreFinalizationOptions, - resetRebuildShieldsStateAfterRecreate, -} from "./rebuild-finalization"; - -function options( - overrides: Partial = {}, -): RebuildPostRestoreFinalizationOptions { - return { - sandboxName: "alpha", - agentExpectedVersion: "2026.6.10", - reportedVersion: "2026.6.10", - rebuiltAgentName: "OpenClaw", - restoredPresets: ["github"], - failedPresets: [], - rebuildMessagingPlan: null, - restoreSucceeded: true, - mutablePermsRepairUnverified: false, - mutableConfigHashRefreshUnverified: false, - staleRecovery: false, - backup: { backupPath: "/tmp/alpha-backup" }, - recoveryRecreate: false, - staleSandboxWasLocked: false, - preparedBackupRecovery: false, - relockShields: vi.fn(() => true), - log: vi.fn(), - bail: vi.fn((message: string) => { - throw new Error(message); - }), - ...overrides, - }; -} - -describe("resetRebuildShieldsStateAfterRecreate", () => { - it("clears prior shields state only after a recovery recreate succeeds", () => { - const clearShieldsState = vi.fn(); - - resetRebuildShieldsStateAfterRecreate("alpha", false, { clearShieldsState }); - resetRebuildShieldsStateAfterRecreate("alpha", true, { clearShieldsState }); - - expect(clearShieldsState).toHaveBeenCalledOnce(); - expect(clearShieldsState).toHaveBeenCalledWith("alpha"); - }); -}); - -describe("finalizeRebuildPostRestore", () => { - it("reconciles policy state, relocks, and verifies forwarding in order", () => { - const calls: string[] = []; - const updateSandbox = vi.fn(() => { - calls.push("registry"); - return true; - }); - const log = vi.fn(() => calls.push("log")); - const relockShields = vi.fn(() => { - calls.push("relock"); - return true; - }); - const ensureMessagingHostForward = vi.fn(() => { - calls.push("forward"); - return true; - }); - const writeLine = vi.fn((message: string) => calls.push(`write:${message}`)); - - const input = options({ relockShields, log, preparedBackupRecovery: true }); - const result = finalizeRebuildPostRestore(input, { - updateSandbox, - ensureMessagingHostForward, - writeLine, - }); - - expect(calls.slice(0, 4)).toEqual(["registry", "log", "relock", "forward"]); - expect(updateSandbox).toHaveBeenCalledWith("alpha", { - agentVersion: "2026.6.10", - policies: ["github"], - }); - expect(writeLine.mock.calls.flat().join("\n")).toContain( - "Sandbox 'alpha' rebuilt successfully", - ); - expect(result).toEqual({ - postRestoreComplete: true, - messagingHostForwardUnverified: false, - }); - expect(input.bail).not.toHaveBeenCalled(); - }); - - it("bails after a failed relock without attempting host forwarding", () => { - const ensureMessagingHostForward = vi.fn(() => true); - - expect(() => - finalizeRebuildPostRestore(options({ relockShields: () => false }), { - updateSandbox: vi.fn(), - ensureMessagingHostForward, - }), - ).toThrow("Failed to re-apply shields lockdown."); - expect(ensureMessagingHostForward).not.toHaveBeenCalled(); - }); - - it("reports every incomplete recovery dimension and the stale shields warning", () => { - const writeLine = vi.fn(); - - const result = finalizeRebuildPostRestore( - options({ - failedPresets: ["messaging-telegram"], - restoreSucceeded: false, - mutablePermsRepairUnverified: true, - mutableConfigHashRefreshUnverified: true, - recoveryRecreate: true, - staleSandboxWasLocked: true, - }), - { - updateSandbox: vi.fn(), - ensureMessagingHostForward: () => false, - writeLine, - }, - ); - - const output = writeLine.mock.calls.flat().join("\n"); - expect(output).toContain("State restore was incomplete"); - expect(output).toContain("Mutable config permissions were not verified"); - expect(output).toContain("Mutable OpenClaw config hash was not refreshed"); - expect(output).toContain("Messaging webhook forward was not verified"); - expect(output).toContain("Policy presets failed to reapply: messaging-telegram"); - expect(output).toContain("Shields were previously enabled"); - const orderedFragments = [ - "State restore was incomplete", - "Mutable config permissions were not verified", - "Mutable OpenClaw config hash was not refreshed", - "Messaging webhook forward was not verified", - "Policy presets failed to reapply", - "Shields were previously enabled", - ]; - const fragmentOffsets = orderedFragments.map((fragment) => output.indexOf(fragment)); - expect(fragmentOffsets).toEqual([...fragmentOffsets].sort((left, right) => left - right)); - expect(result).toEqual({ - postRestoreComplete: false, - messagingHostForwardUnverified: true, - }); - }); - - it("fails closed when prepared recovery finishes with unverified state", () => { - const events: string[] = []; - const writeLine = vi.fn((message: string) => events.push(`write:${message}`)); - const bail = vi.fn((message: string): never => { - events.push(`bail:${message}`); - throw new Error(message); - }); - - expect(() => - finalizeRebuildPostRestore( - options({ - preparedBackupRecovery: true, - mutablePermsRepairUnverified: true, - recoveryRecreate: true, - staleSandboxWasLocked: true, - bail, - }), - { - updateSandbox: vi.fn(), - ensureMessagingHostForward: () => true, - writeLine, - }, - ), - ).toThrow("Prepared backup recovery for 'alpha' completed with unverified post-restore state."); - expect(events.at(-1)).toContain("bail:Prepared backup recovery"); - expect(events.at(-2)).toContain("Shields were previously enabled"); - }); - - it("reports stale recovery success without backup state", () => { - const writeLine = vi.fn(); - - finalizeRebuildPostRestore(options({ staleRecovery: true, backup: null }), { - updateSandbox: vi.fn(), - ensureMessagingHostForward: () => true, - writeLine, - }); - - expect(writeLine.mock.calls.flat().join("\n")).toContain( - "Recovered from a stale registry entry", - ); - }); -}); diff --git a/src/lib/actions/sandbox/rebuild-finalization.ts b/src/lib/actions/sandbox/rebuild-finalization.ts deleted file mode 100644 index 66ff03c0a5d..00000000000 --- a/src/lib/actions/sandbox/rebuild-finalization.ts +++ /dev/null @@ -1,168 +0,0 @@ -// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -import { CLI_NAME } from "../../cli/branding"; -import { D, G, R, YW } from "../../cli/terminal-style"; -import type { SandboxMessagingPlan } from "../../messaging"; -import * as shields from "../../shields"; -import * as registry from "../../state/registry"; -import { ensureMessagingHostForwardAfterRebuild } from "./messaging-host-forward-lifecycle"; - -interface RebuildShieldsResetDeps { - clearShieldsState?: typeof shields.clearShieldsState; -} - -export function resetRebuildShieldsStateAfterRecreate( - sandboxName: string, - recoveryRecreate: boolean, - deps: RebuildShieldsResetDeps = {}, -): void { - if (!recoveryRecreate) return; - (deps.clearShieldsState ?? shields.clearShieldsState)(sandboxName); -} - -export interface RebuildPostRestoreFinalizationOptions { - sandboxName: string; - agentExpectedVersion: string | null; - reportedVersion: string | null; - rebuiltAgentName: string; - restoredPresets: string[]; - failedPresets: string[]; - rebuildMessagingPlan: SandboxMessagingPlan | null; - restoreSucceeded: boolean; - mutablePermsRepairUnverified: boolean; - mutableConfigHashRefreshUnverified: boolean; - staleRecovery: boolean; - backup: { readonly backupPath: string } | null; - recoveryRecreate: boolean; - staleSandboxWasLocked: boolean; - preparedBackupRecovery: boolean; - relockShields: () => boolean; - log: (message: string) => void; - bail: (message: string, code?: number) => never; -} - -interface RebuildPostRestoreFinalizationDeps { - updateSandbox?: typeof registry.updateSandbox; - ensureMessagingHostForward?: typeof ensureMessagingHostForwardAfterRebuild; - writeLine?: (message: string) => void; -} - -export interface RebuildPostRestoreFinalizationResult { - postRestoreComplete: boolean; - messagingHostForwardUnverified: boolean; -} - -/** - * Reconcile rebuilt state and report its recovery posture in one fixed order. - * Keep this boundary after all restore/migration work: the restored preset set - * is authoritative, shields must relock before host forwarding is verified, - * and prepared recovery must fail closed on any unverified post-restore step. - */ -export function finalizeRebuildPostRestore( - options: RebuildPostRestoreFinalizationOptions, - deps: RebuildPostRestoreFinalizationDeps = {}, -): RebuildPostRestoreFinalizationResult { - const updateSandbox = deps.updateSandbox ?? registry.updateSandbox; - const ensureMessagingHostForward = - deps.ensureMessagingHostForward ?? ensureMessagingHostForwardAfterRebuild; - const writeLine = deps.writeLine ?? console.log; - - // Source-of-truth reconciliation for `policies`: - // - // - Invalid state: `registry.policies` retained a preset name after the - // reapply loop pruned it (disabled messaging channel) or skipped it - // (failed `applyPreset`), so `policy-list` showed a marker for a preset - // whose rules were absent from the gateway. - // - Source boundary: `policies.applyPreset` only appends to - // `registry.policies`; nothing else writes the canonical post-rebuild - // set. The reapply loop is the only place that knows which presets were - // actually reapplied. - // - Source-fix constraint: this must run after the reapply loop and use the - // successfully restored subset, not the saved set (which still includes - // failures). - // - Regression tests: `rebuild-flow.test.ts` asserts the successful subset - // reaches `registry.updateSandbox`; this module's tests also pin the - // reconciliation and finalization order. - // - Removal condition: drop this once `applyPreset` writes the canonical - // post-apply set itself (replacing its append-only contract), making this - // rebuild reconciliation redundant. - updateSandbox(options.sandboxName, { - agentVersion: options.agentExpectedVersion || null, - policies: options.restoredPresets, - }); - options.log( - `Registry updated: agentVersion=${options.agentExpectedVersion}, policies=[${options.restoredPresets.join(",")}]`, - ); - - if (!options.relockShields()) { - return options.bail("Failed to re-apply shields lockdown."); - } - - const messagingHostForwardUnverified = !ensureMessagingHostForward( - options.sandboxName, - options.rebuildMessagingPlan, - ); - const policyPresetRestoreIncomplete = options.failedPresets.length > 0; - const postRestoreComplete = - options.restoreSucceeded && - !options.mutablePermsRepairUnverified && - !options.mutableConfigHashRefreshUnverified && - !messagingHostForwardUnverified && - !policyPresetRestoreIncomplete; - - writeLine(""); - if (postRestoreComplete) { - writeLine(` ${G}\u2713${R} Sandbox '${options.sandboxName}' rebuilt successfully`); - if (options.staleRecovery && !options.backup) { - writeLine( - ` ${D}Recovered from a stale registry entry \u2014 no prior workspace state was available to restore.${R}`, - ); - } - if (options.reportedVersion) { - writeLine(` Now running: ${options.rebuiltAgentName} v${options.reportedVersion}`); - } - } else { - writeLine( - ` ${YW}\u26a0${R} Sandbox '${options.sandboxName}' rebuilt but some post-restore steps were incomplete`, - ); - if (!options.restoreSucceeded && options.backup) { - writeLine( - ` State restore was incomplete \u2014 backup available at: ${options.backup.backupPath}`, - ); - } - if (options.mutablePermsRepairUnverified) { - writeLine( - ` Mutable config permissions were not verified \u2014 run \`${CLI_NAME} ${options.sandboxName} doctor --fix\` to restore the OpenClaw config permission contract`, - ); - } - if (options.mutableConfigHashRefreshUnverified) { - writeLine( - ` Mutable OpenClaw config hash was not refreshed \u2014 restart the sandbox or re-run \`${CLI_NAME} ${options.sandboxName} rebuild\` before relying on config integrity checks`, - ); - } - if (messagingHostForwardUnverified) { - writeLine( - ` Messaging webhook forward was not verified \u2014 run \`${CLI_NAME} ${options.sandboxName} connect\` after resolving the port conflict`, - ); - } - if (policyPresetRestoreIncomplete) { - writeLine( - ` Policy presets failed to reapply: ${options.failedPresets.join(", ")} \u2014 re-apply manually with \`${CLI_NAME} ${options.sandboxName} policy-add\``, - ); - } - } - - if (options.recoveryRecreate && options.staleSandboxWasLocked) { - writeLine( - ` ${YW}\u26a0${R} Shields were previously enabled but the recreated sandbox starts unlocked \u2014 run \`${CLI_NAME} ${options.sandboxName} shields up\` to restore lockdown.`, - ); - } - if (options.preparedBackupRecovery && !postRestoreComplete) { - options.bail( - `Prepared backup recovery for '${options.sandboxName}' completed with unverified post-restore state.`, - ); - } - - return { postRestoreComplete, messagingHostForwardUnverified }; -} diff --git a/src/lib/actions/sandbox/rebuild-mcp-phase.ts b/src/lib/actions/sandbox/rebuild-mcp-phase.ts index 0a555a01c6a..8c6ff0dd805 100644 --- a/src/lib/actions/sandbox/rebuild-mcp-phase.ts +++ b/src/lib/actions/sandbox/rebuild-mcp-phase.ts @@ -164,24 +164,6 @@ export async function restoreMcpAfterRebuild( } } -export function postRestoreCompleted(status: { - messagingHostForwardUnverified: boolean; - mcpBridgeRestoreUnverified: boolean; - mutableConfigHashRefreshUnverified: boolean; - mutablePermsRepairUnverified: boolean; - policyPresetRestoreIncomplete: boolean; - restoreSucceeded: boolean; -}): boolean { - return ( - status.restoreSucceeded && - !status.mutablePermsRepairUnverified && - !status.mutableConfigHashRefreshUnverified && - !status.messagingHostForwardUnverified && - !status.mcpBridgeRestoreUnverified && - !status.policyPresetRestoreIncomplete - ); -} - export function printMcpRestoreRecovery( sandboxName: string, mcpBridgeRestoreUnverified: boolean, diff --git a/src/lib/actions/sandbox/rebuild-pipeline.ts b/src/lib/actions/sandbox/rebuild-pipeline.ts index dbb783827b4..c6058a82a65 100644 --- a/src/lib/actions/sandbox/rebuild-pipeline.ts +++ b/src/lib/actions/sandbox/rebuild-pipeline.ts @@ -329,7 +329,7 @@ async function rebuildSandboxUnlocked( reconcileManagedDcodeObservability: rebuildAgent === DCODE_AGENT_NAME, log, }); - await runRebuildPostRestorePhase({ + const verification = await runRebuildPostRestorePhase({ sandboxName, sandboxEntry, messagingPlan, @@ -342,14 +342,16 @@ async function rebuildSandboxUnlocked( policyPresetReconciliationVerified: restored.policyPresetReconciliationVerified, staleRecovery, recoveryRecreate, - preparedBackupRecovery, staleSandboxWasLocked: originalShieldsLocked, versionCheck, relockShieldsIfNeeded, log, - bail, }); - await transaction.complete(); + if (!(await transaction.finalize(verification))) { + bail( + `Rebuild for '${sandboxName}' has unverified required post-restore state: ${verification.required.join(", ")}. Correct the reported conditions, then retry the rebuild.`, + ); + } } finally { if (!rebuildShieldsWindow.relocked) relockShieldsIfNeeded(sandboxStillExists); } diff --git a/src/lib/actions/sandbox/rebuild-post-restore-phase.ts b/src/lib/actions/sandbox/rebuild-post-restore-phase.ts index de3e727793d..fe94d53533f 100644 --- a/src/lib/actions/sandbox/rebuild-post-restore-phase.ts +++ b/src/lib/actions/sandbox/rebuild-post-restore-phase.ts @@ -14,11 +14,10 @@ import { ensureMessagingHostForwardAfterRebuild } from "./messaging-host-forward import { executeSandboxCommand } from "./process-recovery"; import type { RebuildBackupManifest } from "./rebuild-backup-phase"; import { refreshMutableOpenClawConfigHashAfterPostRestoreWrites } from "./rebuild-config-hash"; -import type { RebuildBail, RebuildLog } from "./rebuild-credential-preflight"; +import type { RebuildLog } from "./rebuild-credential-preflight"; import type { RebuildSandboxEntry } from "./rebuild-flow-helpers"; import { type McpRebuildPreparation, - postRestoreCompleted, printMcpRestoreRecovery, restoreMcpAfterRebuild, } from "./rebuild-mcp-phase"; @@ -37,12 +36,77 @@ export interface RebuildPostRestorePhaseInput { policyPresetReconciliationVerified: boolean; staleRecovery: boolean; recoveryRecreate: boolean; - preparedBackupRecovery: boolean; staleSandboxWasLocked: boolean; versionCheck: ReturnType; relockShieldsIfNeeded: (sandboxStillExists: boolean) => boolean; log: RebuildLog; - bail: RebuildBail; +} + +export type RebuildPostRestoreRequiredFinding = + | "STATE_RESTORE_INCOMPLETE" + | "MUTABLE_CONFIG_PERMISSIONS_UNVERIFIED" + | "MUTABLE_CONFIG_HASH_UNVERIFIED" + | "MCP_BRIDGE_RESTORE_UNVERIFIED" + | "POLICY_PRESET_RESTORE_INCOMPLETE" + | "POLICY_RECONCILIATION_UNVERIFIED" + | "REGISTRY_RECONCILIATION_UNVERIFIED" + | "SHIELDS_RELOCK_UNVERIFIED" + | "MESSAGING_HOST_FORWARD_UNVERIFIED"; + +export type RebuildPostRestoreAdvisoryFinding = + | "OPENCLAW_DOCTOR_UNVERIFIED" + | "RECOVERY_SHIELDS_UNLOCKED"; + +export interface RebuildPostRestoreVerification { + complete: boolean; + required: RebuildPostRestoreRequiredFinding[]; + advisory: RebuildPostRestoreAdvisoryFinding[]; +} + +interface RebuildPostRestoreObservations { + restoreSucceeded: boolean; + mutablePermsVerified: boolean; + mutableConfigHashVerified: boolean; + mcpBridgeRestoreVerified: boolean; + policyPresetRestoreVerified: boolean; + policyReconciliationVerified: boolean; + registryReconciliationVerified: boolean; + shieldsRelocked: boolean; + messagingHostForwardVerified: boolean; + openClawDoctorVerified: boolean; + recoveryShieldsUnlocked: boolean; +} + +/** Convert phase observations into the only contract allowed to gate completion. */ +export function verifyRebuildPostRestore( + observations: RebuildPostRestoreObservations, +): RebuildPostRestoreVerification { + const required: RebuildPostRestoreRequiredFinding[] = []; + const advisory: RebuildPostRestoreAdvisoryFinding[] = []; + if (!observations.restoreSucceeded) required.push("STATE_RESTORE_INCOMPLETE"); + if (!observations.mutablePermsVerified) { + required.push("MUTABLE_CONFIG_PERMISSIONS_UNVERIFIED"); + } + if (!observations.mutableConfigHashVerified) { + required.push("MUTABLE_CONFIG_HASH_UNVERIFIED"); + } + if (!observations.mcpBridgeRestoreVerified) required.push("MCP_BRIDGE_RESTORE_UNVERIFIED"); + if (!observations.policyPresetRestoreVerified) { + required.push("POLICY_PRESET_RESTORE_INCOMPLETE"); + } + if (!observations.policyReconciliationVerified) { + required.push("POLICY_RECONCILIATION_UNVERIFIED"); + } + if (!observations.registryReconciliationVerified) { + required.push("REGISTRY_RECONCILIATION_UNVERIFIED"); + } + if (!observations.shieldsRelocked) required.push("SHIELDS_RELOCK_UNVERIFIED"); + if (!observations.messagingHostForwardVerified) { + required.push("MESSAGING_HOST_FORWARD_UNVERIFIED"); + } + if (!observations.openClawDoctorVerified) advisory.push("OPENCLAW_DOCTOR_UNVERIFIED"); + if (observations.recoveryShieldsUnlocked) advisory.push("RECOVERY_SHIELDS_UNLOCKED"); + return { complete: required.length === 0, required, advisory }; } export function resolveRestoredPolicyRegistryState( @@ -63,13 +127,13 @@ export function resolveRestoredPolicyRegistryState( } /** - * Repair agent state, restore MCP/forwarding, reconcile the registry, and report - * the final transaction result. Boundary coverage: rebuild-flow.test.ts and - * rebuild-config-hash.test.ts cover the complete/incomplete post-restore paths. + * Repair and verify rebuilt state, returning the authoritative completion + * result. The coordinator alone decides whether to retain or complete the + * durable transaction from this result. */ export async function runRebuildPostRestorePhase( input: RebuildPostRestorePhaseInput, -): Promise { +): Promise { const { sandboxName, sandboxEntry: sb, @@ -83,23 +147,17 @@ export async function runRebuildPostRestorePhase( policyPresetReconciliationVerified, staleRecovery, recoveryRecreate, - preparedBackupRecovery, staleSandboxWasLocked, versionCheck, relockShieldsIfNeeded, log, - bail, } = input; const rebuiltAgent = agentRuntime.getSessionAgent(sandboxName); const rebuiltAgentName = agentRuntime.getAgentDisplayName(rebuiltAgent); const agentDef = rebuiltAgent ? loadAgent(rebuiltAgent.name) : loadAgent("openclaw"); let mutablePermsRepairUnverified = false; let mutableConfigHashRefreshUnverified = false; - let messagingHostForwardUnverified = false; - const policyPresetRestoreIncomplete = - failedPresets.length > 0 || - failedPresetRemovals.length > 0 || - !policyPresetReconciliationVerified; + let openClawDoctorVerified = true; if (agentDef.name === "openclaw") { log("Running openclaw doctor --fix inside sandbox for post-upgrade structure repair"); @@ -108,8 +166,9 @@ export async function runRebuildPostRestorePhase( `doctor --fix: exit=${doctorResult?.status}, stdout=${(doctorResult?.stdout || "").substring(0, 200)}`, ); if (doctorResult && doctorResult.status === 0) { - console.log(` ${G}\u2713${R} Post-upgrade structure check passed`); + console.log(` ${G}✓${R} Post-upgrade structure check passed`); } else { + openClawDoctorVerified = false; console.log( ` ${D}Post-upgrade structure check skipped (doctor returned ${doctorResult?.status ?? "null"})${R}`, ); @@ -128,7 +187,7 @@ export async function runRebuildPostRestorePhase( } catch (error) { mutablePermsRepairUnverified = true; console.error( - ` ${YW}\u26a0${R} Mutable config permission repair errored: ${error instanceof Error ? error.message : String(error)}`, + ` ${YW}⚠${R} Mutable config permission repair errored: ${error instanceof Error ? error.message : String(error)}`, ); } if (permRepair === null) { @@ -136,64 +195,70 @@ export async function runRebuildPostRestorePhase( } else if (!permRepair.applied) { if (permRepair.skipReason === "unreadable") { mutablePermsRepairUnverified = true; - console.error( - ` ${YW}\u26a0${R} Mutable config permissions not restored: ${permRepair.reason}`, - ); + console.error(` ${YW}⚠${R} Mutable config permissions not restored: ${permRepair.reason}`); } else { log(`Mutable config permission repair skipped: ${permRepair.reason}`); } } else if (permRepair.verified) { - console.log(` ${G}\u2713${R} Mutable config permissions restored`); + console.log(` ${G}✓${R} Mutable config permissions restored`); } else { mutablePermsRepairUnverified = true; console.error( - ` ${YW}\u26a0${R} Mutable config permission repair incomplete: ${permRepair.errors.join("; ")}`, + ` ${YW}⚠${R} Mutable config permission repair incomplete: ${permRepair.errors.join("; ")}`, ); } } - const mcpBridgeRestoreUnverified = !(await restoreMcpAfterRebuild(sandboxName, mcpEntries)); + const mcpBridgeRestoreVerified = await restoreMcpAfterRebuild(sandboxName, mcpEntries); const { policies: restoredBuiltinPresets, policyPresetsFinalized } = resolveRestoredPolicyRegistryState( - { - policyPresetsFinalized: sb.policyPresetsFinalized, - }, + { policyPresetsFinalized: sb.policyPresetsFinalized }, finalBuiltinPresets, failedPresets, policyPresetReconciliationVerified, ); - registry.updateSandbox(sandboxName, { - agentVersion: agentDef.expectedVersion || null, - policies: restoredBuiltinPresets, - policyTier: normalizePolicyTierName(sb.policyTier), - policyPresetsFinalized, - }); - log( - `Registry updated: agentVersion=${agentDef.expectedVersion}, policies=[${restoredBuiltinPresets.join(",")}], policyPresetsFinalized=${String(policyPresetsFinalized === true)}`, - ); - - if (!relockShieldsIfNeeded(true)) { - bail("Failed to re-apply shields lockdown."); - return; + let registryReconciliationVerified = false; + try { + registryReconciliationVerified = registry.updateSandbox(sandboxName, { + agentVersion: agentDef.expectedVersion || null, + policies: restoredBuiltinPresets, + policyTier: normalizePolicyTierName(sb.policyTier), + policyPresetsFinalized, + }); + } catch { + // The finding code below is the redacted durable recovery contract. } - if (!ensureMessagingHostForwardAfterRebuild(sandboxName, messagingPlan)) { - messagingHostForwardUnverified = true; + if (registryReconciliationVerified) { + log( + `Registry updated: agentVersion=${agentDef.expectedVersion}, policies=[${restoredBuiltinPresets.join(",")}], policyPresetsFinalized=${String(policyPresetsFinalized === true)}`, + ); } - console.log(""); - const postRestoreComplete = postRestoreCompleted({ - messagingHostForwardUnverified, - mcpBridgeRestoreUnverified, - mutableConfigHashRefreshUnverified, - mutablePermsRepairUnverified, - policyPresetRestoreIncomplete, + const shieldsRelocked = relockShieldsIfNeeded(true); + const messagingHostForwardVerified = + messagingPlan === null || + (shieldsRelocked && ensureMessagingHostForwardAfterRebuild(sandboxName, messagingPlan)); + const verification = verifyRebuildPostRestore({ restoreSucceeded, + mutablePermsVerified: !mutablePermsRepairUnverified, + mutableConfigHashVerified: !mutableConfigHashRefreshUnverified, + mcpBridgeRestoreVerified, + policyPresetRestoreVerified: failedPresets.length === 0, + policyReconciliationVerified: + failedPresetRemovals.length === 0 && policyPresetReconciliationVerified, + registryReconciliationVerified, + shieldsRelocked, + messagingHostForwardVerified, + openClawDoctorVerified, + recoveryShieldsUnlocked: recoveryRecreate && staleSandboxWasLocked, }); - if (postRestoreComplete) { - console.log(` ${G}\u2713${R} Sandbox '${sandboxName}' rebuilt successfully`); + + console.log(""); + if (verification.complete) { + console.log(` ${G}✓${R} Sandbox '${sandboxName}' rebuilt successfully`); if (staleRecovery && !backupManifest) { console.log( - ` ${D}Recovered from a stale registry entry \u2014 no prior workspace state was available to restore.${R}`, + ` ${D}Recovered from a stale registry entry — no prior workspace state was available to restore.${R}`, ); } if (versionCheck.expectedVersion) { @@ -201,54 +266,50 @@ export async function runRebuildPostRestorePhase( } } else { console.log( - ` ${YW}\u26a0${R} Sandbox '${sandboxName}' rebuilt but some post-restore steps were incomplete`, + ` ${YW}⚠${R} Sandbox '${sandboxName}' rebuilt but some post-restore steps were incomplete`, ); - if (!restoreSucceeded && backupManifest) { + if (verification.required.includes("STATE_RESTORE_INCOMPLETE") && backupManifest) { console.log( - ` State restore was incomplete \u2014 backup available at: ${backupManifest.backupPath}`, + ` State restore was incomplete — backup available at: ${backupManifest.backupPath}`, ); } - if (mutablePermsRepairUnverified) { + if (verification.required.includes("MUTABLE_CONFIG_PERMISSIONS_UNVERIFIED")) { console.log( - ` Mutable config permissions were not verified \u2014 run \`${CLI_NAME} ${sandboxName} doctor --fix\` to restore the OpenClaw config permission contract`, + ` Mutable config permissions were not verified — run \`${CLI_NAME} ${sandboxName} doctor --fix\` to restore the OpenClaw config permission contract`, ); } - if (mutableConfigHashRefreshUnverified) { + if (verification.required.includes("MUTABLE_CONFIG_HASH_UNVERIFIED")) { console.log( - ` Mutable OpenClaw config hash was not refreshed \u2014 restart the sandbox or re-run \`${CLI_NAME} ${sandboxName} rebuild\` before relying on config integrity checks`, + ` Mutable OpenClaw config hash was not refreshed — restart the sandbox or re-run \`${CLI_NAME} ${sandboxName} rebuild\` before relying on config integrity checks`, ); } - if (messagingHostForwardUnverified) { + if (verification.required.includes("MESSAGING_HOST_FORWARD_UNVERIFIED")) { console.log( - ` Messaging webhook forward was not verified \u2014 run \`${CLI_NAME} ${sandboxName} connect\` after resolving the port conflict`, + ` Messaging webhook forward was not verified — run \`${CLI_NAME} ${sandboxName} connect\` after resolving the port conflict`, ); } - printMcpRestoreRecovery(sandboxName, mcpBridgeRestoreUnverified); - if (policyPresetRestoreIncomplete) { - if (failedPresets.length > 0) { - console.log( - ` Policy presets failed to reapply: ${failedPresets.join(", ")} \u2014 re-apply manually with \`${CLI_NAME} ${sandboxName} policy-add\``, - ); - } - if (failedPresetRemovals.length > 0 || !policyPresetReconciliationVerified) { - console.log( - ` Exact live policy reconciliation was incomplete${failedPresetRemovals.length > 0 ? `; remove failed: ${failedPresetRemovals.join(", ")}` : ""} \u2014 reconcile manually with \`${CLI_NAME} ${sandboxName} policy-add\` or \`${CLI_NAME} ${sandboxName} policy-remove\``, - ); - } + printMcpRestoreRecovery( + sandboxName, + verification.required.includes("MCP_BRIDGE_RESTORE_UNVERIFIED"), + ); + if (verification.required.includes("POLICY_PRESET_RESTORE_INCOMPLETE")) { + console.log( + ` Policy presets failed to reapply: ${failedPresets.join(", ")} — re-apply manually with \`${CLI_NAME} ${sandboxName} policy-add\``, + ); + } + if (verification.required.includes("POLICY_RECONCILIATION_UNVERIFIED")) { + console.log( + ` Exact live policy reconciliation was incomplete${failedPresetRemovals.length > 0 ? `; remove failed: ${failedPresetRemovals.join(", ")}` : ""} — reconcile manually with \`${CLI_NAME} ${sandboxName} policy-add\` or \`${CLI_NAME} ${sandboxName} policy-remove\``, + ); + } + if (verification.required.includes("REGISTRY_RECONCILIATION_UNVERIFIED")) { + console.log(` Rebuilt registry metadata was not verified — retry the rebuild.`); } } - if (recoveryRecreate && staleSandboxWasLocked) { + if (verification.advisory.includes("RECOVERY_SHIELDS_UNLOCKED")) { console.log( - ` ${YW}\u26a0${R} Shields were previously enabled but the recreated sandbox starts unlocked \u2014 run \`${CLI_NAME} ${sandboxName} shields up\` to restore lockdown.`, - ); - } - if (failedPresetRemovals.length > 0 || !policyPresetReconciliationVerified) { - bail(`Rebuild completed with unverified live policy reconciliation for '${sandboxName}'.`); - return; - } - if (preparedBackupRecovery && !postRestoreComplete) { - bail( - `Prepared backup recovery for '${sandboxName}' completed with unverified post-restore state.`, + ` ${YW}⚠${R} Shields were previously enabled but the recreated sandbox starts unlocked — run \`${CLI_NAME} ${sandboxName} shields up\` to restore lockdown.`, ); } + return verification; } diff --git a/src/lib/actions/sandbox/rebuild-post-restore-verification.test.ts b/src/lib/actions/sandbox/rebuild-post-restore-verification.test.ts new file mode 100644 index 00000000000..d591269cce3 --- /dev/null +++ b/src/lib/actions/sandbox/rebuild-post-restore-verification.test.ts @@ -0,0 +1,96 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; + +import { verifyRebuildPostRestore } from "./rebuild-post-restore-phase"; + +const verified = { + restoreSucceeded: true, + mutablePermsVerified: true, + mutableConfigHashVerified: true, + mcpBridgeRestoreVerified: true, + policyPresetRestoreVerified: true, + policyReconciliationVerified: true, + registryReconciliationVerified: true, + shieldsRelocked: true, + messagingHostForwardVerified: true, + openClawDoctorVerified: true, + recoveryShieldsUnlocked: false, +}; + +const requiredCases = [ + ["restoreSucceeded", "STATE_RESTORE_INCOMPLETE"], + ["mutablePermsVerified", "MUTABLE_CONFIG_PERMISSIONS_UNVERIFIED"], + ["mutableConfigHashVerified", "MUTABLE_CONFIG_HASH_UNVERIFIED"], + ["mcpBridgeRestoreVerified", "MCP_BRIDGE_RESTORE_UNVERIFIED"], + ["policyPresetRestoreVerified", "POLICY_PRESET_RESTORE_INCOMPLETE"], + ["policyReconciliationVerified", "POLICY_RECONCILIATION_UNVERIFIED"], + ["registryReconciliationVerified", "REGISTRY_RECONCILIATION_UNVERIFIED"], + ["shieldsRelocked", "SHIELDS_RELOCK_UNVERIFIED"], + ["messagingHostForwardVerified", "MESSAGING_HOST_FORWARD_UNVERIFIED"], +] as const; + +describe("verifyRebuildPostRestore", () => { + it("permits completion only when every required observation is verified", () => { + expect(verifyRebuildPostRestore(verified)).toEqual({ + complete: true, + required: [], + advisory: [], + }); + }); + + it("returns a stable code for every required completion blocker", () => { + const result = verifyRebuildPostRestore({ + restoreSucceeded: false, + mutablePermsVerified: false, + mutableConfigHashVerified: false, + mcpBridgeRestoreVerified: false, + policyPresetRestoreVerified: false, + policyReconciliationVerified: false, + registryReconciliationVerified: false, + shieldsRelocked: false, + messagingHostForwardVerified: false, + openClawDoctorVerified: true, + recoveryShieldsUnlocked: false, + }); + + expect(result).toEqual({ + complete: false, + required: [ + "STATE_RESTORE_INCOMPLETE", + "MUTABLE_CONFIG_PERMISSIONS_UNVERIFIED", + "MUTABLE_CONFIG_HASH_UNVERIFIED", + "MCP_BRIDGE_RESTORE_UNVERIFIED", + "POLICY_PRESET_RESTORE_INCOMPLETE", + "POLICY_RECONCILIATION_UNVERIFIED", + "REGISTRY_RECONCILIATION_UNVERIFIED", + "SHIELDS_RELOCK_UNVERIFIED", + "MESSAGING_HOST_FORWARD_UNVERIFIED", + ], + advisory: [], + }); + }); + + it.each(requiredCases)("blocks completion when only %s is unverified", (field, code) => { + expect(verifyRebuildPostRestore({ ...verified, [field]: false })).toEqual({ + complete: false, + required: [code], + advisory: [], + }); + }); + + it("keeps advisory findings visible without blocking completion", () => { + expect( + verifyRebuildPostRestore({ + ...verified, + openClawDoctorVerified: false, + recoveryShieldsUnlocked: true, + }), + ).toEqual({ + complete: true, + required: [], + advisory: ["OPENCLAW_DOCTOR_UNVERIFIED", "RECOVERY_SHIELDS_UNLOCKED"], + }); + }); +}); diff --git a/src/lib/actions/sandbox/rebuild-transaction-boundary.test.ts b/src/lib/actions/sandbox/rebuild-transaction-boundary.test.ts index 1af2971a013..c94f88c1d81 100644 --- a/src/lib/actions/sandbox/rebuild-transaction-boundary.test.ts +++ b/src/lib/actions/sandbox/rebuild-transaction-boundary.test.ts @@ -145,6 +145,32 @@ describe("rebuild transaction boundary", () => { }); }); + it("keeps finalization guidance when its failure metadata cannot be persisted", async () => { + const harness = createRebuildFlowHarness({ + restoreSandboxState: () => ({ + success: false, + restoredDirs: [], + restoredFiles: [], + failedDirs: ["config"], + failedFiles: [], + }), + }); + vi.spyOn(harness.transactionStore, "recordFailure").mockRejectedValueOnce( + new Error("simulated journal write failure"), + ); + + await expect( + harness.rebuildSandbox("alpha", ["--yes"], { throwOnError: true }), + ).rejects.toThrow("unverified required post-restore state"); + + expect(harness.logSpy.mock.calls.flat().join("\n")).toContain("State restore was incomplete"); + expect(harness.transactionStore.load("alpha")).toMatchObject({ + status: "active", + phase: "replacement_created", + failure: null, + }); + }); + it("leaves a prepared transaction when delete fails after compensation", async () => { const harness = createRebuildFlowHarness({ runOpenshell: (args) => diff --git a/src/lib/actions/sandbox/rebuild-transaction-coordinator.ts b/src/lib/actions/sandbox/rebuild-transaction-coordinator.ts index 24e2ff5dfee..f68c8bda613 100644 --- a/src/lib/actions/sandbox/rebuild-transaction-coordinator.ts +++ b/src/lib/actions/sandbox/rebuild-transaction-coordinator.ts @@ -15,6 +15,7 @@ import type { RebuildBackupManifest } from "./rebuild-backup-phase"; import type { RebuildSandboxEntry } from "./rebuild-flow-helpers"; import type { RebuildRecreateOnboardOpts } from "./rebuild-gpu-opt-out"; import type { RebuildTargetConfig } from "./rebuild-target-preflight"; +import type { RebuildPostRestoreVerification } from "./rebuild-post-restore-phase"; function stableJson(value: unknown): string { if (Array.isArray(value)) return `[${value.map(stableJson).join(",")}]`; @@ -237,8 +238,26 @@ export class RebuildTransactionCoordinator { }); } - async complete(): Promise { - if (this.transaction?.phase !== "replacement_created") return; - this.transaction = await this.store.complete(this.sandboxName, this.transaction.revision); + async finalize(verification: RebuildPostRestoreVerification): Promise { + if (!verification.complete) { + const code = verification.required[0]; + if (this.transaction?.phase === "replacement_created" && code) { + try { + this.transaction = await this.store.recordFailure( + this.sandboxName, + this.transaction.revision, + { code, recordedAt: new Date().toISOString(), retryable: true }, + ); + } catch { + // Completion remains blocked even when best-effort failure metadata + // cannot be published; the caller must still emit recovery guidance. + } + } + return false; + } + if (this.transaction?.phase === "replacement_created") { + this.transaction = await this.store.complete(this.sandboxName, this.transaction.revision); + } + return true; } } diff --git a/test/helpers/rebuild-flow-credential-preflight-cases.ts b/test/helpers/rebuild-flow-credential-preflight-cases.ts index 9bfb10ac9b4..5b4e37d3bf3 100644 --- a/test/helpers/rebuild-flow-credential-preflight-cases.ts +++ b/test/helpers/rebuild-flow-credential-preflight-cases.ts @@ -340,6 +340,7 @@ export function registerRebuildFlowCredentialPreflightTests(): void { it("copies the staged Hermes messaging plan into the rebuild resume session", async () => { const plan = makeMessagingPlan(); const harness = createRebuildFlowHarness({ + applyPreset: () => true, sandboxEntry: { agent: "hermes", provider: "nvidia-prod", diff --git a/test/helpers/rebuild-flow-lifecycle-cases.ts b/test/helpers/rebuild-flow-lifecycle-cases.ts index dab13d05f3c..1c8366c3c00 100644 --- a/test/helpers/rebuild-flow-lifecycle-cases.ts +++ b/test/helpers/rebuild-flow-lifecycle-cases.ts @@ -44,6 +44,7 @@ export function registerRebuildFlowLifecycleTests(): void { }; const harness = createRebuildFlowHarness({ applyPreset: () => true, + backupPolicyPresets: ["npm", "bad", "throw"], sandboxEntry: { policyPresetsFinalized: true, policyTier: "balanced" }, mcpPreparation: { entries: [mcpEntry], diff --git a/test/helpers/rebuild-flow-recovery-cases.ts b/test/helpers/rebuild-flow-recovery-cases.ts index 9a8424224cf..c5cbb8e8b2e 100644 --- a/test/helpers/rebuild-flow-recovery-cases.ts +++ b/test/helpers/rebuild-flow-recovery-cases.ts @@ -226,12 +226,17 @@ export function registerRebuildFlowRecoveryTests(): void { throwOnError: true, recoveryManifest: makePreparedRecoveryManifest(), }), - ).rejects.toThrow("Prepared backup recovery"); + ).rejects.toThrow("MCP_BRIDGE_RESTORE_UNVERIFIED"); expect(harness.errorSpy).toHaveBeenCalledWith( expect.stringContaining("MCP bridge restore incomplete: MCP restore boom"), ); expect(harness.relockSpy).toHaveBeenCalled(); + expect(harness.transactionStore.load("alpha")).toMatchObject({ + status: "active", + phase: "replacement_created", + failure: { code: "MCP_BRIDGE_RESTORE_UNVERIFIED", retryable: true }, + }); }); it("prunes the disabled Teams preset from the final registry policies after rebuild", async () => { @@ -370,8 +375,9 @@ export function registerRebuildFlowRecoveryTests(): void { ).toBeGreaterThan(harness.onboardSpy.mock.invocationCallOrder[0]); }); - it("finishes the rebuild while surfacing incomplete post-restore work", async () => { + it("retains the transaction when required post-restore work is incomplete", async () => { const harness = createRebuildFlowHarness({ + backupPolicyPresets: ["npm", "bad", "throw"], sandboxEntry: { policyPresetsFinalized: true, policyTier: "balanced" }, executeSandboxCommand: () => ({ status: 1, stdout: "", stderr: "hash refresh failed" }), repairMutableConfigPerms: () => ({ @@ -390,7 +396,7 @@ export function registerRebuildFlowRecoveryTests(): void { await expect( harness.rebuildSandbox("alpha", ["--yes"], { throwOnError: true }), - ).resolves.toBeUndefined(); + ).rejects.toThrow("unverified required post-restore state"); const output = harness.logSpy.mock.calls.map((call) => String(call[0])).join("\n"); expect(output).toContain("rebuilt but some post-restore steps were incomplete"); @@ -408,6 +414,11 @@ export function registerRebuildFlowRecoveryTests(): void { policyPresetsFinalized: undefined, }); expect(output).toContain("Policy presets failed to reapply: bad, throw"); + expect(harness.transactionStore.load("alpha")).toMatchObject({ + status: "active", + phase: "replacement_created", + failure: { code: "STATE_RESTORE_INCOMPLETE", retryable: true }, + }); }); it("reports both MCP and policy recovery when both restores are incomplete", async () => { @@ -427,7 +438,7 @@ export function registerRebuildFlowRecoveryTests(): void { await expect( harness.rebuildSandbox("alpha", ["--yes"], { throwOnError: true }), - ).resolves.toBeUndefined(); + ).rejects.toThrow("unverified required post-restore state"); const output = harness.logSpy.mock.calls.map((call) => String(call[0])).join("\n"); expect(output).toContain("rebuilt but some post-restore steps were incomplete"); @@ -437,6 +448,11 @@ export function registerRebuildFlowRecoveryTests(): void { expect(harness.errorSpy).toHaveBeenCalledWith( expect.stringContaining("MCP bridge restore incomplete: MCP restore boom"), ); + expect(harness.transactionStore.load("alpha")).toMatchObject({ + status: "active", + phase: "replacement_created", + failure: { code: "MCP_BRIDGE_RESTORE_UNVERIFIED", retryable: true }, + }); }); }); } diff --git a/test/helpers/rebuild-flow-target-image-cases.ts b/test/helpers/rebuild-flow-target-image-cases.ts index 0829855e8a8..fedc694d918 100644 --- a/test/helpers/rebuild-flow-target-image-cases.ts +++ b/test/helpers/rebuild-flow-target-image-cases.ts @@ -291,6 +291,7 @@ export function registerRebuildFlowTargetImageTests(): void { try { const harness = createRebuildFlowHarness({ applyPreset: () => true, + backupPolicyPresets: ["npm", "bad", "throw"], sandboxEntry: { provider: "nvidia-prod", model: "nvidia/nemotron" }, sessionSandboxName: "some-other-sandbox", }); diff --git a/test/helpers/rebuild-flow-test-harness.ts b/test/helpers/rebuild-flow-test-harness.ts index bc186024c43..789e4e87d34 100644 --- a/test/helpers/rebuild-flow-test-harness.ts +++ b/test/helpers/rebuild-flow-test-harness.ts @@ -338,7 +338,7 @@ export function createRebuildFlowHarness(overrides: RebuildFlowOverrides = {}): manifest: { backupPath: "/tmp/nemoclaw-rebuild-backup", timestamp: "2026-06-01T00:00:00.000Z", - policyPresets: overrides.backupPolicyPresets ?? ["npm", "bad", "throw"], + policyPresets: overrides.backupPolicyPresets ?? ["npm"], }, }; }); From d9586cecb28de956e7cec2687463304085f3af5b Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Wed, 8 Jul 2026 19:07:20 -0400 Subject: [PATCH 2/4] fix(rebuild): preserve finalization failure evidence --- .../sandbox/rebuild-dcode-recovery.test.ts | 7 +- .../sandbox/rebuild-post-restore-phase.ts | 18 ++- .../rebuild-transaction-boundary.test.ts | 26 ----- ...-transaction-finalization-boundary.test.ts | 106 ++++++++++++++++++ test/helpers/rebuild-flow-harness.ts | 5 +- test/helpers/rebuild-flow-test-harness.ts | 13 ++- test/helpers/rebuild-flow-test-support.ts | 3 + 7 files changed, 144 insertions(+), 34 deletions(-) create mode 100644 src/lib/actions/sandbox/rebuild-transaction-finalization-boundary.test.ts diff --git a/src/lib/actions/sandbox/rebuild-dcode-recovery.test.ts b/src/lib/actions/sandbox/rebuild-dcode-recovery.test.ts index 36c72dedaaa..db399d6c113 100644 --- a/src/lib/actions/sandbox/rebuild-dcode-recovery.test.ts +++ b/src/lib/actions/sandbox/rebuild-dcode-recovery.test.ts @@ -366,7 +366,7 @@ describe("rebuildSandbox DCode flow: recovery", () => { await expect( harness.rebuildSandbox("alpha", ["--yes"], { throwOnError: true }), - ).rejects.toThrow("Rebuild completed with unverified live policy reconciliation for 'alpha'."); + ).rejects.toThrow("POLICY_RECONCILIATION_UNVERIFIED"); expect(harness.registryUpdateSpy).toHaveBeenCalledWith( "alpha", @@ -376,6 +376,11 @@ describe("rebuildSandbox DCode flow: recovery", () => { policyPresetsFinalized: undefined, }), ); + expect(harness.transactionStore.load("alpha")).toMatchObject({ + status: "active", + phase: "replacement_created", + failure: { code: "POLICY_RECONCILIATION_UNVERIFIED", retryable: true }, + }); expect(harness.relockSpy).toHaveBeenCalled(); }); diff --git a/src/lib/actions/sandbox/rebuild-post-restore-phase.ts b/src/lib/actions/sandbox/rebuild-post-restore-phase.ts index fe94d53533f..67d275be78e 100644 --- a/src/lib/actions/sandbox/rebuild-post-restore-phase.ts +++ b/src/lib/actions/sandbox/rebuild-post-restore-phase.ts @@ -8,6 +8,7 @@ import { D, G, R, YW } from "../../cli/terminal-style"; import type { SandboxMessagingPlan } from "../../messaging"; import { normalizePolicyTierName } from "../../onboard/policy-tier-suppression"; import type * as sandboxVersion from "../../sandbox/version"; +import { redactFull } from "../../security/redact"; import * as shields from "../../shields"; import * as registry from "../../state/registry"; import { ensureMessagingHostForwardAfterRebuild } from "./messaging-host-forward-lifecycle"; @@ -225,8 +226,21 @@ export async function runRebuildPostRestorePhase( policyTier: normalizePolicyTierName(sb.policyTier), policyPresetsFinalized, }); - } catch { - // The finding code below is the redacted durable recovery contract. + } catch (error) { + // Source-of-truth boundary review: + // - Invalid state: the replacement is live while its registry metadata is + // absent or stale, so the transaction must not be marked completed. + // - Source boundary: registry.updateSandbox owns the atomic registry write; + // post-restore cannot safely reconstruct or bypass a failed write. + // - Source-fix constraint: retain replacement_created and retry the same + // authoritative update instead of introducing a second registry writer. + // - Regression evidence: rebuild-transaction-finalization-boundary.test.ts + // covers a throwing update, redacted cause, guidance, and durable failure. + // - Removal condition: remove this adapter only when the registry boundary + // returns a typed failure carrying equivalent redacted diagnostics. + console.error( + ` ${YW}⚠${R} Registry reconciliation failed: ${redactFull(error instanceof Error ? error.message : String(error))}`, + ); } if (registryReconciliationVerified) { log( diff --git a/src/lib/actions/sandbox/rebuild-transaction-boundary.test.ts b/src/lib/actions/sandbox/rebuild-transaction-boundary.test.ts index c94f88c1d81..1af2971a013 100644 --- a/src/lib/actions/sandbox/rebuild-transaction-boundary.test.ts +++ b/src/lib/actions/sandbox/rebuild-transaction-boundary.test.ts @@ -145,32 +145,6 @@ describe("rebuild transaction boundary", () => { }); }); - it("keeps finalization guidance when its failure metadata cannot be persisted", async () => { - const harness = createRebuildFlowHarness({ - restoreSandboxState: () => ({ - success: false, - restoredDirs: [], - restoredFiles: [], - failedDirs: ["config"], - failedFiles: [], - }), - }); - vi.spyOn(harness.transactionStore, "recordFailure").mockRejectedValueOnce( - new Error("simulated journal write failure"), - ); - - await expect( - harness.rebuildSandbox("alpha", ["--yes"], { throwOnError: true }), - ).rejects.toThrow("unverified required post-restore state"); - - expect(harness.logSpy.mock.calls.flat().join("\n")).toContain("State restore was incomplete"); - expect(harness.transactionStore.load("alpha")).toMatchObject({ - status: "active", - phase: "replacement_created", - failure: null, - }); - }); - it("leaves a prepared transaction when delete fails after compensation", async () => { const harness = createRebuildFlowHarness({ runOpenshell: (args) => diff --git a/src/lib/actions/sandbox/rebuild-transaction-finalization-boundary.test.ts b/src/lib/actions/sandbox/rebuild-transaction-finalization-boundary.test.ts new file mode 100644 index 00000000000..2464cb1c3e6 --- /dev/null +++ b/src/lib/actions/sandbox/rebuild-transaction-finalization-boundary.test.ts @@ -0,0 +1,106 @@ +// 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 { + createRebuildFlowHarness, + installRebuildFlowTestHooks, +} from "../../../../test/helpers/rebuild-flow-test-harness"; +import type { RebuildFlowHarness } from "../../../../test/helpers/rebuild-flow-test-support"; +import { makeActiveTeamsMessagingPlan } from "./rebuild-flow-test-fixtures"; + +installRebuildFlowTestHooks(); + +function expectRetainedFailure(harness: RebuildFlowHarness, code: string): void { + expect(harness.transactionStore.load("alpha")).toMatchObject({ + status: "active", + phase: "replacement_created", + failure: { code, retryable: true }, + }); +} + +describe("rebuild transaction finalization boundary", () => { + it("keeps guidance when finalization failure metadata cannot be persisted", async () => { + const harness = createRebuildFlowHarness({ + restoreSandboxState: () => ({ + success: false, + restoredDirs: [], + restoredFiles: [], + failedDirs: ["config"], + failedFiles: [], + }), + }); + vi.spyOn(harness.transactionStore, "recordFailure").mockRejectedValueOnce( + new Error("simulated journal write failure"), + ); + + await expect( + harness.rebuildSandbox("alpha", ["--yes"], { throwOnError: true }), + ).rejects.toThrow("STATE_RESTORE_INCOMPLETE"); + + expect(harness.logSpy.mock.calls.flat().join("\n")).toContain("State restore was incomplete"); + expect(harness.transactionStore.load("alpha")).toMatchObject({ + status: "active", + phase: "replacement_created", + failure: null, + }); + }); + + it("retains replacement_created when registry reconciliation throws", async () => { + const secret = "nvapi-abcdefghijklmnopqrstuvwxyz012345"; + const harness = createRebuildFlowHarness({ + updateSandbox: (_name, updates) => { + const fields = updates as Record; + if (!("agentVersion" in fields && "policies" in fields && "policyTier" in fields)) { + return true; + } + throw new Error(`ENOSPC while writing ${secret}`); + }, + }); + + await expect( + harness.rebuildSandbox("alpha", ["--yes"], { throwOnError: true }), + ).rejects.toThrow("REGISTRY_RECONCILIATION_UNVERIFIED"); + + const errors = harness.errorSpy.mock.calls.flat().join("\n"); + expect(errors).toContain("Registry reconciliation failed: ENOSPC while writing "); + expect(errors).not.toContain(secret); + expect(harness.logSpy.mock.calls.flat().join("\n")).toContain( + "Rebuilt registry metadata was not verified", + ); + expectRetainedFailure(harness, "REGISTRY_RECONCILIATION_UNVERIFIED"); + }); + + it("retains replacement_created when shields relock is unverified", async () => { + const harness = createRebuildFlowHarness({ + shieldsWasLocked: true, + relockShieldsWindow: () => false, + }); + + await expect( + harness.rebuildSandbox("alpha", ["--yes"], { throwOnError: true }), + ).rejects.toThrow("SHIELDS_RELOCK_UNVERIFIED"); + + expect(harness.relockSpy).toHaveBeenCalledWith("alpha", expect.any(Object), true, "nemoclaw"); + expectRetainedFailure(harness, "SHIELDS_RELOCK_UNVERIFIED"); + }); + + it("retains replacement_created when configured messaging forwarding is unverified", async () => { + const plan = makeActiveTeamsMessagingPlan(); + const harness = createRebuildFlowHarness({ + applyPreset: () => true, + buildMessagingRebuildPlan: () => plan, + ensureMessagingHostForwardAfterRebuild: () => false, + }); + + await expect( + harness.rebuildSandbox("alpha", ["--yes"], { throwOnError: true }), + ).rejects.toThrow("MESSAGING_HOST_FORWARD_UNVERIFIED"); + + expect(harness.ensureMessagingHostForwardAfterRebuildSpy).toHaveBeenCalledWith("alpha", plan); + expect(harness.logSpy.mock.calls.flat().join("\n")).toContain( + "Messaging webhook forward was not verified", + ); + expectRetainedFailure(harness, "MESSAGING_HOST_FORWARD_UNVERIFIED"); + }); +}); diff --git a/test/helpers/rebuild-flow-harness.ts b/test/helpers/rebuild-flow-harness.ts index c9f7f012089..6732f07f702 100644 --- a/test/helpers/rebuild-flow-harness.ts +++ b/test/helpers/rebuild-flow-harness.ts @@ -7,6 +7,7 @@ import os from "node:os"; import path from "node:path"; import { type MockInstance, vi } from "vitest"; +import type { RebuildTransactionStore as RebuildTransactionStoreType } from "../../src/lib/state/rebuild-transaction"; type RebuildSandbox = typeof import("../../src/lib/actions/sandbox/rebuild")["rebuildSandbox"]; @@ -128,6 +129,7 @@ export type RebuildFlowOverrides = { export type RebuildFlowHarness = { rebuildSandbox: RebuildSandbox; + transactionStore: RebuildTransactionStoreType; applyPresetSpy: MockInstance; applyPresetContentSpy: MockInstance; backupSandboxStateSpy: MockInstance; @@ -438,7 +440,7 @@ export function createRebuildFlowHarness(overrides: RebuildFlowOverrides = {}): manifest: { backupPath: "/tmp/nemoclaw-rebuild-backup", timestamp: "2026-06-01T00:00:00.000Z", - policyPresets: overrides.backupPolicyPresets ?? ["npm", "bad", "throw"], + policyPresets: overrides.backupPolicyPresets ?? ["npm"], }, }); vi.spyOn(sandboxState, "validateRebuildRecoveryManifest").mockImplementation( @@ -618,6 +620,7 @@ export function createRebuildFlowHarness(overrides: RebuildFlowOverrides = {}): }); return { rebuildSandbox, + transactionStore, applyPresetSpy, applyPresetContentSpy, backupSandboxStateSpy, diff --git a/test/helpers/rebuild-flow-test-harness.ts b/test/helpers/rebuild-flow-test-harness.ts index 789e4e87d34..fde2c2f7e8a 100644 --- a/test/helpers/rebuild-flow-test-harness.ts +++ b/test/helpers/rebuild-flow-test-harness.ts @@ -243,7 +243,11 @@ export function createRebuildFlowHarness(overrides: RebuildFlowOverrides = {}): }; }); vi.spyOn(registry, "listSandboxes").mockReturnValue({ sandboxes: [] }); - const registryUpdateSpy = vi.spyOn(registry, "updateSandbox").mockReturnValue(true); + const registryUpdateSpy = vi + .spyOn(registry, "updateSandbox") + .mockImplementation( + (...args: unknown[]) => overrides.updateSandbox?.(String(args[0]), args[1]) ?? true, + ); const restoreSandboxEntrySpy = vi .spyOn(registry, "restoreSandboxEntry") .mockImplementation((...args: unknown[]) => { @@ -321,9 +325,10 @@ export function createRebuildFlowHarness(overrides: RebuildFlowOverrides = {}): const relockSpy = vi .spyOn(rebuildShields, "relockRebuildShieldsWindow") .mockImplementation((...args: unknown[]) => { + const relocked = overrides.relockShieldsWindow?.() ?? true; const window = args[1] as typeof rebuildShieldsWindow; - window.relocked = true; - return true; + window.relocked = relocked; + return relocked; }); const backupSandboxStateSpy = vi .spyOn(sandboxState, "backupSandboxState") @@ -428,7 +433,7 @@ export function createRebuildFlowHarness(overrides: RebuildFlowOverrides = {}): .mockImplementation(overrides.buildMessagingRebuildPlan ?? (() => null)); const ensureMessagingHostForwardAfterRebuildSpy = vi .spyOn(messagingHostForwardLifecycle, "ensureMessagingHostForwardAfterRebuild") - .mockReturnValue(true); + .mockImplementation(overrides.ensureMessagingHostForwardAfterRebuild ?? (() => true)); const prepareMcpBridgesForRebuildSpy = vi .spyOn(mcpBridge, "prepareMcpBridgesForRebuild") .mockResolvedValue( diff --git a/test/helpers/rebuild-flow-test-support.ts b/test/helpers/rebuild-flow-test-support.ts index 147194ddfab..12f1b18e5bd 100644 --- a/test/helpers/rebuild-flow-test-support.ts +++ b/test/helpers/rebuild-flow-test-support.ts @@ -51,6 +51,9 @@ export type RebuildFlowOverrides = { failedFiles: string[]; }; restoreMcpBridgesAfterRebuild?: () => Promise; + updateSandbox?: (name: string, updates: unknown) => boolean; + relockShieldsWindow?: () => boolean; + ensureMessagingHostForwardAfterRebuild?: () => boolean; buildMessagingRebuildPlan?: () => Promise | unknown; sandboxEntry?: Record; registryEntryMissing?: boolean; From ea45195e7bd431e8e87b24ad911642d3f122dbe9 Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Wed, 8 Jul 2026 19:09:35 -0400 Subject: [PATCH 3/4] test(rebuild): keep finalization boundary linear --- ...build-transaction-finalization-boundary.test.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/lib/actions/sandbox/rebuild-transaction-finalization-boundary.test.ts b/src/lib/actions/sandbox/rebuild-transaction-finalization-boundary.test.ts index 2464cb1c3e6..458ff279438 100644 --- a/src/lib/actions/sandbox/rebuild-transaction-finalization-boundary.test.ts +++ b/src/lib/actions/sandbox/rebuild-transaction-finalization-boundary.test.ts @@ -48,14 +48,14 @@ describe("rebuild transaction finalization boundary", () => { it("retains replacement_created when registry reconciliation throws", async () => { const secret = "nvapi-abcdefghijklmnopqrstuvwxyz012345"; - const harness = createRebuildFlowHarness({ - updateSandbox: (_name, updates) => { - const fields = updates as Record; - if (!("agentVersion" in fields && "policies" in fields && "policyTier" in fields)) { - return true; - } + const updateSandbox = vi + .fn() + .mockReturnValueOnce(true) + .mockImplementation(() => { throw new Error(`ENOSPC while writing ${secret}`); - }, + }); + const harness = createRebuildFlowHarness({ + updateSandbox, }); await expect( From 61e8c5dbdd883b4d8b02b8c6469c3693709c6498 Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Wed, 8 Jul 2026 19:15:23 -0400 Subject: [PATCH 4/4] test(rebuild): cover rejected registry reconciliation --- .../actions/sandbox/rebuild-post-restore-phase.ts | 3 ++- ...build-transaction-finalization-boundary.test.ts | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/lib/actions/sandbox/rebuild-post-restore-phase.ts b/src/lib/actions/sandbox/rebuild-post-restore-phase.ts index 67d275be78e..458695833a9 100644 --- a/src/lib/actions/sandbox/rebuild-post-restore-phase.ts +++ b/src/lib/actions/sandbox/rebuild-post-restore-phase.ts @@ -235,7 +235,8 @@ export async function runRebuildPostRestorePhase( // - Source-fix constraint: retain replacement_created and retry the same // authoritative update instead of introducing a second registry writer. // - Regression evidence: rebuild-transaction-finalization-boundary.test.ts - // covers a throwing update, redacted cause, guidance, and durable failure. + // covers false and throwing updates, redacted cause, guidance, and + // durable failure. // - Removal condition: remove this adapter only when the registry boundary // returns a typed failure carrying equivalent redacted diagnostics. console.error( diff --git a/src/lib/actions/sandbox/rebuild-transaction-finalization-boundary.test.ts b/src/lib/actions/sandbox/rebuild-transaction-finalization-boundary.test.ts index 458ff279438..e32f8b2e774 100644 --- a/src/lib/actions/sandbox/rebuild-transaction-finalization-boundary.test.ts +++ b/src/lib/actions/sandbox/rebuild-transaction-finalization-boundary.test.ts @@ -71,6 +71,20 @@ describe("rebuild transaction finalization boundary", () => { expectRetainedFailure(harness, "REGISTRY_RECONCILIATION_UNVERIFIED"); }); + it("retains replacement_created when registry reconciliation returns false", async () => { + const updateSandbox = vi.fn().mockReturnValueOnce(true).mockReturnValue(false); + const harness = createRebuildFlowHarness({ updateSandbox }); + + await expect( + harness.rebuildSandbox("alpha", ["--yes"], { throwOnError: true }), + ).rejects.toThrow("REGISTRY_RECONCILIATION_UNVERIFIED"); + + expect(harness.logSpy.mock.calls.flat().join("\n")).toContain( + "Rebuilt registry metadata was not verified", + ); + expectRetainedFailure(harness, "REGISTRY_RECONCILIATION_UNVERIFIED"); + }); + it("retains replacement_created when shields relock is unverified", async () => { const harness = createRebuildFlowHarness({ shieldsWasLocked: true,