From 8c82e93132879583eb7cbd62265167a6dfbfaffb Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Mon, 14 Sep 2026 10:35:21 -0700 Subject: [PATCH] fix(gateway): migrate --multiplex rolls back or resumes when the default cannot come up; every installed unit counts `hermes gateway migrate --multiplex` ran its one fallible step LAST (install + start the default gateway) with nothing around it. On a fleet whose secondary ran a system unit as root (#110850) that step raised, leaving the flag on, the secondary's unit removed and no gateway anywhere, and the re-run hit the "already multiplexing (flag on)" short-circuit over an empty fleet. - apply_migration(): the default bring-up runs inside a rollback. On failure the manifest written before the first destructive step restores the flag and reinstalls every recorded per-profile gateway with its recorded User=. - MigrationPlan.interrupted: flag on + manifest present + no live default gateway is a half-applied migration, not "already multiplexed"; the re-run resumes from the manifest (target manager and User= read from it, since the units themselves are gone) instead of refusing. Flag off + leftover manifest refuses to overwrite it and points at --standalone. - ProfileGateway.services records EVERY installed unit (user and system) and the manifest carries them; apply stops/uninstalls all of them and rollback reinstalls all of them, so a second owner is never left live beside the multiplexer. The unattended hook treats a two-unit profile as an ambiguous topology and refuses (review finding on #110205). - gateway_identity(): an unresolvable User= on a system unit stays None instead of borrowing the profile directory's owner; the unattended hook treats the unknown principal as a boundary (review finding on #110205). - auto_migration_opted_out(): reads the effective config (load_config_readonly under the default home), so a managed `false` wins over a user `true` and a YAML string "false" is an opt-out, not a truthy value (review finding on #110205). Builds on KoNit-K's #110854 (run_as_user threaded through install, preserved from the removed system unit). --- hermes_cli/gateway_migrate.py | 195 ++++++++++++------ hermes_cli/gateway_migrate_guards.py | 63 +++--- .../test_gateway_migrate_multiplex.py | 147 ++++++++++++- .../hermes_cli/test_profile_clone_channels.py | 2 +- .../docs/user-guide/multi-profile-gateways.md | 18 +- 5 files changed, 334 insertions(+), 91 deletions(-) diff --git a/hermes_cli/gateway_migrate.py b/hermes_cli/gateway_migrate.py index 5e88e7a485..bd1c7ae860 100644 --- a/hermes_cli/gateway_migrate.py +++ b/hermes_cli/gateway_migrate.py @@ -36,7 +36,9 @@ class ProfileGateway: name: str home: Path pid: Optional[int] = None - service: Optional[tuple[str, bool]] = None # ("systemd", system) | ("launchd", False) + # EVERY installed unit: [("systemd", system), ("launchd", False)]. A profile can carry a user AND a + # system unit at once; recording only the first found left the other live beside the multiplexer. + services: list[tuple[str, bool]] = field(default_factory=list) run_as_user: Optional[str] = None # User= recorded by a system-scope systemd unit uid: Optional[int] = None # owner of the gateway process/unit; None = unknown (never "different") runtime_home: Optional[Path] = None # HERMES_HOME the installed unit pins, when it differs from ``home`` @@ -45,31 +47,50 @@ class ProfileGateway: def is_default(self) -> bool: return self.name == "default" + @property + def service(self) -> Optional[tuple[str, bool]]: + """The unit the default gateway is (re)started through; None when nothing is installed.""" + return self.services[0] if self.services else None + @property def has_gateway(self) -> bool: - return self.pid is not None or self.service is not None + return self.pid is not None or bool(self.services) + + @property + def has_system_unit(self) -> bool: + return ("systemd", True) in self.services def service_label(self) -> str: - if self.service is None: - return "none" - kind, system = self.service - return f"{kind} ({'system' if system else 'user'})" if kind == "systemd" else kind + return " + ".join(_service_label(s) for s in self.services) if self.services else "none" def to_dict(self) -> dict: return { "profile": self.name, "home": str(self.home), "pid": self.pid, - "service": None if self.service is None else {"kind": self.service[0], "system": self.service[1]}, + "service": None if self.service is None else _service_dict(self.service), + "services": [_service_dict(s) for s in self.services], "run_as_user": self.run_as_user, "uid": self.uid, "runtime_home": None if self.runtime_home is None else str(self.runtime_home), } +def _service_label(service: tuple[str, bool]) -> str: + kind, system = service + return f"{kind} ({'system' if system else 'user'})" if kind == "systemd" else kind + + +def _service_dict(service: tuple[str, bool]) -> dict: + return {"kind": service[0], "system": service[1]} + + @dataclass class MigrationPlan: default_home: Path profiles: list[ProfileGateway] multiplex_flag_on: bool live_served: Optional[list[str]] # served_profiles the live default gateway recorded, if any + # A manifest with the flag on and no live default gateway: an earlier apply died between flipping + # the flag and bringing the multiplexer up (#110850). Not "already multiplexed" — resumable. + interrupted: bool = False blockers: list[str] = field(default_factory=list) notices: list[str] = field(default_factory=list) @@ -83,6 +104,8 @@ class MigrationPlan: @property def already_multiplexed(self) -> bool: + if self.interrupted: + return False return self.multiplex_flag_on or bool(self.live_served and len(self.live_served) > 1) @property @@ -113,6 +136,7 @@ class MigrationPlan: "multiplex_flag_on": self.multiplex_flag_on, "live_served": self.live_served, "already_multiplexed": self.already_multiplexed, + "interrupted": self.interrupted, "blockers": list(self.blockers), "notices": list(self.notices), "eligible": self.eligible_for_migration(), @@ -173,27 +197,26 @@ def _live_gateway_pid(home: Path) -> Optional[int]: return None -def _gateway_identity(home: Path, pid: Optional[int], service: Optional[tuple[str, bool]]) -> tuple[Optional[int], Path]: +def _gateway_identity(home: Path, pid: Optional[int], services: list[tuple[str, bool]]) -> tuple[Optional[int], Path]: from hermes_cli.gateway_migrate_guards import gateway_identity - return gateway_identity(home, pid, service) + return gateway_identity(home, pid, services) -def _installed_service(home: Path) -> Optional[tuple[str, bool]]: - """Installed service kind for ``home``'s gateway (unit / plist on disk), else None.""" +def _installed_services(home: Path) -> list[tuple[str, bool]]: + """Every installed service for ``home``'s gateway (units / plist on disk), user scope first.""" from hermes_cli import gateway as gw + found: list[tuple[str, bool]] = [] with _home_env(home): if gw.supports_systemd_services(): - for system in (False, True): - if gw.get_systemd_unit_path(system=system).exists(): - return ("systemd", system) + found.extend(("systemd", system) for system in (False, True) if gw.get_systemd_unit_path(system=system).exists()) if gw.is_macos() and gw.get_launchd_plist_path().exists(): - return ("launchd", False) - return None + found.append(("launchd", False)) + return found -def _systemd_service_user(home: Path, service: Optional[tuple[str, bool]]) -> Optional[str]: +def _systemd_service_user(home: Path, services: list[tuple[str, bool]]) -> Optional[str]: """Read ``User=`` before migration removes a system-scope unit.""" - if service != ("systemd", True): + if ("systemd", True) not in services: return None from hermes_cli import gateway as gw with _home_env(home): @@ -416,16 +439,17 @@ def build_migration_plan() -> MigrationPlan: default_home = _default_home() profiles = [] for name, home in _profile_homes(): - pid, service = _live_gateway_pid(home), _installed_service(home) - uid, runtime_home = _gateway_identity(home, pid, service) - profiles.append(ProfileGateway(name=name, home=home, pid=pid, service=service, - run_as_user=_systemd_service_user(home, service), uid=uid, + pid, services = _live_gateway_pid(home), _installed_services(home) + uid, runtime_home = _gateway_identity(home, pid, services) + profiles.append(ProfileGateway(name=name, home=home, pid=pid, services=services, + run_as_user=_systemd_service_user(home, services), uid=uid, runtime_home=None if runtime_home == home else runtime_home)) plan = MigrationPlan( default_home=default_home, profiles=profiles, multiplex_flag_on=_read_multiplex_flag(default_home), live_served=recorded_served_profiles(default_home), ) + plan.interrupted = plan.multiplex_flag_on and not plan.default.has_gateway and _read_manifest(default_home) is not None if len(plan.profiles) < 2: plan.notices.append("Only one profile exists: nothing to multiplex.") return plan @@ -461,9 +485,12 @@ def format_plan(plan: MigrationPlan, *, dry_run: bool) -> list[str]: lines.append(" ✓ The default gateway is already multiplexing" + (f" (serving {', '.join(plan.live_served)})" if plan.live_served else " (flag on)") + ".") return lines + if plan.interrupted: + lines.append(f" ↻ An earlier migration was interrupted before the default gateway came up " + f"(flag on, no gateway; manifest {plan.default_home / MANIFEST_NAME}); this run resumes it.") steps = [] for p in plan.standalone_secondaries: - what = " + ".join(x for x in (f"stop pid {p.pid}" if p.pid else "", f"uninstall {p.service_label()}" if p.service else "") if x) + what = " + ".join(x for x in (f"stop pid {p.pid}" if p.pid else "", f"uninstall {p.service_label()}" if p.services else "") if x) steps.append(f" - {p.name}: {what}") if len(plan.profiles) < 2: # the notice already says "only one profile exists" return lines + _plan_tail(plan) @@ -503,13 +530,36 @@ def _manifest_secondaries(manifest: dict) -> Optional[list[dict]]: return recs -def _secondary_service(rec: dict) -> Optional[tuple[str, bool]]: - service = rec.get("service") +def _service_from_dict(service) -> Optional[tuple[str, bool]]: if isinstance(service, dict) and service.get("kind"): return str(service["kind"]), bool(service.get("system")) return None +def _recorded_services(rec: dict) -> list[tuple[str, bool]]: + """Every service a manifest record names: ``services`` (all installed units) or, in a manifest + written before that key existed, the single ``service``.""" + recorded = rec.get("services") + if isinstance(recorded, list): + return [s for s in (_service_from_dict(r) for r in recorded) if s is not None] + service = _service_from_dict(rec.get("service")) + return [service] if service is not None else [] + + +def _recorded_run_as_user(rec: dict) -> Optional[str]: + user = rec.get("run_as_user") + return user if isinstance(user, str) and user else None + + +def _target_from_manifest(manifest: dict) -> tuple[Optional[tuple[str, bool]], Optional[str]]: + """Service manager + ``User=`` the resumed default install should use, read from the recorded + footprint (the units themselves are gone by the time an interrupted apply is re-run).""" + recs = [r for r in (manifest.get("default"), *(_manifest_secondaries(manifest) or [])) if isinstance(r, dict)] + target = next((s for r in recs for s in _recorded_services(r)), None) + user = next((u for r in recs if (u := _recorded_run_as_user(r)) is not None), None) + return target, user + + _NOTHING_RECORDED = "no gateway was recorded; nothing to restore" @@ -522,9 +572,9 @@ def format_rollback_plan(default_home: Path, manifest: dict, *, dry_run: bool) - if secondaries is None: return lines + [f" ✗ malformed secondary records in {_manifest_path(default_home)}; fix or delete the manifest"] for rec in secondaries: - service = _secondary_service(rec) - if service is not None: - action = f"reinstall and start its {service[0]} service" + services = _recorded_services(rec) + if services: + action = "reinstall and start its " + " + ".join(_service_label(s) for s in services) + " service" elif rec.get("pid"): action = "start its standalone gateway (detached)" else: @@ -630,42 +680,72 @@ def _restart_default( return f"{verb} the default gateway (detached; no service manager was in use)" +def _remove_secondary_gateways(plan: MigrationPlan) -> None: + for p in plan.standalone_secondaries: + for kind, system in p.services: + _service_op(kind, system, "stop", p.home) + _service_op(kind, system, "uninstall", p.home) + print(f" ✓ {p.name}: stopped and removed its {_service_label((kind, system))} service") + if p.pid is not None: + _stop_gateway_process(p.home) + print(f" ✓ {p.name}: stopped standalone gateway (pid {p.pid})") + + def apply_migration(plan: MigrationPlan, *, served_wait: float = _SERVED_WAIT_SECONDS) -> bool: """Stop/uninstall every secondary gateway, flip the flag, bring up the multiplexer, verify. - Returns True when the multiplexer verifiably serves every profile.""" + Returns True when the multiplexer verifiably serves every profile. + + Bringing the default up is the one step that can fail after the destructive ones (a system unit + that needs ``--run-as-user``, an unreachable user bus). It runs inside a rollback: on failure the + manifest written before the first destructive step restores the flag and every recorded per-profile + gateway (#110850), so the fleet never ends with the flag on and no gateway at all.""" if plan.blocked: _print(["✗ Migration refused:", *[f" • {b}" for b in plan.blockers]]) return False if plan.already_multiplexed: print("✓ Already multiplexed — nothing to do.") return True - manifest = { - "version": 1, "migrated_at": time.strftime("%Y-%m-%dT%H:%M:%S%z"), - "flag_was": plan.multiplex_flag_on, - "default": plan.default.to_dict(), - "secondaries": [p.to_dict() for p in plan.standalone_secondaries], - } - # Recovery metadata must exist before the first destructive operation; the manifest never - # changes afterwards, so this is the only write it needs. - _write_manifest(plan.default_home, manifest) - for p in plan.standalone_secondaries: - if p.service is not None: - kind, system = p.service - _service_op(kind, system, "stop", p.home) - _service_op(kind, system, "uninstall", p.home) - print(f" ✓ {p.name}: stopped and removed its {p.service_label()} service") - if p.pid is not None: - _stop_gateway_process(p.home) - print(f" ✓ {p.name}: stopped standalone gateway (pid {p.pid})") + target, run_as_user = plan.target_service_kind(), plan.target_run_as_user() + if plan.interrupted: + # An earlier apply removed the secondaries and flipped the flag but never brought the default + # up; the manifest is the only record of the units that existed. Finish from it, don't rewrite it. + manifest = _read_manifest(plan.default_home) or {} + target, run_as_user = _target_from_manifest(manifest) + print(f" ↻ resuming an interrupted migration recorded in {_manifest_path(plan.default_home)}") + elif _read_manifest(plan.default_home) is not None: + # Flag off + manifest present = a rollback that did not finish. Overwriting the manifest would + # discard the only record of the units that rollback still has to restore. + _print([f"✗ A previous migration's manifest is still at {_manifest_path(plan.default_home)} (its rollback did not finish).", + " Finish it with: hermes gateway migrate --standalone (or delete the manifest to start over)"]) + return False + else: + manifest = { + "version": 1, "migrated_at": time.strftime("%Y-%m-%dT%H:%M:%S%z"), + "flag_was": plan.multiplex_flag_on, + "default": plan.default.to_dict(), + "secondaries": [p.to_dict() for p in plan.standalone_secondaries], + } + # Recovery metadata must exist before the first destructive operation; the manifest never + # changes afterwards, so this is the only write it needs. + _write_manifest(plan.default_home, manifest) + _remove_secondary_gateways(plan) _write_multiplex_flag(plan.default_home, True) print(f" ✓ default: gateway.multiplex_profiles: true ({plan.default_home / 'config.yaml'})") - print(f" ✓ {_restart_default(plan.default, plan.target_service_kind(), plan.default_home, run_as_user=plan.target_run_as_user())}") + try: + print(f" ✓ {_restart_default(plan.default, target, plan.default_home, run_as_user=run_as_user)}") + except Exception as exc: + _print([f" ✗ default: could not bring up the multiplexed gateway ({exc})", + " ↩ Rolling back to per-profile gateways so no profile is left without one..."]) + rolled_back = rollback_migration(plan.default_home) + if not rolled_back: + print(f" Re-run {MIGRATE_COMMAND} to resume, or hermes gateway migrate --standalone to roll back.") + return False expected = {p.name for p in plan.profiles} served = _wait_for_served(plan.default_home, expected, served_wait) if served is not None and expected <= set(served): _print(["", f"✓ Migrated: the default gateway now serves {len(served)} profiles: {', '.join(served)}", - f" Rollback any time with: hermes gateway migrate --standalone", + " Rollback any time with: hermes gateway migrate --standalone", *[f" • {n}" for n in plan.notices]]) return True missing = sorted(expected - set(served or [])) @@ -697,10 +777,9 @@ def rollback_migration(default_home: Optional[Path] = None) -> bool: return False default_rec = manifest.get("default") - default_service = _secondary_service(default_rec) if isinstance(default_rec, dict) else None default_gw = ProfileGateway( "default", default_home, pid=_live_gateway_pid(default_home), - service=default_service or _installed_service(default_home), + services=(_recorded_services(default_rec) if isinstance(default_rec, dict) else []) or _installed_services(default_home), ) # 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. @@ -715,14 +794,12 @@ def rollback_migration(default_home: Optional[Path] = None) -> bool: for rec in secondaries: name, home = str(rec["profile"]), Path(str(rec["home"])) try: - service = _secondary_service(rec) - if service is not None: - kind, system = service - run_as_user = rec.get("run_as_user") - _service_op(kind, system, "install", home, - run_as_user=run_as_user if isinstance(run_as_user, str) else None) - _service_op(kind, system, "start", home) - print(f" ✓ {name}: reinstalled and started its {kind} service") + services = _recorded_services(rec) + if services: + for kind, system in services: + _service_op(kind, system, "install", home, run_as_user=_recorded_run_as_user(rec)) + _service_op(kind, system, "start", home) + print(f" ✓ {name}: reinstalled and started its {_service_label((kind, system))} service") elif rec.get("pid"): if _live_gateway_pid(home) is not None: # re-run after a partial rollback print(f" ✓ {name}: standalone gateway already running") diff --git a/hermes_cli/gateway_migrate_guards.py b/hermes_cli/gateway_migrate_guards.py index db62a62f59..a7a04b3d99 100644 --- a/hermes_cli/gateway_migrate_guards.py +++ b/hermes_cli/gateway_migrate_guards.py @@ -49,28 +49,35 @@ def _system_unit_uid(unit_path: Path) -> Optional[int]: return None -def gateway_identity(home: Path, pid: Optional[int], service: Optional[tuple[str, bool]]) -> tuple[Optional[int], Path]: +def gateway_identity(home: Path, pid: Optional[int], services: list[tuple[str, bool]]) -> tuple[Optional[int], Path]: """``(uid, runtime_home)`` of the gateway that serves ``home``. - uid: the live process owner, else the system unit's ``User=``, else the owner of the profile - directory (user-scope systemd / launchd / detached gateways run as the account that owns it). - None means unknown — never a different user. runtime_home: the HERMES_HOME the installed unit - pins, which is where the gateway really runs; ``home`` when there is no unit or no pin. + uid: the live process owner; else, for a system unit, its ``User=`` — and ONLY that: a system + unit's principal is whatever systemd will run, so an unresolvable ``User=`` stays None rather than + borrowing the profile directory's owner (a stopped unit pinned to an absent NSS user is not the + account that owns the files). Without a system unit (user-scope systemd / launchd / detached), the + profile directory owner is the account the gateway runs as. None means unknown. runtime_home: the + HERMES_HOME an installed unit pins, which is where the gateway really runs; ``home`` otherwise. """ from hermes_cli.gateway import _hermes_home_pinned_by_unit, get_systemd_unit_path from hermes_cli.gateway_migrate import _home_env uid: Optional[int] = _pid_uid(pid) if pid is not None else None runtime_home = home - if service is not None and service[0] == "systemd": + has_system_unit = False + for kind, system in services: + if kind != "systemd": + continue with _home_env(home): - unit_path = get_systemd_unit_path(system=service[1]) + unit_path = get_systemd_unit_path(system=system) pinned = _hermes_home_pinned_by_unit(unit_path) - if pinned: + if pinned and runtime_home == home: runtime_home = Path(pinned).expanduser() - if uid is None and service[1]: - uid = _system_unit_uid(unit_path) - if uid is None: + if system: + has_system_unit = True + if uid is None: + uid = _system_unit_uid(unit_path) + if uid is None and not has_system_unit: with contextlib.suppress(OSError): uid = home.stat().st_uid return uid, runtime_home @@ -80,13 +87,17 @@ def gateway_identity(home: Path, pid: Optional[int], service: Optional[tuple[str def _service_label(profile: ProfileGateway) -> str: - return profile.service_label() if profile.service is not None else "no service manager (detached)" + return profile.service_label() if profile.services else "no service manager (detached)" def _guard_service_domain(plan: MigrationPlan, profile: ProfileGateway) -> Optional[str]: """Different manager or scope than the default gateway (system vs user systemd, launchd vs systemd, - or any service when the default is detached: the auto path never elects a secondary's manager).""" - if profile.service == plan.default.service: + or any service when the default is detached: the auto path never elects a secondary's manager). + Two units on one profile is an ambiguous topology the unattended path does not resolve either.""" + if len(profile.services) > 1: + return (f"Profile '{profile.name}' has more than one installed service ({profile.service_label()}): " + f"an ambiguous service topology is not folded automatically.") + if set(profile.services) == set(plan.default.services): return None return (f"Profile '{profile.name}' runs under {_service_label(profile)} while the default gateway " f"runs under {_service_label(plan.default)}: a different service domain is not folded automatically.") @@ -94,6 +105,10 @@ def _guard_service_domain(plan: MigrationPlan, profile: ProfileGateway) -> Optio def _guard_unix_user(plan: MigrationPlan, profile: ProfileGateway) -> Optional[str]: default_uid = plan.default.uid + if profile.uid is None and profile.has_system_unit: + # Unknown principal is not "same user": the unit names an account this host cannot resolve. + return (f"Profile '{profile.name}' runs a system unit whose User= cannot be resolved on this host: " + f"an unknown service principal is not folded automatically.") if default_uid is None or profile.uid is None or profile.uid == default_uid: return None return (f"Profile '{profile.name}' runs as uid {profile.uid} while the default gateway runs as uid " @@ -131,15 +146,15 @@ def auto_migration_blockers(plan: MigrationPlan) -> list[str]: def auto_migration_opted_out(default_home: Path) -> bool: - """``gateway.auto_multiplex_migration: false`` in the DEFAULT profile's config.yaml. Absent means - opted in (the ``DEFAULT_CONFIG`` value); only the nested key counts, there is no top-level alias.""" - cfg_path = default_home / "config.yaml" - if not cfg_path.exists(): - return False - from hermes_cli.config import read_user_config_raw - cfg = read_user_config_raw(cfg_path) or {} - gateway_section = cfg.get("gateway") + """``gateway.auto_multiplex_migration: false`` in the DEFAULT profile's EFFECTIVE config: the same + ``load_config`` the rest of the CLI reads (``DEFAULT_CONFIG`` + config.yaml + the managed overlay), so + an administrator's managed ``false`` wins over a user's ``true`` and a YAML string ``"false"`` is + false, not truthy. Only the nested key counts, there is no top-level alias.""" + from hermes_cli.config import load_config_readonly + from hermes_cli.gateway_migrate import _home_env + from utils import is_truthy_value + with _home_env(default_home): + gateway_section = load_config_readonly().get("gateway") if not isinstance(gateway_section, dict): return False - value = gateway_section.get("auto_multiplex_migration") - return value is not None and not bool(value) + return not is_truthy_value(gateway_section.get("auto_multiplex_migration"), default=True) diff --git a/tests/hermes_cli/test_gateway_migrate_multiplex.py b/tests/hermes_cli/test_gateway_migrate_multiplex.py index 55e9e65ce7..d1f1fe008f 100644 --- a/tests/hermes_cli/test_gateway_migrate_multiplex.py +++ b/tests/hermes_cli/test_gateway_migrate_multiplex.py @@ -1,6 +1,6 @@ """``hermes gateway migrate``: preflight verdicts, apply/rollback bookkeeping, and the update hook. -Service layer is faked through the module's ``_installed_service`` / ``_service_op`` seams (the same +Service layer is faked through the module's ``_installed_services`` / ``_service_op`` seams (the same shape ``hermes gateway install`` tests use); the default gateway boot is faked by writing the ``served_profiles`` record the real multiplexer writes. Blockers reuse the gateway's own credential fingerprint and port-binding predicates, so the tests assert verdict → effect, not internal lists. @@ -36,6 +36,7 @@ def fleet(tmp_path, monkeypatch): monkeypatch.setattr(hermes_constants, "_default_hermes_root_memo", None) state = SimpleNamespace( + # profile -> installed unit(s); a tuple is one unit, a list is every installed unit. services={"coder": ("systemd", False), "ops": ("systemd", False)}, pids={"coder": 4101, "ops": 4102}, ops=[], @@ -51,7 +52,11 @@ def fleet(tmp_path, monkeypatch): 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) + remaining = [u for u in _units(state.services.get(name)) if u != (kind, system)] + if remaining: + state.services[name] = remaining + else: + state.services.pop(name, None) elif verb == "install": state.services[name] = (kind, system) elif verb in ("start", "restart") and name == "default": @@ -68,7 +73,7 @@ def fleet(tmp_path, monkeypatch): import gateway.status as status # The default gateway the fixture "starts" is this process; the served probe verifies identity. monkeypatch.setattr(status, "_read_process_cmdline", lambda pid: "hermes gateway run") - monkeypatch.setattr(gm, "_installed_service", lambda home: state.services.get(_name(home))) + monkeypatch.setattr(gm, "_installed_services", lambda home: _units(state.services.get(_name(home)))) monkeypatch.setattr(gm, "_live_gateway_pid", lambda home: state.pids.get(_name(home))) monkeypatch.setattr(gm, "_service_op", _service_op) monkeypatch.setattr(gm, "_stop_gateway_process", lambda home: state.pids.pop(_name(home), None)) @@ -81,6 +86,12 @@ def _name(home: Path) -> str: return hermes_constants.profile_name_for_home(home) or "default" +def _units(recorded) -> list: + if recorded is None: + return [] + return list(recorded) if isinstance(recorded, list) else [recorded] + + def _config_flag(root: Path): import yaml raw = yaml.safe_load((root / "config.yaml").read_text(encoding="utf-8")) or {} @@ -154,7 +165,7 @@ def test_migration_preserves_root_system_service_user_for_default_install(fleet, monkeypatch.setattr( gm, "_systemd_service_user", - lambda home, service: "root" if _name(home) == "coder" and service == ("systemd", True) else None, + lambda home, services: "root" if _name(home) == "coder" and ("systemd", True) in services else None, ) plan = gm.build_migration_plan() root_secondary = next(p for p in plan.standalone_secondaries if p.name == "coder") @@ -425,3 +436,131 @@ def test_explicit_migrate_with_no_standalone_secondaries_still_flips_flag_and_re assert "serves 3 profiles" in out # The same manifest rolls it back: flag restored, nothing to reinstall. assert gm.rollback_migration(fleet.root) is True and _config_flag(fleet.root) is False + + +def test_failed_default_bringup_rolls_back_to_per_profile_gateways(fleet, monkeypatch, capsys): + """#110850: the last step (install/start the default) is the one that can fail after the destructive + ones. It must not leave the flag on with no gateway anywhere: the manifest rolls the fleet back.""" + real_op = gm._service_op + + def _refusing(kind, system, verb, home, *, run_as_user=None): + if verb == "install" and _name(home) == "default": + raise ValueError("Refusing to install the gateway system service as root; pass --run-as-user root") + real_op(kind, system, verb, home, run_as_user=run_as_user) + + monkeypatch.setattr(gm, "_service_op", _refusing) + assert gm.apply_migration(gm.build_migration_plan(), served_wait=0.1) is False + out = capsys.readouterr().out + assert "Rolling back" in out and "Rolled back" in out + assert _config_flag(fleet.root) is False + assert fleet.services == {"coder": ("systemd", False), "ops": ("systemd", False)} + assert not (fleet.root / gm.MANIFEST_NAME).exists() + # The fleet is back where it started, so a corrected re-run is a fresh migration, not a refusal. + assert gm.build_migration_plan().eligible_for_migration() + + +def test_interrupted_apply_is_resumed_from_the_manifest_not_short_circuited(fleet, monkeypatch, capsys): + """Flag flipped, secondaries gone, default never came up (the process died mid-apply): the re-run + must finish the migration from the manifest — with the recorded User= — instead of reporting + 'already multiplexed' over a fleet with no gateway at all.""" + fleet.services["coder"] = ("systemd", True) + monkeypatch.setattr(gm, "_systemd_service_user", lambda home, services: "root" if _name(home) == "coder" else None) + with pytest.MonkeyPatch.context() as dying: + dying.setattr(gm, "_restart_default", lambda *a, **k: (_ for _ in ()).throw(KeyboardInterrupt())) + with pytest.raises(KeyboardInterrupt): + gm.apply_migration(gm.build_migration_plan(), served_wait=0.1) + assert _config_flag(fleet.root) is True and "coder" not in fleet.services and "default" not in fleet.services + installs = [] + real_op = gm._service_op + + def _recording(kind, system, verb, home, *, run_as_user=None): + if verb == "install": + installs.append((_name(home), kind, system, run_as_user)) + real_op(kind, system, verb, home, run_as_user=run_as_user) + + monkeypatch.setattr(gm, "_service_op", _recording) + plan = gm.build_migration_plan() + assert plan.interrupted and not plan.already_multiplexed + assert gm.apply_migration(plan, served_wait=5.0) is True + assert installs == [("default", "systemd", True, "root")] + assert "serves 3 profiles" in capsys.readouterr().out + + +def test_every_installed_unit_of_a_secondary_is_removed_and_restored(fleet, capsys): + """A profile carrying a user AND a system unit: both are stopped/uninstalled (recording only the first + found left the other live beside the multiplexer) and rollback reinstalls both.""" + fleet.services["coder"] = [("systemd", False), ("systemd", True)] + plan = gm.build_migration_plan() + coder = next(p for p in plan.standalone_secondaries if p.name == "coder") + assert coder.services == [("systemd", False), ("systemd", True)] + assert gm.apply_migration(plan, served_wait=5.0) is True + assert "coder" not in fleet.services + manifest = json.loads((fleet.root / gm.MANIFEST_NAME).read_text(encoding="utf-8")) + coder_rec = next(r for r in manifest["secondaries"] if r["profile"] == "coder") + assert [(s["kind"], s["system"]) for s in coder_rec["services"]] == [("systemd", False), ("systemd", True)] + capsys.readouterr() + assert gm.rollback_migration(fleet.root) is True + assert fleet.services["coder"] == ("systemd", True) or set(_units(fleet.services["coder"])) == {("systemd", False), ("systemd", True)} + assert [op for op in fleet.ops if op[0] == "coder"].count(("coder", "install")) == 2 + + # The unattended hook does not resolve an ambiguous two-unit topology on its own. + for f in (gm.MANIFEST_NAME, "gateway.pid", "gateway_state.json"): + (fleet.root / f).unlink(missing_ok=True) + (fleet.root / "config.yaml").write_text("model:\n default: x\n", encoding="utf-8") + fleet.services.update({"default": ("systemd", False), "coder": [("systemd", False), ("systemd", True)]}) + fleet.pids.update({"coder": 4101, "ops": 4102}); fleet.ops.clear() + gm.maybe_auto_migrate_after_update() + assert "more than one installed service" in capsys.readouterr().out and fleet.ops == [] + + +def test_unresolvable_system_unit_user_is_unknown_principal_not_directory_owner(fleet, tmp_path, monkeypatch, capsys): + """A system unit pinned to a User= this host cannot resolve: the principal is unknown, never the + profile directory's owner, and unknown blocks the unattended path.""" + from hermes_cli import gateway as gw + from hermes_cli.gateway_migrate_guards import gateway_identity + unit_dir = tmp_path / "system"; unit_dir.mkdir() + monkeypatch.setattr(gw, "_SYSTEM_UNIT_DIR", unit_dir) + coder_home = fleet.root / "profiles/coder" + with gm._home_env(coder_home): + gw.get_systemd_unit_path(system=True).write_text("[Service]\nUser=nobody-such-user-xyz\n", encoding="utf-8") + uid, _home = gateway_identity(coder_home, None, [("systemd", True)]) + assert uid is None # NOT coder_home.stat().st_uid + # Same unit shape without a system unit keeps the directory-owner answer for user-scope gateways. + assert gateway_identity(coder_home, None, [("systemd", False)])[0] == coder_home.stat().st_uid + + fleet.services.update({"default": ("systemd", True), "coder": ("systemd", True)}) + monkeypatch.setattr(gm, "_gateway_identity", + lambda home, pid, services: (None if _name(home) == "coder" else 1000, home)) + gm.maybe_auto_migrate_after_update() + out = capsys.readouterr().out + assert "cannot be resolved" in out and gm.MIGRATE_COMMAND in out + assert fleet.ops == [] and _config_flag(fleet.root) is None + + +def test_opt_out_reads_effective_config_managed_false_wins_and_string_false_is_false(fleet, tmp_path, monkeypatch): + """The opt-out authorizes an unattended destructive action, so it reads the same effective config + the CLI does: a managed ``false`` overrides the user's ``true``; a hand-written ``"false"`` string is + an opt-out, not a truthy value; the declared default keeps absent == opted in.""" + from hermes_cli import config as cfg + from hermes_cli.config_defaults import DEFAULT_CONFIG + from hermes_cli.gateway_migrate_guards import auto_migration_opted_out + from hermes_cli import managed_scope + assert DEFAULT_CONFIG["gateway"]["auto_multiplex_migration"] is True + assert auto_migration_opted_out(fleet.root) is False # absent -> DEFAULT_CONFIG value + + (fleet.root / "config.yaml").write_text( + "model:\n default: x\ngateway:\n auto_multiplex_migration: 'false'\n", encoding="utf-8") + assert auto_migration_opted_out(fleet.root) is True + + (fleet.root / "config.yaml").write_text( + "model:\n default: x\ngateway:\n auto_multiplex_migration: true\n", encoding="utf-8") + assert auto_migration_opted_out(fleet.root) is False + managed = tmp_path / "managed"; managed.mkdir() + (managed / "config.yaml").write_text("gateway:\n auto_multiplex_migration: false\n", encoding="utf-8") + monkeypatch.setenv("HERMES_MANAGED_DIR", str(managed)) + managed_scope.invalidate_managed_cache() + with gm._home_env(fleet.root): + assert cfg.load_config()["gateway"]["auto_multiplex_migration"] is False + assert auto_migration_opted_out(fleet.root) is True + gm.maybe_auto_migrate_after_update() + assert fleet.ops == [] and _config_flag(fleet.root) is None diff --git a/tests/hermes_cli/test_profile_clone_channels.py b/tests/hermes_cli/test_profile_clone_channels.py index 8c82eb2a0a..8ce8d72648 100644 --- a/tests/hermes_cli/test_profile_clone_channels.py +++ b/tests/hermes_cli/test_profile_clone_channels.py @@ -56,7 +56,7 @@ def home(tmp_path, monkeypatch): (root / ".env").write_text(_SOURCE_ENV, encoding="utf-8") (root / "config.yaml").write_text(yaml.safe_dump(_SOURCE_CONFIG), encoding="utf-8") (root / "SOUL.md").write_text("Be helpful.", encoding="utf-8") - monkeypatch.setattr(gm, "_installed_service", lambda home: None) + monkeypatch.setattr(gm, "_installed_services", lambda home: []) monkeypatch.setattr(gm, "_live_gateway_pid", lambda home: None) return root diff --git a/website/docs/user-guide/multi-profile-gateways.md b/website/docs/user-guide/multi-profile-gateways.md index a3b45cb85d..07bf441f53 100644 --- a/website/docs/user-guide/multi-profile-gateways.md +++ b/website/docs/user-guide/multi-profile-gateways.md @@ -832,7 +832,8 @@ A standalone secondary behind any of these boundaries stops the automatic path: | boundary | example | |---|---| | different service manager or scope | default on user systemd, a secondary on **system** systemd (or launchd), or the default detached with a service-managed secondary | -| different UNIX user | a system unit with its own `User=`, or a live gateway owned by another uid | +| more than one installed unit on a profile | a user **and** a system unit for the same profile (the explicit command removes both) | +| different UNIX user | a system unit with its own `User=`, or a live gateway owned by another uid; a system unit whose `User=` this host cannot resolve counts as unknown, never as "same user" | | `HERMES_HOME` outside `/profiles/` | a unit pinning `HERMES_HOME=/opt/hermes/profiles/emma` | In that case `hermes update` prints the boundary it found plus @@ -855,7 +856,9 @@ hermes config set gateway.auto_multiplex_migration false `hermes update` then leaves per-profile gateways exactly as they are, with no output and no changes, however eligible the install looks. The setting lives in config, so it survives updates — the decision is made once rather than -re-litigated on every release. It governs the **automatic** path only: +re-litigated on every release. It is read from the effective config like every +other setting, so a value pinned in the managed scope (`/etc/hermes/config.yaml`) +wins over the profile's own file. It governs the **automatic** path only: `hermes gateway migrate --multiplex` is an explicit request and still migrates (and is the supported way to opt back in). Absent or `true` keeps the default behaviour described above. @@ -930,7 +933,16 @@ hermes gateway migrate --standalone reads `gateway_migration.json`, sets `gateway.multiplex_profiles` back to its previous value, restarts the default gateway, and reinstalls/starts every -recorded per-profile service. The manifest is removed once everything is back. +recorded per-profile service (a system unit comes back with the `User=` it had). +The manifest is removed once everything is back. + +The forward migration is transactional in the same way: if bringing the default +gateway up fails after the per-profile gateways were removed (for example a +system unit that has to run as root), `--multiplex` rolls back through the +manifest on the spot so no profile is left without a gateway. Should the +process die between flipping the flag and starting the default, the next +`hermes gateway migrate --multiplex` sees the manifest with no live gateway and +resumes from it instead of reporting "already multiplexed". If no manifest exists (you enabled multiplexing by hand), leave multiplex mode with `hermes config set gateway.multiplex_profiles false && hermes gateway restart` and reinstall the per-profile services you want.