diff --git a/agents/hermes/Dockerfile b/agents/hermes/Dockerfile index ac09dd6a791..3467f1f66d9 100644 --- a/agents/hermes/Dockerfile +++ b/agents/hermes/Dockerfile @@ -584,7 +584,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=e8593cf1580bffa4663e91c079ba0ce31c3d26391f5b1718872701138ce250b0 # hadolint ignore=DL4006 RUN printf '%s %s\n' \ "$NEMOCLAW_HERMES_VALIDATOR_SHA256" /usr/local/lib/nemoclaw/validate-hermes-env-secret-boundary.py \ @@ -1189,6 +1189,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 3f334aecc65..9d4af474166 100644 --- a/agents/hermes/cron-restore-control.py +++ b/agents/hermes/cron-restore-control.py @@ -3,10 +3,17 @@ """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 +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. + +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 @@ -30,7 +37,11 @@ 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" +DRAIN_MARKER_ROLLBACK_FAILED_CODE = "drain-marker-rollback-failed" BEGIN_TIMEOUT_SECONDS = 60.0 RELEASE_TIMEOUT_SECONDS = 15.0 POLL_SECONDS = 0.1 @@ -43,11 +54,33 @@ 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 +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") @@ -66,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() @@ -95,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 @@ -103,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 @@ -143,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}, @@ -166,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) @@ -179,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) @@ -198,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]]: @@ -423,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): @@ -480,7 +658,19 @@ def _complete_release( drain_token: str, **fields: Any, ) -> None: - _remove_owned_drain(drain_token) + _require_drained_idle(status_module, pid, start_time) + _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, @@ -490,14 +680,29 @@ 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" + "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 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, @@ -511,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) @@ -554,18 +786,63 @@ def validate_restore(pid: int, start_time: int, drain_token: str) -> None: ) -def release_drain(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) - _require_drained_idle(status_module, pid, start_time) + _, 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( + 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( - "release", + "complete", drain_control, status_module, - pid=pid, - start_time=start_time, + pid=replacement_pid, + start_time=replacement_start_time, drain_token=drain_token, + **counts, ) @@ -573,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( @@ -613,12 +890,18 @@ 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", "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) 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) @@ -630,17 +913,27 @@ 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": validate_restore(args.pid, args.start_time, args.drain_token) - elif args.action == "release": - release_drain(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.replacement_pid, + args.replacement_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/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx b/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx index 44405846a1e..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. @@ -221,9 +222,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. @@ -234,12 +236,18 @@ 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. -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. +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 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, 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. -After you correct the reported restore problem, run `$$nemoclaw recover` to validate the restored cron tree and clear the NemoClaw gate. +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 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 d3195169f75..0c4029f2038 100644 --- a/src/lib/actions/sandbox/rebuild-hermes-cron-restore.test.ts +++ b/src/lib/actions/sandbox/rebuild-hermes-cron-restore.test.ts @@ -29,13 +29,18 @@ vi.mock("../../sandbox/privileged-exec", async (importOriginal) => ({ import { beginHermesCronRestore, + completeHermesCronRestoreAfterGatewayReplacement, + HERMES_CRON_RESTORE_DRAIN_MARKER_ROLLBACK_FAILED_CODE, + isHermesCronRestoreDrainMarkerRollbackFailure, + observeHermesCronReplacement, + prepareHermesCronRestoreRecovery, 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 }); @@ -47,7 +52,7 @@ function writeScript(target: string): void { writeFileSync(target, "print('ok')\n", { mode: 0o600 }); } -type ReceiptAction = "begin" | "validate" | "release" | "recover"; +type ReceiptAction = "begin" | "validate" | "observe" | "complete" | "recover"; function receipt( action: ReceiptAction, @@ -69,11 +74,19 @@ function receipt( profiles: 1, script_jobs: 1, }, - release: { + observe: { active_agents: 0, + disposition: "replacement-observed", + operator_drain_active: false, + }, + complete: { + active_agents: 0, + active_jobs: 1, disposition: "dispatch-reactivated", operator_drain_active: false, preserved_drain: false, + profiles: 1, + script_jobs: 1, }, recover: { active_agents: 0, @@ -97,6 +110,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, @@ -112,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; @@ -174,22 +214,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", @@ -202,7 +237,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", () => { @@ -220,18 +254,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, @@ -246,29 +268,190 @@ 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") - ? "validate" - : argv.includes("release") - ? "release" - : "begin"; + const action = argv.includes("validate") ? "validate" : "begin"; events.push(action); 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", + }, + { pid: 77, start_time: 903, 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", + "--replacement-pid", + "77", + "--replacement-start-time", + "903", + ], + 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", + }, + { 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, + }, + { pid: 77, start_time: 903, drain_token: "restore-token" }, + ), + ).toThrow("requires the held drain token"); + expect(processMocks.dockerSpawnSync).not.toHaveBeenCalled(); + }); + + it("rejects completion when the replacement carries 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("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, + stdout: receipt("complete", 77, 903, "restore-token", { active_agents: 1 }), + stderr: "", + }); + + expect(() => + 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", + }), + ).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([ ["dispatch-reactivated", false], ["operator-drain-preserved", true], @@ -297,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.test.ts b/src/lib/actions/sandbox/rebuild-hermes-post-restore.test.ts index b0f37168115..1a0176cedd9 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,164 @@ 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, + 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(); + }); + + 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("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() + .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", () => { it("accepts a gateway that becomes healthy after an inconclusive recovery check (#7084)", () => { @@ -26,6 +183,7 @@ describe("Hermes gateway post-restore recheck", () => { expect( ensureHermesGatewayAfterStateRestore("alpha", "hermes", { + restartSandboxGateway: () => RESTART_SUCCEEDED, checkAndRecoverSandboxProcesses, }), ).toBe("healthy"); @@ -47,6 +205,7 @@ describe("Hermes gateway post-restore recheck", () => { expect( ensureHermesGatewayAfterStateRestore("alpha", "hermes", { + restartSandboxGateway: () => RESTART_SUCCEEDED, checkAndRecoverSandboxProcesses, }), ).toBe("unverified"); @@ -63,6 +222,7 @@ describe("Hermes gateway post-restore recheck", () => { expect( ensureHermesGatewayAfterStateRestore("alpha", "hermes", { + restartSandboxGateway: () => RESTART_SUCCEEDED, checkAndRecoverSandboxProcesses, }), ).toBe("unverified"); @@ -264,6 +424,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 5146ac599f0..da68305db87 100644 --- a/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts +++ b/src/lib/actions/sandbox/rebuild-hermes-post-restore.ts @@ -3,20 +3,31 @@ 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"; 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" | "release" | "recover"; +type HermesCronRestoreAction = + | "begin" + | "validate" + | "observe" + | "complete" + | "prepare-recover" + | "recover"; +type HermesCronRestoreReceiptAction = Exclude; type HermesCronRestoreDisposition = | "drain-acquired" | "restore-validated" + | "replacement-observed" | "dispatch-reactivated" | "operator-drain-preserved" | "not-required"; @@ -37,17 +48,24 @@ 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" | "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"); @@ -75,37 +93,123 @@ interface HermesPostRestoreGatewayDeps { sandboxName: string, options: { quiet: boolean }, ) => GatewayRecoveryObservation; + restartSandboxGateway?: ( + sandboxName: string, + options: { quiet: boolean }, + ) => GatewayRestartResult; + observeHermesCronReplacement?: ( + sandboxName: string, + originalIdentity: HermesCronRestoreIdentity, + ) => HermesCronRestoreIdentity; +} + +export interface HermesPostRestoreGatewayVerification { + state: HermesPostRestoreGatewayState; + replacementIdentity?: HermesCronRestoreIdentity; } /** - * Re-prove Hermes gateway health after workspace state restoration. + * Bind the running Hermes gateway to the state this rebuild just restored. * - * 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. + * 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. + * + * 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, 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; - if (observation.wasRunning === true) return "healthy"; - if (observation.recovered) return "recovered"; + // 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) { + 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( @@ -115,7 +219,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`, ); } @@ -146,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) { @@ -211,15 +315,28 @@ function parseCronRestoreReceipt( "script_jobs", ]); break; - case "release": + case "observe": actionValid = receipt.drain_acquired === true && + receipt.disposition === "replacement-observed" && receipt.active_agents === 0 && + hasExactReceiptFields(receipt, [...baseFields, ...tokenFields, "active_agents"]); + 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; @@ -255,35 +372,117 @@ function parseCronRestoreReceipt( return receipt as unknown as HermesCronRestoreReceipt; } -class HermesCronRestoreControlFailure extends Error { - constructor( - action: HermesCronRestoreAction, - readonly stderr: string, +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) + .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; } } -function runCronRestoreControl( +export function isHermesCronRestoreDrainMarkerRollbackFailure(error: unknown): boolean { + return ( + error instanceof HermesCronRestoreControlFailure && + error.action === "complete" && + error.controlCode === HERMES_CRON_RESTORE_DRAIN_MARKER_ROLLBACK_FAILED_CODE + ); +} + +function executeCronRestoreControl( sandboxName: string, action: HermesCronRestoreAction, identity?: HermesCronRestoreIdentity, -): HermesCronRestoreReceipt { + replacementIdentity?: HermesCronRestoreIdentity, +): string { 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 === "recover" || action === "complete" ? RECOVERY_TIMEOUT_MS : CONTROL_TIMEOUT_MS, ); @@ -299,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 { @@ -325,35 +536,95 @@ export function validateHermesCronRestore( } } -export function releaseHermesCronRestore( +export function completeHermesCronRestoreAfterGatewayReplacement( sandboxName: string, - identity: HermesCronRestoreIdentity, -): void { - const receipt = runCronRestoreControl(sandboxName, "release", identity); + originalIdentity: HermesCronRestoreIdentity, + verifiedReplacementIdentity: HermesCronRestoreIdentity, +): HermesCronRestoreIdentity { + if (!originalIdentity.drain_token) { + throw new Error("Hermes cron completion requires the held drain token"); + } + 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.pid !== identity.pid || - receipt.start_time !== identity.start_time || - receipt.drain_token !== identity.drain_token + 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 || + sameGatewayIdentity(receipt, originalIdentity) ) { - throw new Error("Hermes cron release receipt changed gateway identity"); + throw new Error("Hermes cron observation did not bind to a 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 { +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 ( @@ -369,11 +640,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(); @@ -381,7 +649,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..edf08ee5189 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,21 @@ 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", + ).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, ); @@ -115,6 +130,234 @@ 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.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(() => { + 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( + 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.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(); + 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("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", + ); + vi.mocked( + rebuildHermesPostRestore.completeHermesCronRestoreAfterGatewayReplacement, + ).mockImplementation(() => { + throw rollbackFailure; + }); + vi.mocked( + rebuildHermesPostRestore.isHermesCronRestoreDrainMarkerRollbackFailure, + ).mockImplementation((error) => error === rollbackFailure); + const args = { + ...input(), + hermesCronRestoreIdentity: { + pid: 41, + start_time: 902, + drain_token: "restore-token", + }, + }; + + await runRebuildPostRestorePhase(args); + + expect(args.bail).toHaveBeenCalledWith( + "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("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( + rebuildHermesPostRestore.isHermesCronRestoreDrainMarkerRollbackFailure, + ).toHaveBeenCalledWith(rollbackFailure); + }); + + 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("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 6f24c0b4fbe..f002f8da7bd 100644 --- a/src/lib/actions/sandbox/rebuild-post-restore-phase.ts +++ b/src/lib/actions/sandbox/rebuild-post-restore-phase.ts @@ -18,7 +18,11 @@ import { refreshMutableOpenClawConfigHashAfterPostRestoreWrites } from "./rebuil import type { RebuildBail, RebuildLog } from "./rebuild-credential-preflight"; import type { RebuildSandboxEntry } from "./rebuild-flow-helpers"; import { + completeHermesCronRestoreAfterGatewayReplacement, ensureHermesGatewayAfterStateRestore, + ensureHermesGatewayAfterStateRestoreForCronGate, + type HermesCronRestoreIdentity, + isHermesCronRestoreDrainMarkerRollbackFailure, printHermesGatewayRestoreRecovery, } from "./rebuild-hermes-post-restore"; import { @@ -31,6 +35,7 @@ import { reapplyMessagingManifestAfterOpenClawDoctor } from "./rebuild-messaging import { reconcileStalePinnedSessionModelsAfterRebuild } from "./reconcile-session-models"; export { + type HermesCronRestoreIdentity, HermesCronRestoreIncompleteError, recoverHermesCronRestore, runHermesCronRestoreTransaction, @@ -45,6 +50,23 @@ export function printHermesCronRestoreRecoveryCommand( ); } +function bailAfterHermesCronRestoreFailure( + sandboxName: string, + backupManifest: RebuildBackupManifest, + 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); +} + export interface RebuildPostRestorePhaseInput { sandboxName: string; sandboxEntry: RebuildSandboxEntry; @@ -53,6 +75,7 @@ export interface RebuildPostRestorePhaseInput { backupManifest: RebuildBackupManifest; mcpEntries: McpRebuildPreparation["entries"]; restoreSucceeded: boolean; + hermesCronRestoreIdentity?: HermesCronRestoreIdentity; backupWasForceSkipped: boolean; failedPresets: string[]; finalBuiltinPresets: string[]; @@ -155,6 +178,7 @@ export async function runRebuildPostRestorePhase( backupManifest, mcpEntries, restoreSucceeded, + hermesCronRestoreIdentity, backupWasForceSkipped, failedPresets, finalBuiltinPresets, @@ -182,6 +206,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 bailAfterHermesCronRestoreFailure( + 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; } @@ -251,13 +284,76 @@ 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) { + const replacementIdentity = hermesGatewayVerification.replacementIdentity; + if ( + hermesGatewayRestoreUnverified || + hermesGatewayRestoreState === "not-applicable" || + !replacementIdentity + ) { + return bailAfterHermesCronRestoreFailure( + 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, + mcpBridgeRestoreUnverified ? () => printMcpRestoreRecovery(sandboxName, true) : undefined, + ); + } + if (mcpBridgeRestoreUnverified) { + return bailAfterHermesCronRestoreFailure( + 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 { + completedIdentity = completeHermesCronRestoreAfterGatewayReplacement( + sandboxName, + hermesCronRestoreIdentity, + replacementIdentity, + ); + } catch (error) { + const errorDetail = error instanceof Error ? error.message : String(error); + if (isHermesCronRestoreDrainMarkerRollbackFailure(error)) { + return bailAfterHermesCronRestoreFailure( + sandboxName, + backupManifest, + ` 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, + ); + } + return bailAfterHermesCronRestoreFailure( + sandboxName, + backupManifest, + ` Hermes cron restore could not validate the replacement gateway and reactivate dispatch: ${errorDetail}`, + "Hermes cron restore validation failed; dispatch was not re-enabled.", + bail, + ); + } + log( + `Hermes cron restore gate released: pid=${String(completedIdentity.pid)}, startTime=${String(completedIdentity.start_time)}`, + ); + } 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/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/e2e/live/rebuild-hermes-cron-restore.ts b/test/e2e/live/rebuild-hermes-cron-restore.ts index 2039e2d0f51..d5472be24d8 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"), ); @@ -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/e2e/live/rebuild-hermes.test.ts b/test/e2e/live/rebuild-hermes.test.ts index a1d1ca61b9b..64636f66910 100644 --- a/test/e2e/live/rebuild-hermes.test.ts +++ b/test/e2e/live/rebuild-hermes.test.ts @@ -1216,6 +1216,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 124f6cff6e1..ea64af4382a 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"; import { makePreparedRecoveryManifest } from "../../src/lib/actions/sandbox/rebuild-flow-test-fixtures"; import { snapshotEnv } from "./rebuild-flow-test-support"; @@ -106,6 +107,7 @@ export type RebuildFlowOverrides = { secretBoundaryRefused?: boolean; mcpReconciliationRefused?: boolean; }; + restartSandboxGateway?: () => GatewayRestartResult; onboard?: (session: RebuildFlowSession) => Promise | void; repairMutableConfigPerms?: () => | { applied: false; skipReason: "agent" | "locked" | "unreadable"; reason: string } @@ -170,6 +172,7 @@ export type RebuildFlowHarness = { restoreTrustedAgentRemoteBaseImageOverrideSpy: MockInstance; executeSandboxCommandSpy: MockInstance; checkAndRecoverSandboxProcessesSpy: MockInstance; + restartSandboxGatewaySpy: MockInstance; ensureMessagingHostForwardAfterRebuildSpy: MockInstance; logSpy: MockInstance; finalizeIncompleteOnboardStepSpy: MockInstance; @@ -762,6 +765,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: [] })), ); @@ -818,6 +832,7 @@ export function createRebuildFlowHarness(overrides: RebuildFlowOverrides = {}): restoreTrustedAgentRemoteBaseImageOverrideSpy, executeSandboxCommandSpy, checkAndRecoverSandboxProcessesSpy, + restartSandboxGatewaySpy, ensureMessagingHostForwardAfterRebuildSpy, logSpy, finalizeIncompleteOnboardStepSpy, diff --git a/test/helpers/rebuild-flow-test-harness.ts b/test/helpers/rebuild-flow-test-harness.ts index c56310de928..665d84e8c12 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; diff --git a/test/hermes-cron-restore-control.test.ts b/test/hermes-cron-restore-control.test.ts index 4311b707116..065f834f612 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 @@ -41,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): @@ -116,24 +138,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 +177,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 +197,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 +207,227 @@ 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) + status.payload["pid"] = 77 + status.payload["start_time"] = 903 + 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 + 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, 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 + 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": + drain.marker = {"principal": "operator"} + 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": + drain.marker = {"principal": "operator"} + 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 == "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 == "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 @@ -198,7 +444,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}") @@ -206,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"]) `; @@ -269,6 +521,31 @@ describe("Hermes in-sandbox cron restore validator", () => { | "unsafe-lock-metadata" | "replacement-owned-marker" | "rollback-operator" + | "complete" + | "complete-same-identity" + | "complete-validation-failure" + | "complete-substitution" + | "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", @@ -363,7 +640,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(""); @@ -372,7 +649,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", @@ -498,6 +775,308 @@ 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", + "observe", + "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("OPERATOR_MUTATIONS:0:0"); + expect(result.stdout).toContain("FINAL_MARKER:operator"); + 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("OPERATOR_MUTATIONS:0:0"); + expect(result.stdout).toContain("FINAL_MARKER:operator"); + 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("OPERATOR_MUTATIONS:0:0"); + expect(result.stdout).toContain("FINAL_MARKER:operator"); + 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("OPERATOR_MUTATIONS:0:0"); + expect(result.stdout).toContain("FINAL_MARKER:operator"); + 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("OPERATOR_MUTATIONS:0:0"); + expect(result.stdout).toContain("FINAL_MARKER:operator"); + 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"); + 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", () => { const result = runLifecycle("recover"); diff --git a/test/hermes-final-image-layout.test.ts b/test/hermes-final-image-layout.test.ts index 1be381cb733..5c81777e55d 100644 --- a/test/hermes-final-image-layout.test.ts +++ b/test/hermes-final-image-layout.test.ts @@ -484,6 +484,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 \\");