mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-22 16:25:58 +00:00
fix(gateway): preserve external supervisor ownership
This commit is contained in:
parent
7d8c499893
commit
caf5f27e30
9 changed files with 207 additions and 22 deletions
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
82
tests/hermes_cli/test_gateway_external_supervisor.py
Normal file
82
tests/hermes_cli/test_gateway_external_supervisor.py
Normal file
|
|
@ -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)]
|
||||
|
|
@ -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"):
|
||||
|
|
|
|||
|
|
@ -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`.
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue