mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(gateway): drop --replace + Restart=on-failure in generated service units
Under a process supervisor (systemd/launchd), --replace makes each supervised restart kill its predecessor, producing self-kill loops, and Restart=always revives even clean manual stops. Generated units now run plain 'gateway run' with Restart=on-failure (+RestartForceExitStatus=75 for drain-restarts); the Nix module default follows suit. --replace stays on the manual/detached fallback paths where no supervisor owns the lifecycle.
This commit is contained in:
parent
7426c09bee
commit
0fc14fc2e9
3 changed files with 41 additions and 14 deletions
|
|
@ -250,8 +250,11 @@ def _graceful_restart_via_sigusr1(pid: int, drain_timeout: float) -> bool:
|
|||
|
||||
SIGUSR1 is wired in gateway/run.py to ``request_restart(via_service=True)``
|
||||
which drains in-flight agent runs (up to ``agent.restart_drain_timeout``
|
||||
seconds), then exits. Both systemd (``Restart=always``) and launchd
|
||||
(unconditional ``<key>KeepAlive</key><true/>``) restart on any exit.
|
||||
seconds), then exits with code 75. Systemd units generated by Hermes use
|
||||
``Restart=on-failure`` together with ``RestartForceExitStatus=75`` so the
|
||||
service is relaunched after the graceful exit without reviving clean
|
||||
``--replace`` takeovers. launchd (unconditional ``<key>KeepAlive</key>
|
||||
<true/>``) restarts on any exit.
|
||||
|
||||
This is the drain-aware alternative to ``systemctl restart`` / ``SIGTERM``,
|
||||
which SIGKILL in-flight agents after a short timeout.
|
||||
|
|
@ -652,7 +655,7 @@ def _gateway_run_args_for_profile(profile: str) -> list[str]:
|
|||
args = [get_python_path(), "-m", "hermes_cli.main"]
|
||||
if profile != "default":
|
||||
args.extend(["--profile", profile])
|
||||
args.extend(["gateway", "run", "--replace"])
|
||||
args.extend(["gateway", "run"])
|
||||
return args
|
||||
|
||||
|
||||
|
|
@ -2729,7 +2732,7 @@ Environment="LOGNAME={username}"
|
|||
Environment="PATH={sane_path}"
|
||||
Environment="VIRTUAL_ENV={venv_dir}"
|
||||
Environment="HERMES_HOME={hermes_home}"
|
||||
Restart=always
|
||||
Restart=on-failure
|
||||
RestartSec=5
|
||||
RestartForceExitStatus={GATEWAY_SERVICE_RESTART_EXIT_CODE}
|
||||
KillMode=mixed
|
||||
|
|
@ -2763,7 +2766,7 @@ WorkingDirectory={working_dir}
|
|||
Environment="PATH={sane_path}"
|
||||
Environment="VIRTUAL_ENV={venv_dir}"
|
||||
Environment="HERMES_HOME={hermes_home}"
|
||||
Restart=always
|
||||
Restart=on-failure
|
||||
RestartSec=5
|
||||
RestartForceExitStatus={GATEWAY_SERVICE_RESTART_EXIT_CODE}
|
||||
KillMode=mixed
|
||||
|
|
@ -3852,7 +3855,6 @@ def generate_launchd_plist() -> str:
|
|||
[
|
||||
"<string>gateway</string>",
|
||||
"<string>run</string>",
|
||||
"<string>--replace</string>",
|
||||
]
|
||||
)
|
||||
prog_args_xml = "\n ".join(prog_args)
|
||||
|
|
@ -4705,7 +4707,7 @@ def run_gateway(verbose: int = 0, quiet: bool = False, replace: bool = False, fo
|
|||
print()
|
||||
|
||||
# Exit with code 1 if gateway fails to connect any platform,
|
||||
# so systemd Restart=always will retry on transient errors
|
||||
# so systemd Restart=on-failure will retry on transient errors
|
||||
verbosity = None if quiet else verbose
|
||||
|
||||
# ── Exit-path diagnostics ────────────────────────────────────────────
|
||||
|
|
|
|||
|
|
@ -541,7 +541,7 @@
|
|||
|
||||
restart = mkOption {
|
||||
type = types.str;
|
||||
default = "always";
|
||||
default = "on-failure";
|
||||
description = "systemd Restart= policy.";
|
||||
};
|
||||
|
||||
|
|
@ -983,7 +983,7 @@
|
|||
--env MESSAGING_CWD=${containerWorkDir} \
|
||||
${lib.concatStringsSep " " cfg.container.extraOptions} \
|
||||
${cfg.container.image} \
|
||||
${containerDataDir}/current-package/bin/hermes gateway run --replace ${lib.concatStringsSep " " cfg.extraArgs}
|
||||
${containerDataDir}/current-package/bin/hermes gateway run ${lib.concatStringsSep " " cfg.extraArgs}
|
||||
|
||||
echo "${containerIdentity}" > ${identityFile}
|
||||
fi
|
||||
|
|
|
|||
|
|
@ -426,6 +426,8 @@ class TestGeneratedSystemdUnits:
|
|||
assert "ExecStart=" in unit
|
||||
assert "ExecStop=" not in unit
|
||||
assert "ExecReload=/bin/kill -USR1 $MAINPID" in unit
|
||||
assert "Restart=on-failure" in unit
|
||||
assert "Restart=always" not in unit
|
||||
assert f"RestartForceExitStatus={GATEWAY_SERVICE_RESTART_EXIT_CODE}" in unit
|
||||
# TimeoutStopSec must exceed the default drain_timeout (60s) so
|
||||
# systemd doesn't SIGKILL the cgroup before post-interrupt cleanup
|
||||
|
|
@ -537,6 +539,8 @@ class TestGeneratedSystemdUnits:
|
|||
assert "ExecStart=" in unit
|
||||
assert "ExecStop=" not in unit
|
||||
assert "ExecReload=/bin/kill -USR1 $MAINPID" in unit
|
||||
assert "Restart=on-failure" in unit
|
||||
assert "Restart=always" not in unit
|
||||
assert f"RestartForceExitStatus={GATEWAY_SERVICE_RESTART_EXIT_CODE}" in unit
|
||||
# TimeoutStopSec must exceed the default drain_timeout (60s) so
|
||||
# systemd doesn't SIGKILL the cgroup before post-interrupt cleanup
|
||||
|
|
@ -664,7 +668,10 @@ class TestLaunchdServiceRecovery:
|
|||
|
||||
label = gateway_cli.get_launchd_label()
|
||||
domain = gateway_cli._launchd_domain()
|
||||
assert "--replace" in plist_path.read_text(encoding="utf-8")
|
||||
plist_text = plist_path.read_text(encoding="utf-8")
|
||||
# The generator is stubbed above; assert the repair path rewrote the
|
||||
# plist with the (synthetic) generator output.
|
||||
assert plist_text == gateway_cli.generate_launchd_plist()
|
||||
# The calls list includes launchctl print probes from _launchd_domain()
|
||||
# before the bootout/bootstrap calls. Filter to only bootout/bootstrap.
|
||||
service_calls = [c for c in calls if "bootout" in c or "bootstrap" in c]
|
||||
|
|
@ -2412,10 +2419,10 @@ class TestProfileArg:
|
|||
unit = gateway_cli.generate_systemd_unit(system=False)
|
||||
assert "--profile mybot" in unit
|
||||
assert "gateway run" in unit
|
||||
# Under a process supervisor (Restart=always), --replace makes each
|
||||
# restart kill its predecessor → self-kill loop. The systemd unit must
|
||||
# NOT use --replace; the supervisor owns the lifecycle. (--replace stays
|
||||
# on the manual launchd fallback path — see test_launchd_plist_includes_profile.)
|
||||
# Under a process supervisor, --replace makes each restart kill its
|
||||
# predecessor → self-kill loop. Neither the systemd unit nor the
|
||||
# launchd plist may use --replace; the supervisor owns the lifecycle.
|
||||
# (--replace stays on the manual/detached fallback paths only.)
|
||||
assert "--replace" not in unit
|
||||
|
||||
def test_systemd_unit_for_target_user_includes_named_profile(self, tmp_path, monkeypatch):
|
||||
|
|
@ -2450,6 +2457,24 @@ class TestProfileArg:
|
|||
plist = gateway_cli.generate_launchd_plist()
|
||||
assert "<string>--profile</string>" in plist
|
||||
assert "<string>mybot</string>" in plist
|
||||
assert "<string>--replace</string>" not in plist
|
||||
|
||||
def test_gateway_run_args_for_profile_omit_replace(self, monkeypatch):
|
||||
monkeypatch.setattr(gateway_cli, "get_python_path", lambda: "/venv/bin/python")
|
||||
|
||||
default_args = gateway_cli._gateway_run_args_for_profile("default")
|
||||
named_args = gateway_cli._gateway_run_args_for_profile("mybot")
|
||||
|
||||
assert default_args == ["/venv/bin/python", "-m", "hermes_cli.main", "gateway", "run"]
|
||||
assert named_args == [
|
||||
"/venv/bin/python",
|
||||
"-m",
|
||||
"hermes_cli.main",
|
||||
"--profile",
|
||||
"mybot",
|
||||
"gateway",
|
||||
"run",
|
||||
]
|
||||
|
||||
def test_launchd_plist_supports_aqua_and_background_sessions(self):
|
||||
# macOS 26+ only loads the agent in non-Aqua sessions when the plist
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue