diff --git a/gateway/restart.py b/gateway/restart.py index 97830872b9ef..240937032eba 100644 --- a/gateway/restart.py +++ b/gateway/restart.py @@ -1,4 +1,7 @@ -"""Shared gateway restart constants and parsing helpers.""" +"""Shared gateway restart constants and supervisor detection helpers.""" + +import os +from collections.abc import Mapping from hermes_cli.config import DEFAULT_CONFIG @@ -12,11 +15,36 @@ GATEWAY_SERVICE_RESTART_EXIT_CODE = 75 # restarting the gateway. See #51228. GATEWAY_FATAL_CONFIG_EXIT_CODE = 78 +# Set by ``hermes gateway run --external-supervisor``. Unlike systemd's +# INVOCATION_ID and launchd's XPC_SERVICE_NAME, this survives wrappers that +# intentionally replace the child environment (for example ``sudo env -i``). +EXTERNAL_GATEWAY_SUPERVISOR_ENV = "HERMES_GATEWAY_EXTERNAL_SUPERVISOR" + DEFAULT_GATEWAY_RESTART_DRAIN_TIMEOUT = float( DEFAULT_CONFIG["agent"]["restart_drain_timeout"] ) +def is_gateway_supervisor_process( + environ: Mapping[str, str] | None = None, +) -> bool: + """Return whether this gateway process is owned by a supervisor.""" + env = os.environ if environ is None else environ + if env.get("INVOCATION_ID"): + return True + if env.get("HERMES_S6_SUPERVISED_CHILD"): + return True + xpc_service = env.get("XPC_SERVICE_NAME", "") + if xpc_service and xpc_service != "0": + return True + return str(env.get(EXTERNAL_GATEWAY_SUPERVISOR_ENV, "")).strip().lower() in { + "1", + "true", + "yes", + "on", + } + + def parse_restart_drain_timeout(raw: object) -> float: """Parse a configured drain timeout, falling back to the shared default.""" try: diff --git a/gateway/slash_commands.py b/gateway/slash_commands.py index 682d1431028c..c3c977d8be9d 100644 --- a/gateway/slash_commands.py +++ b/gateway/slash_commands.py @@ -1313,15 +1313,12 @@ class GatewaySlashCommandsMixin: # us. The detached subprocess approach (setsid + bash) doesn't work # under systemd (KillMode=mixed kills the cgroup) or Docker (tini # exits when the gateway dies, taking the detached helper with it). - # systemd sets INVOCATION_ID; launchd sets XPC_SERVICE_NAME to the - # job label. Without the launchd check, macOS /restart takes the - # detached path and exits 0, which KeepAlive.SuccessfulExit=false - # treats as a deliberate stop — the gateway stays dead until next - # login. Interactive macOS shells inherit XPC_SERVICE_NAME=0, so - # "0" must count as not-under-launchd. - _under_service = bool(os.environ.get("INVOCATION_ID")) or os.environ.get( - "XPC_SERVICE_NAME", "0" - ) not in ("", "0") + # Native supervisor markers cover direct systemd/launchd starts. The + # explicit marker covers wrappers such as ``sudo env -i`` that strip + # those markers before execing the foreground gateway. + from gateway.restart import is_gateway_supervisor_process + + _under_service = is_gateway_supervisor_process() _in_container = os.path.exists("/.dockerenv") or os.path.exists("/run/.containerenv") if _under_service or _in_container: self.request_restart(detached=False, via_service=True) diff --git a/hermes_cli/gateway.py b/hermes_cli/gateway.py index f45a0e85fb43..7024cf32e76f 100644 --- a/hermes_cli/gateway.py +++ b/hermes_cli/gateway.py @@ -32,8 +32,10 @@ PROJECT_ROOT = Path(__file__).parent.parent.resolve() from gateway.status import terminate_pid from gateway.restart import ( DEFAULT_GATEWAY_RESTART_DRAIN_TIMEOUT, + EXTERNAL_GATEWAY_SUPERVISOR_ENV, GATEWAY_FATAL_CONFIG_EXIT_CODE, GATEWAY_SERVICE_RESTART_EXIT_CODE, + is_gateway_supervisor_process, parse_restart_drain_timeout, ) from hermes_cli.config import ( @@ -696,6 +698,22 @@ def _capture_gateway_argv(pid: int) -> list[str] | None: return argv +def _prepare_profile_gateway_update_restart(profile: str, pid: int) -> str | None: + """Choose who relaunches a profile gateway after ``hermes update``. + + A gateway started with ``--external-supervisor`` must exit back to that + manager. Starting Hermes's detached watcher as well would escape the + manager and race its replacement process. Ordinary foreground gateways + retain the existing detached-watcher behavior. + """ + argv = _capture_gateway_argv(pid) + if argv and "--external-supervisor" in argv: + return "external-supervisor" + if launch_detached_profile_gateway_restart(profile, pid): + return "detached" + return None + + def launch_detached_gateway_restart_by_cmdline( old_pid: int, run_argv: list[str] ) -> bool: @@ -4503,15 +4521,10 @@ def _running_under_gateway_supervisor() -> bool: - launchd sets ``XPC_SERVICE_NAME`` to the job label for jobs it spawns; interactive shells inherit the sentinel ``"0"`` instead. - the s6-overlay container longrun exports ``HERMES_S6_SUPERVISED_CHILD``. + - wrapped services can opt in with ``--external-supervisor`` when their + launcher strips the native systemd/launchd marker. """ - if os.environ.get("INVOCATION_ID"): - return True - if os.environ.get("HERMES_S6_SUPERVISED_CHILD"): - return True - xpc_service = os.environ.get("XPC_SERVICE_NAME", "") - if xpc_service and xpc_service != "0": - return True - return False + return is_gateway_supervisor_process() def _guard_named_profile_under_multiplexer(force: bool = False) -> None: @@ -6543,6 +6556,8 @@ def _gateway_command_inner(args): if subcmd is None or subcmd == "run": if _maybe_redirect_run_to_s6_supervision(args): return # unreachable; execvp doesn't return + if getattr(args, "external_supervisor", False): + os.environ[EXTERNAL_GATEWAY_SUPERVISOR_ENV] = "1" verbose = getattr(args, "verbose", 0) quiet = getattr(args, "quiet", False) replace = getattr(args, "replace", False) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 1712d7888d11..81c8de4aa1e6 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -10581,7 +10581,7 @@ def _cmd_update_impl(args, gateway_mode: bool): _ensure_user_systemd_env, find_gateway_pids, find_profile_gateway_processes, - launch_detached_profile_gateway_restart, + _prepare_profile_gateway_update_restart, _get_service_pids, _graceful_restart_via_sigusr1, _wait_for_gateway_exit, @@ -10763,6 +10763,7 @@ def _cmd_update_impl(args, gateway_mode: bool): restarted_services = [] killed_pids = set() relaunched_profiles = [] + externally_supervised_profiles = [] # --- Systemd services (Linux) --- # Discover all hermes-gateway* units (default + profiles) @@ -11094,7 +11095,10 @@ def _cmd_update_impl(args, gateway_mode: bool): if proc.pid in manual_pids } for pid, proc in profile_processes.items(): - if not launch_detached_profile_gateway_restart(proc.profile, pid): + restart_mode = _prepare_profile_gateway_update_restart( + proc.profile, pid + ) + if restart_mode is None: continue # Prefer a graceful SIGUSR1 drain so in-flight agent runs # finish before the watcher respawns the gateway. If the @@ -11134,7 +11138,10 @@ def _cmd_update_impl(args, gateway_mode: bool): # live when the new gateway polls. _wait_for_gateway_exit(timeout=5.0, force_after=None) killed_pids.add(pid) - relaunched_profiles.append(proc.profile) + if restart_mode == "external-supervisor": + externally_supervised_profiles.append(proc.profile) + else: + relaunched_profiles.append(proc.profile) for pid in manual_pids: if pid in profile_processes: @@ -11152,7 +11159,17 @@ def _cmd_update_impl(args, gateway_mode: bool): if relaunched_profiles: names = ", ".join(relaunched_profiles) print(f" ✓ Restarting manual gateway profile(s): {names}") - unmapped_count = len(killed_pids) - len(relaunched_profiles) + if externally_supervised_profiles: + names = ", ".join(externally_supervised_profiles) + print( + " ✓ Handed gateway profile(s) back to their external " + f"supervisor: {names}" + ) + unmapped_count = ( + len(killed_pids) + - len(relaunched_profiles) + - len(externally_supervised_profiles) + ) if unmapped_count: print(f" → Stopped {unmapped_count} manual gateway process(es)") print(" Restart manually: hermes gateway run") diff --git a/hermes_cli/subcommands/gateway.py b/hermes_cli/subcommands/gateway.py index 6b975d4390db..d541b6955494 100644 --- a/hermes_cli/subcommands/gateway.py +++ b/hermes_cli/subcommands/gateway.py @@ -85,6 +85,16 @@ def build_gateway_parser( "gateway's exit code. No effect outside an s6 container." ), ) + gateway_run.add_argument( + "--external-supervisor", + action="store_true", + help=( + "Declare that an external process manager owns this foreground " + "gateway. In-chat restarts and updates exit back to that manager " + "instead of spawning a detached replacement. Use this when a " + "launchd/systemd wrapper strips its native environment markers." + ), + ) add_accept_hooks_flag(gateway_run) add_accept_hooks_flag(gateway_parser) diff --git a/tests/gateway/test_restart_service_detection.py b/tests/gateway/test_restart_service_detection.py index 10ea2ff28f7e..72b7b7db1983 100644 --- a/tests/gateway/test_restart_service_detection.py +++ b/tests/gateway/test_restart_service_detection.py @@ -20,6 +20,7 @@ import pytest import gateway.run as gateway_run from gateway.platforms.base import MessageEvent, MessageType +from gateway.restart import EXTERNAL_GATEWAY_SUPERVISOR_ENV from tests.gateway.restart_test_helpers import make_restart_runner, make_restart_source @@ -37,6 +38,8 @@ def _make_runner_with_mock_restart(tmp_path, monkeypatch): monkeypatch.setattr(gateway_run, "_hermes_home", tmp_path) monkeypatch.delenv("INVOCATION_ID", raising=False) monkeypatch.delenv("XPC_SERVICE_NAME", raising=False) + monkeypatch.delenv("HERMES_S6_SUPERVISED_CHILD", raising=False) + monkeypatch.delenv(EXTERNAL_GATEWAY_SUPERVISOR_ENV, raising=False) runner, _adapter = make_restart_runner() runner.request_restart = MagicMock(return_value=True) return runner @@ -83,3 +86,29 @@ async def test_restart_under_systemd_uses_service_path(tmp_path, monkeypatch): await runner._handle_restart_command(_make_restart_event()) runner.request_restart.assert_called_once_with(detached=False, via_service=True) + + +@pytest.mark.asyncio +async def test_restart_with_external_supervisor_marker_uses_service_path( + tmp_path, monkeypatch +): + """Wrapped supervisors can retain restart ownership without native markers.""" + runner = _make_runner_with_mock_restart(tmp_path, monkeypatch) + monkeypatch.setenv(EXTERNAL_GATEWAY_SUPERVISOR_ENV, "1") + + await runner._handle_restart_command(_make_restart_event()) + + runner.request_restart.assert_called_once_with(detached=False, via_service=True) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("value", ["", "0", "false", "off"]) +async def test_false_external_supervisor_marker_keeps_detached_path( + value, tmp_path, monkeypatch +): + runner = _make_runner_with_mock_restart(tmp_path, monkeypatch) + monkeypatch.setenv(EXTERNAL_GATEWAY_SUPERVISOR_ENV, value) + + await runner._handle_restart_command(_make_restart_event()) + + runner.request_restart.assert_called_once_with(detached=True, via_service=False) diff --git a/tests/hermes_cli/test_gateway_external_supervisor.py b/tests/hermes_cli/test_gateway_external_supervisor.py new file mode 100644 index 000000000000..76b74ee4b176 --- /dev/null +++ b/tests/hermes_cli/test_gateway_external_supervisor.py @@ -0,0 +1,82 @@ +"""Tests for explicit ownership by a wrapped external gateway supervisor.""" + +from types import SimpleNamespace + +import pytest + +import hermes_cli.gateway as gateway + + +def _clear_native_supervisor_markers(monkeypatch): + monkeypatch.delenv("INVOCATION_ID", raising=False) + monkeypatch.delenv("HERMES_S6_SUPERVISED_CHILD", raising=False) + monkeypatch.setenv("XPC_SERVICE_NAME", "0") + + +def test_external_marker_identifies_supervisor_process(monkeypatch): + _clear_native_supervisor_markers(monkeypatch) + monkeypatch.setenv(gateway.EXTERNAL_GATEWAY_SUPERVISOR_ENV, "1") + + assert gateway._running_under_gateway_supervisor() is True + + +def test_gateway_run_external_supervisor_flag_marks_process(monkeypatch): + monkeypatch.delenv(gateway.EXTERNAL_GATEWAY_SUPERVISOR_ENV, raising=False) + monkeypatch.setattr( + gateway, "_maybe_redirect_run_to_s6_supervision", lambda _args: False + ) + observed = [] + monkeypatch.setattr( + gateway, + "run_gateway", + lambda *_args, **_kwargs: observed.append( + gateway.os.environ.get(gateway.EXTERNAL_GATEWAY_SUPERVISOR_ENV) + ), + ) + + gateway._gateway_command_inner( + SimpleNamespace(gateway_command="run", external_supervisor=True) + ) + + assert observed == ["1"] + + +def test_update_hands_external_supervisor_gateway_back_without_watcher(monkeypatch): + monkeypatch.setattr( + gateway, + "_capture_gateway_argv", + lambda _pid: [ + "python", + "-m", + "hermes_cli.main", + "gateway", + "run", + "--external-supervisor", + ], + ) + monkeypatch.setattr( + gateway, + "launch_detached_profile_gateway_restart", + lambda *_args: pytest.fail("detached watcher must not be launched"), + ) + + assert gateway._prepare_profile_gateway_update_restart("work", 1234) == ( + "external-supervisor" + ) + + +def test_update_keeps_detached_restart_for_ordinary_foreground_gateway(monkeypatch): + monkeypatch.setattr( + gateway, + "_capture_gateway_argv", + lambda _pid: ["python", "-m", "hermes_cli.main", "gateway", "run"], + ) + calls = [] + monkeypatch.setattr( + gateway, + "launch_detached_profile_gateway_restart", + lambda profile, pid: calls.append((profile, pid)) or True, + ) + + assert gateway._prepare_profile_gateway_update_restart("work", 1234) == "detached" + assert calls == [("work", 1234)] diff --git a/tests/hermes_cli/test_subcommands_profile_gateway.py b/tests/hermes_cli/test_subcommands_profile_gateway.py index 4040c1f67ae8..afe11745ee0b 100644 --- a/tests/hermes_cli/test_subcommands_profile_gateway.py +++ b/tests/hermes_cli/test_subcommands_profile_gateway.py @@ -92,6 +92,12 @@ def test_gateway_accept_hooks_flag(): assert ns.accept_hooks is True +def test_gateway_run_accepts_external_supervisor_flag(): + p = _gateway_parser() + ns = p.parse_args(["gateway", "run", "--external-supervisor"]) + assert ns.external_supervisor is True + + def test_gateway_lifecycle_accepts_legacy_platform_flag(): p = _gateway_parser() for action in ("start", "restart", "status"): diff --git a/website/docs/reference/cli-commands.md b/website/docs/reference/cli-commands.md index ae6fe70fc21f..55ca8a1cd9cb 100644 --- a/website/docs/reference/cli-commands.md +++ b/website/docs/reference/cli-commands.md @@ -239,6 +239,7 @@ Options: |--------|-------------| | `--all` | On `start` / `restart` / `stop`: act on **every profile's** gateway, not just the active `HERMES_HOME`. Useful if you run multiple profiles side-by-side and want to restart them all after `hermes update`. | | `--no-supervise` | On `run`: inside the s6-overlay Docker image, opt out of auto-supervision and use pre-s6 foreground semantics — gateway runs as the container's main process with no auto-restart. No-op outside the s6 image. Equivalent to setting `HERMES_GATEWAY_NO_SUPERVISE=1`. | +| `--external-supervisor` | On `run`: declare that a wrapper-provided process manager owns the foreground gateway. Use this when `sudo`, `env -i`, or another wrapper strips launchd/systemd's native environment marker. In-chat restarts and updates exit back to that manager instead of spawning a detached replacement. | `hermes gateway enroll` accepts `--token`, `--connector-url`, `--gateway-id`, and `--wake-url`. It exchanges the enrollment token with the connector and writes the resulting `GATEWAY_RELAY_ID`, `GATEWAY_RELAY_SECRET`, `GATEWAY_RELAY_DELIVERY_KEY`, optional `GATEWAY_RELAY_URL`, and (when `--wake-url` is given) `GATEWAY_RELAY_WAKE_URL` values to the active profile's `.env`.