RomanNum3ral
/
hermes-agent
mirror of https://github.com/NousResearch/hermes-agent.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(launchd): require a supervised PID to call a reload successful 

The reload retry loop treated `launchctl list <label>` exit 0 as success,
but exit 0 also covers a registered-but-not-running definition (macOS 26+
`state = not running`) — the same trap _probe_launchd_service_running
already guards against. Require a PID so success means launchd is
supervising a live process, in both the Python loop and the shell helper.

Verified against live launchd: a RunAtLoad=false job reports exit 0 with
no PID, which the old check accepted and the new one rejects.

Note this is NOT what distinguishes a draining instance — measured, the
label deregisters within ~1s of bootout while the old process drains on.
Waiting for the old PID to exit is what covers that.
This commit is contained in:
Rob Hilgefort 2026-08-05 21:07:52 -06:00 committed by kshitij
parent a1e4c905f5
commit 65b7151dbd
2 changed files with 79 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
hermes_cli/gateway.py
View File

@ -3899,17 +3899,34 @@ def _append_launchd_reload_log(message: str) -> None:
pass
def _launchctl_label_registered(label: str) -> bool:
"""True when ``launchctl list <label>`` reports the job as registered."""
def _launchctl_label_supervising_process(label: str) -> bool:
"""True when launchd both knows ``label`` AND is running a process for it.
A bare ``launchctl list <label>`` exit-0 only proves a *definition* is
registered — it also returns 0 for ``state = not running`` (macOS 26+),
which is why :func:`_probe_launchd_service_running` already insists on a
PID. The reload's success check needs the same standard: ending the retry
loop on "a definition exists" can report success for a job launchd is not
actually running.
Measured against live launchd (2026-08-05): immediately after ``bootout``
the label deregisters within ~1s (rc=113) while the old process keeps
draining, so this is NOT what distinguishes a draining instance from a
fresh one — waiting for the old PID to exit before bootstrapping is what
does that. This check is the narrower guarantee: success means launchd is
supervising a live process.
"""
try:
result = subprocess.run(
["launchctl", "list", label],
check=False,
timeout=10,
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
capture_output=True,
text=True, encoding='utf-8', errors='replace',
)
return result.returncode == 0
if result.returncode != 0:
return False
return _parse_launchd_pid_from_list_output(result.stdout) is not None
except (subprocess.TimeoutExpired, OSError):
return False
@ -3939,11 +3956,11 @@ def _retry_launchctl_bootstrap_until_registered(
attempt += 1
try:
_launchctl_bootstrap(domain, plist_path, label, timeout=30)
if _launchctl_label_registered(label):
if _launchctl_label_supervising_process(label):
return True
_append_launchd_reload_log(
f"bootstrap attempt {attempt} exited 0 but {domain}/{label} "
f"is not registered (launchctl list) — retrying"
f"has no supervised process (launchctl list) — retrying"
)
except subprocess.CalledProcessError as exc:
_append_launchd_reload_log(
@ -4295,12 +4312,15 @@ def refresh_launchd_plist_if_needed() -> bool:
f"_deadline=$(($(date +%s) + {_reload_budget})); "
f"while :; do "
f" launchctl bootstrap {shlex.quote(domain)} {shlex.quote(str(plist_path))} 2>/dev/null; "
f" if launchctl list {shlex.quote(label)} >/dev/null 2>&1; then break; fi; "
# Require a PID, not just exit 0: a bare `launchctl list` also
# succeeds for a registered-but-not-running definition, which would
# end this loop reporting success for a job launchd isn't running.
f" if launchctl list {shlex.quote(label)} 2>/dev/null | grep -q '\"PID\"'; then break; fi; "
f" echo \"[$(date '+%Y-%m-%d %H:%M:%S %z')] bootstrap not yet registered for {shlex.quote(target)} — retrying\" >> {shlex.quote(str(reload_log_path))}; "
f" if [ $(date +%s) -ge $_deadline ]; then break; fi; "
f" sleep 2; "
f"done; "
f"if ! launchctl list {shlex.quote(label)} >/dev/null 2>&1; then "
f"if ! launchctl list {shlex.quote(label)} 2>/dev/null | grep -q '\"PID\"'; then "
f" echo \"[$(date '+%Y-%m-%d %H:%M:%S %z')] FAILED launchd reload for {shlex.quote(target)} — service NOT registered after {_reload_budget}s of retries\" >> {shlex.quote(str(reload_log_path))}; "
f"fi; "
# Submitted jobs stay registered with launchd after the script

86
tests/hermes_cli/test_gateway_service.py
View File

@ -525,38 +525,6 @@ class TestLaunchdServiceRecovery:
assert waited and waited[0][0] == 4242
def test_registered_but_not_running_is_not_success(self, monkeypatch):
"""A definition with no PID must not end the loop.
`launchctl list` exits 0 for a registered-but-not-running job (macOS
26+ `state = not running`), so exit-0 alone would report success for a
gateway launchd is not actually running. Verified against live launchd
on 2026-08-05.
"""
list_calls = {"n": 0}
def fake_run(cmd, check=False, **kwargs):
if cmd[:2] == ["launchctl", "list"]:
list_calls["n"] += 1
# Registered (exit 0) but no PID line — never running.
return SimpleNamespace(
returncode=0,
stdout='{\n\t"Label" = "ai.hermes.gateway";\n};',
stderr="",
)
return SimpleNamespace(returncode=0, stdout="", stderr="")
monkeypatch.setattr(gateway_cli.subprocess, "run", fake_run)
monkeypatch.setattr(gateway_cli.time, "sleep", lambda *_a, **_k: None)
ok = gateway_cli._retry_launchctl_bootstrap_until_registered(
self.DOMAIN, self.PLIST, self.LABEL,
deadline=gateway_cli.time.monotonic() - 1, # already expired
)
assert ok is False
assert list_calls["n"] >= 1
def test_launchd_domain_uses_user_domain(self, monkeypatch):
# The user/<uid> domain (not gui/<uid>) is the one reachable from
# non-Aqua/background sessions on macOS 26+ (issue #23387).
@ -1937,14 +1905,24 @@ class TestRetryLaunchctlBootstrapUntilRegistered:
PLIST = "/tmp/ai.hermes.gateway.plist"
LABEL = "ai.hermes.gateway"
# `launchctl list <label>` output for a job launchd is actively running.
# Success requires a PID here, not just exit 0 — exit 0 alone also covers a
# registered-but-not-running definition (macOS 26+ `state = not running`).
RUNNING_LIST_OUTPUT = '{\n\t"PID" = 4242;\n\t"Label" = "ai.hermes.gateway";\n};'
def test_returns_true_once_label_is_registered(self, monkeypatch):
"""Success requires launchctl list to confirm registration, not just
a zero bootstrap exit."""
"""Success requires launchctl list to confirm a supervised process, not
just a zero bootstrap exit."""
list_results = iter([1, 0]) # first check: not registered, second: registered
def fake_run(cmd, check=False, **kwargs):
if cmd[:2] == ["launchctl", "list"]:
return SimpleNamespace(returncode=next(list_results))
rc = next(list_results)
return SimpleNamespace(
returncode=rc,
stdout=self.RUNNING_LIST_OUTPUT if rc == 0 else "",
stderr="",
)
return SimpleNamespace(returncode=0, stdout="", stderr="")
monkeypatch.setattr(gateway_cli.subprocess, "run", fake_run)
@ -1969,7 +1947,12 @@ class TestRetryLaunchctlBootstrapUntilRegistered:
return SimpleNamespace(returncode=0, stdout="", stderr="")
if cmd[:2] == ["launchctl", "list"]:
# registered only after the second (successful) bootstrap
return SimpleNamespace(returncode=0 if attempts["bootstrap"] >= 2 else 1)
ok = attempts["bootstrap"] >= 2
return SimpleNamespace(
returncode=0 if ok else 1,
stdout=self.RUNNING_LIST_OUTPUT if ok else "",
stderr="",
)
return SimpleNamespace(returncode=0, stdout="", stderr="")
monkeypatch.setattr(gateway_cli.subprocess, "run", fake_run)
@ -1982,3 +1965,34 @@ class TestRetryLaunchctlBootstrapUntilRegistered:
assert ok is True
assert attempts["bootstrap"] >= 2 # the timeout was retried, not raised
def test_registered_but_not_running_is_not_success(self, monkeypatch):
"""A definition with no PID must not end the loop.
`launchctl list` exits 0 for a registered-but-not-running job (macOS
26+ `state = not running`), so exit-0 alone would report success for a
gateway launchd is not actually running. Verified against live launchd
on 2026-08-05.
"""
list_calls = {"n": 0}
def fake_run(cmd, check=False, **kwargs):
if cmd[:2] == ["launchctl", "list"]:
list_calls["n"] += 1
# Registered (exit 0) but no PID line — never running.
return SimpleNamespace(
returncode=0,
stdout='{\n\t"Label" = "ai.hermes.gateway";\n};',
stderr="",
)
return SimpleNamespace(returncode=0, stdout="", stderr="")
monkeypatch.setattr(gateway_cli.subprocess, "run", fake_run)
monkeypatch.setattr(gateway_cli.time, "sleep", lambda *_a, **_k: None)
ok = gateway_cli._retry_launchctl_bootstrap_until_registered(
self.DOMAIN, self.PLIST, self.LABEL,
deadline=gateway_cli.time.monotonic() - 1, # already expired
)
assert ok is False
assert list_calls["n"] >= 1