David/HERMES-GIELIE
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: harden gateway restart recovery targeting

Browse source
This commit is contained in:
Frank Song 2026-07-14 16:01:56 +08:00
parent ba1a058f87
commit d08116b4d2
4 changed files with 226 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

59
api/agent_health.py
View file

@ -252,44 +252,65 @@ def _gateway_running_pid(gateway_status: Any, pid_path: Path | None) -> int | No
return get_running_pid()
def _accepts_positional_pid_path(get_running_pid: Any) -> bool:
def _declared_pid_path_parameter(
get_running_pid: Any,
) -> inspect.Parameter | None:
try:
signature = inspect.signature(get_running_pid)
except (TypeError, ValueError):
return False
return None
for param in signature.parameters.values():
if param.kind not in (
inspect.Parameter.POSITIONAL_ONLY,
inspect.Parameter.POSITIONAL_OR_KEYWORD,
inspect.Parameter.KEYWORD_ONLY,
):
continue
name = str(param.name or "").lower()
return "path" in name and "pid" in name
return False
if "path" in name and "pid" in name:
return param
return None
def _gateway_running_pid_strict_path(gateway_status: Any, pid_path: Path) -> int | None:
"""Read a PID from an explicit path only; never fall back to ambient state."""
get_running_pid = gateway_status.get_running_pid
for kwargs in (
{"pid_path": pid_path, "cleanup_stale": False},
{"pid_path": pid_path},
):
try:
return get_running_pid(**kwargs)
except TypeError:
pass
if not _accepts_positional_pid_path(get_running_pid):
pid_path_param = _declared_pid_path_parameter(get_running_pid)
if pid_path_param is None:
return None
try:
return get_running_pid(pid_path, cleanup_stale=False)
except TypeError:
if pid_path_param.kind is inspect.Parameter.KEYWORD_ONLY:
kwargs = {pid_path_param.name: pid_path}
try:
return get_running_pid(pid_path)
return get_running_pid(**kwargs, cleanup_stale=False)
except TypeError:
return None
try:
return get_running_pid(**kwargs)
except TypeError:
return None
if pid_path_param.kind is inspect.Parameter.POSITIONAL_OR_KEYWORD:
kwargs = {pid_path_param.name: pid_path}
try:
return get_running_pid(**kwargs, cleanup_stale=False)
except TypeError:
try:
return get_running_pid(**kwargs)
except TypeError:
pass
if pid_path_param.kind in (
inspect.Parameter.POSITIONAL_ONLY,
inspect.Parameter.POSITIONAL_OR_KEYWORD,
):
try:
return get_running_pid(pid_path, cleanup_stale=False)
except TypeError:
try:
return get_running_pid(pid_path)
except TypeError:
return None
return None
def get_active_profile_gateway_running_pid(profile: str | None = None) -> int | None:

34
api/gateway_restart.py
View file

@ -52,17 +52,23 @@ def _release_lock() -> None:
pass
def _gateway_restart_profile_context(profile: str | None = None) -> tuple[Path, str]:
def _gateway_restart_profile_context(profile: str | None = None) -> tuple[Path, str | None]:
"""Return the HERMES_HOME and CLI profile arg for a gateway restart."""
if profile is None:
raw_profile = str(get_active_profile_name() or "default").strip()
active_home = Path(get_active_hermes_home())
else:
raw_profile = str(profile or "").strip()
raw_profile = str(profile or "")
if not raw_profile or not _PROFILE_ID_RE.fullmatch(raw_profile):
raise ValueError(f"Invalid profile for gateway restart: {profile!r}")
active_home = Path(get_hermes_home_for_profile(raw_profile))
if (
raw_profile == "default"
and active_home.name == "default"
and active_home.parent.name == "profiles"
):
return active_home, None
if not raw_profile or not _PROFILE_ID_RE.fullmatch(raw_profile) or _is_root_profile(raw_profile):
return active_home, "default"
return active_home, raw_profile
@ -93,14 +99,24 @@ def restart_active_profile_gateway(
env = os.environ.copy()
env["HERMES_HOME"] = str(active_home)
hermes_cmd = _resolve_hermes_command()
cmd = [hermes_cmd, "--profile", cli_profile, "gateway", "restart"]
cmd = [hermes_cmd]
if cli_profile is not None:
cmd.extend(["--profile", cli_profile])
cmd.extend(["gateway", "restart"])
logger.info(
"Restarting gateway service via CLI command: %s --profile %s gateway restart (HERMES_HOME=%s)",
hermes_cmd,
cli_profile,
active_home,
)
if cli_profile is None:
logger.info(
"Restarting gateway service via CLI command: %s gateway restart (HERMES_HOME=%s)",
hermes_cmd,
active_home,
)
else:
logger.info(
"Restarting gateway service via CLI command: %s --profile %s gateway restart (HERMES_HOME=%s)",
hermes_cmd,
cli_profile,
active_home,
)
proc = subprocess.Popen(
cmd,
stdout=subprocess.PIPE,

55
tests/test_health_restart.py
View file

@ -125,6 +125,30 @@ def test_restart_active_profile_gateway_pins_explicit_default_profile(monkeypatc
assert called["env"]["HERMES_HOME"] == "/mock/hermes/default"
def test_restart_active_profile_gateway_omits_profile_for_isolated_default_home(monkeypatch):
gateway_restart._GATEWAY_RESTART_LOCK = threading.Lock()
called = {}
def fake_popen(args, stdout=None, stderr=None, text=True, env=None):
called["args"] = args
called["env"] = env
return MockPopen(args, stdout_text="ok", returncode=0, env=env)
monkeypatch.setattr(
gateway_restart,
"get_hermes_home_for_profile",
lambda profile: "/mock/hermes/profiles/default",
)
monkeypatch.setattr(gateway_restart.shutil, "which", lambda cmd: "/mock/bin/hermes")
monkeypatch.setattr(gateway_restart.subprocess, "Popen", fake_popen)
result = gateway_restart.restart_active_profile_gateway(profile="default")
assert result["status"] == "completed"
assert called["args"] == ["/mock/bin/hermes", "gateway", "restart"]
assert called["env"]["HERMES_HOME"] == "/mock/hermes/profiles/default"
def test_restart_active_profile_gateway_rejects_malformed_explicit_profile(monkeypatch):
gateway_restart._GATEWAY_RESTART_LOCK = threading.Lock()
@ -133,7 +157,7 @@ def test_restart_active_profile_gateway_rejects_malformed_explicit_profile(monke
monkeypatch.setattr(gateway_restart.subprocess, "Popen", fail_popen)
for profile in ("", "../bad"):
for profile in ("", " default", "default ", "default\n", "../bad", "bad;echo"):
result = gateway_restart.restart_active_profile_gateway(profile=profile)
assert result["status"] == "failed"
@ -141,6 +165,35 @@ def test_restart_active_profile_gateway_rejects_malformed_explicit_profile(monke
assert gateway_restart._GATEWAY_RESTART_LOCK.locked() is False
def test_restart_active_profile_gateway_accepts_renamed_root_alias(monkeypatch):
gateway_restart._GATEWAY_RESTART_LOCK = threading.Lock()
called = {}
def fake_popen(args, stdout=None, stderr=None, text=True, env=None):
called["args"] = args
called["env"] = env
return MockPopen(args, stdout_text="ok", returncode=0, env=env)
monkeypatch.setattr(
gateway_restart,
"get_hermes_home_for_profile",
lambda profile: "/mock/hermes/root" if profile == "rootalias" else "/mock/hermes/other",
)
monkeypatch.setattr(
gateway_restart,
"_is_root_profile",
lambda profile: profile in {"default", "rootalias"},
)
monkeypatch.setattr(gateway_restart.shutil, "which", lambda cmd: "/mock/bin/hermes")
monkeypatch.setattr(gateway_restart.subprocess, "Popen", fake_popen)
result = gateway_restart.restart_active_profile_gateway(profile="rootalias")
assert result["status"] == "completed"
assert called["args"] == ["/mock/bin/hermes", "--profile", "default", "gateway", "restart"]
assert called["env"]["HERMES_HOME"] == "/mock/hermes/root"
def test_restart_active_profile_gateway_failure_preserves_empty_output_contract(monkeypatch):
gateway_restart._GATEWAY_RESTART_LOCK = threading.Lock()

107
tests/test_update_banner_fixes.py
View file

@ -1091,6 +1091,113 @@ class TestAgentUpdateRequiresGatewayRestart:
]
assert [call[1]["HERMES_HOME"] for call in calls["popen"]] == [str(root_home), str(root_home)]
def test_agent_gateway_restart_kwargs_wrapper_pid_change_fails_closed(
self,
monkeypatch,
tmp_path,
):
from api import agent_health, gateway_restart, profiles
import api.updates as upd
root_home = tmp_path / ".hermes"
sticky_home = root_home / "profiles" / "work"
sticky_home.mkdir(parents=True)
calls = {"popen": [], "ambient_pid": 0}
class FailedRestartProcess:
returncode = 7
def communicate(self, timeout=None):
return "", "restart failed"
class AmbientKwargsGatewayStatus:
def __init__(self):
self._pids = iter([201, 202])
def get_running_pid(self, **kwargs):
calls["ambient_pid"] += 1
return next(self._pids)
def fake_popen(args, stdout=None, stderr=None, text=True, env=None):
calls["popen"].append((args, dict(env or {})))
return FailedRestartProcess()
monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", root_home)
monkeypatch.setattr(profiles, "_active_profile", "work")
monkeypatch.setattr(gateway_restart, "_GATEWAY_RESTART_LOCK", threading.Lock())
monkeypatch.setattr(gateway_restart.shutil, "which", lambda cmd: "/mock/bin/hermes")
monkeypatch.setattr(gateway_restart.subprocess, "Popen", fake_popen)
monkeypatch.setattr(agent_health, "_gateway_status_module", AmbientKwargsGatewayStatus)
monkeypatch.setattr(upd.time, 'sleep', lambda seconds: None)
profiles.set_request_profile("default")
try:
ok, result = upd._ensure_gateway_restart_for_agent_update()
finally:
profiles.clear_request_profile()
assert ok is False
assert result["status"] == "failed"
assert calls["ambient_pid"] == 0
assert [call[0] for call in calls["popen"]] == [
["/mock/bin/hermes", "--profile", "default", "gateway", "restart"],
["/mock/bin/hermes", "--profile", "default", "gateway", "restart"],
]
assert [call[1]["HERMES_HOME"] for call in calls["popen"]] == [str(root_home), str(root_home)]
def test_agent_gateway_restart_isolated_default_home_omits_profile_flag(
self,
monkeypatch,
tmp_path,
):
from api import agent_health, gateway_restart, profiles
import api.updates as upd
base_home = tmp_path / ".hermes"
isolated_home = base_home / "profiles" / "default"
isolated_home.mkdir(parents=True)
calls = {"popen": [], "pid_paths": []}
class FailedRestartProcess:
returncode = 7
def communicate(self, timeout=None):
return "", "restart failed"
class PathStrictGatewayStatus:
def get_running_pid(self, pid_path=None, cleanup_stale=False):
path = pathlib.Path(pid_path) if pid_path is not None else None
calls["pid_paths"].append(path)
if path == isolated_home / "gateway.pid":
return 101
return None
def fake_popen(args, stdout=None, stderr=None, text=True, env=None):
calls["popen"].append((args, dict(env or {})))
return FailedRestartProcess()
monkeypatch.setattr(profiles, "_INITIAL_HERMES_HOME", str(isolated_home))
monkeypatch.setattr(profiles, "_INITIAL_ISOLATED_PROFILE_OPT_IN", "1")
monkeypatch.setattr(gateway_restart, "_GATEWAY_RESTART_LOCK", threading.Lock())
monkeypatch.setattr(gateway_restart.shutil, "which", lambda cmd: "/mock/bin/hermes")
monkeypatch.setattr(gateway_restart.subprocess, "Popen", fake_popen)
monkeypatch.setattr(agent_health, "_gateway_status_module", lambda: PathStrictGatewayStatus())
monkeypatch.setattr(upd.time, 'sleep', lambda seconds: None)
ok, result = upd._ensure_gateway_restart_for_agent_update()
assert ok is False
assert result["status"] == "failed"
assert calls["pid_paths"] == [isolated_home / "gateway.pid", isolated_home / "gateway.pid"]
assert [call[0] for call in calls["popen"]] == [
["/mock/bin/hermes", "gateway", "restart"],
["/mock/bin/hermes", "gateway", "restart"],
]
assert [call[1]["HERMES_HOME"] for call in calls["popen"]] == [
str(isolated_home),
str(isolated_home),
]
def test_apply_update_agent_requires_gateway_restart(self, tmp_path, monkeypatch):
import api.updates as upd