diff --git a/hermes_cli/gateway_migrate.py b/hermes_cli/gateway_migrate.py index 9343c6b816..b851a6d623 100644 --- a/hermes_cli/gateway_migrate.py +++ b/hermes_cli/gateway_migrate.py @@ -465,6 +465,7 @@ def format_rollback_plan(default_home: Path, manifest: dict, *, dry_run: bool) - head = "Rollback plan (dry run — nothing changed)" if dry_run else "Rollback plan" lines = [head, f" default home: {default_home}", "", " Steps:"] lines.append(" - default: restore gateway.multiplex_profiles to its pre-migration value") + lines.append(" - default: clear multiplex-owned runtime status") for rec in manifest.get("secondaries", []): if not isinstance(rec, dict): continue @@ -478,7 +479,6 @@ def format_rollback_plan(default_home: Path, manifest: dict, *, dry_run: bool) - action = "restore its recorded standalone gateway" lines.append(f" - {name}: {action}") lines += [ - " - default: clear multiplex-owned runtime status", f" - remove rollback manifest {default_home / MANIFEST_NAME}", " - default: restart the standalone gateway last", ] @@ -639,19 +639,25 @@ def rollback_migration(default_home: Optional[Path] = None) -> bool: "default", default_home, pid=_live_gateway_pid(default_home), service=(default_service["kind"], bool(default_service.get("system"))) if default_service else _installed_service(default_home), ) - ok = True - secondary_names: set[str] = set() - for rec in manifest.get("secondaries", []): - if not isinstance(rec, dict): - ok = False - print(" ✗ invalid secondary record in migration manifest") - continue + secondaries = [rec for rec in manifest.get("secondaries", []) if isinstance(rec, dict)] + ok = len(secondaries) == len(manifest.get("secondaries", [])) + if not ok: + print(" ✗ invalid secondary record in migration manifest") + # The live multiplexer's record still claims every secondary; a per-profile gateway started + # while it does is refused (exit 78, parked by RestartPreventExitStatus) — clear it FIRST. + try: + _reconcile_standalone_runtime(default_home, {str(rec.get("profile") or "") for rec in secondaries}) + print(" ✓ default: cleared multiplex-owned runtime status") + except Exception as exc: + print(f" ✗ default: could not clear multiplex-owned runtime status ({exc})") + print(f"⚠ Rollback incomplete; manifest kept at {_manifest_path(default_home)}.") + return False + for rec in secondaries: name = str(rec.get("profile") or "") try: home = Path(rec["home"]) if not name: raise ValueError("missing profile name") - secondary_names.add(name) service = rec.get("service") if service: kind, system = service["kind"], bool(service.get("system")) @@ -667,32 +673,27 @@ def rollback_migration(default_home: Optional[Path] = None) -> bool: except Exception as exc: ok = False print(f" ✗ {name or ''}: {exc}") - if not ok: - print(f"⚠ Rollback incomplete; manifest kept at {_manifest_path(default_home)}.") - return False - - try: - _reconcile_standalone_runtime(default_home, secondary_names) - print(" ✓ default: cleared multiplex-owned runtime status") + if ok: _manifest_path(default_home).unlink(missing_ok=True) - except Exception as exc: - print(f" ✗ default: could not finish rollback cleanup ({exc})") - print(f"⚠ Rollback incomplete; manifest kept at {_manifest_path(default_home)}.") - return False - + # The flag is already off, so the default must come back standalone even when a secondary + # failed (otherwise config and the live process disagree). The restart is LAST: from inside + # the gateway's cgroup a service-manager restart kills this process, so nothing after it runs. if default_gw.has_gateway: try: print(f" ✓ {_restart_default(default_gw, None, default_home)}") except Exception as exc: - try: - _write_manifest(default_home, manifest) - except Exception as manifest_exc: - print(f" ✗ default: could not restore rollback manifest ({manifest_exc})") + if ok: + try: + _write_manifest(default_home, manifest) + except Exception as manifest_exc: + print(f" ✗ default: could not restore rollback manifest ({manifest_exc})") + ok = False print(f" ✗ default: could not restart its standalone gateway ({exc})") - print(f"⚠ Rollback incomplete; manifest kept at {_manifest_path(default_home)}.") - return False - print("✓ Rolled back to per-profile gateways.") - return True + if ok: + print("✓ Rolled back to per-profile gateways.") + else: + print(f"⚠ Rollback incomplete; manifest kept at {_manifest_path(default_home)}.") + return ok # --------------------------------------------------------------------------- CLI + update hook diff --git a/tests/hermes_cli/test_gateway_migrate_multiplex.py b/tests/hermes_cli/test_gateway_migrate_multiplex.py index b9de6edaa9..ea3141280c 100644 --- a/tests/hermes_cli/test_gateway_migrate_multiplex.py +++ b/tests/hermes_cli/test_gateway_migrate_multiplex.py @@ -39,6 +39,7 @@ def fleet(tmp_path, monkeypatch): services={"coder": ("systemd", False), "ops": ("systemd", False)}, pids={"coder": 4101, "ops": 4102}, ops=[], + refused_at_start={}, ) def _name(home: Path) -> str: @@ -47,6 +48,11 @@ def fleet(tmp_path, monkeypatch): def _service_op(kind, system, verb, home): name = _name(home) state.ops.append((name, verb)) + if verb == "start" and name != "default": + # What the real `hermes -p gateway run` checks first: is a live multiplexer + # still recorded as serving me? (exit 78 if so — the unit is then parked for good). + from hermes_cli.gateway import named_profile_served_by_running_multiplexer + state.refused_at_start[name] = named_profile_served_by_running_multiplexer(name) if verb == "uninstall": state.services.pop(name, None) elif verb == "install": @@ -128,6 +134,8 @@ def test_apply_records_manifest_flips_flag_and_rollback_restores(fleet, capsys): assert [op for op in fleet.ops if op[0] != "default"] == [ ("coder", "install"), ("coder", "start"), ("ops", "install"), ("ops", "start")] assert fleet.ops[-1] == ("default", "restart") + # Each secondary's own gateway must have been startable at the moment it was started. + assert fleet.refused_at_start == {"coder": False, "ops": False} runtime = json.loads(runtime_path.read_text(encoding="utf-8")) assert runtime["served_profiles"] == [] assert runtime["platforms"] == {"telegram": {"state": "connected"}} @@ -136,6 +144,29 @@ def test_apply_records_manifest_flips_flag_and_rollback_restores(fleet, capsys): assert not (fleet.root / gm.MANIFEST_NAME).exists() +def test_rollback_with_failed_secondary_still_restarts_default_and_keeps_manifest(fleet, monkeypatch): + assert gm.apply_migration(gm.build_migration_plan(), served_wait=5.0) is True + fleet.ops.clear() + real_op = gm._service_op + + def _flaky(kind, system, verb, home): + if verb == "start" and _name_of(home) == "coder": + raise RuntimeError("systemctl start failed") + real_op(kind, system, verb, home) + + monkeypatch.setattr(gm, "_service_op", _flaky) + assert gm.rollback_migration(fleet.root) is False + # The flag is off, so the default must not be left multiplexing; the manifest stays for a re-run. + assert _config_flag(fleet.root) is False + assert fleet.ops[-1] == ("default", "restart") + assert ("ops", "start") in fleet.ops + assert (fleet.root / gm.MANIFEST_NAME).exists() + + +def _name_of(home: Path) -> str: + return hermes_constants.profile_name_for_home(home) or "default" + + def test_standalone_dry_run_prints_rollback_plan_without_mutation(fleet, capsys): assert gm.apply_migration(gm.build_migration_plan(), served_wait=5.0) is True capsys.readouterr()