fix(gateway/macos): preserve launchd plist runtime on auto-repair + verify job live after self-restart

Salvaged from #40413 + #40299; re-verified on main, tightened, tested.

Co-authored-by: izumi0uu <izumi0uu@users.noreply.github.com>
Co-authored-by: responseops-ai <responseops-ai@users.noreply.github.com>

hermes_cli/gateway.py

@@ -7,6 +7,7 @@ Handles: hermes gateway [run|start|stop|restart|status|install|uninstall|setup]
 import asyncio
 import logging
 import os
+import plistlib
 import shutil
 import signal
 import subprocess
@@ -2450,6 +2451,42 @@ def _normalize_launchd_plist_for_comparison(text: str) -> str:
     )
 
 
+def _extract_launchd_runtime_overrides(text: str) -> tuple[str | None, str | None]:
+    """Return the installed launchd runtime (python path, VIRTUAL_ENV) if present.
+
+    launchd service ownership is runtime-sensitive: a user-installed gateway
+    plist should keep pointing at the Python/venv it was explicitly installed
+    with unless the operator forces a reinstall. This avoids one Hermes runtime
+    (for example a bundled desktop app) silently stealing another runtime's
+    service definition during auto-repair.
+    """
+    try:
+        plist = plistlib.loads(text.encode("utf-8"))
+    except Exception:
+        return None, None
+
+    if not isinstance(plist, dict):
+        return None, None
+
+    program_arguments = plist.get("ProgramArguments")
+    if isinstance(program_arguments, list) and program_arguments:
+        python_candidate = program_arguments[0]
+        python_path = python_candidate if isinstance(python_candidate, str) else None
+    else:
+        python_path = None
+
+    # Only trust the structured launchd env payload; malformed or unexpected
+    # shapes should fall back to the current runtime rather than guessing.
+    environment = plist.get("EnvironmentVariables")
+    if isinstance(environment, dict):
+        venv_candidate = environment.get("VIRTUAL_ENV")
+        venv_dir = venv_candidate if isinstance(venv_candidate, str) else None
+    else:
+        venv_dir = None
+
+    return python_path, venv_dir
+
+
 def systemd_unit_is_current(system: bool = False) -> bool:
     unit_path = get_systemd_unit_path(system=system)
     if not unit_path.exists():
@@ -3003,8 +3040,12 @@ def _launchd_domain() -> str:
     return f"gui/{os.getuid()}"  # windows-footgun: ok — POSIX launchd (macOS) helper, never invoked on Windows
 
 
-def generate_launchd_plist() -> str:
-    python_path = get_python_path()
+def generate_launchd_plist(
+    *,
+    python_path_override: str | None = None,
+    venv_dir_override: str | None = None,
+) -> str:
+    python_path = python_path_override or get_python_path()
     # Stable cwd anchor — never the volatile source checkout. See
     # _stable_service_working_dir() for the rationale (same rot risk applies
     # to launchd's WorkingDirectory as to systemd's).
@@ -3020,7 +3061,8 @@ def generate_launchd_plist() -> str:
     # the systemd unit), then capture the user's full shell PATH so every
     # user-installed tool (node, ffmpeg, …) is reachable.
     detected_venv = _detect_venv_dir()
-    venv_dir = str(detected_venv) if detected_venv else str(PROJECT_ROOT / "venv")
+    detected_venv_dir = str(detected_venv) if detected_venv else str(PROJECT_ROOT / "venv")
+    venv_dir = venv_dir_override or detected_venv_dir
     # Resolve the directory containing the node binary (e.g. Homebrew, nvm)
     # so it's explicitly in PATH even if the user's shell PATH changes later.
     priority_dirs = _build_service_path_dirs()
@@ -3101,7 +3143,11 @@ def launchd_plist_is_current() -> bool:
         return False
 
     installed = plist_path.read_text(encoding="utf-8")
-    expected = generate_launchd_plist()
+    installed_python, installed_venv = _extract_launchd_runtime_overrides(installed)
+    expected = generate_launchd_plist(
+        python_path_override=installed_python,
+        venv_dir_override=installed_venv,
+    )
     return _normalize_launchd_plist_for_comparison(
         installed
     ) == _normalize_launchd_plist_for_comparison(expected)
@@ -3118,7 +3164,15 @@ def refresh_launchd_plist_if_needed() -> bool:
     if not plist_path.exists() or launchd_plist_is_current():
         return False
 
-    plist_path.write_text(generate_launchd_plist(), encoding="utf-8")
+    installed = plist_path.read_text(encoding="utf-8")
+    installed_python, installed_venv = _extract_launchd_runtime_overrides(installed)
+    plist_path.write_text(
+        generate_launchd_plist(
+            python_path_override=installed_python,
+            venv_dir_override=installed_venv,
+        ),
+        encoding="utf-8",
+    )
     label = get_launchd_label()
     # Bootout/bootstrap so launchd picks up the new definition
     subprocess.run(
@@ -3320,6 +3374,22 @@ def launchd_restart():
         pid = get_running_pid()
         if pid is not None and _request_gateway_self_restart(pid):
             print("✓ Service restart requested")
+            # A self-restart request only asks the running gateway to drain and
+            # exit.  Do not assume launchd will definitely bring it back: after
+            # some update paths the job can be absent/unloaded, leaving Telegram
+            # polling dead even though the restart request succeeded.  Wait for
+            # the old process to leave, then explicitly kickstart/verify the
+            # launchd job.  If launchd reports the job missing, the outer
+            # CalledProcessError handler bootstraps the plist and starts fresh.
+            exited = _wait_for_gateway_exit(timeout=drain_timeout + 10.0, force_after=None)
+            if not exited:
+                print(
+                    f"⚠ Gateway drain timed out after {drain_timeout:.0f}s — forcing launchd restart"
+                )
+                subprocess.run(["launchctl", "kickstart", "-k", target], check=True, timeout=90)
+            else:
+                subprocess.run(["launchctl", "kickstart", target], check=True, timeout=30)
+            print("✓ Service restarted")
             return
         if pid is not None:
             try:

tests/hermes_cli/test_gateway_launchd_restart.py

@@ -0,0 +1,98 @@
+"""Regression tests for macOS launchd gateway restart recovery.
+
+These cover PR #40299: a successful gateway *self-restart* request used to
+return immediately and report success, even though launchd might have the job
+absent/unloaded — leaving the gateway (and Telegram polling) dead while the CLI
+claimed the restart succeeded.
+"""
+
+import subprocess
+
+from hermes_cli import gateway as gw
+
+
+def _patch_launchd_basics(monkeypatch, tmp_path, *, pid=12345, exited=True):
+    plist = tmp_path / "ai.hermes.gateway.plist"
+    plist.write_text("<plist/>", encoding="utf-8")
+
+    monkeypatch.setattr(gw, "get_launchd_label", lambda: "ai.hermes.gateway")
+    monkeypatch.setattr(gw, "_launchd_domain", lambda: "gui/501")
+    monkeypatch.setattr(gw, "get_launchd_plist_path", lambda: plist)
+    monkeypatch.setattr(gw, "_get_restart_drain_timeout", lambda: 1.0)
+    monkeypatch.setattr(gw, "_request_gateway_self_restart", lambda actual_pid: actual_pid == pid)
+    monkeypatch.setattr(gw, "_wait_for_gateway_exit", lambda timeout, force_after: exited)
+
+    import gateway.status as status
+
+    monkeypatch.setattr(status, "get_running_pid", lambda: pid)
+
+
+def test_launchd_restart_self_restart_waits_then_kickstarts(monkeypatch, tmp_path, capsys):
+    """A successful self-restart request must still explicitly revive launchd.
+
+    Regression for the Telegram outage: the update path requested a gateway
+    self-restart, returned immediately, and Telegram never came back because
+    nothing verified launchd actually had the job loaded/running.
+    """
+    _patch_launchd_basics(monkeypatch, tmp_path, exited=True)
+    calls = []
+
+    def fake_run(cmd, **kwargs):
+        calls.append(cmd)
+        return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+
+    monkeypatch.setattr(gw.subprocess, "run", fake_run)
+
+    gw.launchd_restart()
+
+    # On a clean drain we issue a plain kickstart to ensure the job is running,
+    # NOT a force-kill kickstart (-k).
+    assert ["launchctl", "kickstart", "gui/501/ai.hermes.gateway"] in calls
+    assert ["launchctl", "kickstart", "-k", "gui/501/ai.hermes.gateway"] not in calls
+    out = capsys.readouterr().out
+    assert "Service restart requested" in out
+    assert "Service restarted" in out
+
+
+def test_launchd_restart_self_restart_bootstraps_if_job_unloaded(monkeypatch, tmp_path):
+    """If kickstart reports the job is missing, bootstrap the plist and start."""
+    _patch_launchd_basics(monkeypatch, tmp_path, exited=True)
+    calls = []
+
+    def fake_run(cmd, **kwargs):
+        calls.append(cmd)
+        if (
+            cmd == ["launchctl", "kickstart", "gui/501/ai.hermes.gateway"]
+            and calls.count(cmd) == 1
+        ):
+            # First (post-drain) kickstart: launchd has no such job loaded.
+            raise subprocess.CalledProcessError(3, cmd, stderr="Could not find service")
+        return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+
+    monkeypatch.setattr(gw.subprocess, "run", fake_run)
+
+    gw.launchd_restart()
+
+    assert [
+        "launchctl",
+        "bootstrap",
+        "gui/501",
+        str(tmp_path / "ai.hermes.gateway.plist"),
+    ] in calls
+    assert ["launchctl", "kickstart", "gui/501/ai.hermes.gateway"] in calls
+
+
+def test_launchd_restart_self_restart_force_kicks_if_drain_times_out(monkeypatch, tmp_path):
+    """If the old gateway does not exit, force launchd to restart the job."""
+    _patch_launchd_basics(monkeypatch, tmp_path, exited=False)
+    calls = []
+
+    def fake_run(cmd, **kwargs):
+        calls.append(cmd)
+        return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
+
+    monkeypatch.setattr(gw.subprocess, "run", fake_run)
+
+    gw.launchd_restart()
+
+    assert ["launchctl", "kickstart", "-k", "gui/501/ai.hermes.gateway"] in calls

tests/hermes_cli/test_gateway_service.py

@@ -451,6 +451,72 @@ class TestGatewayStopCleanup:
 
 
 class TestLaunchdServiceRecovery:
+    def test_extract_launchd_runtime_overrides_reads_structured_plist(self, tmp_path, monkeypatch):
+        user_venv = tmp_path / "user-venv"
+        (user_venv / "bin").mkdir(parents=True)
+        python_path = str(user_venv / "bin" / "python")
+        (user_venv / "bin" / "python").write_text("", encoding="utf-8")
+
+        monkeypatch.setattr(gateway_cli, "get_python_path", lambda: python_path)
+        monkeypatch.setattr(gateway_cli, "_detect_venv_dir", lambda: user_venv)
+        plist_text = gateway_cli.generate_launchd_plist()
+
+        assert gateway_cli._extract_launchd_runtime_overrides(plist_text) == (
+            python_path,
+            str(user_venv),
+        )
+
+    def test_extract_launchd_runtime_overrides_ignores_malformed_plist(self):
+        malformed = (
+            '<?xml version="1.0"?><plist><dict><key>ProgramArguments</key>'
+            '<array><string>/tmp/python</string>'
+        )
+
+        assert gateway_cli._extract_launchd_runtime_overrides(malformed) == (None, None)
+
+    def test_launchd_install_does_not_overwrite_existing_runtime_without_force(
+        self, tmp_path, monkeypatch
+    ):
+        """Auto-repair must not steal runtime ownership from an existing plist.
+
+        Issue #40278: Hermes Studio.app can run the same gateway service
+        management code under its bundled Python, notice that the installed
+        plist differs, and rewrite ProgramArguments[0]/VIRTUAL_ENV to point at
+        the app bundle. A user-installed gateway plist should keep its existing
+        runtime unless the operator explicitly forces a reinstall.
+        """
+        plist_path = tmp_path / "ai.hermes.gateway.plist"
+        user_venv = tmp_path / "user-venv"
+        studio_runtime = tmp_path / "Hermes Studio.app" / "Contents" / "Resources" / "python"
+        (user_venv / "bin").mkdir(parents=True)
+        (studio_runtime / "bin").mkdir(parents=True)
+        (user_venv / "bin" / "python").write_text("", encoding="utf-8")
+        (studio_runtime / "bin" / "python").write_text("", encoding="utf-8")
+
+        monkeypatch.setattr(gateway_cli, "get_python_path", lambda: str(user_venv / "bin" / "python"))
+        plist_path.write_text(gateway_cli.generate_launchd_plist(), encoding="utf-8")
+
+        monkeypatch.setattr(gateway_cli, "get_launchd_plist_path", lambda: plist_path)
+        monkeypatch.setattr(
+            gateway_cli, "get_python_path", lambda: str(studio_runtime / "bin" / "python")
+        )
+        calls = []
+
+        def fake_run(cmd, check=False, **kwargs):
+            calls.append(cmd)
+            return SimpleNamespace(returncode=0, stdout="", stderr="")
+
+        monkeypatch.setattr(gateway_cli.subprocess, "run", fake_run)
+
+        gateway_cli.launchd_install(force=False)
+
+        installed = plist_path.read_text(encoding="utf-8")
+        assert str(user_venv / "bin" / "python") in installed
+        assert str(studio_runtime / "bin" / "python") not in installed
+        assert str(user_venv) in installed
+        assert str(studio_runtime) not in installed
+        assert calls == []
+
     def test_get_restart_drain_timeout_prefers_env_then_config_then_default(self, monkeypatch):
         monkeypatch.delenv("HERMES_RESTART_DRAIN_TIMEOUT", raising=False)
         monkeypatch.setattr(gateway_cli, "read_raw_config", lambda: {})