From f29ee96dd3b5bc4cd7a33ff16d69a1b4c911d344 Mon Sep 17 00:00:00 2001 From: PT Date: Tue, 28 Jul 2026 11:24:15 -0700 Subject: [PATCH] fix(update): restart all macOS launchd gateways on hermes update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The macOS branch of the update's fleet-restart step only restarted the invoking profile's LaunchAgent. Sibling ai.hermes.gateway- services kept pre-update modules cached in sys.modules and died on their next agent turn (ImportError on new lazy imports, or TypeError/ AttributeError with garbled tracebacks on wider version gaps). The systemd branch already iterates every hermes-gateway* unit; this brings launchd to parity: - _restart_macos_launchd_gateways(): the invoking profile keeps the existing launchd_restart() path; every other gateway of this install is drained via SIGUSR1 (same as systemd siblings), then hard- kickstarted unless KeepAlive already respawned it, then verified on a fresh PID. TimeoutExpired is isolated per label (#68523 parity) and counts toward failed_or_stale_units — including timeouts during liveness discovery, which must not read as "unloaded". - Install-scoped fleet enumeration: launchd_gateway_labels_for_install() derives labels from THIS install's profiles (get_default_hermes_root), not by globbing the shared per-user ~/Library/LaunchAgents — a sandboxed HERMES_HOME (tests, capture sandboxes, side-by-side installs) must never enumerate, let alone restart, another install's fleet. This also keeps the hermetic test suite blind to a dev machine's real gateways. - Domain-explicit sibling handling via _locate_launchd_gateway_service(): liveness, kickstart, and fresh-PID verification all use the domain the service was actually located in (gui/ vs user/ probed per label via `launchctl print`). This addresses the #41403 review defect: the process-wide _launchd_domain() cache resolves the current profile's domain and must never be reused for a sibling. _launchd_domain() itself becomes a thin caching wrapper; behavior unchanged. - _get_service_pids(all_profiles=...): the update path's manual-process sweep excludes every gateway service PID (mirror of the systemd hermes-gateway* pattern) so it cannot mistake a freshly respawned sibling service for a stale manual gateway. Default-scope callers (gateway status, cron checks, stop_profile_gateway's orphan reaper — which kills what it is fed) keep the current-profile-only contract. - _warn_incomplete_gateway_fleet_restart() prints launchctl recovery hints for launchd labels alongside the systemctl ones. Supersedes and completes #41403, addressing its review feedback (per-label domain resolution + mocked regression tests). Co-authored-by: David Neyra Co-Authored-By: Claude Fable 5 --- hermes_cli/gateway.py | 292 +++++--- hermes_cli/update_cmd.py | 148 +++- .../test_update_launchd_fleet_restart.py | 642 ++++++++++++++++++ 3 files changed, 983 insertions(+), 99 deletions(-) create mode 100644 tests/hermes_cli/test_update_launchd_fleet_restart.py diff --git a/hermes_cli/gateway.py b/hermes_cli/gateway.py index c64080ab0c..dced8278a5 100644 --- a/hermes_cli/gateway.py +++ b/hermes_cli/gateway.py @@ -107,11 +107,14 @@ def _get_service_pids(all_profiles: bool = False) -> set: returns (true for both systemd and launchd in practice). ``all_profiles`` widens the launchd branch to every installed - ``ai.hermes.gateway*`` agent — the update path needs the whole fleet - excluded from its sweep so sibling-profile launchd gateways found by the - ps scan aren't misclassified as manual processes (#73626). Default-scope - callers (``gateway status``, cron checks) keep seeing only the current - profile's service. + ``ai.hermes.gateway*`` LaunchAgent — the update path needs the whole + fleet excluded from its sweep (#41403, #73626): sibling-profile launchd + gateways found by the (BSD-fixed) ps scan must not be misclassified as + manual processes and killed. Default-scope callers (``gateway status``, + cron checks) keep seeing only the current profile's service; the orphan + reaper passes all_profiles=True for the same friendly-fire reason. The + systemd branch has always been fleet-wide (``hermes-gateway*``) and is + unaffected. """ pids: set = set() @@ -155,13 +158,29 @@ def _get_service_pids(all_profiles: bool = False) -> set: # --- launchd (macOS) --- if is_macos(): - try: - if all_profiles: - # Enumerate every ai.hermes.gateway* agent across profiles - # so the update sweep's exclude set is complete (#73626). - # Without this, sibling-profile launchd gateways found by the - # (now-working) ps scan would be misclassified as manual and - # killed, racing with KeepAlive → duplicate gateways. + labels = {get_launchd_label()} + if all_profiles: + # Every gateway LaunchAgent, not just the invoking profile's — + # mirrors the systemd branch's ``hermes-gateway*`` pattern above. + # The update path restarts the whole fleet, and its stale-process + # sweep must not mistake a sibling service's fresh PID for a + # manual gateway it should kill (#41403). + labels.update(launchd_gateway_labels_for_install()) + for label in sorted(labels): + try: + _domain, pid = _locate_launchd_gateway_service(label) + except subprocess.TimeoutExpired: + continue + if pid is not None and pid > 0: + pids.add(pid) + if all_profiles: + # Belt-and-suspenders for the EXCLUDE use case (#74075): a bare + # ``launchctl list`` prefix scan also catches ai.hermes.gateway* + # agents the label derivation can't map (renamed profiles, other + # installs sharing this user). Over-inclusion is safe here — + # these PIDs are only ever protected from the kill sweep, never + # targeted. Restart paths use the label-derived set only. + try: result = subprocess.run( ["launchctl", "list"], capture_output=True, @@ -180,33 +199,8 @@ def _get_service_pids(all_profiles: bool = False) -> set: pids.add(pid) except ValueError: pass - else: - label = get_launchd_label() - result = subprocess.run( - ["launchctl", "list", label], - capture_output=True, - text=True, encoding='utf-8', errors='replace', - timeout=5, - ) - if result.returncode == 0: - # Try plist format first (macOS 26+): "PID" = ; - pid = _parse_launchd_pid_from_list_output(result.stdout) - if pid is not None and pid > 0: - pids.add(pid) - else: - # Fall back to legacy tab-separated format: - # "PID\tStatus\tLabel" - for line in result.stdout.strip().splitlines(): - parts = line.split() - if len(parts) >= 3 and parts[2] == label: - try: - pid = int(parts[0]) - if pid > 0: - pids.add(pid) - except ValueError: - pass - except (FileNotFoundError, subprocess.TimeoutExpired): - pass + except (FileNotFoundError, subprocess.TimeoutExpired): + pass return pids @@ -1437,6 +1431,87 @@ def _parse_launchd_pid_from_list_output(output: str) -> int | None: return None +def _parse_launchd_pid_from_print_output(output: str) -> int | None: + """Extract the live PID from ``launchctl print`` output (``pid = ``). + + A bootstrapped-but-not-running service prints no ``pid =`` line; the + first (service-level) occurrence wins over any nested endpoint state. + Returns ``None`` when no PID is found or the PID is non-positive. + """ + for line in output.splitlines(): + stripped = line.strip() + if stripped.startswith("pid = "): + try: + pid = int(stripped[len("pid = "):].strip()) + return pid if pid > 0 else None + except ValueError: + return None + return None + + +def _launchd_print_service_pid(domain: str, label: str) -> tuple[bool, int | None]: + """Return ``(loaded, pid)`` for ``domain/label`` via ``launchctl print``. + + Domain-explicit on purpose: legacy ``launchctl list`` infers its domain + from the caller's execution context, which is exactly the ambiguity that + sank the first fleet-restart attempt (#41403 review). ``TimeoutExpired`` + propagates — fleet-restart callers own per-label failure accounting (a + wedged launchctl call must be reported, not read as "unloaded"). + """ + try: + result = subprocess.run( + ["launchctl", "print", f"{domain}/{label}"], + capture_output=True, + text=True, encoding='utf-8', errors='replace', + timeout=5, + ) + except FileNotFoundError: + return (False, None) + if result.returncode != 0: + return (False, None) + return (True, _parse_launchd_pid_from_print_output(result.stdout)) + + +def _launchd_service_registered(label: str) -> bool: + """True when launchd knows ``label`` (``launchctl list