Compare commits

...

1 Commits

Author SHA1 Message Date
teknium1
0fc14fc2e9 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.
2026-07-06 05:22:02 -07:00
3 changed files with 41 additions and 14 deletions

View File

@@ -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 ────────────────────────────────────────────

View File

@@ -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

View File

@@ -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