From cc9a3dd2501808524dfdc8bd55f777c24fb94068 Mon Sep 17 00:00:00 2001 From: Hung Le Date: Thu, 6 Aug 2026 17:31:50 +0530 Subject: [PATCH 01/11] fix(rebuild): restart the Hermes gateway after state restore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebuild starts the gateway during recreation and restores durable state about 27 seconds later. An adapter that reads its state once at startup keeps the pre-restore result for the life of the process — Hermes' WhatsApp bridge reads its paired session that way — so the gateway can be alive and healthy while still serving the state the rebuild replaced. The post-restore step proved only liveness, which was never in doubt, so it reported success and left the channel unpaired until an operator ran `gateway restart` by hand. Restart the gateway first and let the existing check report on the process that restart produced. A gateway that stays up through a failed restart is now unverified instead of healthy; one the recovery check replaced is still accepted, because that process is new. `relaunchManagedSupervisorSession` and `restartRestoredSandboxGateway` already restart after their own restore, so rebuild was the outlier. The restart stays outside `runHermesCronRestoreTransaction`, whose drain gate compares the gateway pid and start time on validate and release. Signed-off-by: Hung Le --- .../recover-rebuild-sandboxes.mdx | 5 +- .../rebuild-hermes-post-restore.test.ts | 103 +++++++++++++++++- .../sandbox/rebuild-hermes-post-restore.ts | 34 +++++- .../sandbox/rebuild-post-restore-phase.ts | 2 +- test/e2e/live/rebuild-hermes.test.ts | 7 ++ test/helpers/rebuild-flow-harness.ts | 15 +++ 6 files changed, 156 insertions(+), 10 deletions(-) diff --git a/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx b/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx index 1b6be097676..c0c36b9306d 100644 --- a/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx +++ b/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx @@ -183,9 +183,10 @@ $$nemoclaw gateway-token --quiet ``` Before post-restore repairs, NemoClaw verifies that the recreated sandbox still identifies as Hermes and exits nonzero if its identity does not match the rebuild target. -After state restore, NemoClaw restores managed MCP configuration through the normal lifecycle, then re-proves or recovers gateway health and performs final MCP reconciliation. +After state restore, NemoClaw restores managed MCP configuration through the normal lifecycle, then restarts the Hermes gateway and verifies or recovers its health before performing final MCP reconciliation. +The gateway starts during recreation and reads its durable state before the restore replaces it, so the restart is what binds the running gateway to the restored state. `rebuild` exits nonzero instead of reporting success when it cannot verify final gateway health or managed MCP state. -Follow the printed recovery guidance, using `$$nemoclaw recover` for gateway health and `$$nemoclaw mcp restart` for incomplete managed MCP restoration. +Follow the printed recovery guidance, using `$$nemoclaw gateway restart` first for gateway health, `$$nemoclaw recover` when the restart does not restore verified health, and `$$nemoclaw mcp restart` for incomplete managed MCP restoration. When the rebuild backup contains active Hermes cron jobs that reference scripts, NemoClaw validates those script references before it deletes the existing sandbox. The check covers the default profile and named profiles. diff --git a/src/lib/actions/sandbox/rebuild-hermes-post-restore.test.ts b/src/lib/actions/sandbox/rebuild-hermes-post-restore.test.ts index e6aba209276..222deabc393 100644 --- a/src/lib/actions/sandbox/rebuild-hermes-post-restore.test.ts +++ b/src/lib/actions/sandbox/rebuild-hermes-post-restore.test.ts @@ -1,12 +1,91 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { createRebuildFlowHarness, resetRebuildFlowTestEnvironment, restoreRebuildFlowTestEnvironment, } from "../../../../test/helpers/rebuild-flow-harness"; +import { ensureHermesGatewayAfterStateRestore } from "./rebuild-hermes-post-restore"; + +const RESTART_SUCCEEDED = { + ok: true, + restarted: true, + healthPassed: true, + forwardRecovered: false, +} as const; +const RESTART_FAILED = { + ok: false, + failureLayer: "health timeout", + detail: "gateway did not become healthy", +} as const; + +describe("binding the Hermes gateway to restored state", () => { + it("restarts the gateway before reading its health (#8184)", () => { + const order: string[] = []; + const state = ensureHermesGatewayAfterStateRestore("alpha", "hermes", { + restartSandboxGateway: () => { + order.push("restart"); + return RESTART_SUCCEEDED; + }, + checkAndRecoverSandboxProcesses: () => { + order.push("check"); + return { checked: true, wasRunning: true, recovered: false }; + }, + }); + + expect(state).toBe("healthy"); + expect(order).toEqual(["restart", "check"]); + }); + + // The bug this replaces: the gateway read its durable state at startup, the + // restore replaced that state afterwards, and a live process satisfied the + // old liveness check while still serving what it read before the restore. + it("refuses a gateway that stayed up through a failed restart (#8184)", () => { + const state = ensureHermesGatewayAfterStateRestore("alpha", "hermes", { + restartSandboxGateway: () => RESTART_FAILED, + checkAndRecoverSandboxProcesses: () => ({ + checked: true, + wasRunning: true, + recovered: false, + }), + }); + + expect(state).toBe("unverified"); + }); + + it("accepts a gateway the recovery check replaced after a failed restart (#8184)", () => { + const state = ensureHermesGatewayAfterStateRestore("alpha", "hermes", { + restartSandboxGateway: () => RESTART_FAILED, + checkAndRecoverSandboxProcesses: () => ({ + checked: true, + wasRunning: false, + recovered: true, + }), + }); + + expect(state).toBe("recovered"); + }); + + it("leaves a non-Hermes rebuild without a gateway restart (#8184)", () => { + const restartSandboxGateway = vi.fn(() => RESTART_SUCCEEDED); + const checkAndRecoverSandboxProcesses = vi.fn(() => ({ + checked: true, + wasRunning: true, + recovered: false, + })); + + const state = ensureHermesGatewayAfterStateRestore("alpha", "openclaw", { + restartSandboxGateway, + checkAndRecoverSandboxProcesses, + }); + + expect(state).toBe("not-applicable"); + expect(restartSandboxGateway).not.toHaveBeenCalled(); + expect(checkAndRecoverSandboxProcesses).not.toHaveBeenCalled(); + }); +}); describe("Hermes rebuild post-restore verification", () => { beforeEach(resetRebuildFlowTestEnvironment); @@ -201,6 +280,28 @@ describe("Hermes rebuild post-restore verification", () => { ); }); + it("restarts the gateway between the state restore and the health check (#8184)", async () => { + const harness = createRebuildFlowHarness({ + agentName: "hermes", + sandboxEntry: { agent: "hermes" }, + }); + + await expect( + harness.rebuildSandbox("alpha", ["--yes"], { throwOnError: true }), + ).resolves.toBeUndefined(); + + expect(harness.restartSandboxGatewaySpy).toHaveBeenCalledWith("alpha", { quiet: true }); + expect(harness.restoreSandboxStateSpy.mock.invocationCallOrder[0]).toBeLessThan( + harness.restartSandboxGatewaySpy.mock.invocationCallOrder[0], + ); + expect(harness.restartSandboxGatewaySpy.mock.invocationCallOrder[0]).toBeLessThan( + harness.checkAndRecoverSandboxProcessesSpy.mock.invocationCallOrder[0], + ); + expect(harness.logSpy).toHaveBeenCalledWith( + expect.stringContaining("Hermes gateway restarted and verified after state restore"), + ); + }); + it("fails before recovery when recreated Hermes identity mismatches (#7084)", async () => { const harness = createRebuildFlowHarness({ agentName: "hermes", diff --git a/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts b/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts index d03284106f8..f1fe804ca82 100644 --- a/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts +++ b/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts @@ -3,6 +3,7 @@ import { CLI_NAME } from "../../cli/branding"; import { isDirectSandboxFallbackUnavailableError } from "../../sandbox/privileged-exec"; +import type { GatewayRestartResult } from "./gateway-restart"; import * as processRecovery from "./process-recovery"; const HERMES_CRON_CONTROL = "/usr/local/lib/nemoclaw/hermes-cron-restore-control.py"; @@ -74,14 +75,28 @@ interface HermesPostRestoreGatewayDeps { sandboxName: string, options: { quiet: boolean }, ) => GatewayRecoveryObservation; + restartSandboxGateway?: ( + sandboxName: string, + options: { quiet: boolean }, + ) => GatewayRestartResult; } /** - * Re-prove Hermes gateway health after workspace state restoration. + * Bind the running Hermes gateway to the state this rebuild just restored. + * + * Recreation starts the gateway, and the restore replaces its durable state + * afterwards. An adapter that reads that state once at startup keeps the + * pre-restore result for the life of the process — the WhatsApp bridge reads + * its paired session that way — so the gateway can be alive and healthy while + * still serving the state the rebuild replaced. A liveness check cannot see + * that difference, so restart first and let the check report on the process + * that restart produced. `relaunchManagedSupervisorSession` already restarts + * after its own restore for the same reason. * - * Inner onboarding verifies the fresh image before rebuild restores the prior - * state. That restore can still stop or wedge the gateway, so its earlier - * readiness message is not authoritative for rebuild completion. + * The restart belongs here rather than inside `runHermesCronRestoreTransaction`: + * that gate records the gateway `pid` and `start_time` when it drains dispatch + * and compares them again on validate and release, so a restart inside it would + * fail its own identity check. */ export function ensureHermesGatewayAfterStateRestore( sandboxName: string, @@ -89,6 +104,8 @@ export function ensureHermesGatewayAfterStateRestore( deps: HermesPostRestoreGatewayDeps = {}, ): HermesPostRestoreGatewayState { if (agentName !== "hermes") return "not-applicable"; + const restart = deps.restartSandboxGateway ?? processRecovery.restartSandboxGateway; + const restarted = restart(sandboxName, { quiet: true }).ok; const checkAndRecover = deps.checkAndRecoverSandboxProcesses ?? processRecovery.checkAndRecoverSandboxProcesses; const observation: GatewayRecoveryObservation = checkAndRecover(sandboxName, { quiet: true }); @@ -100,8 +117,13 @@ export function ensureHermesGatewayAfterStateRestore( ) { return "unverified"; } - if (observation.wasRunning === true) return "healthy"; + // Recovery replaces the process, so a recovered gateway reads the restored + // state whatever the restart reported. A gateway that stayed up through a + // failed restart is still serving what it read before the restore, which is + // the state this step exists to replace. if (observation.recovered) return "recovered"; + if (!restarted) return "unverified"; + if (observation.wasRunning === true) return "healthy"; return "unverified"; } @@ -112,7 +134,7 @@ export function printHermesGatewayRestoreRecovery( ): void { if (state !== "unverified") return; writeLine( - ` Hermes gateway health was not verified after state restore — run \`${CLI_NAME} ${sandboxName} recover\` before relying on this sandbox`, + ` Hermes gateway health was not verified after state restore — it can still be serving the state this rebuild replaced; run \`${CLI_NAME} ${sandboxName} gateway restart\`, then \`${CLI_NAME} ${sandboxName} recover\` if that fails`, ); } diff --git a/src/lib/actions/sandbox/rebuild-post-restore-phase.ts b/src/lib/actions/sandbox/rebuild-post-restore-phase.ts index 99ddcf85d5d..c9c1734ff4e 100644 --- a/src/lib/actions/sandbox/rebuild-post-restore-phase.ts +++ b/src/lib/actions/sandbox/rebuild-post-restore-phase.ts @@ -255,7 +255,7 @@ export async function runRebuildPostRestorePhase( ); const hermesGatewayRestoreUnverified = hermesGatewayRestoreState === "unverified"; if (hermesGatewayRestoreState === "healthy") { - console.log(` ${G}\u2713${R} Hermes gateway health verified after state restore`); + console.log(` ${G}\u2713${R} Hermes gateway restarted and verified after state restore`); } else if (hermesGatewayRestoreState === "recovered") { console.log(` ${G}\u2713${R} Hermes gateway recovered after state restore`); } diff --git a/test/e2e/live/rebuild-hermes.test.ts b/test/e2e/live/rebuild-hermes.test.ts index 26f49736367..58322e69668 100644 --- a/test/e2e/live/rebuild-hermes.test.ts +++ b/test/e2e/live/rebuild-hermes.test.ts @@ -1220,6 +1220,13 @@ test(STALE_BASE_REBUILD expect(rebuildOutput).toContain(`Using Hermes Agent base image: ${phase1BaseResolution.ref}`); expect(rebuildOutput).not.toContain("Rebuilding Hermes Agent base image"); expect(rebuildOutput).not.toMatch(/provider credential not found/i); + // The gateway starts during recreation and reads its durable state before the + // restore replaces it, so rebuild must hand back a process that started after + // the restore. Either post-restore path reports one; a live gateway that was + // only checked reports neither. + expect(rebuildOutput, "rebuild must report a Hermes gateway bound to the restored state").toMatch( + /Hermes gateway (?:restarted and verified|recovered) after state restore/u, + ); await waitForSandboxReady(host, apiKey, activeOpenshellBin, "phase-6-post-rebuild"); const backupPathText = rebuildOutput.match(/^\s*Backup:\s+(.+)$/mu)?.[1]?.trim(); diff --git a/test/helpers/rebuild-flow-harness.ts b/test/helpers/rebuild-flow-harness.ts index 0e5d8131ba0..b811a796958 100644 --- a/test/helpers/rebuild-flow-harness.ts +++ b/test/helpers/rebuild-flow-harness.ts @@ -5,6 +5,7 @@ import { createRequire } from "node:module"; import path from "node:path"; import { type MockInstance, vi } from "vitest"; +import type { GatewayRestartResult } from "../../src/lib/actions/sandbox/gateway-restart"; type RebuildSandbox = typeof import("../../src/lib/actions/sandbox/rebuild")["rebuildSandbox"]; @@ -102,6 +103,7 @@ export type RebuildFlowOverrides = { secretBoundaryRefused?: boolean; mcpReconciliationRefused?: boolean; }; + restartSandboxGateway?: () => GatewayRestartResult; onboard?: (session: RebuildFlowSession) => Promise | void; repairMutableConfigPerms?: () => | { applied: false; skipReason: "agent" | "locked" | "unreadable"; reason: string } @@ -166,6 +168,7 @@ export type RebuildFlowHarness = { restoreTrustedAgentRemoteBaseImageOverrideSpy: MockInstance; executeSandboxCommandSpy: MockInstance; checkAndRecoverSandboxProcessesSpy: MockInstance; + restartSandboxGatewaySpy: MockInstance; ensureMessagingHostForwardAfterRebuildSpy: MockInstance; logSpy: MockInstance; finalizeIncompleteOnboardStepSpy: MockInstance; @@ -777,6 +780,17 @@ export function createRebuildFlowHarness(overrides: RebuildFlowOverrides = {}): forwardRecovered: false, })), ); + const restartSandboxGatewaySpy = vi + .spyOn(processRecovery, "restartSandboxGateway") + .mockImplementation( + overrides.restartSandboxGateway ?? + (() => ({ + ok: true, + restarted: true, + healthPassed: true, + forwardRecovered: false, + })), + ); vi.spyOn(shields, "repairMutableConfigPerms").mockImplementation( overrides.repairMutableConfigPerms ?? (() => ({ applied: true, verified: true, errors: [] })), ); @@ -833,6 +847,7 @@ export function createRebuildFlowHarness(overrides: RebuildFlowOverrides = {}): restoreTrustedAgentRemoteBaseImageOverrideSpy, executeSandboxCommandSpy, checkAndRecoverSandboxProcessesSpy, + restartSandboxGatewaySpy, ensureMessagingHostForwardAfterRebuildSpy, logSpy, finalizeIncompleteOnboardStepSpy, From a19a17dd199b1b6694b2e1c06023fd8b3777a720 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Thu, 6 Aug 2026 06:13:18 -0700 Subject: [PATCH 02/11] test(rebuild): stub post-restore gateway restart Signed-off-by: Apurv Kumaria --- test/helpers/rebuild-flow-test-harness.ts | 12 ++++++++++++ test/helpers/rebuild-flow-test-support.ts | 5 ++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/test/helpers/rebuild-flow-test-harness.ts b/test/helpers/rebuild-flow-test-harness.ts index f862f1fe45f..3a4bfeb469e 100644 --- a/test/helpers/rebuild-flow-test-harness.ts +++ b/test/helpers/rebuild-flow-test-harness.ts @@ -587,6 +587,17 @@ export function createRebuildFlowHarness(overrides: RebuildFlowOverrides = {}): forwardRecovered: false, })), ); + const restartSandboxGatewaySpy = vi + .spyOn(processRecovery, "restartSandboxGateway") + .mockImplementation( + overrides.restartSandboxGateway ?? + (() => ({ + ok: true, + restarted: true, + healthPassed: true, + forwardRecovered: false, + })), + ); vi.spyOn(shields, "repairMutableConfigPerms").mockImplementation( overrides.repairMutableConfigPerms ?? (() => ({ applied: true, verified: true, errors: [] })), ); @@ -633,6 +644,7 @@ export function createRebuildFlowHarness(overrides: RebuildFlowOverrides = {}): applyPresetSpy, backupSandboxStateSpy, checkAndRecoverSandboxProcessesSpy, + restartSandboxGatewaySpy, errorSpy, executeSandboxCommandSpy, ensureMessagingHostForwardAfterRebuildSpy, diff --git a/test/helpers/rebuild-flow-test-support.ts b/test/helpers/rebuild-flow-test-support.ts index de70b076532..ad21b1b593d 100644 --- a/test/helpers/rebuild-flow-test-support.ts +++ b/test/helpers/rebuild-flow-test-support.ts @@ -2,14 +2,15 @@ // SPDX-License-Identifier: Apache-2.0 import { type MockInstance, vi } from "vitest"; +import type { GatewayRestartResult } from "../../src/lib/actions/sandbox/gateway-restart"; import type { SandboxGatewayState } from "../../src/lib/actions/sandbox/gateway-state"; import type { finalizePreparedRebuildImageMessagingPlan, RebuildImagePreflightResult, } from "../../src/lib/actions/sandbox/rebuild-custom-image-preflight"; -import type { PreservedEnvFile } from "../../src/lib/state/preserved-env"; import type { RebuildRecreateOnboardOpts } from "../../src/lib/actions/sandbox/rebuild-gpu-opt-out"; import type { VersionCheckResult } from "../../src/lib/sandbox/version"; +import type { PreservedEnvFile } from "../../src/lib/state/preserved-env"; import type { SandboxRemovalReceipt } from "../../src/lib/state/registry"; export type RebuildSandbox = @@ -51,6 +52,7 @@ export type RebuildFlowOverrides = { secretBoundaryRefused?: boolean; mcpReconciliationRefused?: boolean; }; + restartSandboxGateway?: () => GatewayRestartResult; onboard?: ( session: RebuildFlowSession, options: RebuildRecreateOnboardOpts, @@ -132,6 +134,7 @@ export type RebuildFlowHarness = { applyPresetSpy: MockInstance; backupSandboxStateSpy: MockInstance; checkAndRecoverSandboxProcessesSpy: MockInstance; + restartSandboxGatewaySpy: MockInstance; errorSpy: MockInstance; executeSandboxCommandSpy: MockInstance; ensureMessagingHostForwardAfterRebuildSpy: MockInstance; From 46940cb2a5bb4034d090661a8d2bbdcd937f9010 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Thu, 6 Aug 2026 10:14:47 -0700 Subject: [PATCH 03/11] fix(rebuild): restore gateway recheck bound Signed-off-by: Apurv Kumaria --- src/lib/actions/sandbox/rebuild-hermes-post-restore.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts b/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts index 02551c48aea..0a30ad030d9 100644 --- a/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts +++ b/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts @@ -12,6 +12,7 @@ const RECEIPT_PREFIX = "NEMOCLAW_HERMES_CRON_RESTORE_V1:"; const BEGIN_TIMEOUT_MS = 70_000; const CONTROL_TIMEOUT_MS = 25_000; const RECOVERY_TIMEOUT_MS = BEGIN_TIMEOUT_MS + CONTROL_TIMEOUT_MS * 2 + 10_000; +const HERMES_GATEWAY_RECHECK_ATTEMPTS = 2; type HermesCronRestoreAction = "begin" | "validate" | "release" | "recover"; type HermesCronRestoreDisposition = From 8ffba2dfe9cbcbe02ed422a8ab04cae64a518a73 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Fri, 7 Aug 2026 09:01:12 -0700 Subject: [PATCH 04/11] fix(rebuild): hold cron gate through gateway replacement Signed-off-by: Apurv Kumaria --- agents/hermes/cron-restore-control.py | 40 +++++- .../recover-rebuild-sandboxes.mdx | 5 +- .../rebuild-hermes-cron-restore.test.ts | 100 ++++++++++++++- .../sandbox/rebuild-hermes-post-restore.ts | 68 +++++++--- src/lib/actions/sandbox/rebuild-pipeline.ts | 19 ++- .../rebuild-post-restore-phase.test.ts | 118 ++++++++++++++++++ .../sandbox/rebuild-post-restore-phase.ts | 58 +++++++++ test/e2e/live/rebuild-hermes-cron-restore.ts | 2 +- test/hermes-cron-restore-control.test.ts | 61 +++++++++ 9 files changed, 438 insertions(+), 33 deletions(-) diff --git a/agents/hermes/cron-restore-control.py b/agents/hermes/cron-restore-control.py index 3f334aecc65..c96a8e0dc21 100644 --- a/agents/hermes/cron-restore-control.py +++ b/agents/hermes/cron-restore-control.py @@ -3,10 +3,11 @@ """Control Hermes cron dispatch while NemoClaw restores durable state. Cron restore control is the rebuild-time gate that keeps dispatch disabled until -backed-up scripts and job definitions are valid and the gateway identity is -unchanged. The gateway identity is the (PID, start_time) tuple pinned across -begin, validate, and release. A drain token is the client-side secret proving -ownership of the server-side persisted drain marker. +backed-up scripts and job definitions are valid and the replacement gateway is +ready. The initial gateway identity is pinned across begin and validate. The +complete action requires a different live identity before releasing the gate. A +drain token is the client-side secret proving ownership of the server-side +persisted drain marker. """ from __future__ import annotations @@ -569,6 +570,33 @@ def release_drain(pid: int, start_time: int, drain_token: str) -> None: ) +def complete_replacement(pid: int, start_time: int, drain_token: str) -> None: + with _control_lock(): + drain_control, status_module = _load_gateway_modules() + _require_owned_drain(drain_token) + _, replacement_pid, replacement_start_time = _gateway_identity(status_module) + if replacement_pid == pid and replacement_start_time == start_time: + raise ControlError("Hermes gateway identity did not change during cron restore") + _wait_for_state( + status_module, + pid=replacement_pid, + start_time=replacement_start_time, + state="draining", + require_idle=True, + timeout_seconds=BEGIN_TIMEOUT_SECONDS, + ) + counts = validate_cron_tree() + _complete_release( + "complete", + drain_control, + status_module, + pid=replacement_pid, + start_time=replacement_start_time, + drain_token=drain_token, + **counts, + ) + + def recover_drain() -> None: with _control_lock(): drain_control, status_module = _load_gateway_modules() @@ -614,7 +642,7 @@ def _parser() -> argparse.ArgumentParser: subparsers = parser.add_subparsers(dest="action", required=True) subparsers.add_parser("begin") subparsers.add_parser("recover") - for action in ("validate", "release"): + for action in ("validate", "complete", "release"): subparser = subparsers.add_parser(action) subparser.add_argument("--pid", required=True, type=int) subparser.add_argument("--start-time", required=True, type=int) @@ -634,6 +662,8 @@ def main() -> int: recover_drain() elif args.action == "validate": validate_restore(args.pid, args.start_time, args.drain_token) + elif args.action == "complete": + complete_replacement(args.pid, args.start_time, args.drain_token) elif args.action == "release": release_drain(args.pid, args.start_time, args.drain_token) else: diff --git a/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx b/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx index 1d6d89c00d3..6fbb46b09eb 100644 --- a/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx +++ b/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx @@ -235,9 +235,10 @@ If this validation fails, the rebuild keeps the existing sandbox and reports the After NemoClaw creates the replacement, it acquires an independent root-owned gate that blocks new Hermes turns and cron dispatch. The gate remains active across gateway and container restarts in the replacement sandbox. NemoClaw waits for active agent work to finish before restoring state. -It validates the restored jobs and scripts against the same running gateway before it clears its gate. +It validates the restored jobs and scripts before the gateway replacement, then keeps dispatch blocked while it restarts and verifies that replacement. +Before it clears its gate, it validates the restored cron tree against the replacement gateway. If an operator already drained the gateway, NemoClaw clears only its gate and leaves the operator drain active. -If state restore or cron validation fails after gate acquisition, the command exits nonzero, preserves the backup, and retains the NemoClaw gate. +If state restore, gateway replacement, or cron validation fails after gate acquisition, the command exits nonzero, preserves the backup, and retains the NemoClaw gate. Failures before gate acquisition do not create a new gate. Do not manually remove the root-owned cron restore marker because removal bypasses restored cron validation. After you correct the reported restore problem, run `$$nemoclaw recover` to validate the restored cron tree and clear the NemoClaw gate. diff --git a/src/lib/actions/sandbox/rebuild-hermes-cron-restore.test.ts b/src/lib/actions/sandbox/rebuild-hermes-cron-restore.test.ts index d3195169f75..c00797a90f5 100644 --- a/src/lib/actions/sandbox/rebuild-hermes-cron-restore.test.ts +++ b/src/lib/actions/sandbox/rebuild-hermes-cron-restore.test.ts @@ -29,6 +29,7 @@ vi.mock("../../sandbox/privileged-exec", async (importOriginal) => ({ import { beginHermesCronRestore, + completeHermesCronRestoreAfterGatewayReplacement, recoverHermesCronRestore, releaseHermesCronRestore, runHermesCronRestoreTransaction, @@ -47,7 +48,7 @@ function writeScript(target: string): void { writeFileSync(target, "print('ok')\n", { mode: 0o600 }); } -type ReceiptAction = "begin" | "validate" | "release" | "recover"; +type ReceiptAction = "begin" | "validate" | "complete" | "release" | "recover"; function receipt( action: ReceiptAction, @@ -69,6 +70,15 @@ function receipt( profiles: 1, script_jobs: 1, }, + complete: { + active_agents: 0, + active_jobs: 1, + disposition: "dispatch-reactivated", + operator_drain_active: false, + preserved_drain: false, + profiles: 1, + script_jobs: 1, + }, release: { active_agents: 0, disposition: "dispatch-reactivated", @@ -246,7 +256,7 @@ describe("Hermes cron rebuild restore contract", () => { expect(processMocks.privilegedSandboxExecArgv.mock.calls[0]?.[1]).toContain("begin"); }); - it("orders drain, restore, validation, and release", () => { + it("keeps dispatch held after restore validation until gateway replacement (#8472)", () => { const events: string[] = []; processMocks.dockerSpawnSync.mockImplementation((argv: string[]) => { const action = argv.includes("validate") @@ -258,17 +268,97 @@ describe("Hermes cron rebuild restore contract", () => { return { status: 0, stdout: receipt(action), stderr: "" }; }); - runHermesCronRestoreTransaction( + const transaction = runHermesCronRestoreTransaction( "alpha", () => { events.push("restore"); - return { restoreSucceeded: true }; + return { restoreSucceeded: true, restored: "state" }; }, (state) => events.push(state), ); - expect(events).toEqual(["begin", "acquired", "restore", "validate", "release", "released"]); + expect(events).toEqual(["begin", "acquired", "restore", "validate"]); + expect(transaction).toEqual({ + identity: { drain_token: "restore-token", pid: 41, start_time: 902 }, + result: { restoreSucceeded: true, restored: "state" }, + }); + }); + + it("completes the held gate against the replacement gateway identity (#8472)", () => { + processMocks.dockerSpawnSync.mockReturnValue({ + status: 0, + stdout: receipt("complete", 77, 903), + stderr: "", + }); + + expect( + completeHermesCronRestoreAfterGatewayReplacement("alpha", { + pid: 41, + start_time: 902, + drain_token: "restore-token", + }), + ).toEqual({ pid: 77, start_time: 903, drain_token: "restore-token" }); + expect(processMocks.privilegedSandboxExecArgv).toHaveBeenCalledWith( + "alpha", + [ + "/opt/hermes/.venv/bin/python", + "-I", + "/usr/local/lib/nemoclaw/hermes-cron-restore-control.py", + "complete", + "--pid", + "41", + "--start-time", + "902", + "--drain-token", + "restore-token", + ], + false, + true, + ); + }); + + it("rejects completion that did not bind to a replacement identity (#8472)", () => { + processMocks.dockerSpawnSync.mockReturnValue({ + status: 0, + stdout: receipt("complete"), + stderr: "", + }); + + expect(() => + completeHermesCronRestoreAfterGatewayReplacement("alpha", { + pid: 41, + start_time: 902, + drain_token: "restore-token", + }), + ).toThrow("did not bind to the replacement gateway identity"); }); + + it("rejects completion without the held drain token before transport (#8472)", () => { + expect(() => + completeHermesCronRestoreAfterGatewayReplacement("alpha", { + pid: 41, + start_time: 902, + }), + ).toThrow("requires the held drain token"); + expect(processMocks.dockerSpawnSync).not.toHaveBeenCalled(); + }); + + it("rejects completion while replacement agents are still active (#8472)", () => { + processMocks.dockerSpawnSync.mockReturnValue({ + status: 0, + stdout: receipt("complete", 77, 903, "restore-token", { active_agents: 1 }), + stderr: "", + }); + + expect(() => + completeHermesCronRestoreAfterGatewayReplacement("alpha", { + pid: 41, + start_time: 902, + drain_token: "restore-token", + }), + ).toThrow("receipt failed validation"); + }); + it.each([ ["dispatch-reactivated", false], ["operator-drain-preserved", true], diff --git a/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts b/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts index 0a30ad030d9..c65888bb042 100644 --- a/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts +++ b/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts @@ -14,7 +14,7 @@ const CONTROL_TIMEOUT_MS = 25_000; const RECOVERY_TIMEOUT_MS = BEGIN_TIMEOUT_MS + CONTROL_TIMEOUT_MS * 2 + 10_000; const HERMES_GATEWAY_RECHECK_ATTEMPTS = 2; -type HermesCronRestoreAction = "begin" | "validate" | "release" | "recover"; +type HermesCronRestoreAction = "begin" | "validate" | "complete" | "release" | "recover"; type HermesCronRestoreDisposition = | "drain-acquired" | "restore-validated" @@ -38,11 +38,16 @@ interface HermesCronRestoreReceipt { preserved_drain?: boolean; } -type HermesCronRestoreIdentity = Pick< +export type HermesCronRestoreIdentity = Pick< HermesCronRestoreReceipt, "pid" | "start_time" | "drain_token" >; +export interface PendingHermesCronRestore { + result: T; + identity: HermesCronRestoreIdentity; +} + export type HermesCronRestoreRecoveryOutcome = | "dispatch-reactivated" | "operator-drain-preserved" @@ -94,10 +99,9 @@ interface HermesPostRestoreGatewayDeps { * that restart produced. `relaunchManagedSupervisorSession` already restarts * after its own restore for the same reason. * - * The restart belongs here rather than inside `runHermesCronRestoreTransaction`: - * that gate records the gateway `pid` and `start_time` when it drains dispatch - * and compares them again on validate and release, so a restart inside it would - * fail its own identity check. + * A gated rebuild keeps the root-owned cron drain active while this function + * replaces and verifies the gateway. The caller then completes the held + * transaction against the replacement process before dispatch can resume. */ export function ensureHermesGatewayAfterStateRestore( sandboxName: string, @@ -244,6 +248,24 @@ function parseCronRestoreReceipt( "preserved_drain", ]); break; + case "complete": + actionValid = + receipt.drain_acquired === true && + receipt.active_agents === 0 && + isNonNegativeInteger(receipt.profiles) && + isNonNegativeInteger(receipt.active_jobs) && + isNonNegativeInteger(receipt.script_jobs) && + isReleaseDispositionValid(receipt) && + hasExactReceiptFields(receipt, [ + ...baseFields, + ...tokenFields, + "active_agents", + "profiles", + "active_jobs", + "script_jobs", + "preserved_drain", + ]); + break; case "recover": if (receipt.drain_acquired) { actionValid = @@ -304,7 +326,7 @@ function runCronRestoreControl( command, action === "begin" ? BEGIN_TIMEOUT_MS - : action === "recover" + : action === "recover" || action === "complete" ? RECOVERY_TIMEOUT_MS : CONTROL_TIMEOUT_MS, ); @@ -360,6 +382,27 @@ export function releaseHermesCronRestore( } } +export function completeHermesCronRestoreAfterGatewayReplacement( + sandboxName: string, + originalIdentity: HermesCronRestoreIdentity, +): HermesCronRestoreIdentity { + if (!originalIdentity.drain_token) { + throw new Error("Hermes cron completion requires the held drain token"); + } + const receipt = runCronRestoreControl(sandboxName, "complete", originalIdentity); + if ( + receipt.drain_token !== originalIdentity.drain_token || + (receipt.pid === originalIdentity.pid && receipt.start_time === originalIdentity.start_time) + ) { + throw new Error("Hermes cron completion did not bind to the replacement gateway identity"); + } + return { + pid: receipt.pid, + start_time: receipt.start_time, + ...(receipt.drain_token ? { drain_token: receipt.drain_token } : {}), + }; +} + function isLegacyCronRestoreControl(error: unknown): boolean { if (!(error instanceof HermesCronRestoreControlFailure)) return false; return ( @@ -390,11 +433,8 @@ export function recoverHermesCronRestore(sandboxName: string): HermesCronRestore export function runHermesCronRestoreTransaction( sandboxName: string, restore: () => T, - onGateTransition: ( - state: "acquired" | "released", - identity: HermesCronRestoreIdentity, - ) => void = () => {}, -): T { + onGateTransition: (state: "acquired", identity: HermesCronRestoreIdentity) => void = () => {}, +): PendingHermesCronRestore { const identity = beginHermesCronRestore(sandboxName); onGateTransition("acquired", identity); const result = restore(); @@ -402,7 +442,5 @@ export function runHermesCronRestoreTransaction { try { - return runHermesCronRestoreTransaction(sandboxName, restore, (state, identity) => { - log( - `Hermes cron restore gate ${state}: pid=${String(identity.pid)}, startTime=${String(identity.start_time)}`, - ); - }); + const transaction = runHermesCronRestoreTransaction( + sandboxName, + restore, + (state, identity) => { + log( + `Hermes cron restore gate ${state}: pid=${String(identity.pid)}, startTime=${String(identity.start_time)}`, + ); + }, + ); + hermesCronRestoreIdentity = transaction.identity; + return transaction.result; } catch (error) { console.error(""); console.error( @@ -506,6 +514,7 @@ async function rebuildSandboxUnlocked( backupManifest: backup.backupManifest, mcpEntries: mcpPreparation.entries, restoreSucceeded: restored.restoreSucceeded, + hermesCronRestoreIdentity, backupWasForceSkipped: backup.backupWasForceSkipped, failedPresets: restored.failedPresets, finalBuiltinPresets: restored.finalBuiltinPresets, diff --git a/src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts b/src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts index 05516025858..6a26adbc112 100644 --- a/src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts +++ b/src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts @@ -62,6 +62,10 @@ describe("rebuild post-restore phase", () => { (_sandboxName, targetAgentName) => targetAgentName === "hermes" ? "healthy" : "not-applicable", ); + vi.spyOn( + rebuildHermesPostRestore, + "completeHermesCronRestoreAfterGatewayReplacement", + ).mockReturnValue({ pid: 77, start_time: 903, drain_token: "restore-token" }); vi.spyOn(registry, "getSandbox").mockImplementation( () => ({ agent: agentName === "openclaw" ? null : agentName }) as never, ); @@ -115,6 +119,120 @@ describe("rebuild post-restore phase", () => { expect(processRecovery.executeSandboxCommand).not.toHaveBeenCalled(); }); + it("keeps cron dispatch blocked through replacement health verification (#8472)", async () => { + agentName = "hermes"; + const events: string[] = []; + let dispatchHeld = true; + const attemptDispatch = () => events.push(dispatchHeld ? "dispatch-blocked" : "dispatch-ran"); + vi.mocked(rebuildMcp.restoreMcpAfterRebuild).mockImplementation(async () => { + events.push("mcp"); + attemptDispatch(); + return true; + }); + vi.mocked(rebuildHermesPostRestore.ensureHermesGatewayAfterStateRestore).mockImplementation( + () => { + events.push("restart"); + attemptDispatch(); + events.push("health-verified"); + return "healthy"; + }, + ); + vi.mocked( + rebuildHermesPostRestore.completeHermesCronRestoreAfterGatewayReplacement, + ).mockImplementation(() => { + events.push("release"); + dispatchHeld = false; + return { pid: 77, start_time: 903, drain_token: "restore-token" }; + }); + vi.mocked(messagingHostForward.ensureMessagingHostForwardAfterRebuild).mockImplementation( + () => { + attemptDispatch(); + return true; + }, + ); + const args = { + ...input(), + hermesCronRestoreIdentity: { + pid: 41, + start_time: 902, + drain_token: "restore-token", + }, + }; + + await runRebuildPostRestorePhase(args); + + expect(events).toEqual([ + "mcp", + "dispatch-blocked", + "restart", + "dispatch-blocked", + "health-verified", + "release", + "dispatch-ran", + ]); + expect(args.log).toHaveBeenCalledWith( + "Hermes cron restore gate released: pid=77, startTime=903", + ); + expect(args.bail).not.toHaveBeenCalled(); + }); + + it("leaves the cron gate active when replacement verification fails (#8472)", async () => { + agentName = "hermes"; + vi.mocked(rebuildHermesPostRestore.ensureHermesGatewayAfterStateRestore).mockReturnValue( + "unverified", + ); + const args = { + ...input(), + hermesCronRestoreIdentity: { + pid: 41, + start_time: 902, + drain_token: "restore-token", + }, + }; + + await runRebuildPostRestorePhase(args); + + expect( + rebuildHermesPostRestore.completeHermesCronRestoreAfterGatewayReplacement, + ).not.toHaveBeenCalled(); + expect(args.bail).toHaveBeenCalledWith( + "Hermes cron restore validation failed; dispatch was not re-enabled.", + ); + expect(messagingHostForward.ensureMessagingHostForwardAfterRebuild).not.toHaveBeenCalled(); + expect(vi.mocked(console.error).mock.calls.flat().join("\n")).toContain( + "Hermes cron dispatch remains drained", + ); + }); + + it("leaves the cron gate active when replacement completion fails (#8472)", async () => { + agentName = "hermes"; + vi.mocked( + rebuildHermesPostRestore.completeHermesCronRestoreAfterGatewayReplacement, + ).mockImplementation(() => { + throw new Error("replacement cron tree is invalid"); + }); + const args = { + ...input(), + backupManifest: { backupPath: "/tmp/alpha-backup" } as never, + hermesCronRestoreIdentity: { + pid: 41, + start_time: 902, + drain_token: "restore-token", + }, + }; + + await runRebuildPostRestorePhase(args); + + expect(args.bail).toHaveBeenCalledWith( + "Hermes cron restore validation failed; dispatch was not re-enabled.", + ); + expect(messagingHostForward.ensureMessagingHostForwardAfterRebuild).not.toHaveBeenCalled(); + const output = vi.mocked(console.error).mock.calls.flat().join("\n"); + expect(output).toContain("replacement cron tree is invalid"); + expect(output).toContain("Backup is preserved at: /tmp/alpha-backup"); + expect(output).toContain("nemoclaw alpha recover"); + }); + it("discloses carried-over baseline exclusions in the successful rebuild summary (#7194)", async () => { const logSpy = vi.spyOn(console, "log").mockImplementation(() => undefined); vi.spyOn(registry, "getBaselineExclusions").mockReturnValue([ diff --git a/src/lib/actions/sandbox/rebuild-post-restore-phase.ts b/src/lib/actions/sandbox/rebuild-post-restore-phase.ts index 4feda7a79c1..9deee381a10 100644 --- a/src/lib/actions/sandbox/rebuild-post-restore-phase.ts +++ b/src/lib/actions/sandbox/rebuild-post-restore-phase.ts @@ -18,7 +18,9 @@ import { refreshMutableOpenClawConfigHashAfterPostRestoreWrites } from "./rebuil import type { RebuildBail, RebuildLog } from "./rebuild-credential-preflight"; import type { RebuildSandboxEntry } from "./rebuild-flow-helpers"; import { + completeHermesCronRestoreAfterGatewayReplacement, ensureHermesGatewayAfterStateRestore, + type HermesCronRestoreIdentity, printHermesGatewayRestoreRecovery, } from "./rebuild-hermes-post-restore"; import { @@ -31,6 +33,7 @@ import { reapplyMessagingManifestAfterOpenClawDoctor } from "./rebuild-messaging import { reconcileStalePinnedSessionModelsAfterRebuild } from "./reconcile-session-models"; export { + type HermesCronRestoreIdentity, HermesCronRestoreIncompleteError, recoverHermesCronRestore, runHermesCronRestoreTransaction, @@ -45,6 +48,21 @@ export function printHermesCronRestoreRecoveryCommand( ); } +function bailWithHeldHermesCronRestore( + sandboxName: string, + backupManifest: RebuildBackupManifest, + detail: string, + bailMessage: string, + bail: RebuildBail, +): never { + console.error(detail); + if (backupManifest) { + console.error(` Backup is preserved at: ${backupManifest.backupPath}`); + } + printHermesCronRestoreRecoveryCommand(sandboxName); + return bail(bailMessage); +} + export interface RebuildPostRestorePhaseInput { sandboxName: string; sandboxEntry: RebuildSandboxEntry; @@ -53,6 +71,7 @@ export interface RebuildPostRestorePhaseInput { backupManifest: RebuildBackupManifest; mcpEntries: McpRebuildPreparation["entries"]; restoreSucceeded: boolean; + hermesCronRestoreIdentity?: HermesCronRestoreIdentity; backupWasForceSkipped: boolean; failedPresets: string[]; finalBuiltinPresets: string[]; @@ -155,6 +174,7 @@ export async function runRebuildPostRestorePhase( backupManifest, mcpEntries, restoreSucceeded, + hermesCronRestoreIdentity, backupWasForceSkipped, failedPresets, finalBuiltinPresets, @@ -182,6 +202,15 @@ export async function runRebuildPostRestorePhase( console.error( ` ${YW}\u26a0${R} Recreated sandbox agent identity could not be verified against the rebuild target.`, ); + if (hermesCronRestoreIdentity) { + return bailWithHeldHermesCronRestore( + sandboxName, + backupManifest, + " Hermes cron dispatch remains drained because the replacement identity is unverified.", + "Recreated sandbox agent identity did not match the authoritative rebuild target.", + bail, + ); + } bail("Recreated sandbox agent identity did not match the authoritative rebuild target."); return; } @@ -256,6 +285,35 @@ export async function runRebuildPostRestorePhase( targetAgentName, ); const hermesGatewayRestoreUnverified = hermesGatewayRestoreState === "unverified"; + if (hermesCronRestoreIdentity) { + if (hermesGatewayRestoreUnverified || hermesGatewayRestoreState === "not-applicable") { + return bailWithHeldHermesCronRestore( + sandboxName, + backupManifest, + " Hermes cron dispatch remains drained because the replacement gateway was not verified.", + "Hermes cron restore validation failed; dispatch was not re-enabled.", + bail, + ); + } + let replacementIdentity: HermesCronRestoreIdentity; + try { + replacementIdentity = completeHermesCronRestoreAfterGatewayReplacement( + sandboxName, + hermesCronRestoreIdentity, + ); + } catch (error) { + return bailWithHeldHermesCronRestore( + sandboxName, + backupManifest, + ` Hermes cron restore could not validate the replacement gateway and reactivate dispatch: ${error instanceof Error ? error.message : String(error)}`, + "Hermes cron restore validation failed; dispatch was not re-enabled.", + bail, + ); + } + log( + `Hermes cron restore gate released: pid=${String(replacementIdentity.pid)}, startTime=${String(replacementIdentity.start_time)}`, + ); + } if (hermesGatewayRestoreState === "healthy") { console.log(` ${G}\u2713${R} Hermes gateway restarted and verified after state restore`); } else if (hermesGatewayRestoreState === "recovered") { diff --git a/test/e2e/live/rebuild-hermes-cron-restore.ts b/test/e2e/live/rebuild-hermes-cron-restore.ts index 2039e2d0f51..cbf255ec235 100644 --- a/test/e2e/live/rebuild-hermes-cron-restore.ts +++ b/test/e2e/live/rebuild-hermes-cron-restore.ts @@ -577,7 +577,7 @@ export function createRebuildHermesCronRestoreFixture({ ); expect(acquired, "rebuild output must report cron restore gate acquisition").not.toBeNull(); expect(released, "rebuild output must report cron restore gate release").not.toBeNull(); - expect(released?.slice(1)).toEqual(acquired?.slice(1)); + expect(released?.slice(1)).not.toEqual(acquired?.slice(1)); expect(rebuildOutput.indexOf(released?.[0] ?? "released")).toBeGreaterThan( rebuildOutput.indexOf(acquired?.[0] ?? "acquired"), ); diff --git a/test/hermes-cron-restore-control.test.ts b/test/hermes-cron-restore-control.test.ts index 4311b707116..018583fbf3c 100644 --- a/test/hermes-cron-restore-control.test.ts +++ b/test/hermes-cron-restore-control.test.ts @@ -182,6 +182,25 @@ try: raise module.ControlError("simulated reactivation failure") module._wait_for_release_disposition = fail_after_operator_drain module.release_drain(41, 902, token) + elif scenario == "complete": + token = module.begin_drain() + module.validate_restore(41, 902, token) + status.payload["pid"] = 77 + status.payload["start_time"] = 903 + module.complete_replacement(41, 902, token) + elif scenario == "complete-same-identity": + token = module.begin_drain() + module.validate_restore(41, 902, token) + module.complete_replacement(41, 902, token) + elif scenario == "complete-validation-failure": + token = module.begin_drain() + module.validate_restore(41, 902, token) + status.payload["pid"] = 77 + status.payload["start_time"] = 903 + def fail_validation(): + raise module.ControlError("replacement cron tree is invalid") + module.validate_cron_tree = fail_validation + module.complete_replacement(41, 902, token) elif scenario == "recover": module.begin_drain() status.payload["pid"] = 77 @@ -269,6 +288,9 @@ describe("Hermes in-sandbox cron restore validator", () => { | "unsafe-lock-metadata" | "replacement-owned-marker" | "rollback-operator" + | "complete" + | "complete-same-identity" + | "complete-validation-failure" | "recover" | "recover-operator" | "recover-noop", @@ -498,6 +520,45 @@ describe("Hermes in-sandbox cron restore validator", () => { expect(result.stdout).toContain("OWN_MARKER:present"); }); + it("keeps the owned drain through gateway replacement and releases the validated replacement (#8472)", () => { + const result = runLifecycle("complete"); + + expect(result.stderr).toBe(""); + expect(result.status).toBe(0); + const receipts = result.stdout + .split("\n") + .filter((line) => line.startsWith(RECEIPT_PREFIX)) + .map((line) => JSON.parse(line.slice(RECEIPT_PREFIX.length))); + expect(receipts.map((receipt) => receipt.action)).toEqual(["begin", "validate", "complete"]); + expect(receipts.at(-1)).toEqual( + expect.objectContaining({ + active_jobs: 1, + disposition: "dispatch-reactivated", + pid: 77, + profiles: 1, + script_jobs: 1, + start_time: 903, + }), + ); + expect(result.stdout).toContain("OWN_MARKER:absent"); + }); + + it("keeps dispatch drained when the gateway identity was not replaced (#8472)", () => { + const result = runLifecycle("complete-same-identity"); + + expect(result.status).toBe(1); + expect(result.stderr).toContain("gateway identity did not change during cron restore"); + expect(result.stdout).toContain("OWN_MARKER:present"); + }); + + it("keeps dispatch drained when replacement validation fails (#8472)", () => { + const result = runLifecycle("complete-validation-failure"); + + expect(result.status).toBe(1); + expect(result.stderr).toContain("replacement cron tree is invalid"); + expect(result.stdout).toContain("OWN_MARKER:present"); + }); + it("re-pins a restarted gateway before validating and reactivating dispatch", () => { const result = runLifecycle("recover"); From 64c4d458f1e2a4f0d975829eb6a4c210f87fc6e0 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Fri, 7 Aug 2026 09:29:12 -0700 Subject: [PATCH 05/11] fix(rebuild): bind cron release to verified gateway Signed-off-by: Apurv Kumaria --- agents/hermes/cron-restore-control.py | 56 ++++++- .../recover-rebuild-sandboxes.mdx | 7 +- .../rebuild-hermes-cron-restore.test.ts | 101 +++++++++--- .../rebuild-hermes-post-restore.test.ts | 57 ++++++- .../sandbox/rebuild-hermes-post-restore.ts | 145 ++++++++++++++++-- .../rebuild-post-restore-phase.test.ts | 72 +++++++-- .../sandbox/rebuild-post-restore-phase.ts | 42 ++++- test/hermes-cron-restore-control.test.ts | 75 ++++++++- 8 files changed, 495 insertions(+), 60 deletions(-) diff --git a/agents/hermes/cron-restore-control.py b/agents/hermes/cron-restore-control.py index c96a8e0dc21..cd5dbd643c0 100644 --- a/agents/hermes/cron-restore-control.py +++ b/agents/hermes/cron-restore-control.py @@ -5,7 +5,8 @@ Cron restore control is the rebuild-time gate that keeps dispatch disabled until backed-up scripts and job definitions are valid and the replacement gateway is ready. The initial gateway identity is pinned across begin and validate. The -complete action requires a different live identity before releasing the gate. A +replacement identity is observed around managed health verification, and the +complete action requires that same live identity before releasing the gate. A drain token is the client-side secret proving ownership of the server-side persisted drain marker. """ @@ -481,6 +482,7 @@ def _complete_release( drain_token: str, **fields: Any, ) -> None: + _require_drained_idle(status_module, pid, start_time) _remove_owned_drain(drain_token) try: payload, operator_drain_active, disposition = _wait_for_release_disposition( @@ -570,11 +572,44 @@ def release_drain(pid: int, start_time: int, drain_token: str) -> None: ) -def complete_replacement(pid: int, start_time: int, drain_token: str) -> None: +def observe_replacement(pid: int, start_time: int, drain_token: str) -> None: with _control_lock(): drain_control, status_module = _load_gateway_modules() _require_owned_drain(drain_token) _, replacement_pid, replacement_start_time = _gateway_identity(status_module) + if replacement_pid == pid and replacement_start_time == start_time: + raise ControlError("Hermes gateway identity did not change during cron restore") + payload = _wait_for_state( + status_module, + pid=replacement_pid, + start_time=replacement_start_time, + state="draining", + require_idle=True, + timeout_seconds=BEGIN_TIMEOUT_SECONDS, + ) + _receipt( + "observe", + replacement_pid, + replacement_start_time, + drain_token, + active_agents=status_module.parse_active_agents( + payload.get("active_agents") + ), + disposition="replacement-observed", + operator_drain_active=_operator_drain_active(drain_control), + ) + + +def complete_replacement( + pid: int, + start_time: int, + replacement_pid: int, + replacement_start_time: int, + drain_token: str, +) -> None: + with _control_lock(): + drain_control, status_module = _load_gateway_modules() + _require_owned_drain(drain_token) if replacement_pid == pid and replacement_start_time == start_time: raise ControlError("Hermes gateway identity did not change during cron restore") _wait_for_state( @@ -642,11 +677,16 @@ def _parser() -> argparse.ArgumentParser: subparsers = parser.add_subparsers(dest="action", required=True) subparsers.add_parser("begin") subparsers.add_parser("recover") - for action in ("validate", "complete", "release"): + for action in ("validate", "observe", "complete", "release"): subparser = subparsers.add_parser(action) subparser.add_argument("--pid", required=True, type=int) subparser.add_argument("--start-time", required=True, type=int) subparser.add_argument("--drain-token", required=True) + if action == "complete": + subparser.add_argument("--replacement-pid", required=True, type=int) + subparser.add_argument( + "--replacement-start-time", required=True, type=int + ) tree = subparsers.add_parser("validate-tree") tree.add_argument("--home", required=True, type=Path) tree.add_argument("--sandbox-home", required=True, type=Path) @@ -662,8 +702,16 @@ def main() -> int: recover_drain() elif args.action == "validate": validate_restore(args.pid, args.start_time, args.drain_token) + elif args.action == "observe": + observe_replacement(args.pid, args.start_time, args.drain_token) elif args.action == "complete": - complete_replacement(args.pid, args.start_time, args.drain_token) + complete_replacement( + args.pid, + args.start_time, + args.replacement_pid, + args.replacement_start_time, + args.drain_token, + ) elif args.action == "release": release_drain(args.pid, args.start_time, args.drain_token) else: diff --git a/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx b/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx index 6fbb46b09eb..0772db1b647 100644 --- a/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx +++ b/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx @@ -236,12 +236,13 @@ After NemoClaw creates the replacement, it acquires an independent root-owned ga The gate remains active across gateway and container restarts in the replacement sandbox. NemoClaw waits for active agent work to finish before restoring state. It validates the restored jobs and scripts before the gateway replacement, then keeps dispatch blocked while it restarts and verifies that replacement. -Before it clears its gate, it validates the restored cron tree against the replacement gateway. +It records the replacement process identity around managed health verification and clears the gate only if that same live process completes the final cron validation. If an operator already drained the gateway, NemoClaw clears only its gate and leaves the operator drain active. -If state restore, gateway replacement, or cron validation fails after gate acquisition, the command exits nonzero, preserves the backup, and retains the NemoClaw gate. +If state restore, managed MCP restoration, gateway replacement, or cron validation fails after gate acquisition, the command exits nonzero, preserves the backup, and retains the NemoClaw gate. Failures before gate acquisition do not create a new gate. Do not manually remove the root-owned cron restore marker because removal bypasses restored cron validation. -After you correct the reported restore problem, run `$$nemoclaw recover` to validate the restored cron tree and clear the NemoClaw gate. +If managed MCP restoration failed, correct the reported cause and run `$$nemoclaw mcp restart` first. +Then run `$$nemoclaw recover` to repair and probe the gateway, validate the restored cron tree, and clear the NemoClaw gate. diff --git a/src/lib/actions/sandbox/rebuild-hermes-cron-restore.test.ts b/src/lib/actions/sandbox/rebuild-hermes-cron-restore.test.ts index c00797a90f5..625017fe95d 100644 --- a/src/lib/actions/sandbox/rebuild-hermes-cron-restore.test.ts +++ b/src/lib/actions/sandbox/rebuild-hermes-cron-restore.test.ts @@ -30,6 +30,7 @@ vi.mock("../../sandbox/privileged-exec", async (importOriginal) => ({ import { beginHermesCronRestore, completeHermesCronRestoreAfterGatewayReplacement, + observeHermesCronReplacement, recoverHermesCronRestore, releaseHermesCronRestore, runHermesCronRestoreTransaction, @@ -48,7 +49,7 @@ function writeScript(target: string): void { writeFileSync(target, "print('ok')\n", { mode: 0o600 }); } -type ReceiptAction = "begin" | "validate" | "complete" | "release" | "recover"; +type ReceiptAction = "begin" | "validate" | "observe" | "complete" | "release" | "recover"; function receipt( action: ReceiptAction, @@ -70,6 +71,11 @@ function receipt( profiles: 1, script_jobs: 1, }, + observe: { + active_agents: 0, + disposition: "replacement-observed", + operator_drain_active: false, + }, complete: { active_agents: 0, active_jobs: 1, @@ -292,11 +298,15 @@ describe("Hermes cron rebuild restore contract", () => { }); expect( - completeHermesCronRestoreAfterGatewayReplacement("alpha", { - pid: 41, - start_time: 902, - drain_token: "restore-token", - }), + completeHermesCronRestoreAfterGatewayReplacement( + "alpha", + { + pid: 41, + start_time: 902, + drain_token: "restore-token", + }, + { pid: 77, start_time: 903, drain_token: "restore-token" }, + ), ).toEqual({ pid: 77, start_time: 903, drain_token: "restore-token" }); expect(processMocks.privilegedSandboxExecArgv).toHaveBeenCalledWith( "alpha", @@ -311,6 +321,10 @@ describe("Hermes cron rebuild restore contract", () => { "902", "--drain-token", "restore-token", + "--replacement-pid", + "77", + "--replacement-start-time", + "903", ], false, true, @@ -325,24 +339,43 @@ describe("Hermes cron rebuild restore contract", () => { }); expect(() => - completeHermesCronRestoreAfterGatewayReplacement("alpha", { - pid: 41, - start_time: 902, - drain_token: "restore-token", - }), - ).toThrow("did not bind to the replacement gateway identity"); + completeHermesCronRestoreAfterGatewayReplacement( + "alpha", + { + pid: 41, + start_time: 902, + drain_token: "restore-token", + }, + { pid: 77, start_time: 903, drain_token: "restore-token" }, + ), + ).toThrow("changed the verified replacement gateway identity"); }); it("rejects completion without the held drain token before transport (#8472)", () => { expect(() => - completeHermesCronRestoreAfterGatewayReplacement("alpha", { - pid: 41, - start_time: 902, - }), + completeHermesCronRestoreAfterGatewayReplacement( + "alpha", + { + pid: 41, + start_time: 902, + }, + { pid: 77, start_time: 903, drain_token: "restore-token" }, + ), ).toThrow("requires the held drain token"); expect(processMocks.dockerSpawnSync).not.toHaveBeenCalled(); }); + it("rejects a replacement observation with a different drain token (#8472)", () => { + expect(() => + completeHermesCronRestoreAfterGatewayReplacement( + "alpha", + { pid: 41, start_time: 902, drain_token: "restore-token" }, + { pid: 77, start_time: 903, drain_token: "different-token" }, + ), + ).toThrow("changed the held drain token"); + expect(processMocks.dockerSpawnSync).not.toHaveBeenCalled(); + }); + it("rejects completion while replacement agents are still active (#8472)", () => { processMocks.dockerSpawnSync.mockReturnValue({ status: 0, @@ -351,12 +384,44 @@ describe("Hermes cron rebuild restore contract", () => { }); expect(() => - completeHermesCronRestoreAfterGatewayReplacement("alpha", { + completeHermesCronRestoreAfterGatewayReplacement( + "alpha", + { + pid: 41, + start_time: 902, + drain_token: "restore-token", + }, + { pid: 77, start_time: 903, drain_token: "restore-token" }, + ), + ).toThrow("receipt failed validation"); + }); + + it("observes the replacement identity without releasing the held gate (#8472)", () => { + processMocks.dockerSpawnSync.mockReturnValue({ + status: 0, + stdout: receipt("observe", 77, 903), + stderr: "", + }); + + expect( + observeHermesCronReplacement("alpha", { pid: 41, start_time: 902, drain_token: "restore-token", }), - ).toThrow("receipt failed validation"); + ).toEqual({ pid: 77, start_time: 903, drain_token: "restore-token" }); + expect(processMocks.privilegedSandboxExecArgv.mock.calls[0]?.[1]).toEqual([ + "/opt/hermes/.venv/bin/python", + "-I", + "/usr/local/lib/nemoclaw/hermes-cron-restore-control.py", + "observe", + "--pid", + "41", + "--start-time", + "902", + "--drain-token", + "restore-token", + ]); }); it.each([ diff --git a/src/lib/actions/sandbox/rebuild-hermes-post-restore.test.ts b/src/lib/actions/sandbox/rebuild-hermes-post-restore.test.ts index 4ba3192df43..570259807d0 100644 --- a/src/lib/actions/sandbox/rebuild-hermes-post-restore.test.ts +++ b/src/lib/actions/sandbox/rebuild-hermes-post-restore.test.ts @@ -7,7 +7,10 @@ import { resetRebuildFlowTestEnvironment, restoreRebuildFlowTestEnvironment, } from "../../../../test/helpers/rebuild-flow-harness"; -import { ensureHermesGatewayAfterStateRestore } from "./rebuild-hermes-post-restore"; +import { + ensureHermesGatewayAfterStateRestore, + ensureHermesGatewayAfterStateRestoreForCronGate, +} from "./rebuild-hermes-post-restore"; const RESTART_SUCCEEDED = { ok: true, @@ -85,6 +88,58 @@ describe("binding the Hermes gateway to restored state", () => { expect(restartSandboxGateway).not.toHaveBeenCalled(); expect(checkAndRecoverSandboxProcesses).not.toHaveBeenCalled(); }); + + it("binds managed health to one observed replacement identity (#8472)", () => { + const order: string[] = []; + const replacement = { pid: 77, start_time: 903, drain_token: "restore-token" }; + const verification = ensureHermesGatewayAfterStateRestoreForCronGate( + "alpha", + "hermes", + { pid: 41, start_time: 902, drain_token: "restore-token" }, + { + restartSandboxGateway: () => { + order.push("restart"); + return RESTART_SUCCEEDED; + }, + observeHermesCronReplacement: () => { + order.push("observe"); + return replacement; + }, + checkAndRecoverSandboxProcesses: () => { + order.push("health"); + return { checked: true, wasRunning: true, recovered: false }; + }, + }, + ); + + expect(verification).toEqual({ state: "healthy", replacementIdentity: replacement }); + expect(order).toEqual(["restart", "observe", "health", "observe"]); + }); + + it("fails closed when another gateway replaces the process during health verification (#8472)", () => { + const observeHermesCronReplacement = vi + .fn() + .mockReturnValueOnce({ pid: 77, start_time: 903, drain_token: "restore-token" }) + .mockReturnValueOnce({ pid: 88, start_time: 904, drain_token: "restore-token" }); + + expect( + ensureHermesGatewayAfterStateRestoreForCronGate( + "alpha", + "hermes", + { pid: 41, start_time: 902, drain_token: "restore-token" }, + { + restartSandboxGateway: () => RESTART_SUCCEEDED, + observeHermesCronReplacement, + checkAndRecoverSandboxProcesses: () => ({ + checked: true, + wasRunning: true, + recovered: false, + }), + }, + ), + ).toEqual({ state: "unverified" }); + expect(observeHermesCronReplacement).toHaveBeenCalledTimes(2); + }); }); describe("Hermes gateway post-restore recheck", () => { diff --git a/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts b/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts index c65888bb042..92c44940511 100644 --- a/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts +++ b/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts @@ -14,10 +14,17 @@ const CONTROL_TIMEOUT_MS = 25_000; const RECOVERY_TIMEOUT_MS = BEGIN_TIMEOUT_MS + CONTROL_TIMEOUT_MS * 2 + 10_000; const HERMES_GATEWAY_RECHECK_ATTEMPTS = 2; -type HermesCronRestoreAction = "begin" | "validate" | "complete" | "release" | "recover"; +type HermesCronRestoreAction = + | "begin" + | "validate" + | "observe" + | "complete" + | "release" + | "recover"; type HermesCronRestoreDisposition = | "drain-acquired" | "restore-validated" + | "replacement-observed" | "dispatch-reactivated" | "operator-drain-preserved" | "not-required"; @@ -85,6 +92,15 @@ interface HermesPostRestoreGatewayDeps { sandboxName: string, options: { quiet: boolean }, ) => GatewayRestartResult; + observeHermesCronReplacement?: ( + sandboxName: string, + originalIdentity: HermesCronRestoreIdentity, + ) => HermesCronRestoreIdentity; +} + +export interface HermesPostRestoreGatewayVerification { + state: HermesPostRestoreGatewayState; + replacementIdentity?: HermesCronRestoreIdentity; } /** @@ -108,29 +124,87 @@ export function ensureHermesGatewayAfterStateRestore( agentName: string, deps: HermesPostRestoreGatewayDeps = {}, ): HermesPostRestoreGatewayState { - if (agentName !== "hermes") return "not-applicable"; + return ensureHermesGatewayAfterStateRestoreImpl(sandboxName, agentName, deps).state; +} + +export function ensureHermesGatewayAfterStateRestoreForCronGate( + sandboxName: string, + agentName: string, + originalIdentity: HermesCronRestoreIdentity, + deps: HermesPostRestoreGatewayDeps = {}, +): HermesPostRestoreGatewayVerification { + return ensureHermesGatewayAfterStateRestoreImpl(sandboxName, agentName, deps, originalIdentity); +} + +function sameGatewayIdentity( + left: HermesCronRestoreIdentity, + right: HermesCronRestoreIdentity, +): boolean { + return left.pid === right.pid && left.start_time === right.start_time; +} + +function ensureHermesGatewayAfterStateRestoreImpl( + sandboxName: string, + agentName: string, + deps: HermesPostRestoreGatewayDeps, + originalIdentity?: HermesCronRestoreIdentity, +): HermesPostRestoreGatewayVerification { + if (agentName !== "hermes") return { state: "not-applicable" }; const restart = deps.restartSandboxGateway ?? processRecovery.restartSandboxGateway; const restarted = restart(sandboxName, { quiet: true }).ok; const checkAndRecover = deps.checkAndRecoverSandboxProcesses ?? processRecovery.checkAndRecoverSandboxProcesses; - for (let attempt = 1; attempt <= HERMES_GATEWAY_RECHECK_ATTEMPTS; attempt += 1) { + const observeReplacement = deps.observeHermesCronReplacement ?? observeHermesCronReplacement; + const maxAttempts = originalIdentity + ? HERMES_GATEWAY_RECHECK_ATTEMPTS + 1 + : HERMES_GATEWAY_RECHECK_ATTEMPTS; + let recovered = false; + for (let attempt = 1; attempt <= maxAttempts; attempt += 1) { + let identityBeforeHealth: HermesCronRestoreIdentity | undefined; + if (originalIdentity) { + try { + identityBeforeHealth = observeReplacement(sandboxName, originalIdentity); + } catch { + // The recovery check may still create the replacement process. A + // later iteration must observe it both before and after health. + } + } const observation: GatewayRecoveryObservation = checkAndRecover(sandboxName, { quiet: true }); if ( observation.forwardRecoveryFailed === true || observation.secretBoundaryRefused === true || observation.mcpReconciliationRefused === true ) { - return "unverified"; + return { state: "unverified" }; } if (!observation.checked) continue; // Recovery replaces the process, so a recovered gateway reads the restored // state whatever the restart reported. A gateway that stayed up through a // failed restart is still serving what it read before the restore, which is // the state this step exists to replace. - if (observation.recovered) return "recovered"; - if (restarted && observation.wasRunning === true) return "healthy"; + if (observation.recovered) { + if (!originalIdentity) return { state: "recovered" }; + recovered = true; + continue; + } + if ((!restarted && !recovered) || observation.wasRunning !== true) continue; + if (!originalIdentity) return { state: recovered ? "recovered" : "healthy" }; + if (!identityBeforeHealth) continue; + let identityAfterHealth: HermesCronRestoreIdentity; + try { + identityAfterHealth = observeReplacement(sandboxName, originalIdentity); + } catch { + return { state: "unverified" }; + } + if (!sameGatewayIdentity(identityBeforeHealth, identityAfterHealth)) { + return { state: "unverified" }; + } + return { + state: recovered ? "recovered" : "healthy", + replacementIdentity: identityAfterHealth, + }; } - return "unverified"; + return { state: "unverified" }; } export function printHermesGatewayRestoreRecovery( @@ -236,6 +310,13 @@ function parseCronRestoreReceipt( "script_jobs", ]); break; + case "observe": + actionValid = + receipt.drain_acquired === true && + receipt.disposition === "replacement-observed" && + receipt.active_agents === 0 && + hasExactReceiptFields(receipt, [...baseFields, ...tokenFields, "active_agents"]); + break; case "release": actionValid = receipt.drain_acquired === true && @@ -313,18 +394,27 @@ function runCronRestoreControl( sandboxName: string, action: HermesCronRestoreAction, identity?: HermesCronRestoreIdentity, + replacementIdentity?: HermesCronRestoreIdentity, ): HermesCronRestoreReceipt { const command = [HERMES_PYTHON, "-I", HERMES_CRON_CONTROL, action]; if (identity) { command.push("--pid", String(identity.pid), "--start-time", String(identity.start_time)); if (identity.drain_token) command.push("--drain-token", identity.drain_token); } + if (replacementIdentity) { + command.push( + "--replacement-pid", + String(replacementIdentity.pid), + "--replacement-start-time", + String(replacementIdentity.start_time), + ); + } let result: processRecovery.SandboxCommandResult | null; try { result = processRecovery.executePrivilegedSandboxCommand( sandboxName, command, - action === "begin" + action === "begin" || action === "observe" ? BEGIN_TIMEOUT_MS : action === "recover" || action === "complete" ? RECOVERY_TIMEOUT_MS @@ -385,16 +475,49 @@ export function releaseHermesCronRestore( export function completeHermesCronRestoreAfterGatewayReplacement( sandboxName: string, originalIdentity: HermesCronRestoreIdentity, + verifiedReplacementIdentity: HermesCronRestoreIdentity, ): HermesCronRestoreIdentity { if (!originalIdentity.drain_token) { throw new Error("Hermes cron completion requires the held drain token"); } - const receipt = runCronRestoreControl(sandboxName, "complete", originalIdentity); + if (sameGatewayIdentity(originalIdentity, verifiedReplacementIdentity)) { + throw new Error("Hermes cron completion requires a replacement gateway identity"); + } + if (verifiedReplacementIdentity.drain_token !== originalIdentity.drain_token) { + throw new Error("Hermes cron completion changed the held drain token"); + } + const receipt = runCronRestoreControl( + sandboxName, + "complete", + originalIdentity, + verifiedReplacementIdentity, + ); + if ( + receipt.drain_token !== originalIdentity.drain_token || + !sameGatewayIdentity(receipt, verifiedReplacementIdentity) + ) { + throw new Error("Hermes cron completion changed the verified replacement gateway identity"); + } + return { + pid: receipt.pid, + start_time: receipt.start_time, + ...(receipt.drain_token ? { drain_token: receipt.drain_token } : {}), + }; +} + +export function observeHermesCronReplacement( + sandboxName: string, + originalIdentity: HermesCronRestoreIdentity, +): HermesCronRestoreIdentity { + if (!originalIdentity.drain_token) { + throw new Error("Hermes cron replacement observation requires the held drain token"); + } + const receipt = runCronRestoreControl(sandboxName, "observe", originalIdentity); if ( receipt.drain_token !== originalIdentity.drain_token || - (receipt.pid === originalIdentity.pid && receipt.start_time === originalIdentity.start_time) + sameGatewayIdentity(receipt, originalIdentity) ) { - throw new Error("Hermes cron completion did not bind to the replacement gateway identity"); + throw new Error("Hermes cron observation did not bind to a replacement gateway identity"); } return { pid: receipt.pid, diff --git a/src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts b/src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts index 6a26adbc112..ee86bc3b67a 100644 --- a/src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts +++ b/src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts @@ -62,6 +62,13 @@ describe("rebuild post-restore phase", () => { (_sandboxName, targetAgentName) => targetAgentName === "hermes" ? "healthy" : "not-applicable", ); + vi.spyOn( + rebuildHermesPostRestore, + "ensureHermesGatewayAfterStateRestoreForCronGate", + ).mockReturnValue({ + state: "healthy", + replacementIdentity: { pid: 77, start_time: 903, drain_token: "restore-token" }, + }); vi.spyOn( rebuildHermesPostRestore, "completeHermesCronRestoreAfterGatewayReplacement", @@ -129,14 +136,17 @@ describe("rebuild post-restore phase", () => { attemptDispatch(); return true; }); - vi.mocked(rebuildHermesPostRestore.ensureHermesGatewayAfterStateRestore).mockImplementation( - () => { - events.push("restart"); - attemptDispatch(); - events.push("health-verified"); - return "healthy"; - }, - ); + vi.mocked( + rebuildHermesPostRestore.ensureHermesGatewayAfterStateRestoreForCronGate, + ).mockImplementation(() => { + events.push("restart"); + attemptDispatch(); + events.push("health-verified"); + return { + state: "healthy", + replacementIdentity: { pid: 77, start_time: 903, drain_token: "restore-token" }, + }; + }); vi.mocked( rebuildHermesPostRestore.completeHermesCronRestoreAfterGatewayReplacement, ).mockImplementation(() => { @@ -173,14 +183,21 @@ describe("rebuild post-restore phase", () => { expect(args.log).toHaveBeenCalledWith( "Hermes cron restore gate released: pid=77, startTime=903", ); + expect( + rebuildHermesPostRestore.completeHermesCronRestoreAfterGatewayReplacement, + ).toHaveBeenCalledWith( + "alpha", + { pid: 41, start_time: 902, drain_token: "restore-token" }, + { pid: 77, start_time: 903, drain_token: "restore-token" }, + ); expect(args.bail).not.toHaveBeenCalled(); }); it("leaves the cron gate active when replacement verification fails (#8472)", async () => { agentName = "hermes"; - vi.mocked(rebuildHermesPostRestore.ensureHermesGatewayAfterStateRestore).mockReturnValue( - "unverified", - ); + vi.mocked( + rebuildHermesPostRestore.ensureHermesGatewayAfterStateRestoreForCronGate, + ).mockReturnValue({ state: "unverified" }); const args = { ...input(), hermesCronRestoreIdentity: { @@ -233,6 +250,39 @@ describe("rebuild post-restore phase", () => { expect(output).toContain("nemoclaw alpha recover"); }); + it("keeps the gate active and repairs MCP before cron recovery (#8472)", async () => { + agentName = "hermes"; + vi.mocked(rebuildMcp.restoreMcpAfterRebuild).mockResolvedValue(false); + const args = { + ...input(), + hermesCronRestoreIdentity: { + pid: 41, + start_time: 902, + drain_token: "restore-token", + }, + }; + + await runRebuildPostRestorePhase(args); + + expect( + rebuildHermesPostRestore.completeHermesCronRestoreAfterGatewayReplacement, + ).not.toHaveBeenCalled(); + expect(args.bail).toHaveBeenCalledWith( + "Hermes MCP restoration failed; cron dispatch was not re-enabled.", + ); + const mcpCall = vi + .mocked(console.log) + .mock.calls.findIndex((call) => String(call[0]).includes("nemoclaw alpha mcp restart")); + const recoverCall = vi + .mocked(console.error) + .mock.calls.findIndex((call) => String(call[0]).includes("nemoclaw alpha recover")); + expect(mcpCall).toBeGreaterThanOrEqual(0); + expect(recoverCall).toBeGreaterThanOrEqual(0); + expect(vi.mocked(console.log).mock.invocationCallOrder[mcpCall]).toBeLessThan( + vi.mocked(console.error).mock.invocationCallOrder[recoverCall] ?? 0, + ); + }); + it("discloses carried-over baseline exclusions in the successful rebuild summary (#7194)", async () => { const logSpy = vi.spyOn(console, "log").mockImplementation(() => undefined); vi.spyOn(registry, "getBaselineExclusions").mockReturnValue([ diff --git a/src/lib/actions/sandbox/rebuild-post-restore-phase.ts b/src/lib/actions/sandbox/rebuild-post-restore-phase.ts index 9deee381a10..3e2e50c3950 100644 --- a/src/lib/actions/sandbox/rebuild-post-restore-phase.ts +++ b/src/lib/actions/sandbox/rebuild-post-restore-phase.ts @@ -20,6 +20,7 @@ import type { RebuildSandboxEntry } from "./rebuild-flow-helpers"; import { completeHermesCronRestoreAfterGatewayReplacement, ensureHermesGatewayAfterStateRestore, + ensureHermesGatewayAfterStateRestoreForCronGate, type HermesCronRestoreIdentity, printHermesGatewayRestoreRecovery, } from "./rebuild-hermes-post-restore"; @@ -54,11 +55,13 @@ function bailWithHeldHermesCronRestore( detail: string, bailMessage: string, bail: RebuildBail, + beforeCronRecovery?: () => void, ): never { console.error(detail); if (backupManifest) { console.error(` Backup is preserved at: ${backupManifest.backupPath}`); } + beforeCronRecovery?.(); printHermesCronRestoreRecoveryCommand(sandboxName); return bail(bailMessage); } @@ -280,13 +283,25 @@ export async function runRebuildPostRestorePhase( } const mcpBridgeRestoreUnverified = !(await restoreMcpAfterRebuild(sandboxName, mcpEntries)); - const hermesGatewayRestoreState = ensureHermesGatewayAfterStateRestore( - sandboxName, - targetAgentName, - ); + const hermesGatewayVerification = hermesCronRestoreIdentity + ? ensureHermesGatewayAfterStateRestoreForCronGate( + sandboxName, + targetAgentName, + hermesCronRestoreIdentity, + ) + : { + state: ensureHermesGatewayAfterStateRestore(sandboxName, targetAgentName), + replacementIdentity: undefined, + }; + const hermesGatewayRestoreState = hermesGatewayVerification.state; const hermesGatewayRestoreUnverified = hermesGatewayRestoreState === "unverified"; if (hermesCronRestoreIdentity) { - if (hermesGatewayRestoreUnverified || hermesGatewayRestoreState === "not-applicable") { + const replacementIdentity = hermesGatewayVerification.replacementIdentity; + if ( + hermesGatewayRestoreUnverified || + hermesGatewayRestoreState === "not-applicable" || + !replacementIdentity + ) { return bailWithHeldHermesCronRestore( sandboxName, backupManifest, @@ -295,11 +310,22 @@ export async function runRebuildPostRestorePhase( bail, ); } - let replacementIdentity: HermesCronRestoreIdentity; + if (mcpBridgeRestoreUnverified) { + return bailWithHeldHermesCronRestore( + sandboxName, + backupManifest, + " Hermes cron dispatch remains drained because managed MCP restoration was not verified.", + "Hermes MCP restoration failed; cron dispatch was not re-enabled.", + bail, + () => printMcpRestoreRecovery(sandboxName, true), + ); + } + let completedIdentity: HermesCronRestoreIdentity; try { - replacementIdentity = completeHermesCronRestoreAfterGatewayReplacement( + completedIdentity = completeHermesCronRestoreAfterGatewayReplacement( sandboxName, hermesCronRestoreIdentity, + replacementIdentity, ); } catch (error) { return bailWithHeldHermesCronRestore( @@ -311,7 +337,7 @@ export async function runRebuildPostRestorePhase( ); } log( - `Hermes cron restore gate released: pid=${String(replacementIdentity.pid)}, startTime=${String(replacementIdentity.start_time)}`, + `Hermes cron restore gate released: pid=${String(completedIdentity.pid)}, startTime=${String(completedIdentity.start_time)}`, ); } if (hermesGatewayRestoreState === "healthy") { diff --git a/test/hermes-cron-restore-control.test.ts b/test/hermes-cron-restore-control.test.ts index 018583fbf3c..bae87eb2db1 100644 --- a/test/hermes-cron-restore-control.test.ts +++ b/test/hermes-cron-restore-control.test.ts @@ -187,20 +187,55 @@ try: module.validate_restore(41, 902, token) status.payload["pid"] = 77 status.payload["start_time"] = 903 - module.complete_replacement(41, 902, token) + module.observe_replacement(41, 902, token) + module.complete_replacement(41, 902, 77, 903, token) elif scenario == "complete-same-identity": token = module.begin_drain() module.validate_restore(41, 902, token) - module.complete_replacement(41, 902, token) + module.complete_replacement(41, 902, 41, 902, token) elif scenario == "complete-validation-failure": token = module.begin_drain() module.validate_restore(41, 902, token) status.payload["pid"] = 77 status.payload["start_time"] = 903 + module.observe_replacement(41, 902, token) def fail_validation(): raise module.ControlError("replacement cron tree is invalid") module.validate_cron_tree = fail_validation - module.complete_replacement(41, 902, token) + module.complete_replacement(41, 902, 77, 903, token) + elif scenario == "complete-substitution": + token = module.begin_drain() + module.validate_restore(41, 902, token) + status.payload["pid"] = 77 + status.payload["start_time"] = 903 + module.observe_replacement(41, 902, token) + status.payload["pid"] = 88 + status.payload["start_time"] = 904 + module.complete_replacement(41, 902, 77, 903, token) + elif scenario == "complete-release-substitution": + token = module.begin_drain() + module.validate_restore(41, 902, token) + status.payload["pid"] = 77 + status.payload["start_time"] = 903 + module.observe_replacement(41, 902, token) + validate_cron_tree = module.validate_cron_tree + def substitute_after_validation(): + counts = validate_cron_tree() + status.payload["pid"] = 88 + status.payload["start_time"] = 904 + return counts + module.validate_cron_tree = substitute_after_validation + module.complete_replacement(41, 902, 77, 903, token) + elif scenario == "complete-release-failure": + token = module.begin_drain() + module.validate_restore(41, 902, token) + status.payload["pid"] = 77 + status.payload["start_time"] = 903 + module.observe_replacement(41, 902, token) + def fail_release(*_args, **_kwargs): + raise module.ControlError("simulated replacement release failure") + module._wait_for_release_disposition = fail_release + module.complete_replacement(41, 902, 77, 903, token) elif scenario == "recover": module.begin_drain() status.payload["pid"] = 77 @@ -291,6 +326,9 @@ describe("Hermes in-sandbox cron restore validator", () => { | "complete" | "complete-same-identity" | "complete-validation-failure" + | "complete-substitution" + | "complete-release-substitution" + | "complete-release-failure" | "recover" | "recover-operator" | "recover-noop", @@ -529,7 +567,12 @@ describe("Hermes in-sandbox cron restore validator", () => { .split("\n") .filter((line) => line.startsWith(RECEIPT_PREFIX)) .map((line) => JSON.parse(line.slice(RECEIPT_PREFIX.length))); - expect(receipts.map((receipt) => receipt.action)).toEqual(["begin", "validate", "complete"]); + expect(receipts.map((receipt) => receipt.action)).toEqual([ + "begin", + "validate", + "observe", + "complete", + ]); expect(receipts.at(-1)).toEqual( expect.objectContaining({ active_jobs: 1, @@ -559,6 +602,30 @@ describe("Hermes in-sandbox cron restore validator", () => { expect(result.stdout).toContain("OWN_MARKER:present"); }); + it("keeps dispatch drained when the health-bound replacement is substituted (#8472)", () => { + const result = runLifecycle("complete-substitution"); + + expect(result.status).toBe(1); + expect(result.stderr).toContain("gateway identity changed during cron restore"); + expect(result.stdout).toContain("OWN_MARKER:present"); + }); + + it("keeps the drain marker when substitution races final release (#8472)", () => { + const result = runLifecycle("complete-release-substitution"); + + expect(result.status).toBe(1); + expect(result.stderr).toContain("gateway identity changed during cron restore"); + expect(result.stdout).toContain("OWN_MARKER:present"); + }); + + it("restores the drain marker when replacement release verification fails (#8472)", () => { + const result = runLifecycle("complete-release-failure"); + + expect(result.status).toBe(1); + expect(result.stderr).toContain("simulated replacement release failure"); + expect(result.stdout).toContain("OWN_MARKER:present"); + }); + it("re-pins a restarted gateway before validating and reactivating dispatch", () => { const result = runLifecycle("recover"); From 2f7c35b49cef523ac9c2afc39bb1417eddb0fba1 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Fri, 7 Aug 2026 09:36:58 -0700 Subject: [PATCH 06/11] fix(rebuild): order held-gate recovery guidance Signed-off-by: Apurv Kumaria --- .../rebuild-post-restore-phase.test.ts | 33 +++++++++++++++++++ .../sandbox/rebuild-post-restore-phase.ts | 1 + 2 files changed, 34 insertions(+) diff --git a/src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts b/src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts index ee86bc3b67a..594ee17b7e8 100644 --- a/src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts +++ b/src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts @@ -283,6 +283,39 @@ describe("rebuild post-restore phase", () => { ); }); + it("repairs MCP before cron recovery when gateway verification also fails (#8472)", async () => { + agentName = "hermes"; + vi.mocked(rebuildMcp.restoreMcpAfterRebuild).mockResolvedValue(false); + vi.mocked( + rebuildHermesPostRestore.ensureHermesGatewayAfterStateRestoreForCronGate, + ).mockReturnValue({ state: "unverified" }); + const args = { + ...input(), + hermesCronRestoreIdentity: { + pid: 41, + start_time: 902, + drain_token: "restore-token", + }, + }; + + await runRebuildPostRestorePhase(args); + + expect( + rebuildHermesPostRestore.completeHermesCronRestoreAfterGatewayReplacement, + ).not.toHaveBeenCalled(); + const mcpCall = vi + .mocked(console.log) + .mock.calls.findIndex((call) => String(call[0]).includes("nemoclaw alpha mcp restart")); + const recoverCall = vi + .mocked(console.error) + .mock.calls.findIndex((call) => String(call[0]).includes("nemoclaw alpha recover")); + expect(mcpCall).toBeGreaterThanOrEqual(0); + expect(recoverCall).toBeGreaterThanOrEqual(0); + expect(vi.mocked(console.log).mock.invocationCallOrder[mcpCall]).toBeLessThan( + vi.mocked(console.error).mock.invocationCallOrder[recoverCall] ?? 0, + ); + }); + it("discloses carried-over baseline exclusions in the successful rebuild summary (#7194)", async () => { const logSpy = vi.spyOn(console, "log").mockImplementation(() => undefined); vi.spyOn(registry, "getBaselineExclusions").mockReturnValue([ diff --git a/src/lib/actions/sandbox/rebuild-post-restore-phase.ts b/src/lib/actions/sandbox/rebuild-post-restore-phase.ts index 3e2e50c3950..d60a6143965 100644 --- a/src/lib/actions/sandbox/rebuild-post-restore-phase.ts +++ b/src/lib/actions/sandbox/rebuild-post-restore-phase.ts @@ -308,6 +308,7 @@ export async function runRebuildPostRestorePhase( " Hermes cron dispatch remains drained because the replacement gateway was not verified.", "Hermes cron restore validation failed; dispatch was not re-enabled.", bail, + mcpBridgeRestoreUnverified ? () => printMcpRestoreRecovery(sandboxName, true) : undefined, ); } if (mcpBridgeRestoreUnverified) { From 54a3b19562bd2e3b4fedfaae43cb8c928b9a0e4b Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Fri, 7 Aug 2026 10:08:25 -0700 Subject: [PATCH 07/11] fix(rebuild): report cron gate rollback uncertainty Signed-off-by: Apurv Kumaria --- .../recover-rebuild-sandboxes.mdx | 4 ++- .../rebuild-post-restore-phase.test.ts | 30 +++++++++++++++++++ .../sandbox/rebuild-post-restore-phase.ts | 22 ++++++++++---- 3 files changed, 49 insertions(+), 7 deletions(-) diff --git a/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx b/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx index 0772db1b647..79c2abe5ee3 100644 --- a/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx +++ b/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx @@ -238,7 +238,9 @@ NemoClaw waits for active agent work to finish before restoring state. It validates the restored jobs and scripts before the gateway replacement, then keeps dispatch blocked while it restarts and verifies that replacement. It records the replacement process identity around managed health verification and clears the gate only if that same live process completes the final cron validation. If an operator already drained the gateway, NemoClaw clears only its gate and leaves the operator drain active. -If state restore, managed MCP restoration, gateway replacement, or cron validation fails after gate acquisition, the command exits nonzero, preserves the backup, and retains the NemoClaw gate. +If state restore, managed MCP restoration, gateway replacement, or cron validation fails after gate acquisition, the command exits nonzero and preserves the backup. +Those failures retain the NemoClaw gate unless the output explicitly reports that release rollback could not restore its marker. +In that exceptional case, do not assume dispatch is blocked; run `$$nemoclaw recover` immediately. Failures before gate acquisition do not create a new gate. Do not manually remove the root-owned cron restore marker because removal bypasses restored cron validation. If managed MCP restoration failed, correct the reported cause and run `$$nemoclaw mcp restart` first. diff --git a/src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts b/src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts index 594ee17b7e8..79aef1f1a41 100644 --- a/src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts +++ b/src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts @@ -250,6 +250,36 @@ describe("rebuild post-restore phase", () => { expect(output).toContain("nemoclaw alpha recover"); }); + it("reports unverified dispatch state when release marker rollback fails (#8472)", async () => { + agentName = "hermes"; + vi.mocked( + rebuildHermesPostRestore.completeHermesCronRestoreAfterGatewayReplacement, + ).mockImplementation(() => { + throw new Error( + "Hermes cron complete failed: Hermes cron restore drain release failed and its marker could not be restored", + ); + }); + const args = { + ...input(), + hermesCronRestoreIdentity: { + pid: 41, + start_time: 902, + drain_token: "restore-token", + }, + }; + + await runRebuildPostRestorePhase(args); + + expect(args.bail).toHaveBeenCalledWith( + "Hermes cron restore gate state is unverified after release rollback failure; recover immediately.", + ); + const output = vi.mocked(console.error).mock.calls.flat().join("\n"); + expect(output).toContain("drain release failed and its marker could not be restored"); + expect(output).toContain("Dispatch gate state is unverified; run recovery immediately"); + expect(output).toContain("nemoclaw alpha recover"); + expect(output).not.toContain("dispatch was not re-enabled"); + }); + it("keeps the gate active and repairs MCP before cron recovery (#8472)", async () => { agentName = "hermes"; vi.mocked(rebuildMcp.restoreMcpAfterRebuild).mockResolvedValue(false); diff --git a/src/lib/actions/sandbox/rebuild-post-restore-phase.ts b/src/lib/actions/sandbox/rebuild-post-restore-phase.ts index d60a6143965..189f22788ce 100644 --- a/src/lib/actions/sandbox/rebuild-post-restore-phase.ts +++ b/src/lib/actions/sandbox/rebuild-post-restore-phase.ts @@ -49,7 +49,7 @@ export function printHermesCronRestoreRecoveryCommand( ); } -function bailWithHeldHermesCronRestore( +function bailAfterHermesCronRestoreFailure( sandboxName: string, backupManifest: RebuildBackupManifest, detail: string, @@ -206,7 +206,7 @@ export async function runRebuildPostRestorePhase( ` ${YW}\u26a0${R} Recreated sandbox agent identity could not be verified against the rebuild target.`, ); if (hermesCronRestoreIdentity) { - return bailWithHeldHermesCronRestore( + return bailAfterHermesCronRestoreFailure( sandboxName, backupManifest, " Hermes cron dispatch remains drained because the replacement identity is unverified.", @@ -302,7 +302,7 @@ export async function runRebuildPostRestorePhase( hermesGatewayRestoreState === "not-applicable" || !replacementIdentity ) { - return bailWithHeldHermesCronRestore( + return bailAfterHermesCronRestoreFailure( sandboxName, backupManifest, " Hermes cron dispatch remains drained because the replacement gateway was not verified.", @@ -312,7 +312,7 @@ export async function runRebuildPostRestorePhase( ); } if (mcpBridgeRestoreUnverified) { - return bailWithHeldHermesCronRestore( + return bailAfterHermesCronRestoreFailure( sandboxName, backupManifest, " Hermes cron dispatch remains drained because managed MCP restoration was not verified.", @@ -329,10 +329,20 @@ export async function runRebuildPostRestorePhase( replacementIdentity, ); } catch (error) { - return bailWithHeldHermesCronRestore( + const errorDetail = error instanceof Error ? error.message : String(error); + if (errorDetail.includes("drain release failed and its marker could not be restored")) { + return bailAfterHermesCronRestoreFailure( + sandboxName, + backupManifest, + ` Hermes cron restore release rollback failed: ${errorDetail}. Dispatch gate state is unverified; run recovery immediately.`, + "Hermes cron restore gate state is unverified after release rollback failure; recover immediately.", + bail, + ); + } + return bailAfterHermesCronRestoreFailure( sandboxName, backupManifest, - ` Hermes cron restore could not validate the replacement gateway and reactivate dispatch: ${error instanceof Error ? error.message : String(error)}`, + ` Hermes cron restore could not validate the replacement gateway and reactivate dispatch: ${errorDetail}`, "Hermes cron restore validation failed; dispatch was not re-enabled.", bail, ); From 867cbe43ac5694ad2bfd5c689a03bca0b59c662f Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Fri, 7 Aug 2026 10:28:59 -0700 Subject: [PATCH 08/11] fix(hermes): refresh cron controller integrity hash Signed-off-by: Apurv Kumaria --- agents/hermes/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agents/hermes/Dockerfile b/agents/hermes/Dockerfile index 1c9ab27753c..439978cde56 100644 --- a/agents/hermes/Dockerfile +++ b/agents/hermes/Dockerfile @@ -569,7 +569,7 @@ ARG NEMOCLAW_HERMES_CLI_ADAPTER_SHA256=989edf54a8c09c6efb348600a8aa2f264c0b71408 ARG NEMOCLAW_HERMES_CLI_ADAPTER_VALIDATOR_SHA256=db4046e79e513eab67b069a8eda20167b8b65529cf26842531d2ad673c670330 ARG NEMOCLAW_HERMES_VALIDATOR_SHA256=822c7e63d068c5d09f3291350771c1a42c9686f51bfa9bc9a1f41fbe15d163b1 ARG NEMOCLAW_HERMES_TIRITH_FINALIZER_SHA256=a1e6b1c53ab297569abb87c29d15c294d729e46005bfd022136b4c447a791819 -ARG NEMOCLAW_HERMES_CRON_RESTORE_CONTROLLER_SHA256=a674de12d1c30d907491aa7c7b5d40711b077c5b2eca69ca58c5c36e8231e4c8 +ARG NEMOCLAW_HERMES_CRON_RESTORE_CONTROLLER_SHA256=5842aff55ffa03bb3a36dc24f9f02f4d4c44993011ba198e3dbacdb8f8fdbe87 # hadolint ignore=DL4006 RUN printf '%s %s\n' \ "$NEMOCLAW_HERMES_VALIDATOR_SHA256" /usr/local/lib/nemoclaw/validate-hermes-env-secret-boundary.py \ From 3e6ca2610749a0840bb514de39a38665c61c6289 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Fri, 7 Aug 2026 11:12:27 -0700 Subject: [PATCH 09/11] fix(rebuild): type cron rollback failures Signed-off-by: Apurv Kumaria --- agents/hermes/Dockerfile | 2 +- agents/hermes/cron-restore-control.py | 45 +++++----- .../rebuild-hermes-cron-restore.test.ts | 84 +++++++++-------- .../rebuild-hermes-post-restore.test.ts | 24 +++++ .../sandbox/rebuild-hermes-post-restore.ts | 90 +++++++++++-------- .../rebuild-post-restore-phase.test.ts | 17 +++- .../sandbox/rebuild-post-restore-phase.ts | 3 +- test/e2e/live/rebuild-hermes-cron-restore.ts | 6 +- test/hermes-cron-restore-control.test.ts | 60 ++++++++++--- 9 files changed, 221 insertions(+), 110 deletions(-) diff --git a/agents/hermes/Dockerfile b/agents/hermes/Dockerfile index 439978cde56..7564394d937 100644 --- a/agents/hermes/Dockerfile +++ b/agents/hermes/Dockerfile @@ -569,7 +569,7 @@ ARG NEMOCLAW_HERMES_CLI_ADAPTER_SHA256=989edf54a8c09c6efb348600a8aa2f264c0b71408 ARG NEMOCLAW_HERMES_CLI_ADAPTER_VALIDATOR_SHA256=db4046e79e513eab67b069a8eda20167b8b65529cf26842531d2ad673c670330 ARG NEMOCLAW_HERMES_VALIDATOR_SHA256=822c7e63d068c5d09f3291350771c1a42c9686f51bfa9bc9a1f41fbe15d163b1 ARG NEMOCLAW_HERMES_TIRITH_FINALIZER_SHA256=a1e6b1c53ab297569abb87c29d15c294d729e46005bfd022136b4c447a791819 -ARG NEMOCLAW_HERMES_CRON_RESTORE_CONTROLLER_SHA256=5842aff55ffa03bb3a36dc24f9f02f4d4c44993011ba198e3dbacdb8f8fdbe87 +ARG NEMOCLAW_HERMES_CRON_RESTORE_CONTROLLER_SHA256=f24a4dc187428530bfec2f95140fbe2eff59d9dd78d8edac352bcdc6b6a82586 # hadolint ignore=DL4006 RUN printf '%s %s\n' \ "$NEMOCLAW_HERMES_VALIDATOR_SHA256" /usr/local/lib/nemoclaw/validate-hermes-env-secret-boundary.py \ diff --git a/agents/hermes/cron-restore-control.py b/agents/hermes/cron-restore-control.py index cd5dbd643c0..6d399a8b705 100644 --- a/agents/hermes/cron-restore-control.py +++ b/agents/hermes/cron-restore-control.py @@ -33,6 +33,9 @@ CONTROL_LOCK_PATH = Path("/run/nemoclaw/hermes-cron-restore-control.lock") CONTROL_MARKER_NAME = "hermes-cron-restore-drain.json" RECEIPT_PREFIX = "NEMOCLAW_HERMES_CRON_RESTORE_V1:" +CONTROL_ERROR_PREFIX = "NEMOCLAW_HERMES_CRON_RESTORE_ERROR_V1:" +CONTROL_ERROR_CODE = "control-failure" +DRAIN_MARKER_ROLLBACK_FAILED_CODE = "drain-marker-rollback-failed" BEGIN_TIMEOUT_SECONDS = 60.0 RELEASE_TIMEOUT_SECONDS = 15.0 POLL_SECONDS = 0.1 @@ -45,6 +48,24 @@ class ControlError(RuntimeError): """Expected fail-closed control or validation error.""" + def __init__(self, message: str, *, code: str = CONTROL_ERROR_CODE) -> None: + super().__init__(message) + self.code = code + + +def _emit_control_error(error: ControlError) -> None: + """Write the stable control signal after the existing human-readable error.""" + print(f"HERMES_CRON_RESTORE_ERROR: {error}", file=sys.stderr) + print( + CONTROL_ERROR_PREFIX + + json.dumps( + {"code": error.code, "message": str(error)}, + separators=(",", ":"), + sort_keys=True, + ), + file=sys.stderr, + ) + def _marker_path() -> Path: return NEMOCLAW_HOME / CONTROL_MARKER_NAME @@ -496,7 +517,8 @@ def _complete_release( _write_owned_drain(drain_token) except ControlError as rollback_error: raise ControlError( - "Hermes cron restore drain release failed and its marker could not be restored" + "Hermes cron restore drain release failed and its marker could not be restored", + code=DRAIN_MARKER_ROLLBACK_FAILED_CODE, ) from rollback_error if isinstance(release_error, ControlError): raise release_error @@ -557,21 +579,6 @@ def validate_restore(pid: int, start_time: int, drain_token: str) -> None: ) -def release_drain(pid: int, start_time: int, drain_token: str) -> None: - with _control_lock(): - drain_control, status_module = _load_gateway_modules() - _require_owned_drain(drain_token) - _require_drained_idle(status_module, pid, start_time) - _complete_release( - "release", - drain_control, - status_module, - pid=pid, - start_time=start_time, - drain_token=drain_token, - ) - - def observe_replacement(pid: int, start_time: int, drain_token: str) -> None: with _control_lock(): drain_control, status_module = _load_gateway_modules() @@ -677,7 +684,7 @@ def _parser() -> argparse.ArgumentParser: subparsers = parser.add_subparsers(dest="action", required=True) subparsers.add_parser("begin") subparsers.add_parser("recover") - for action in ("validate", "observe", "complete", "release"): + for action in ("validate", "observe", "complete"): subparser = subparsers.add_parser(action) subparser.add_argument("--pid", required=True, type=int) subparser.add_argument("--start-time", required=True, type=int) @@ -712,13 +719,11 @@ def main() -> int: args.replacement_start_time, args.drain_token, ) - elif args.action == "release": - release_drain(args.pid, args.start_time, args.drain_token) else: counts = validate_cron_tree(args.home, args.sandbox_home) print(json.dumps(counts, separators=(",", ":"), sort_keys=True)) except ControlError as error: - print(f"HERMES_CRON_RESTORE_ERROR: {error}", file=sys.stderr) + _emit_control_error(error) return 1 return 0 diff --git a/src/lib/actions/sandbox/rebuild-hermes-cron-restore.test.ts b/src/lib/actions/sandbox/rebuild-hermes-cron-restore.test.ts index 625017fe95d..36e1a05792c 100644 --- a/src/lib/actions/sandbox/rebuild-hermes-cron-restore.test.ts +++ b/src/lib/actions/sandbox/rebuild-hermes-cron-restore.test.ts @@ -30,14 +30,16 @@ vi.mock("../../sandbox/privileged-exec", async (importOriginal) => ({ import { beginHermesCronRestore, completeHermesCronRestoreAfterGatewayReplacement, + HERMES_CRON_RESTORE_DRAIN_MARKER_ROLLBACK_FAILED_CODE, + isHermesCronRestoreDrainMarkerRollbackFailure, observeHermesCronReplacement, recoverHermesCronRestore, - releaseHermesCronRestore, runHermesCronRestoreTransaction, validateHermesCronRestore, } from "./rebuild-hermes-post-restore"; const RECEIPT_PREFIX = "NEMOCLAW_HERMES_CRON_RESTORE_V1:"; +const CONTROL_ERROR_PREFIX = "NEMOCLAW_HERMES_CRON_RESTORE_ERROR_V1:"; function writeJson(target: string, payload: unknown): void { mkdirSync(path.dirname(target), { recursive: true }); @@ -49,7 +51,7 @@ function writeScript(target: string): void { writeFileSync(target, "print('ok')\n", { mode: 0o600 }); } -type ReceiptAction = "begin" | "validate" | "observe" | "complete" | "release" | "recover"; +type ReceiptAction = "begin" | "validate" | "observe" | "complete" | "recover"; function receipt( action: ReceiptAction, @@ -85,12 +87,6 @@ function receipt( profiles: 1, script_jobs: 1, }, - release: { - active_agents: 0, - disposition: "dispatch-reactivated", - operator_drain_active: false, - preserved_drain: false, - }, recover: { active_agents: 0, active_jobs: 1, @@ -113,6 +109,20 @@ function receipt( })}`; } +function completionFailure(stderr: string): unknown { + processMocks.dockerSpawnSync.mockReturnValue({ status: 1, stdout: "", stderr }); + try { + completeHermesCronRestoreAfterGatewayReplacement( + "alpha", + { pid: 41, start_time: 902, drain_token: "restore-token" }, + { pid: 77, start_time: 903, drain_token: "restore-token" }, + ); + } catch (error) { + return error; + } + throw new Error("Hermes cron completion unexpectedly succeeded"); +} + function notRequiredRecoveryReceipt(overrides: Record = {}): string { return `${RECEIPT_PREFIX}${JSON.stringify({ version: 1, @@ -190,22 +200,17 @@ describe("Hermes cron rebuild restore contract", () => { ); }); - it("binds validation and release to the begin receipt identity", () => { + it("binds validation to the begin receipt identity", () => { processMocks.dockerSpawnSync.mockImplementation((argv: string[]) => { - const action = argv.includes("validate") - ? "validate" - : argv.includes("release") - ? "release" - : "begin"; + const action = argv.includes("validate") ? "validate" : "begin"; return { status: 0, stdout: receipt(action), stderr: "" }; }); const identity = beginHermesCronRestore("alpha"); validateHermesCronRestore("alpha", identity); - releaseHermesCronRestore("alpha", identity); expect(identity).toEqual({ pid: 41, start_time: 902, drain_token: "restore-token" }); - expect(processMocks.privilegedSandboxExecArgv).toHaveBeenCalledTimes(3); + expect(processMocks.privilegedSandboxExecArgv).toHaveBeenCalledTimes(2); expect(processMocks.privilegedSandboxExecArgv.mock.calls[1]?.[1]).toEqual([ "/opt/hermes/.venv/bin/python", "-I", @@ -218,7 +223,6 @@ describe("Hermes cron rebuild restore contract", () => { "--drain-token", "restore-token", ]); - expect(processMocks.privilegedSandboxExecArgv.mock.calls[2]?.[1]).toContain("release"); }); it("passes an untrusted drain token as one argv value", () => { @@ -236,18 +240,6 @@ describe("Hermes cron rebuild restore contract", () => { expect(validateArgv?.at(-1)).toBe(untrustedToken); }); - it("rejects a control receipt that changes gateway identity", () => { - processMocks.dockerSpawnSync.mockReturnValue({ - status: 0, - stdout: receipt("release", 42, 902), - stderr: "", - }); - - expect(() => releaseHermesCronRestore("alpha", { pid: 41, start_time: 902 })).toThrow( - "changed gateway identity", - ); - }); - it("keeps dispatch drained when state restore is incomplete", () => { processMocks.dockerSpawnSync.mockReturnValue({ status: 0, @@ -265,11 +257,7 @@ describe("Hermes cron rebuild restore contract", () => { it("keeps dispatch held after restore validation until gateway replacement (#8472)", () => { const events: string[] = []; processMocks.dockerSpawnSync.mockImplementation((argv: string[]) => { - const action = argv.includes("validate") - ? "validate" - : argv.includes("release") - ? "release" - : "begin"; + const action = argv.includes("validate") ? "validate" : "begin"; events.push(action); return { status: 0, stdout: receipt(action), stderr: "" }; }); @@ -365,7 +353,7 @@ describe("Hermes cron rebuild restore contract", () => { expect(processMocks.dockerSpawnSync).not.toHaveBeenCalled(); }); - it("rejects a replacement observation with a different drain token (#8472)", () => { + it("rejects completion when the replacement carries a different drain token (#8472)", () => { expect(() => completeHermesCronRestoreAfterGatewayReplacement( "alpha", @@ -376,6 +364,32 @@ describe("Hermes cron rebuild restore contract", () => { expect(processMocks.dockerSpawnSync).not.toHaveBeenCalled(); }); + it("classifies the structured drain-marker rollback failure (#8472)", () => { + const message = "Hermes cron restore drain release failed and its marker could not be restored"; + const failure = completionFailure( + [ + `HERMES_CRON_RESTORE_ERROR: ${message}`, + `${CONTROL_ERROR_PREFIX}${JSON.stringify({ + code: HERMES_CRON_RESTORE_DRAIN_MARKER_ROLLBACK_FAILED_CODE, + message, + })}`, + ].join("\n"), + ); + + expect(isHermesCronRestoreDrainMarkerRollbackFailure(failure)).toBe(true); + expect(failure).toBeInstanceOf(Error); + expect((failure as Error).message).toContain(message); + }); + + it("does not classify matching prose without the structured failure code (#8472)", () => { + const message = "Hermes cron restore drain release failed and its marker could not be restored"; + const failure = completionFailure(`HERMES_CRON_RESTORE_ERROR: ${message}`); + + expect(isHermesCronRestoreDrainMarkerRollbackFailure(failure)).toBe(false); + expect(failure).toBeInstanceOf(Error); + expect((failure as Error).message).toContain(message); + }); + it("rejects completion while replacement agents are still active (#8472)", () => { processMocks.dockerSpawnSync.mockReturnValue({ status: 0, diff --git a/src/lib/actions/sandbox/rebuild-hermes-post-restore.test.ts b/src/lib/actions/sandbox/rebuild-hermes-post-restore.test.ts index 570259807d0..1a0176cedd9 100644 --- a/src/lib/actions/sandbox/rebuild-hermes-post-restore.test.ts +++ b/src/lib/actions/sandbox/rebuild-hermes-post-restore.test.ts @@ -116,6 +116,30 @@ describe("binding the Hermes gateway to restored state", () => { expect(order).toEqual(["restart", "observe", "health", "observe"]); }); + it("binds a recovered cron-gated gateway to its observed replacement identity (#8472)", () => { + const replacement = { pid: 77, start_time: 903, drain_token: "restore-token" }; + const observeHermesCronReplacement = vi.fn(() => replacement); + const checkAndRecoverSandboxProcesses = vi + .fn() + .mockReturnValueOnce({ checked: true, wasRunning: false, recovered: true }) + .mockReturnValueOnce({ checked: true, wasRunning: true, recovered: false }); + + expect( + ensureHermesGatewayAfterStateRestoreForCronGate( + "alpha", + "hermes", + { pid: 41, start_time: 902, drain_token: "restore-token" }, + { + restartSandboxGateway: () => RESTART_FAILED, + observeHermesCronReplacement, + checkAndRecoverSandboxProcesses, + }, + ), + ).toEqual({ state: "recovered", replacementIdentity: replacement }); + expect(checkAndRecoverSandboxProcesses).toHaveBeenCalledTimes(2); + expect(observeHermesCronReplacement).toHaveBeenCalledTimes(3); + }); + it("fails closed when another gateway replaces the process during health verification (#8472)", () => { const observeHermesCronReplacement = vi .fn() diff --git a/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts b/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts index 92c44940511..2725fb88587 100644 --- a/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts +++ b/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts @@ -9,18 +9,14 @@ import * as processRecovery from "./process-recovery"; const HERMES_CRON_CONTROL = "/usr/local/lib/nemoclaw/hermes-cron-restore-control.py"; const HERMES_PYTHON = "/opt/hermes/.venv/bin/python"; const RECEIPT_PREFIX = "NEMOCLAW_HERMES_CRON_RESTORE_V1:"; +const CONTROL_ERROR_PREFIX = "NEMOCLAW_HERMES_CRON_RESTORE_ERROR_V1:"; +export const HERMES_CRON_RESTORE_DRAIN_MARKER_ROLLBACK_FAILED_CODE = "drain-marker-rollback-failed"; const BEGIN_TIMEOUT_MS = 70_000; const CONTROL_TIMEOUT_MS = 25_000; const RECOVERY_TIMEOUT_MS = BEGIN_TIMEOUT_MS + CONTROL_TIMEOUT_MS * 2 + 10_000; const HERMES_GATEWAY_RECHECK_ATTEMPTS = 2; -type HermesCronRestoreAction = - | "begin" - | "validate" - | "observe" - | "complete" - | "release" - | "recover"; +type HermesCronRestoreAction = "begin" | "validate" | "observe" | "complete" | "recover"; type HermesCronRestoreDisposition = | "drain-acquired" | "restore-validated" @@ -317,18 +313,6 @@ function parseCronRestoreReceipt( receipt.active_agents === 0 && hasExactReceiptFields(receipt, [...baseFields, ...tokenFields, "active_agents"]); break; - case "release": - actionValid = - receipt.drain_acquired === true && - receipt.active_agents === 0 && - isReleaseDispositionValid(receipt) && - hasExactReceiptFields(receipt, [ - ...baseFields, - ...tokenFields, - "active_agents", - "preserved_drain", - ]); - break; case "complete": actionValid = receipt.drain_acquired === true && @@ -379,17 +363,61 @@ function parseCronRestoreReceipt( return receipt as unknown as HermesCronRestoreReceipt; } -class HermesCronRestoreControlFailure extends Error { - constructor( - action: HermesCronRestoreAction, - readonly stderr: string, +function parseCronRestoreControlError(stderr: string): { code: string; message: string } | null { + const signalLines = stderr + .split(/\r?\n/u) + .filter((line) => line.startsWith(CONTROL_ERROR_PREFIX)); + if (signalLines.length !== 1) return null; + let payload: unknown; + try { + payload = JSON.parse(signalLines[0].slice(CONTROL_ERROR_PREFIX.length)); + } catch { + return null; + } + if (payload === null || typeof payload !== "object" || Array.isArray(payload)) return null; + const signal = payload as Record; + if ( + !hasExactReceiptFields(signal, ["code", "message"]) || + typeof signal.code !== "string" || + signal.code.length === 0 || + typeof signal.message !== "string" || + signal.message.length === 0 ) { - const detail = stderr.trim().split(/\r?\n/u).at(-1); + return null; + } + return { code: signal.code, message: signal.message }; +} + +class HermesCronRestoreControlFailure extends Error { + readonly action: HermesCronRestoreAction; + readonly stderr: string; + readonly controlCode?: string; + + constructor(action: HermesCronRestoreAction, stderr: string) { + const controlError = parseCronRestoreControlError(stderr); + const detail = + controlError?.message ?? + stderr + .trim() + .split(/\r?\n/u) + .filter((line) => !line.startsWith(CONTROL_ERROR_PREFIX)) + .at(-1); super(`Hermes cron ${action} failed${detail ? `: ${detail}` : ""}`); this.name = "HermesCronRestoreControlFailure"; + this.action = action; + this.stderr = stderr; + this.controlCode = controlError?.code; } } +export function isHermesCronRestoreDrainMarkerRollbackFailure(error: unknown): boolean { + return ( + error instanceof HermesCronRestoreControlFailure && + error.action === "complete" && + error.controlCode === HERMES_CRON_RESTORE_DRAIN_MARKER_ROLLBACK_FAILED_CODE + ); +} + function runCronRestoreControl( sandboxName: string, action: HermesCronRestoreAction, @@ -458,20 +486,6 @@ export function validateHermesCronRestore( } } -export function releaseHermesCronRestore( - sandboxName: string, - identity: HermesCronRestoreIdentity, -): void { - const receipt = runCronRestoreControl(sandboxName, "release", identity); - if ( - receipt.pid !== identity.pid || - receipt.start_time !== identity.start_time || - receipt.drain_token !== identity.drain_token - ) { - throw new Error("Hermes cron release receipt changed gateway identity"); - } -} - export function completeHermesCronRestoreAfterGatewayReplacement( sandboxName: string, originalIdentity: HermesCronRestoreIdentity, diff --git a/src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts b/src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts index 79aef1f1a41..93a2872bedd 100644 --- a/src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts +++ b/src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts @@ -73,6 +73,10 @@ describe("rebuild post-restore phase", () => { rebuildHermesPostRestore, "completeHermesCronRestoreAfterGatewayReplacement", ).mockReturnValue({ pid: 77, start_time: 903, drain_token: "restore-token" }); + vi.spyOn( + rebuildHermesPostRestore, + "isHermesCronRestoreDrainMarkerRollbackFailure", + ).mockReturnValue(false); vi.spyOn(registry, "getSandbox").mockImplementation( () => ({ agent: agentName === "openclaw" ? null : agentName }) as never, ); @@ -252,13 +256,17 @@ describe("rebuild post-restore phase", () => { it("reports unverified dispatch state when release marker rollback fails (#8472)", async () => { agentName = "hermes"; + const rollbackFailure = new Error( + "Hermes cron complete failed: Hermes cron restore drain release failed and its marker could not be restored", + ); vi.mocked( rebuildHermesPostRestore.completeHermesCronRestoreAfterGatewayReplacement, ).mockImplementation(() => { - throw new Error( - "Hermes cron complete failed: Hermes cron restore drain release failed and its marker could not be restored", - ); + throw rollbackFailure; }); + vi.mocked( + rebuildHermesPostRestore.isHermesCronRestoreDrainMarkerRollbackFailure, + ).mockImplementation((error) => error === rollbackFailure); const args = { ...input(), hermesCronRestoreIdentity: { @@ -278,6 +286,9 @@ describe("rebuild post-restore phase", () => { expect(output).toContain("Dispatch gate state is unverified; run recovery immediately"); expect(output).toContain("nemoclaw alpha recover"); expect(output).not.toContain("dispatch was not re-enabled"); + expect( + rebuildHermesPostRestore.isHermesCronRestoreDrainMarkerRollbackFailure, + ).toHaveBeenCalledWith(rollbackFailure); }); it("keeps the gate active and repairs MCP before cron recovery (#8472)", async () => { diff --git a/src/lib/actions/sandbox/rebuild-post-restore-phase.ts b/src/lib/actions/sandbox/rebuild-post-restore-phase.ts index 189f22788ce..074ae84acfd 100644 --- a/src/lib/actions/sandbox/rebuild-post-restore-phase.ts +++ b/src/lib/actions/sandbox/rebuild-post-restore-phase.ts @@ -22,6 +22,7 @@ import { ensureHermesGatewayAfterStateRestore, ensureHermesGatewayAfterStateRestoreForCronGate, type HermesCronRestoreIdentity, + isHermesCronRestoreDrainMarkerRollbackFailure, printHermesGatewayRestoreRecovery, } from "./rebuild-hermes-post-restore"; import { @@ -330,7 +331,7 @@ export async function runRebuildPostRestorePhase( ); } catch (error) { const errorDetail = error instanceof Error ? error.message : String(error); - if (errorDetail.includes("drain release failed and its marker could not be restored")) { + if (isHermesCronRestoreDrainMarkerRollbackFailure(error)) { return bailAfterHermesCronRestoreFailure( sandboxName, backupManifest, diff --git a/test/e2e/live/rebuild-hermes-cron-restore.ts b/test/e2e/live/rebuild-hermes-cron-restore.ts index cbf255ec235..d5472be24d8 100644 --- a/test/e2e/live/rebuild-hermes-cron-restore.ts +++ b/test/e2e/live/rebuild-hermes-cron-restore.ts @@ -602,7 +602,11 @@ export function createRebuildHermesCronRestoreFixture({ expectExitZero(restoredScript, "read restored Hermes cron script"); expect(restoredScript.stdout).toBe(seed.scriptContent); await assertControlMarker(false, "phase-7-verify-cron-restore-marker-released"); - await waitForGatewayState("running", "phase-7-verify-gateway-running-after-cron-restore"); + const liveGateway = await waitForGatewayState( + "running", + "phase-7-verify-gateway-running-after-cron-restore", + ); + expect(released?.slice(1)).toEqual([String(liveGateway.pid), String(liveGateway.start_time)]); await assertExecutionMarkerAbsent(seed, "phase-7-verify-restored-cron-not-auto-executed"); await runCronNow(seed, "phase-7-run-restored-hermes-cron-job"); diff --git a/test/hermes-cron-restore-control.test.ts b/test/hermes-cron-restore-control.test.ts index bae87eb2db1..5c3aa28289f 100644 --- a/test/hermes-cron-restore-control.test.ts +++ b/test/hermes-cron-restore-control.test.ts @@ -16,11 +16,13 @@ import { import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { HERMES_CRON_RESTORE_DRAIN_MARKER_ROLLBACK_FAILED_CODE } from "../src/lib/actions/sandbox/rebuild-hermes-post-restore"; import { validateHermesCronRestoreBackup } from "../src/lib/state/rebuild/hermes-cron-restore-backup"; const HELPER = path.resolve("agents/hermes/cron-restore-control.py"); const HOST_VALIDATOR = path.resolve("src/lib/state/rebuild/hermes-cron-restore-backup.ts"); const RECEIPT_PREFIX = "NEMOCLAW_HERMES_CRON_RESTORE_V1:"; +const CONTROL_ERROR_PREFIX = "NEMOCLAW_HERMES_CRON_RESTORE_ERROR_V1:"; const LIFECYCLE_HARNESS = String.raw` import importlib.util import os @@ -116,24 +118,26 @@ try: if scenario == "success": token = module.begin_drain() module.validate_restore(41, 902, token) - module.release_drain(41, 902, token) + module.recover_drain() elif scenario == "wrong-identity": token = module.begin_drain() module.validate_restore(42, 902, token) elif scenario == "missing-marker": token = module.begin_drain() module._marker_path().unlink() - module.release_drain(41, 902, token) + status.payload["pid"] = 77 + status.payload["start_time"] = 903 + module.complete_replacement(41, 902, 77, 903, token) elif scenario == "preserve-operator": drain.marker = {"principal": "operator"} token = module.begin_drain() module.validate_restore(41, 902, token) - module.release_drain(41, 902, token) + module.recover_drain() elif scenario == "concurrent-operator": token = module.begin_drain() module.validate_restore(41, 902, token) drain.marker = {"principal": "operator"} - module.release_drain(41, 902, token) + module.recover_drain() elif scenario == "existing-owned-marker": marker = module._marker_path() marker.write_text( @@ -153,13 +157,13 @@ try: held = module.NEMOCLAW_HOME / "held-marker.json" marker.rename(held) marker.symlink_to(held.name) - module.release_drain(41, 902, token) + module.recover_drain() elif scenario == "hardlinked-owned-marker": token = module.begin_drain() marker = module._marker_path() held = module.NEMOCLAW_HOME / "held-marker.json" os.link(marker, held) - module.release_drain(41, 902, token) + module.recover_drain() elif scenario == "unsafe-lock-metadata": module.CONTROL_LOCK_PATH.write_text("unsafe", encoding="utf-8") os.chmod(module.CONTROL_LOCK_PATH, 0o644) @@ -173,7 +177,9 @@ try: encoding="utf-8", ) os.chmod(marker, 0o400) - module.release_drain(41, 902, token) + status.payload["pid"] = 77 + status.payload["start_time"] = 903 + module.complete_replacement(41, 902, 77, 903, token) elif scenario == "rollback-operator": token = module.begin_drain() module.validate_restore(41, 902, token) @@ -181,7 +187,7 @@ try: drain.marker = {"principal": "operator"} raise module.ControlError("simulated reactivation failure") module._wait_for_release_disposition = fail_after_operator_drain - module.release_drain(41, 902, token) + module.recover_drain() elif scenario == "complete": token = module.begin_drain() module.validate_restore(41, 902, token) @@ -236,6 +242,19 @@ try: raise module.ControlError("simulated replacement release failure") module._wait_for_release_disposition = fail_release module.complete_replacement(41, 902, 77, 903, token) + elif scenario == "complete-release-rollback-failure": + token = module.begin_drain() + module.validate_restore(41, 902, token) + status.payload["pid"] = 77 + status.payload["start_time"] = 903 + module.observe_replacement(41, 902, token) + def fail_release(*_args, **_kwargs): + raise module.ControlError("simulated replacement release failure") + def fail_rollback(*_args, **_kwargs): + raise module.ControlError("simulated marker rollback failure") + module._wait_for_release_disposition = fail_release + module._write_owned_drain = fail_rollback + module.complete_replacement(41, 902, 77, 903, token) elif scenario == "recover": module.begin_drain() status.payload["pid"] = 77 @@ -252,7 +271,7 @@ try: else: raise RuntimeError(f"unknown scenario: {scenario}") except module.ControlError as error: - print(str(error), file=sys.stderr) + module._emit_control_error(error) raise SystemExit(1) finally: print(f"OPERATOR_MUTATIONS:{drain.write_calls}:{drain.clear_calls}") @@ -329,6 +348,7 @@ describe("Hermes in-sandbox cron restore validator", () => { | "complete-substitution" | "complete-release-substitution" | "complete-release-failure" + | "complete-release-rollback-failure" | "recover" | "recover-operator" | "recover-noop", @@ -423,7 +443,7 @@ describe("Hermes in-sandbox cron restore validator", () => { expect(result.stderr).toContain("active job #1 script is not readable"); }); - it("pins one gateway identity across begin, validation, and release", () => { + it("pins one gateway identity across begin, validation, and recovery", () => { const result = runLifecycle("success"); expect(result.stderr).toBe(""); @@ -432,7 +452,7 @@ describe("Hermes in-sandbox cron restore validator", () => { .split("\n") .filter((line) => line.startsWith(RECEIPT_PREFIX)) .map((line) => JSON.parse(line.slice(RECEIPT_PREFIX.length))); - expect(receipts.map((receipt) => receipt.action)).toEqual(["begin", "validate", "release"]); + expect(receipts.map((receipt) => receipt.action)).toEqual(["begin", "validate", "recover"]); expect(receipts.map((receipt) => receipt.disposition)).toEqual([ "drain-acquired", "restore-validated", @@ -626,6 +646,24 @@ describe("Hermes in-sandbox cron restore validator", () => { expect(result.stdout).toContain("OWN_MARKER:present"); }); + it("emits the structured rollback-failure code when its marker cannot be restored (#8472)", () => { + const result = runLifecycle("complete-release-rollback-failure"); + + expect(result.status).toBe(1); + expect(result.stderr).toContain( + "HERMES_CRON_RESTORE_ERROR: Hermes cron restore drain release failed and its marker could not be restored", + ); + const signals = result.stderr + .split(/\r?\n/u) + .filter((line) => line.startsWith(CONTROL_ERROR_PREFIX)); + expect(signals).toHaveLength(1); + expect(JSON.parse(signals[0].slice(CONTROL_ERROR_PREFIX.length))).toEqual({ + code: HERMES_CRON_RESTORE_DRAIN_MARKER_ROLLBACK_FAILED_CODE, + message: "Hermes cron restore drain release failed and its marker could not be restored", + }); + expect(result.stdout).toContain("OWN_MARKER:absent"); + }); + it("re-pins a restarted gateway before validating and reactivating dispatch", () => { const result = runLifecycle("recover"); From 42bf3cc384ef9ee4692f16cdc169a1b5d63a2a5f Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Fri, 7 Aug 2026 12:14:46 -0700 Subject: [PATCH 10/11] test(hermes): preserve operator drain on restore failures Signed-off-by: Apurv Kumaria --- test/hermes-cron-restore-control.test.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/hermes-cron-restore-control.test.ts b/test/hermes-cron-restore-control.test.ts index 5c3aa28289f..0739b67f472 100644 --- a/test/hermes-cron-restore-control.test.ts +++ b/test/hermes-cron-restore-control.test.ts @@ -196,10 +196,12 @@ try: module.observe_replacement(41, 902, token) module.complete_replacement(41, 902, 77, 903, token) elif scenario == "complete-same-identity": + drain.marker = {"principal": "operator"} token = module.begin_drain() module.validate_restore(41, 902, token) module.complete_replacement(41, 902, 41, 902, token) elif scenario == "complete-validation-failure": + drain.marker = {"principal": "operator"} token = module.begin_drain() module.validate_restore(41, 902, token) status.payload["pid"] = 77 @@ -210,6 +212,7 @@ try: module.validate_cron_tree = fail_validation module.complete_replacement(41, 902, 77, 903, token) elif scenario == "complete-substitution": + drain.marker = {"principal": "operator"} token = module.begin_drain() module.validate_restore(41, 902, token) status.payload["pid"] = 77 @@ -219,6 +222,7 @@ try: status.payload["start_time"] = 904 module.complete_replacement(41, 902, 77, 903, token) elif scenario == "complete-release-substitution": + drain.marker = {"principal": "operator"} token = module.begin_drain() module.validate_restore(41, 902, token) status.payload["pid"] = 77 @@ -233,6 +237,7 @@ try: module.validate_cron_tree = substitute_after_validation module.complete_replacement(41, 902, 77, 903, token) elif scenario == "complete-release-failure": + drain.marker = {"principal": "operator"} token = module.begin_drain() module.validate_restore(41, 902, token) status.payload["pid"] = 77 @@ -611,6 +616,8 @@ describe("Hermes in-sandbox cron restore validator", () => { expect(result.status).toBe(1); expect(result.stderr).toContain("gateway identity did not change during cron restore"); + expect(result.stdout).toContain("OPERATOR_MUTATIONS:0:0"); + expect(result.stdout).toContain("FINAL_MARKER:operator"); expect(result.stdout).toContain("OWN_MARKER:present"); }); @@ -619,6 +626,8 @@ describe("Hermes in-sandbox cron restore validator", () => { expect(result.status).toBe(1); expect(result.stderr).toContain("replacement cron tree is invalid"); + expect(result.stdout).toContain("OPERATOR_MUTATIONS:0:0"); + expect(result.stdout).toContain("FINAL_MARKER:operator"); expect(result.stdout).toContain("OWN_MARKER:present"); }); @@ -627,6 +636,8 @@ describe("Hermes in-sandbox cron restore validator", () => { expect(result.status).toBe(1); expect(result.stderr).toContain("gateway identity changed during cron restore"); + expect(result.stdout).toContain("OPERATOR_MUTATIONS:0:0"); + expect(result.stdout).toContain("FINAL_MARKER:operator"); expect(result.stdout).toContain("OWN_MARKER:present"); }); @@ -635,6 +646,8 @@ describe("Hermes in-sandbox cron restore validator", () => { expect(result.status).toBe(1); expect(result.stderr).toContain("gateway identity changed during cron restore"); + expect(result.stdout).toContain("OPERATOR_MUTATIONS:0:0"); + expect(result.stdout).toContain("FINAL_MARKER:operator"); expect(result.stdout).toContain("OWN_MARKER:present"); }); @@ -643,6 +656,8 @@ describe("Hermes in-sandbox cron restore validator", () => { expect(result.status).toBe(1); expect(result.stderr).toContain("simulated replacement release failure"); + expect(result.stdout).toContain("OPERATOR_MUTATIONS:0:0"); + expect(result.stdout).toContain("FINAL_MARKER:operator"); expect(result.stdout).toContain("OWN_MARKER:present"); }); From 5f29a10479316458a88f14e1e4717ed7abb54702 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Fri, 7 Aug 2026 14:17:49 -0700 Subject: [PATCH 11/11] fix(rebuild): make Hermes cron recovery durable Signed-off-by: Apurv Kumaria --- agents/hermes/Dockerfile | 3 +- agents/hermes/cron-restore-control.py | 272 ++++++++++-- .../recover-rebuild-sandboxes.mdx | 19 +- .../rebuild-hermes-cron-restore.test.ts | 84 +++- .../sandbox/rebuild-hermes-post-restore.ts | 86 +++- .../rebuild-post-restore-phase.test.ts | 7 +- .../sandbox/rebuild-post-restore-phase.ts | 4 +- .../hermes-cron-restore-recovery.test.ts | 46 +- .../runtime/hermes-cron-restore-recovery.ts | 13 +- test/hermes-cron-restore-control.test.ts | 408 +++++++++++++++++- test/hermes-final-image-layout.test.ts | 3 + 11 files changed, 878 insertions(+), 67 deletions(-) diff --git a/agents/hermes/Dockerfile b/agents/hermes/Dockerfile index 7564394d937..aa94360acdd 100644 --- a/agents/hermes/Dockerfile +++ b/agents/hermes/Dockerfile @@ -569,7 +569,7 @@ ARG NEMOCLAW_HERMES_CLI_ADAPTER_SHA256=989edf54a8c09c6efb348600a8aa2f264c0b71408 ARG NEMOCLAW_HERMES_CLI_ADAPTER_VALIDATOR_SHA256=db4046e79e513eab67b069a8eda20167b8b65529cf26842531d2ad673c670330 ARG NEMOCLAW_HERMES_VALIDATOR_SHA256=822c7e63d068c5d09f3291350771c1a42c9686f51bfa9bc9a1f41fbe15d163b1 ARG NEMOCLAW_HERMES_TIRITH_FINALIZER_SHA256=a1e6b1c53ab297569abb87c29d15c294d729e46005bfd022136b4c447a791819 -ARG NEMOCLAW_HERMES_CRON_RESTORE_CONTROLLER_SHA256=f24a4dc187428530bfec2f95140fbe2eff59d9dd78d8edac352bcdc6b6a82586 +ARG NEMOCLAW_HERMES_CRON_RESTORE_CONTROLLER_SHA256=e8593cf1580bffa4663e91c079ba0ce31c3d26391f5b1718872701138ce250b0 # hadolint ignore=DL4006 RUN printf '%s %s\n' \ "$NEMOCLAW_HERMES_VALIDATOR_SHA256" /usr/local/lib/nemoclaw/validate-hermes-env-secret-boundary.py \ @@ -1170,6 +1170,7 @@ RUN check_metadata() { \ && check_absent /sandbox/.cache \ && check_absent /sandbox/.hermes/managed-policy.json \ && check_absent /sandbox/.nemoclaw/hermes-cron-restore-drain.json \ + && check_absent /sandbox/.nemoclaw/hermes-cron-restore-release-recovery.json \ && check_metadata /sandbox/.nemoclaw 'root:root 1755' \ && check_metadata /scripts/patch-bundled-npm-brace-expansion.mts 'root:root 444' \ && check_metadata /scripts/lib/patch-bundled-npm-ip-address.mts 'root:root 444' \ diff --git a/agents/hermes/cron-restore-control.py b/agents/hermes/cron-restore-control.py index 6d399a8b705..9d4af474166 100644 --- a/agents/hermes/cron-restore-control.py +++ b/agents/hermes/cron-restore-control.py @@ -9,6 +9,11 @@ complete action requires that same live identity before releasing the gate. A drain token is the client-side secret proving ownership of the server-side persisted drain marker. + +Before release, the controller durably writes a separate root-owned recovery +record. That write-ahead record survives a failed marker rollback and lets +``prepare-recover`` reacquire the gate before host gateway repair. ``recover`` +then validates cron state before clearing NemoClaw-owned recovery state. """ from __future__ import annotations @@ -32,6 +37,7 @@ NEMOCLAW_HOME = SANDBOX_HOME / ".nemoclaw" CONTROL_LOCK_PATH = Path("/run/nemoclaw/hermes-cron-restore-control.lock") CONTROL_MARKER_NAME = "hermes-cron-restore-drain.json" +RELEASE_RECOVERY_NAME = "hermes-cron-restore-release-recovery.json" RECEIPT_PREFIX = "NEMOCLAW_HERMES_CRON_RESTORE_V1:" CONTROL_ERROR_PREFIX = "NEMOCLAW_HERMES_CRON_RESTORE_ERROR_V1:" CONTROL_ERROR_CODE = "control-failure" @@ -71,6 +77,10 @@ def _marker_path() -> Path: return NEMOCLAW_HOME / CONTROL_MARKER_NAME +def _release_recovery_path() -> Path: + return NEMOCLAW_HOME / RELEASE_RECOVERY_NAME + + def _require_root() -> None: if os.geteuid() != ROOT_UID or os.getegid() != ROOT_GID: raise ControlError("Hermes cron restore control requires root") @@ -89,6 +99,34 @@ def _require_secure_directory(path: Path, label: str) -> None: raise ControlError(f"{label} is writable outside root") +def _fsync_directory(path: Path, label: str) -> None: + """Durably order a state-directory entry transition.""" + _require_secure_directory(path, label) + flags = os.O_RDONLY | os.O_CLOEXEC + if hasattr(os, "O_DIRECTORY"): + flags |= os.O_DIRECTORY + if hasattr(os, "O_NOFOLLOW"): + flags |= os.O_NOFOLLOW + try: + descriptor = os.open(path, flags) + except OSError as error: + raise ControlError(f"{label} could not be opened for durability") from error + try: + metadata = os.fstat(descriptor) + if ( + not stat.S_ISDIR(metadata.st_mode) + or metadata.st_uid != ROOT_UID + or metadata.st_gid != ROOT_GID + or stat.S_IMODE(metadata.st_mode) & 0o022 + ): + raise ControlError(f"{label} metadata is unsafe for durability") + os.fsync(descriptor) + except OSError as error: + raise ControlError(f"{label} durability sync failed") from error + finally: + os.close(descriptor) + + @contextmanager def _control_lock() -> Iterator[None]: _require_root() @@ -118,7 +156,7 @@ def _control_lock() -> Iterator[None]: os.close(descriptor) -def _validate_marker_metadata(metadata: os.stat_result) -> None: +def _validate_marker_metadata(metadata: os.stat_result, label: str) -> None: if ( not stat.S_ISREG(metadata.st_mode) or metadata.st_uid != ROOT_UID @@ -126,38 +164,38 @@ def _validate_marker_metadata(metadata: os.stat_result) -> None: or stat.S_IMODE(metadata.st_mode) != 0o400 or metadata.st_nlink != 1 ): - raise ControlError("NemoClaw cron restore drain marker metadata is unsafe") + raise ControlError(f"{label} metadata is unsafe") if metadata.st_size <= 0 or metadata.st_size > MAX_MARKER_BYTES: - raise ControlError("NemoClaw cron restore drain marker size is invalid") + raise ControlError(f"{label} size is invalid") -def _read_owned_drain_token(*, required: bool = True) -> str | None: +def _read_owned_token(path: Path, label: str, *, required: bool) -> str | None: _require_secure_directory(NEMOCLAW_HOME, "NemoClaw state root") flags = os.O_RDONLY | os.O_CLOEXEC if hasattr(os, "O_NOFOLLOW"): flags |= os.O_NOFOLLOW try: - descriptor = os.open(_marker_path(), flags) + descriptor = os.open(path, flags) except FileNotFoundError as error: if not required: return None - raise ControlError("NemoClaw cron restore drain marker is not active") from error + raise ControlError(f"{label} is not active") from error except OSError as error: - raise ControlError("NemoClaw cron restore drain marker is unreadable") from error + raise ControlError(f"{label} is unreadable") from error try: metadata = os.fstat(descriptor) - _validate_marker_metadata(metadata) + _validate_marker_metadata(metadata, label) raw = os.read(descriptor, MAX_MARKER_BYTES + 1) except OSError as error: - raise ControlError("NemoClaw cron restore drain marker is unreadable") from error + raise ControlError(f"{label} is unreadable") from error finally: os.close(descriptor) try: payload = json.loads(raw.decode("utf-8")) except (UnicodeError, ValueError) as error: - raise ControlError("NemoClaw cron restore drain marker is invalid") from error + raise ControlError(f"{label} is invalid") from error if not isinstance(payload, dict) or set(payload) != {"token", "version"}: - raise ControlError("NemoClaw cron restore drain marker has an invalid schema") + raise ControlError(f"{label} has an invalid schema") token = payload.get("token") if ( payload.get("version") != 1 @@ -166,19 +204,57 @@ def _read_owned_drain_token(*, required: bool = True) -> str | None: or not token.isascii() or not all(character.isalnum() or character in "-_" for character in token) ): - raise ControlError("NemoClaw cron restore drain marker has an invalid token") + raise ControlError(f"{label} has an invalid token") return token -def _require_owned_drain(drain_token: str) -> None: - observed_token = _read_owned_drain_token() +def _read_owned_drain_token(*, required: bool = True) -> str | None: + return _read_owned_token( + _marker_path(), + "NemoClaw cron restore drain marker", + required=required, + ) + + +def _read_release_recovery_token(*, required: bool = True) -> str | None: + return _read_owned_token( + _release_recovery_path(), + "NemoClaw cron restore release recovery record", + required=required, + ) + + +def _require_owned_token( + path: Path, + label: str, + ownership_label: str, + drain_token: str, +) -> None: + observed_token = _read_owned_token(path, label, required=True) if observed_token is None: - raise ControlError("NemoClaw cron restore drain marker is not active") + raise ControlError(f"{label} is not active") if not hmac.compare_digest(observed_token, drain_token): - raise ControlError("NemoClaw cron restore drain ownership changed") + raise ControlError(f"{ownership_label} ownership changed") -def _write_owned_drain(drain_token: str) -> None: +def _require_owned_drain(drain_token: str) -> None: + _require_owned_token( + _marker_path(), + "NemoClaw cron restore drain marker", + "NemoClaw cron restore drain", + drain_token, + ) + + +def _write_owned_token( + path: Path, + label: str, + drain_token: str, + *, + temp_prefix: str, + exists_message: str, + write_message: str, +) -> None: _require_secure_directory(NEMOCLAW_HOME, "NemoClaw state root") payload = json.dumps( {"token": drain_token, "version": 1}, @@ -189,7 +265,7 @@ def _write_owned_drain(drain_token: str) -> None: staged_path: Path | None = None try: descriptor, staged_raw = tempfile.mkstemp( - prefix=".hermes-cron-restore-drain-", + prefix=temp_prefix, dir=NEMOCLAW_HOME, ) staged_path = Path(staged_raw) @@ -202,18 +278,17 @@ def _write_owned_drain(drain_token: str) -> None: os.close(descriptor) descriptor = -1 try: - os.link(staged_path, _marker_path()) + os.link(staged_path, path) except FileExistsError as error: - raise ControlError( - "a NemoClaw cron restore drain already requires recovery" - ) from error + raise ControlError(exists_message) from error staged_path.unlink() staged_path = None - _require_owned_drain(drain_token) + _require_owned_token(path, label, label, drain_token) + _fsync_directory(NEMOCLAW_HOME, "NemoClaw state root") except ControlError: raise except OSError as error: - raise ControlError("NemoClaw cron restore drain could not be acquired") from error + raise ControlError(write_message) from error finally: if descriptor >= 0: os.close(descriptor) @@ -221,12 +296,82 @@ def _write_owned_drain(drain_token: str) -> None: staged_path.unlink(missing_ok=True) -def _remove_owned_drain(drain_token: str) -> None: - _require_owned_drain(drain_token) +def _write_owned_drain(drain_token: str) -> None: + _write_owned_token( + _marker_path(), + "NemoClaw cron restore drain marker", + drain_token, + temp_prefix=".hermes-cron-restore-drain-", + exists_message="a NemoClaw cron restore drain already requires recovery", + write_message="NemoClaw cron restore drain could not be acquired", + ) + + +def _write_release_recovery(drain_token: str) -> None: + _write_owned_token( + _release_recovery_path(), + "NemoClaw cron restore release recovery record", + drain_token, + temp_prefix=".hermes-cron-restore-release-recovery-", + exists_message="a NemoClaw cron restore release recovery already exists", + write_message="NemoClaw cron restore release recovery could not be recorded", + ) + + +def _ensure_release_recovery(drain_token: str) -> None: + observed_token = _read_release_recovery_token(required=False) + if observed_token is None: + _write_release_recovery(drain_token) + return + if not hmac.compare_digest(observed_token, drain_token): + raise ControlError("NemoClaw cron restore release recovery ownership changed") + _fsync_directory(NEMOCLAW_HOME, "NemoClaw state root") + + +def _ensure_owned_drain(drain_token: str) -> None: + observed_token = _read_owned_drain_token(required=False) + if observed_token is None: + _write_owned_drain(drain_token) + return + if not hmac.compare_digest(observed_token, drain_token): + raise ControlError("NemoClaw cron restore drain ownership changed") + _fsync_directory(NEMOCLAW_HOME, "NemoClaw state root") + + +def _remove_owned_token( + path: Path, + label: str, + ownership_label: str, + drain_token: str, + *, + failure_message: str, +) -> None: + _require_owned_token(path, label, ownership_label, drain_token) try: - _marker_path().unlink() + path.unlink() except OSError as error: - raise ControlError("NemoClaw cron restore drain could not be released") from error + raise ControlError(failure_message) from error + _fsync_directory(NEMOCLAW_HOME, "NemoClaw state root") + + +def _remove_owned_drain(drain_token: str) -> None: + _remove_owned_token( + _marker_path(), + "NemoClaw cron restore drain marker", + "NemoClaw cron restore drain", + drain_token, + failure_message="NemoClaw cron restore drain could not be released", + ) + + +def _remove_release_recovery(drain_token: str) -> None: + _remove_owned_token( + _release_recovery_path(), + "NemoClaw cron restore release recovery record", + "NemoClaw cron restore release recovery record", + drain_token, + failure_message="NemoClaw cron restore release recovery could not be cleared", + ) def _profile_homes(home: Path) -> list[tuple[str, Path]]: @@ -446,6 +591,16 @@ def _receipt( print(f"{RECEIPT_PREFIX}{json.dumps(payload, separators=(',', ':'), sort_keys=True)}") +def _prepare_recovery_receipt(drain_acquired: bool) -> None: + payload = { + "version": 1, + "action": "prepare-recover", + "drain_acquired": drain_acquired, + "disposition": "gate-prepared" if drain_acquired else "not-required", + } + print(f"{RECEIPT_PREFIX}{json.dumps(payload, separators=(',', ':'), sort_keys=True)}") + + def _operator_drain_active(drain_control: Any) -> bool: predicate = getattr(drain_control, "operator_drain_requested", None) if not callable(predicate): @@ -504,7 +659,18 @@ def _complete_release( **fields: Any, ) -> None: _require_drained_idle(status_module, pid, start_time) - _remove_owned_drain(drain_token) + _ensure_release_recovery(drain_token) + try: + _remove_owned_drain(drain_token) + except ControlError as release_error: + try: + _ensure_owned_drain(drain_token) + except ControlError as rollback_error: + raise ControlError( + "Hermes cron restore drain release failed and its marker could not be restored", + code=DRAIN_MARKER_ROLLBACK_FAILED_CODE, + ) from rollback_error + raise release_error try: payload, operator_drain_active, disposition = _wait_for_release_disposition( drain_control, @@ -514,7 +680,7 @@ def _complete_release( ) except Exception as release_error: try: - _write_owned_drain(drain_token) + _ensure_owned_drain(drain_token) except ControlError as rollback_error: raise ControlError( "Hermes cron restore drain release failed and its marker could not be restored", @@ -523,6 +689,20 @@ def _complete_release( if isinstance(release_error, ControlError): raise release_error raise + try: + _remove_release_recovery(drain_token) + except ControlError as cleanup_error: + try: + _ensure_owned_drain(drain_token) + except ControlError as rollback_error: + raise ControlError( + "Hermes cron restore drain release failed and its marker could not be restored", + code=DRAIN_MARKER_ROLLBACK_FAILED_CODE, + ) from rollback_error + raise ControlError( + "Hermes cron restore release recovery could not be cleared; " + "the drain marker was restored" + ) from cleanup_error _receipt( action, pid, @@ -536,9 +716,36 @@ def _complete_release( ) +def _prepare_owned_drain() -> str | None: + drain_token = _read_owned_drain_token(required=False) + recovery_token = _read_release_recovery_token(required=False) + if drain_token is not None and recovery_token is not None: + if not hmac.compare_digest(drain_token, recovery_token): + raise ControlError( + "NemoClaw cron restore drain and release recovery ownership differ" + ) + _fsync_directory(NEMOCLAW_HOME, "NemoClaw state root") + elif drain_token is None and recovery_token is not None: + _write_owned_drain(recovery_token) + drain_token = recovery_token + elif drain_token is not None: + _fsync_directory(NEMOCLAW_HOME, "NemoClaw state root") + return drain_token + + +def prepare_recovery() -> None: + """Re-establish any persisted NemoClaw gate before host gateway repair.""" + with _control_lock(): + _prepare_recovery_receipt(_prepare_owned_drain() is not None) + + def begin_drain() -> str: with _control_lock(): drain_control, status_module = _load_gateway_modules() + if _read_release_recovery_token(required=False) is not None: + raise ControlError( + "a NemoClaw cron restore release recovery already requires recovery" + ) _, pid, start_time = _gateway_identity(status_module) drain_token = secrets.token_urlsafe(24) _write_owned_drain(drain_token) @@ -643,7 +850,7 @@ def recover_drain() -> None: with _control_lock(): drain_control, status_module = _load_gateway_modules() payload, pid, start_time = _gateway_identity(status_module) - drain_token = _read_owned_drain_token(required=False) + drain_token = _prepare_owned_drain() if drain_token is None: operator_drain_active = _operator_drain_active(drain_control) _receipt( @@ -683,6 +890,7 @@ def _parser() -> argparse.ArgumentParser: parser = argparse.ArgumentParser() subparsers = parser.add_subparsers(dest="action", required=True) subparsers.add_parser("begin") + subparsers.add_parser("prepare-recover") subparsers.add_parser("recover") for action in ("validate", "observe", "complete"): subparser = subparsers.add_parser(action) @@ -705,6 +913,8 @@ def main() -> int: try: if args.action == "begin": begin_drain() + elif args.action == "prepare-recover": + prepare_recovery() elif args.action == "recover": recover_drain() elif args.action == "validate": diff --git a/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx b/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx index 79c2abe5ee3..4669a240d96 100644 --- a/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx +++ b/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx @@ -79,14 +79,15 @@ It is idempotent and safe to script. If the gateway is already healthy, `recover` does not restart it. If the host forward is already active, recovery accepts it only after OpenShell ownership is reconciled and the local endpoint is reachable. -After it checks gateway health and host forwards, `recover` checks for a NemoClaw cron restore gate left by an interrupted rebuild. +Before it repairs the gateway, `recover` checks for a NemoClaw cron restore gate or release recovery record left by an interrupted rebuild. The gate continues to block new Hermes turns and cron dispatch across gateway and container restarts in the same sandbox. -When the gate exists, `recover` waits for active agent work to finish and validates the restored cron jobs and scripts. -It clears only the NemoClaw gate after validation succeeds. +If release rollback could not restore the gate, `recover` uses the root-owned recovery record to reacquire it before gateway repair can start dispatch. +After gateway repair, `recover` waits for active agent work to finish and validates the restored cron jobs and scripts. +It clears NemoClaw-owned gate and release recovery state only after validation succeeds. If no independent operator drain exists, successful recovery prints `Hermes cron dispatch resumed after restored jobs and scripts were validated.` If an operator drain exists, recovery prints `Hermes cron restore gate cleared; the independent operator drain remains active.` The command does not own or clear the Hermes operator drain, so new Hermes turns and cron dispatch remain blocked while that drain is active. -If cron validation fails, `recover` exits nonzero and retains the NemoClaw gate. +If gate reacquisition or cron validation fails, `recover` exits nonzero and retains the recovery state for another attempt. Use `gateway restart` when you intentionally need a supported Hermes gateway to reload runtime configuration or plugins. @@ -237,14 +238,16 @@ The gate remains active across gateway and container restarts in the replacement NemoClaw waits for active agent work to finish before restoring state. It validates the restored jobs and scripts before the gateway replacement, then keeps dispatch blocked while it restarts and verifies that replacement. It records the replacement process identity around managed health verification and clears the gate only if that same live process completes the final cron validation. -If an operator already drained the gateway, NemoClaw clears only its gate and leaves the operator drain active. +If an operator already drained the gateway, NemoClaw clears its gate and release recovery record while leaving the operator drain active. If state restore, managed MCP restoration, gateway replacement, or cron validation fails after gate acquisition, the command exits nonzero and preserves the backup. Those failures retain the NemoClaw gate unless the output explicitly reports that release rollback could not restore its marker. -In that exceptional case, do not assume dispatch is blocked; run `$$nemoclaw recover` immediately. +In that exceptional case, NemoClaw preserves a root-owned release recovery record, but you must not assume dispatch is blocked. +Run `$$nemoclaw recover` immediately so it can reacquire the gate before validating the restored cron state. +If gate reacquisition fails, recovery exits nonzero and leaves the recovery record in place for another attempt. Failures before gate acquisition do not create a new gate. -Do not manually remove the root-owned cron restore marker because removal bypasses restored cron validation. +Do not manually remove the root-owned cron restore marker or release recovery record because removal bypasses restored cron validation. If managed MCP restoration failed, correct the reported cause and run `$$nemoclaw mcp restart` first. -Then run `$$nemoclaw recover` to repair and probe the gateway, validate the restored cron tree, and clear the NemoClaw gate. +Then run `$$nemoclaw recover` to repair and probe the gateway, validate the restored cron tree, and clear NemoClaw-owned cron restore recovery state. diff --git a/src/lib/actions/sandbox/rebuild-hermes-cron-restore.test.ts b/src/lib/actions/sandbox/rebuild-hermes-cron-restore.test.ts index 36e1a05792c..0c4029f2038 100644 --- a/src/lib/actions/sandbox/rebuild-hermes-cron-restore.test.ts +++ b/src/lib/actions/sandbox/rebuild-hermes-cron-restore.test.ts @@ -33,6 +33,7 @@ import { HERMES_CRON_RESTORE_DRAIN_MARKER_ROLLBACK_FAILED_CODE, isHermesCronRestoreDrainMarkerRollbackFailure, observeHermesCronReplacement, + prepareHermesCronRestoreRecovery, recoverHermesCronRestore, runHermesCronRestoreTransaction, validateHermesCronRestore, @@ -138,6 +139,19 @@ function notRequiredRecoveryReceipt(overrides: Record = {}): st })}`; } +function preparationReceipt( + disposition: "gate-prepared" | "not-required", + overrides: Record = {}, +): string { + return `${RECEIPT_PREFIX}${JSON.stringify({ + version: 1, + action: "prepare-recover", + drain_acquired: disposition === "gate-prepared", + disposition, + ...overrides, + })}`; +} + describe("Hermes cron rebuild restore contract", () => { let backupPath: string; @@ -466,18 +480,78 @@ describe("Hermes cron rebuild restore contract", () => { ); }); - it("composes the recovery transport budget from every controller phase (#7806)", () => { - processMocks.dockerSpawnSync.mockImplementation((argv: string[]) => ({ + it.each([ + "gate-prepared", + "not-required", + ] as const)("returns the %s pre-repair disposition", (disposition) => { + processMocks.dockerSpawnSync.mockReturnValue({ status: 0, - stdout: receipt(argv.includes("recover") ? "recover" : "begin"), + stdout: preparationReceipt(disposition), stderr: "", - })); + }); + + expect(prepareHermesCronRestoreRecovery("alpha")).toBe(disposition); + expect(processMocks.privilegedSandboxExecArgv).toHaveBeenCalledWith( + "alpha", + [ + "/opt/hermes/.venv/bin/python", + "-I", + "/usr/local/lib/nemoclaw/hermes-cron-restore-control.py", + "prepare-recover", + ], + false, + true, + ); + }); + + it("rejects an inconsistent pre-repair receipt", () => { + processMocks.dockerSpawnSync.mockReturnValue({ + status: 0, + stdout: preparationReceipt("gate-prepared", { drain_acquired: false }), + stderr: "", + }); + + expect(() => prepareHermesCronRestoreRecovery("alpha")).toThrow( + "prepare-recover receipt failed validation", + ); + }); + + it.each([ + `/opt/hermes/.venv/bin/python: can't open file '/usr/local/lib/nemoclaw/hermes-cron-restore-control.py': [Errno 2] No such file or directory`, + "hermes-cron-restore-control.py: error: argument action: invalid choice: 'prepare-recover'", + ])("keeps pre-repair compatible with a legacy Hermes sandbox: %s", (stderr) => { + processMocks.dockerSpawnSync.mockReturnValue({ status: 2, stdout: "", stderr }); + + expect(prepareHermesCronRestoreRecovery("alpha")).toBe("unsupported"); + }); + + it("does not hide a current controller pre-repair failure", () => { + processMocks.dockerSpawnSync.mockReturnValue({ + status: 1, + stdout: "", + stderr: "NemoClaw cron restore release recovery record metadata is unsafe", + }); + + expect(() => prepareHermesCronRestoreRecovery("alpha")).toThrow( + "Hermes cron prepare-recover failed: NemoClaw cron restore release recovery record metadata is unsafe", + ); + }); + + it("composes the recovery transport budget from every controller phase (#7806)", () => { + processMocks.dockerSpawnSync.mockImplementation((argv: string[]) => { + const stdout = argv.includes("prepare-recover") + ? preparationReceipt("not-required") + : receipt(argv.includes("recover") ? "recover" : "begin"); + return { status: 0, stdout, stderr: "" }; + }); beginHermesCronRestore("alpha"); + prepareHermesCronRestoreRecovery("alpha"); recoverHermesCronRestore("alpha"); expect(processMocks.dockerSpawnSync.mock.calls[0]?.[1]).toMatchObject({ timeout: 70_000 }); - expect(processMocks.dockerSpawnSync.mock.calls[1]?.[1]).toMatchObject({ timeout: 130_000 }); + expect(processMocks.dockerSpawnSync.mock.calls[1]?.[1]).toMatchObject({ timeout: 25_000 }); + expect(processMocks.dockerSpawnSync.mock.calls[2]?.[1]).toMatchObject({ timeout: 130_000 }); }); it("returns not-required when no NemoClaw recovery gate exists", () => { diff --git a/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts b/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts index 2725fb88587..da68305db87 100644 --- a/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts +++ b/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts @@ -16,7 +16,14 @@ const CONTROL_TIMEOUT_MS = 25_000; const RECOVERY_TIMEOUT_MS = BEGIN_TIMEOUT_MS + CONTROL_TIMEOUT_MS * 2 + 10_000; const HERMES_GATEWAY_RECHECK_ATTEMPTS = 2; -type HermesCronRestoreAction = "begin" | "validate" | "observe" | "complete" | "recover"; +type HermesCronRestoreAction = + | "begin" + | "validate" + | "observe" + | "complete" + | "prepare-recover" + | "recover"; +type HermesCronRestoreReceiptAction = Exclude; type HermesCronRestoreDisposition = | "drain-acquired" | "restore-validated" @@ -57,6 +64,8 @@ export type HermesCronRestoreRecoveryOutcome = | "not-required" | "unsupported"; +export type HermesCronRestorePreparationOutcome = "gate-prepared" | "not-required" | "unsupported"; + export class HermesCronRestoreIncompleteError extends Error { constructor() { super("Hermes state restore was incomplete while cron dispatch was drained"); @@ -241,7 +250,7 @@ function isReleaseDispositionValid(payload: Record): boolean { function parseCronRestoreReceipt( stdout: string, - expectedAction: HermesCronRestoreAction, + expectedAction: HermesCronRestoreReceiptAction, ): HermesCronRestoreReceipt { const receiptLines = stdout.split(/\r?\n/u).filter((line) => line.startsWith(RECEIPT_PREFIX)); if (receiptLines.length !== 1) { @@ -363,6 +372,35 @@ function parseCronRestoreReceipt( return receipt as unknown as HermesCronRestoreReceipt; } +function parseCronRestorePreparationReceipt(stdout: string): HermesCronRestorePreparationOutcome { + const receiptLines = stdout.split(/\r?\n/u).filter((line) => line.startsWith(RECEIPT_PREFIX)); + if (receiptLines.length !== 1) { + throw new Error("Hermes cron prepare-recover returned an invalid receipt"); + } + let payload: unknown; + try { + payload = JSON.parse(receiptLines[0].slice(RECEIPT_PREFIX.length)); + } catch { + throw new Error("Hermes cron prepare-recover returned malformed JSON"); + } + if (payload === null || typeof payload !== "object" || Array.isArray(payload)) { + throw new Error("Hermes cron prepare-recover receipt failed validation"); + } + const receipt = payload as Record; + const validDisposition = + (receipt.drain_acquired === true && receipt.disposition === "gate-prepared") || + (receipt.drain_acquired === false && receipt.disposition === "not-required"); + if ( + receipt.version !== 1 || + receipt.action !== "prepare-recover" || + !validDisposition || + !hasExactReceiptFields(receipt, ["version", "action", "drain_acquired", "disposition"]) + ) { + throw new Error("Hermes cron prepare-recover receipt failed validation"); + } + return receipt.disposition as "gate-prepared" | "not-required"; +} + function parseCronRestoreControlError(stderr: string): { code: string; message: string } | null { const signalLines = stderr .split(/\r?\n/u) @@ -418,12 +456,12 @@ export function isHermesCronRestoreDrainMarkerRollbackFailure(error: unknown): b ); } -function runCronRestoreControl( +function executeCronRestoreControl( sandboxName: string, action: HermesCronRestoreAction, identity?: HermesCronRestoreIdentity, replacementIdentity?: HermesCronRestoreIdentity, -): HermesCronRestoreReceipt { +): string { const command = [HERMES_PYTHON, "-I", HERMES_CRON_CONTROL, action]; if (identity) { command.push("--pid", String(identity.pid), "--start-time", String(identity.start_time)); @@ -460,7 +498,19 @@ function runCronRestoreControl( if (result.status !== 0) { throw new HermesCronRestoreControlFailure(action, result.stderr); } - return parseCronRestoreReceipt(result.stdout, action); + return result.stdout; +} + +function runCronRestoreControl( + sandboxName: string, + action: HermesCronRestoreReceiptAction, + identity?: HermesCronRestoreIdentity, + replacementIdentity?: HermesCronRestoreIdentity, +): HermesCronRestoreReceipt { + return parseCronRestoreReceipt( + executeCronRestoreControl(sandboxName, action, identity, replacementIdentity), + action, + ); } export function beginHermesCronRestore(sandboxName: string): HermesCronRestoreIdentity { @@ -540,21 +590,41 @@ export function observeHermesCronReplacement( }; } -function isLegacyCronRestoreControl(error: unknown): boolean { +function isLegacyCronRestoreControl( + error: unknown, + action: "prepare-recover" | "recover", +): boolean { if (!(error instanceof HermesCronRestoreControlFailure)) return false; + const invalidAction = + action === "prepare-recover" + ? /argument action: invalid choice: ['"]prepare-recover['"]/u + : /argument action: invalid choice: ['"]recover['"]/u; return ( /can't open file ['"]\/usr\/local\/lib\/nemoclaw\/hermes-cron-restore-control\.py['"]: \[Errno 2\] No such file or directory/u.test( error.stderr, - ) || /argument action: invalid choice: ['"]recover['"]/u.test(error.stderr) + ) || invalidAction.test(error.stderr) ); } +export function prepareHermesCronRestoreRecovery( + sandboxName: string, +): HermesCronRestorePreparationOutcome { + let stdout: string; + try { + stdout = executeCronRestoreControl(sandboxName, "prepare-recover"); + } catch (error) { + if (isLegacyCronRestoreControl(error, "prepare-recover")) return "unsupported"; + throw error; + } + return parseCronRestorePreparationReceipt(stdout); +} + export function recoverHermesCronRestore(sandboxName: string): HermesCronRestoreRecoveryOutcome { let receipt: HermesCronRestoreReceipt; try { receipt = runCronRestoreControl(sandboxName, "recover"); } catch (error) { - if (isLegacyCronRestoreControl(error)) return "unsupported"; + if (isLegacyCronRestoreControl(error, "recover")) return "unsupported"; throw error; } if ( diff --git a/src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts b/src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts index 93a2872bedd..edf08ee5189 100644 --- a/src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts +++ b/src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts @@ -254,7 +254,7 @@ describe("rebuild post-restore phase", () => { expect(output).toContain("nemoclaw alpha recover"); }); - it("reports unverified dispatch state when release marker rollback fails (#8472)", async () => { + it("reports preserved recovery authority when release marker rollback fails (#8472)", async () => { agentName = "hermes"; const rollbackFailure = new Error( "Hermes cron complete failed: Hermes cron restore drain release failed and its marker could not be restored", @@ -279,11 +279,12 @@ describe("rebuild post-restore phase", () => { await runRebuildPostRestorePhase(args); expect(args.bail).toHaveBeenCalledWith( - "Hermes cron restore gate state is unverified after release rollback failure; recover immediately.", + "Hermes cron restore release state requires immediate recovery.", ); const output = vi.mocked(console.error).mock.calls.flat().join("\n"); expect(output).toContain("drain release failed and its marker could not be restored"); - expect(output).toContain("Dispatch gate state is unverified; run recovery immediately"); + expect(output).toContain("root-owned recovery state was preserved"); + expect(output).toContain("reacquire the gate and validate restored cron state"); expect(output).toContain("nemoclaw alpha recover"); expect(output).not.toContain("dispatch was not re-enabled"); expect( diff --git a/src/lib/actions/sandbox/rebuild-post-restore-phase.ts b/src/lib/actions/sandbox/rebuild-post-restore-phase.ts index 074ae84acfd..f002f8da7bd 100644 --- a/src/lib/actions/sandbox/rebuild-post-restore-phase.ts +++ b/src/lib/actions/sandbox/rebuild-post-restore-phase.ts @@ -335,8 +335,8 @@ export async function runRebuildPostRestorePhase( return bailAfterHermesCronRestoreFailure( sandboxName, backupManifest, - ` Hermes cron restore release rollback failed: ${errorDetail}. Dispatch gate state is unverified; run recovery immediately.`, - "Hermes cron restore gate state is unverified after release rollback failure; recover immediately.", + ` Hermes cron restore release rollback failed: ${errorDetail}. Dispatch state is unverified, but root-owned recovery state was preserved; run recovery immediately so it can reacquire the gate and validate restored cron state.`, + "Hermes cron restore release state requires immediate recovery.", bail, ); } diff --git a/src/lib/actions/sandbox/runtime/hermes-cron-restore-recovery.test.ts b/src/lib/actions/sandbox/runtime/hermes-cron-restore-recovery.test.ts index b0bd4e2fd4d..f876637a4f3 100644 --- a/src/lib/actions/sandbox/runtime/hermes-cron-restore-recovery.test.ts +++ b/src/lib/actions/sandbox/runtime/hermes-cron-restore-recovery.test.ts @@ -6,6 +6,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => ({ connectSandbox: vi.fn(), getSessionAgent: vi.fn(), + prepareHermesCronRestoreRecovery: vi.fn(), recoverHermesCronRestore: vi.fn(), withMcpLifecycleLock: vi.fn( async (_sandboxName: string, operation: () => Promise, _options: unknown) => operation(), @@ -26,6 +27,7 @@ vi.mock("../connect", () => ({ })); vi.mock("../rebuild-hermes-post-restore", () => ({ + prepareHermesCronRestoreRecovery: mocks.prepareHermesCronRestoreRecovery, recoverHermesCronRestore: mocks.recoverHermesCronRestore, })); @@ -35,17 +37,58 @@ describe("sandbox recovery with a Hermes cron restore gate", () => { beforeEach(() => { vi.clearAllMocks(); mocks.connectSandbox.mockResolvedValue(undefined); + mocks.prepareHermesCronRestoreRecovery.mockReturnValue("not-required"); mocks.recoverHermesCronRestore.mockReturnValue("not-required"); }); - it("serializes gateway and cron recovery with the sandbox mutation lock", async () => { + it("prepares the Hermes gate before gateway repair under the sandbox mutation lock", async () => { mocks.getSessionAgent.mockReturnValue({ name: "hermes" }); + const events: string[] = []; + mocks.prepareHermesCronRestoreRecovery.mockImplementation(() => { + events.push("prepare"); + return "gate-prepared"; + }); + mocks.connectSandbox.mockImplementation(async () => { + events.push("connect"); + }); + mocks.recoverHermesCronRestore.mockImplementation(() => { + events.push("recover"); + return "dispatch-reactivated"; + }); await recoverSandboxWithHermesCronRestore("alpha"); expect(mocks.withMcpLifecycleLock).toHaveBeenCalledWith("alpha", expect.any(Function), { timeoutMs: 30_000, }); + expect(events).toEqual(["prepare", "connect", "recover"]); + expect(mocks.prepareHermesCronRestoreRecovery).toHaveBeenCalledWith("alpha"); + expect(mocks.connectSandbox).toHaveBeenCalledWith("alpha", { probeOnly: true }); + expect(mocks.recoverHermesCronRestore).toHaveBeenCalledWith("alpha"); + }); + + it("does not repair the gateway when Hermes gate preparation fails", async () => { + mocks.getSessionAgent.mockReturnValue({ name: "hermes" }); + mocks.prepareHermesCronRestoreRecovery.mockImplementation(() => { + throw new Error("recovery authority is unsafe"); + }); + + await expect(recoverSandboxWithHermesCronRestore("alpha")).rejects.toThrow( + "recovery authority is unsafe", + ); + + expect(mocks.connectSandbox).not.toHaveBeenCalled(); + expect(mocks.recoverHermesCronRestore).not.toHaveBeenCalled(); + }); + + it("keeps legacy Hermes recovery compatible when preparation is unsupported", async () => { + mocks.getSessionAgent.mockReturnValue({ name: "hermes" }); + mocks.prepareHermesCronRestoreRecovery.mockReturnValue("unsupported"); + mocks.recoverHermesCronRestore.mockReturnValue("unsupported"); + + await recoverSandboxWithHermesCronRestore("alpha"); + + expect(mocks.prepareHermesCronRestoreRecovery).toHaveBeenCalledWith("alpha"); expect(mocks.connectSandbox).toHaveBeenCalledWith("alpha", { probeOnly: true }); expect(mocks.recoverHermesCronRestore).toHaveBeenCalledWith("alpha"); }); @@ -56,6 +99,7 @@ describe("sandbox recovery with a Hermes cron restore gate", () => { await recoverSandboxWithHermesCronRestore("alpha"); expect(mocks.connectSandbox).toHaveBeenCalledWith("alpha", { probeOnly: true }); + expect(mocks.prepareHermesCronRestoreRecovery).not.toHaveBeenCalled(); expect(mocks.recoverHermesCronRestore).not.toHaveBeenCalled(); }); diff --git a/src/lib/actions/sandbox/runtime/hermes-cron-restore-recovery.ts b/src/lib/actions/sandbox/runtime/hermes-cron-restore-recovery.ts index afe21a2d9fa..b318986ab5f 100644 --- a/src/lib/actions/sandbox/runtime/hermes-cron-restore-recovery.ts +++ b/src/lib/actions/sandbox/runtime/hermes-cron-restore-recovery.ts @@ -4,17 +4,24 @@ import * as agentRuntime from "../../../agent/runtime"; import { withMcpLifecycleLock } from "../../../state/mcp-lifecycle-lock"; import { connectSandbox } from "../connect"; -import { recoverHermesCronRestore } from "../rebuild-hermes-post-restore"; +import { + prepareHermesCronRestoreRecovery, + recoverHermesCronRestore, +} from "../rebuild-hermes-post-restore"; const RECOVERY_LOCK_TIMEOUT_MS = 30_000; -/** Repair the gateway first, then validate and release any stranded Hermes cron restore gate. */ +/** Re-establish a Hermes gate before gateway repair, then validate and release it. */ export async function recoverSandboxWithHermesCronRestore(sandboxName: string): Promise { await withMcpLifecycleLock( sandboxName, async () => { + const agent = agentRuntime.getSessionAgent(sandboxName); + if (agent?.name === "hermes") { + prepareHermesCronRestoreRecovery(sandboxName); + } await connectSandbox(sandboxName, { probeOnly: true }); - if (agentRuntime.getSessionAgent(sandboxName)?.name !== "hermes") return; + if (agent?.name !== "hermes") return; const outcome = recoverHermesCronRestore(sandboxName); switch (outcome) { diff --git a/test/hermes-cron-restore-control.test.ts b/test/hermes-cron-restore-control.test.ts index 0739b67f472..065f834f612 100644 --- a/test/hermes-cron-restore-control.test.ts +++ b/test/hermes-cron-restore-control.test.ts @@ -43,11 +43,31 @@ module.NEMOCLAW_HOME.mkdir(mode=0o755) module.CONTROL_LOCK_PATH.parent.mkdir(mode=0o755) os.chmod(module.NEMOCLAW_HOME, 0o755) os.chmod(module.CONTROL_LOCK_PATH.parent, 0o755) -module.validate_cron_tree = lambda: { - "profiles": 1, - "active_jobs": 1, - "script_jobs": 1, -} +cron_validations = 0 +def validate_cron_tree(): + global cron_validations + if not module._marker_path().exists(): + raise AssertionError("cron validation ran without the NemoClaw drain") + cron_validations += 1 + return { + "profiles": 1, + "active_jobs": 1, + "script_jobs": 1, + } +module.validate_cron_tree = validate_cron_tree +durability_sync_calls = 0 +def fail_directory_sync_on(expected_call): + original_fsync_directory = module._fsync_directory + def fsync_directory(path, label): + global durability_sync_calls + durability_sync_calls += 1 + if durability_sync_calls == expected_call: + raise module.ControlError("simulated state directory durability failure") + return original_fsync_directory(path, label) + module._fsync_directory = fsync_directory + +def forbid_gateway_or_validation(*_args, **_kwargs): + raise AssertionError("prepare-recover touched gateway or cron validation") class DrainControl: def __init__(self): @@ -260,6 +280,154 @@ try: module._wait_for_release_disposition = fail_release module._write_owned_drain = fail_rollback module.complete_replacement(41, 902, 77, 903, token) + elif scenario == "complete-durable-order": + token = module.begin_drain() + module.validate_restore(41, 902, token) + status.payload["pid"] = 77 + status.payload["start_time"] = 903 + module.observe_replacement(41, 902, token) + release_events = [] + original_write_release_recovery = module._write_release_recovery + original_remove_owned_drain = module._remove_owned_drain + def write_release_recovery(drain_token): + release_events.append("recovery-write-started") + original_write_release_recovery(drain_token) + release_events.append("recovery-write-durable") + def remove_owned_drain(drain_token): + release_events.append("drain-delete-started") + original_remove_owned_drain(drain_token) + release_events.append("drain-delete-durable") + module._write_release_recovery = write_release_recovery + module._remove_owned_drain = remove_owned_drain + module.complete_replacement(41, 902, 77, 903, token) + print("RELEASE_EVENTS:" + ",".join(release_events)) + elif scenario == "release-recovery-sync-failure": + token = module.begin_drain() + module.validate_restore(41, 902, token) + status.payload["pid"] = 77 + status.payload["start_time"] = 903 + module.observe_replacement(41, 902, token) + fail_directory_sync_on(1) + module.complete_replacement(41, 902, 77, 903, token) + elif scenario == "existing-recovery-sync-failure": + token = module.begin_drain() + module.validate_restore(41, 902, token) + status.payload["pid"] = 77 + status.payload["start_time"] = 903 + module.observe_replacement(41, 902, token) + module._write_release_recovery(token) + fail_directory_sync_on(1) + module.complete_replacement(41, 902, 77, 903, token) + elif scenario == "drain-unlink-sync-failure": + token = module.begin_drain() + module.validate_restore(41, 902, token) + status.payload["pid"] = 77 + status.payload["start_time"] = 903 + module.observe_replacement(41, 902, token) + fail_directory_sync_on(2) + module.complete_replacement(41, 902, 77, 903, token) + elif scenario == "recovery-unlink-sync-failure": + token = module.begin_drain() + module.validate_restore(41, 902, token) + status.payload["pid"] = 77 + status.payload["start_time"] = 903 + module.observe_replacement(41, 902, token) + fail_directory_sync_on(3) + module.complete_replacement(41, 902, 77, 903, token) + elif scenario == "rollback-publication-sync-failure": + token = module.begin_drain() + module.validate_restore(41, 902, token) + status.payload["pid"] = 77 + status.payload["start_time"] = 903 + module.observe_replacement(41, 902, token) + def fail_release(*_args, **_kwargs): + raise module.ControlError("simulated replacement release failure") + module._wait_for_release_disposition = fail_release + fail_directory_sync_on(3) + module.complete_replacement(41, 902, 77, 903, token) + elif scenario == "prepare-recovery-only": + module._write_release_recovery("a" * 32) + module._load_gateway_modules = forbid_gateway_or_validation + module.validate_cron_tree = forbid_gateway_or_validation + module.prepare_recovery() + elif scenario == "prepare-matching": + module._write_owned_drain("a" * 32) + module._write_release_recovery("a" * 32) + module._load_gateway_modules = forbid_gateway_or_validation + module.validate_cron_tree = forbid_gateway_or_validation + module.prepare_recovery() + elif scenario == "prepare-matching-sync-failure": + module._write_owned_drain("a" * 32) + module._write_release_recovery("a" * 32) + module._load_gateway_modules = forbid_gateway_or_validation + module.validate_cron_tree = forbid_gateway_or_validation + fail_directory_sync_on(1) + module.prepare_recovery() + elif scenario == "prepare-noop": + module._load_gateway_modules = forbid_gateway_or_validation + module.validate_cron_tree = forbid_gateway_or_validation + module.prepare_recovery() + elif scenario == "prepare-existing-sync-failure": + module._write_owned_drain("a" * 32) + fail_directory_sync_on(1) + module.prepare_recovery() + elif scenario == "prepare-mismatch": + module._write_owned_drain("a" * 32) + module._write_release_recovery("b" * 32) + module._load_gateway_modules = forbid_gateway_or_validation + module.validate_cron_tree = forbid_gateway_or_validation + module.prepare_recovery() + elif scenario == "prepare-recovery-unsafe-mode": + module._write_release_recovery("a" * 32) + os.chmod(module._release_recovery_path(), 0o600) + module.prepare_recovery() + elif scenario == "prepare-recovery-symlink": + module._write_release_recovery("a" * 32) + recovery = module._release_recovery_path() + held = module.NEMOCLAW_HOME / "held-recovery.json" + recovery.rename(held) + recovery.symlink_to(held.name) + module.prepare_recovery() + elif scenario == "prepare-recovery-hardlink": + module._write_release_recovery("a" * 32) + os.link( + module._release_recovery_path(), + module.NEMOCLAW_HOME / "held-recovery.json", + ) + module.prepare_recovery() + elif scenario == "pending-release-recovery": + module._write_release_recovery("a" * 32) + module.begin_drain() + elif scenario == "mismatched-release-recovery": + module.begin_drain() + module._write_release_recovery("b" * 32) + module.recover_drain() + elif scenario == "recover-release-rollback": + token = module.begin_drain() + module.validate_restore(41, 902, token) + status.payload["pid"] = 77 + status.payload["start_time"] = 903 + module.observe_replacement(41, 902, token) + original_wait_for_release = module._wait_for_release_disposition + original_write_owned_drain = module._write_owned_drain + def fail_release(*_args, **_kwargs): + raise module.ControlError("simulated replacement release failure") + def fail_rollback(*_args, **_kwargs): + raise module.ControlError("simulated marker rollback failure") + module._wait_for_release_disposition = fail_release + module._write_owned_drain = fail_rollback + try: + module.complete_replacement(41, 902, 77, 903, token) + except module.ControlError as error: + if error.code != module.DRAIN_MARKER_ROLLBACK_FAILED_CODE: + raise + module._emit_control_error(error) + else: + raise AssertionError("release rollback unexpectedly succeeded") + finally: + module._wait_for_release_disposition = original_wait_for_release + module._write_owned_drain = original_write_owned_drain + module.recover_drain() elif scenario == "recover": module.begin_drain() status.payload["pid"] = 77 @@ -284,6 +452,12 @@ finally: "OWN_MARKER:" + ("present" if module._marker_path().exists() else "absent") ) + print( + "RECOVERY_STATE:" + + ("present" if module._release_recovery_path().exists() else "absent") + ) + print(f"CRON_VALIDATIONS:{cron_validations}") + print(f"DURABILITY_SYNCS:{durability_sync_calls}") if drain.marker is not None: print("FINAL_MARKER:" + drain.marker["principal"]) `; @@ -354,6 +528,24 @@ describe("Hermes in-sandbox cron restore validator", () => { | "complete-release-substitution" | "complete-release-failure" | "complete-release-rollback-failure" + | "complete-durable-order" + | "release-recovery-sync-failure" + | "existing-recovery-sync-failure" + | "drain-unlink-sync-failure" + | "recovery-unlink-sync-failure" + | "rollback-publication-sync-failure" + | "prepare-recovery-only" + | "prepare-matching" + | "prepare-matching-sync-failure" + | "prepare-noop" + | "prepare-existing-sync-failure" + | "prepare-mismatch" + | "prepare-recovery-unsafe-mode" + | "prepare-recovery-symlink" + | "prepare-recovery-hardlink" + | "pending-release-recovery" + | "mismatched-release-recovery" + | "recover-release-rollback" | "recover" | "recover-operator" | "recover-noop", @@ -677,6 +869,212 @@ describe("Hermes in-sandbox cron restore validator", () => { message: "Hermes cron restore drain release failed and its marker could not be restored", }); expect(result.stdout).toContain("OWN_MARKER:absent"); + expect(result.stdout).toContain("RECOVERY_STATE:present"); + }); + + it("durably publishes recovery authority before deleting the drain marker (#8472)", () => { + const result = runLifecycle("complete-durable-order"); + + expect(result.status).toBe(0); + expect(result.stderr).toBe(""); + expect(result.stdout).toContain( + "RELEASE_EVENTS:recovery-write-started,recovery-write-durable,drain-delete-started,drain-delete-durable", + ); + expect(result.stdout).toContain("OWN_MARKER:absent"); + expect(result.stdout).toContain("RECOVERY_STATE:absent"); + }); + + it("keeps the active marker when recovery-record durability fails (#8472)", () => { + const result = runLifecycle("release-recovery-sync-failure"); + + expect(result.status).toBe(1); + expect(result.stderr).toContain("simulated state directory durability failure"); + expect(result.stdout).toContain("OWN_MARKER:present"); + expect(result.stdout).toContain("RECOVERY_STATE:present"); + expect(result.stdout).toContain("DURABILITY_SYNCS:1"); + expect(result.stdout).not.toContain('"action":"complete"'); + }); + + it("rechecks existing recovery-record durability before marker deletion (#8472)", () => { + const result = runLifecycle("existing-recovery-sync-failure"); + + expect(result.status).toBe(1); + expect(result.stderr).toContain("simulated state directory durability failure"); + expect(result.stdout).toContain("OWN_MARKER:present"); + expect(result.stdout).toContain("RECOVERY_STATE:present"); + expect(result.stdout).toContain("DURABILITY_SYNCS:1"); + expect(result.stdout).not.toContain('"action":"complete"'); + }); + + it("restores the marker when its durable deletion cannot be proved (#8472)", () => { + const result = runLifecycle("drain-unlink-sync-failure"); + + expect(result.status).toBe(1); + expect(result.stderr).toContain("simulated state directory durability failure"); + expect(result.stdout).toContain("OWN_MARKER:present"); + expect(result.stdout).toContain("RECOVERY_STATE:present"); + expect(result.stdout).toContain("DURABILITY_SYNCS:3"); + expect(result.stdout).not.toContain('"action":"complete"'); + }); + + it("restores the marker when recovery-state deletion is not durable (#8472)", () => { + const result = runLifecycle("recovery-unlink-sync-failure"); + + expect(result.status).toBe(1); + expect(result.stderr).toContain("release recovery could not be cleared"); + expect(result.stdout).toContain("OWN_MARKER:present"); + expect(result.stdout).toContain("RECOVERY_STATE:absent"); + expect(result.stdout).toContain("DURABILITY_SYNCS:4"); + expect(result.stdout).not.toContain('"action":"complete"'); + }); + + it("does not report success when rollback publication is not durable (#8472)", () => { + const result = runLifecycle("rollback-publication-sync-failure"); + + expect(result.status).toBe(1); + expect(result.stderr).toContain("drain release failed and its marker could not be restored"); + expect(result.stderr).toContain( + `"code":"${HERMES_CRON_RESTORE_DRAIN_MARKER_ROLLBACK_FAILED_CODE}"`, + ); + expect(result.stdout).toContain("OWN_MARKER:present"); + expect(result.stdout).toContain("RECOVERY_STATE:present"); + expect(result.stdout).toContain("DURABILITY_SYNCS:3"); + expect(result.stdout).not.toContain('"action":"complete"'); + }); + + it("reacquires recovery authority without touching the gateway or cron tree (#8472)", () => { + const result = runLifecycle("prepare-recovery-only"); + + expect(result.status).toBe(0); + expect(result.stderr).toBe(""); + expect(result.stdout).toContain('"action":"prepare-recover"'); + expect(result.stdout).toContain('"disposition":"gate-prepared"'); + expect(result.stdout).toContain('"drain_acquired":true'); + expect(result.stdout).toContain("OWN_MARKER:present"); + expect(result.stdout).toContain("RECOVERY_STATE:present"); + expect(result.stdout).toContain("CRON_VALIDATIONS:0"); + }); + + it("keeps matching prepared recovery authority idempotent (#8472)", () => { + const result = runLifecycle("prepare-matching"); + + expect(result.status).toBe(0); + expect(result.stderr).toBe(""); + expect(result.stdout).toContain('"disposition":"gate-prepared"'); + expect(result.stdout).toContain("OWN_MARKER:present"); + expect(result.stdout).toContain("RECOVERY_STATE:present"); + expect(result.stdout).toContain("CRON_VALIDATIONS:0"); + }); + + it("blocks gateway preparation when matching recovery authority durability is unproved (#8472)", () => { + const result = runLifecycle("prepare-matching-sync-failure"); + + expect(result.status).toBe(1); + expect(result.stderr).toContain("simulated state directory durability failure"); + expect(result.stdout).not.toContain('"action":"prepare-recover"'); + expect(result.stdout).toContain("OWN_MARKER:present"); + expect(result.stdout).toContain("RECOVERY_STATE:present"); + expect(result.stdout).toContain("DURABILITY_SYNCS:1"); + expect(result.stdout).toContain("CRON_VALIDATIONS:0"); + }); + + it("returns a typed no-op when no recovery authority exists (#8472)", () => { + const result = runLifecycle("prepare-noop"); + + expect(result.status).toBe(0); + expect(result.stderr).toBe(""); + expect(result.stdout).toContain('"action":"prepare-recover"'); + expect(result.stdout).toContain('"disposition":"not-required"'); + expect(result.stdout).toContain('"drain_acquired":false'); + expect(result.stdout).toContain("OWN_MARKER:absent"); + expect(result.stdout).toContain("RECOVERY_STATE:absent"); + expect(result.stdout).toContain("CRON_VALIDATIONS:0"); + }); + + it("blocks gateway preparation when existing marker durability is unproved (#8472)", () => { + const result = runLifecycle("prepare-existing-sync-failure"); + + expect(result.status).toBe(1); + expect(result.stderr).toContain("simulated state directory durability failure"); + expect(result.stdout).not.toContain('"action":"prepare-recover"'); + expect(result.stdout).toContain("OWN_MARKER:present"); + expect(result.stdout).toContain("RECOVERY_STATE:absent"); + expect(result.stdout).toContain("DURABILITY_SYNCS:1"); + }); + + it("fails preparation when recovery owners differ (#8472)", () => { + const result = runLifecycle("prepare-mismatch"); + + expect(result.status).toBe(1); + expect(result.stderr).toContain("drain and release recovery ownership differ"); + expect(result.stdout).not.toContain('"action":"prepare-recover"'); + expect(result.stdout).toContain("OWN_MARKER:present"); + expect(result.stdout).toContain("RECOVERY_STATE:present"); + expect(result.stdout).toContain("CRON_VALIDATIONS:0"); + }); + + it.each([ + ["prepare-recovery-unsafe-mode", "metadata is unsafe"], + ["prepare-recovery-symlink", "is unreadable"], + ["prepare-recovery-hardlink", "metadata is unsafe"], + ] as const)("rejects unsafe recovery authority in %s (#8472)", (scenario, message) => { + const result = runLifecycle(scenario); + + expect(result.status).toBe(1); + expect(result.stderr).toContain(message); + expect(result.stdout).not.toContain('"action":"prepare-recover"'); + expect(result.stdout).toContain("OWN_MARKER:absent"); + expect(result.stdout).toContain("RECOVERY_STATE:present"); + expect(result.stdout).toContain("CRON_VALIDATIONS:0"); + }); + + it("blocks a new drain while release recovery remains pending (#8472)", () => { + const result = runLifecycle("pending-release-recovery"); + + expect(result.status).toBe(1); + expect(result.stderr).toContain("release recovery already requires recovery"); + expect(result.stdout).toContain("OWN_MARKER:absent"); + expect(result.stdout).toContain("RECOVERY_STATE:present"); + }); + + it("fails closed when the drain and release recovery owners differ (#8472)", () => { + const result = runLifecycle("mismatched-release-recovery"); + + expect(result.status).toBe(1); + expect(result.stderr).toContain("drain and release recovery ownership differ"); + expect(result.stdout).toContain("OWN_MARKER:present"); + expect(result.stdout).toContain("RECOVERY_STATE:present"); + expect(result.stdout).toContain("CRON_VALIDATIONS:0"); + }); + + it("reacquires and validates the gate from release recovery state (#8472)", () => { + const result = runLifecycle("recover-release-rollback"); + + expect(result.status).toBe(0); + expect(result.stderr).toContain( + "HERMES_CRON_RESTORE_ERROR: Hermes cron restore drain release failed and its marker could not be restored", + ); + const receipts = result.stdout + .split("\n") + .filter((line) => line.startsWith(RECEIPT_PREFIX)) + .map((line) => JSON.parse(line.slice(RECEIPT_PREFIX.length))); + expect(receipts.map((receipt) => receipt.action)).toEqual([ + "begin", + "validate", + "observe", + "recover", + ]); + expect(receipts.at(-1)).toEqual( + expect.objectContaining({ + active_jobs: 1, + disposition: "dispatch-reactivated", + profiles: 1, + script_jobs: 1, + }), + ); + expect(result.stdout).toContain("CRON_VALIDATIONS:3"); + expect(result.stdout).toContain("OWN_MARKER:absent"); + expect(result.stdout).toContain("RECOVERY_STATE:absent"); }); it("re-pins a restarted gateway before validating and reactivating dispatch", () => { diff --git a/test/hermes-final-image-layout.test.ts b/test/hermes-final-image-layout.test.ts index 9a46fb24b22..bd9275f1705 100644 --- a/test/hermes-final-image-layout.test.ts +++ b/test/hermes-final-image-layout.test.ts @@ -472,6 +472,9 @@ describe("Hermes final image layout", () => { expect(finalStage).toContain( "&& check_absent /sandbox/.nemoclaw/hermes-cron-restore-drain.json \\", ); + expect(finalStage).toContain( + "&& check_absent /sandbox/.nemoclaw/hermes-cron-restore-release-recovery.json \\", + ); expect(finalStage).toContain("&& check_absent /sandbox/.cache \\"); expect(finalStage).toContain("&& check_absent /sandbox/.hermes/managed-policy.json \\"); expect(finalStage).toContain("RUN chown root:root /sandbox/.nemoclaw \\");