refactor(gateway): move the on-demand s6 slot helper into service_manager, trim tests
Reshape of the cherry-picked fix from #104194: - The SOUL.md gate + `register_profile_gateway(start_now=False)` now live in `hermes_cli/service_manager.py::register_unregistered_profile_gateway`, next to the s6 manager and `_profile_dir_for_gateway_service` it needs, instead of a private reach-in from the 6.5k-line `hermes_cli/gateway.py` facade. The facade only decides "start repairs, stop/restart re-raise" and keeps ONE error handler (S6Error is a RuntimeError; register's ValueError/RuntimeError/OSError surface as the same `✗` + exit 1). - Tests trimmed from four to two invariants: start on a real profile registers `down` and then starts; stop on an unregistered profile / start on a directory without SOUL.md keep the original error and mint nothing (parametrized). Dropped: the registration-failure traceback test (covered by the single except clause) and the duplicate no-marker/stop split. The test now resolves the profile dir through the real HERMES_HOME mapping instead of monkeypatching `_profile_dir_for_gateway_service`. - Docs: docker.md multi-profile section says `gateway start` inside the container registers a slot for a profile created from the host.
This commit is contained in:
teknium1
+14
-44
@@ -5861,7 +5861,8 @@ def _dispatch_via_service_manager_if_s6(action: str, profile: str | None = None)
|
||||
"""Dispatch start/stop/restart via s6 inside an s6 container; True iff dispatched (caller returns).
|
||||
Profile defaults to the current one; missing slot / s6 errors become actionable CLI messages."""
|
||||
from hermes_cli.service_manager import (
|
||||
GatewayNotRegisteredError, S6CommandError, detect_service_manager, get_service_manager,
|
||||
GatewayNotRegisteredError, detect_service_manager, get_service_manager,
|
||||
register_unregistered_profile_gateway,
|
||||
)
|
||||
|
||||
if detect_service_manager() != "s6":
|
||||
@@ -5871,55 +5872,24 @@ def _dispatch_via_service_manager_if_s6(action: str, profile: str | None = None)
|
||||
mgr = get_service_manager()
|
||||
if action not in ("start", "stop", "restart"):
|
||||
return False
|
||||
service = f"gateway-{profile}"
|
||||
try:
|
||||
getattr(mgr, action)(f"gateway-{profile}")
|
||||
except GatewayNotRegisteredError as exc:
|
||||
# A profile directory can exist without a slot: it was created against a bind-mounted
|
||||
# HERMES_HOME from outside the container, where `profile create` cannot reach
|
||||
# /run/service. Register it here instead of making the operator restart the container
|
||||
# to let the boot reconciler notice — which is what profiles._maybe_register_gateway_service
|
||||
# already promises. Anything else stays an actionable error.
|
||||
if action != "start" or not _register_missing_gateway_slot(mgr, profile):
|
||||
print(f"✗ {exc}")
|
||||
sys.exit(1)
|
||||
except S6CommandError as exc:
|
||||
try:
|
||||
getattr(mgr, action)(service)
|
||||
except GatewayNotRegisteredError:
|
||||
# A profile created from the HOST against a bind-mounted home has a directory but no
|
||||
# slot (`profile create` cannot reach the container's /run/service). Only `start`
|
||||
# repairs that; stop/restart on a missing slot stay an error.
|
||||
if action != "start" or not register_unregistered_profile_gateway(mgr, profile):
|
||||
raise
|
||||
print(f"✓ registered the s6 gateway slot for profile {profile!r}")
|
||||
mgr.start(service)
|
||||
except (RuntimeError, ValueError, OSError) as exc: # S6Error is a RuntimeError
|
||||
print(f"✗ {exc}")
|
||||
sys.exit(1)
|
||||
return True
|
||||
|
||||
|
||||
def _register_missing_gateway_slot(mgr, profile: str) -> bool:
|
||||
"""Register and start an s6 slot for an existing-but-unregistered profile.
|
||||
|
||||
Returns False when ``profile`` is not a real profile, leaving the caller's "not registered"
|
||||
error intact: the guard is the boot reconciler's own marker (``SOUL.md``, seeded by
|
||||
``profile create``), so a mistyped ``-p`` name or a stray directory cannot mint a phantom
|
||||
slot for a profile that does not exist.
|
||||
|
||||
Registers with ``start_now=False`` and then goes through the ordinary ``start`` path, so the
|
||||
``desired_state`` write that lets boot reconciliation restore want-up keeps a single owner.
|
||||
"""
|
||||
from hermes_cli.service_manager import (
|
||||
GatewayNotRegisteredError, S6CommandError, _profile_dir_for_gateway_service,
|
||||
)
|
||||
|
||||
service_name = f"gateway-{profile}"
|
||||
try:
|
||||
profile_dir = _profile_dir_for_gateway_service(service_name)
|
||||
except Exception:
|
||||
return False
|
||||
if not (profile_dir / "SOUL.md").exists():
|
||||
return False
|
||||
try:
|
||||
mgr.register_profile_gateway(profile, start_now=False)
|
||||
mgr.start(service_name)
|
||||
except (ValueError, RuntimeError, GatewayNotRegisteredError, S6CommandError, OSError) as exc:
|
||||
print(f"✗ could not register the gateway slot for profile {profile!r}: {exc}")
|
||||
sys.exit(1)
|
||||
print(f"✓ registered the s6 gateway slot for profile {profile!r}")
|
||||
return True
|
||||
|
||||
|
||||
def _dispatch_all_via_service_manager_if_s6(action: str) -> bool:
|
||||
"""Dispatch ``--all`` stop/restart to every registered profile gateway under s6; True iff dispatched.
|
||||
A bare pkill is seen by s6-supervise as a crash and restarted ~1s later; the service manager flips
|
||||
|
||||
@@ -270,6 +270,24 @@ def _write_gateway_desired_state(name: str, desired_state: str) -> None:
|
||||
return
|
||||
|
||||
|
||||
def register_unregistered_profile_gateway(mgr: ServiceManager, profile: str) -> bool:
|
||||
"""Register a ``down`` s6 slot for a profile whose directory exists but was never registered.
|
||||
|
||||
`hermes profile create` can only register a slot when it runs inside the container; created
|
||||
from the host against a bind-mounted home, the directory lands where the container reads it
|
||||
but no ``/run/service/gateway-<name>`` exists, and the boot reconciler only notices on the
|
||||
next container restart. Returns False without touching anything unless the directory carries
|
||||
``SOUL.md`` — the reconciler's own "real profile" marker — so a mistyped ``-p`` name cannot
|
||||
mint a phantom slot. ``start_now=False``: the caller's ordinary ``start`` stays the single
|
||||
owner of the ``desired_state`` write.
|
||||
"""
|
||||
profile_dir = _profile_dir_for_gateway_service(f"{S6_SERVICE_PREFIX}{profile}")
|
||||
if not (profile_dir / "SOUL.md").exists():
|
||||
return False
|
||||
mgr.register_profile_gateway(profile, start_now=False)
|
||||
return True
|
||||
|
||||
|
||||
# s6-overlay installs its binaries under /command/ and only adds it to PATH inside the supervision
|
||||
# tree. Out-of-tree entry points (``docker exec``, the profile create/delete hooks) inherit the base
|
||||
# PATH, so every s6 invocation uses this absolute prefix. Not ``/usr/bin/s6-*``: the
|
||||
|
||||
@@ -152,107 +152,84 @@ def test_redirect_falls_back_when_sleep_missing(
|
||||
assert "`sleep` is unavailable" in err
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Lazy slot registration — a profile dir that exists but was never registered
|
||||
# On-demand slot registration — a profile dir that exists but was never registered (#111720)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class _UnregisteredRecorder(_CallRecorder):
|
||||
"""Recorder whose slot is missing until ``register_profile_gateway`` runs."""
|
||||
|
||||
def __init__(self, *, register_error: Exception | None = None) -> None:
|
||||
def __init__(self) -> None:
|
||||
super().__init__()
|
||||
self.registered: list[tuple[str, bool]] = []
|
||||
self._register_error = register_error
|
||||
self._slots: set[str] = set()
|
||||
|
||||
def start(self, name: str) -> None:
|
||||
def _svc(self, action: str, name: str) -> None:
|
||||
from hermes_cli.service_manager import GatewayNotRegisteredError
|
||||
if name not in self._slots:
|
||||
raise GatewayNotRegisteredError(name.removeprefix("gateway-"))
|
||||
self.calls.append(("start", name))
|
||||
self.calls.append((action, name))
|
||||
|
||||
def start(self, name: str) -> None:
|
||||
self._svc("start", name)
|
||||
|
||||
def stop(self, name: str) -> None:
|
||||
self._svc("stop", name)
|
||||
|
||||
def register_profile_gateway(self, profile: str, *, start_now: bool = True) -> None:
|
||||
if self._register_error is not None:
|
||||
raise self._register_error
|
||||
self.registered.append((profile, start_now))
|
||||
self._slots.add(f"gateway-{profile}")
|
||||
|
||||
|
||||
def _arrange(monkeypatch, tmp_path, mgr, *, seed_soul: bool):
|
||||
"""Point the helper at ``tmp_path`` as the profile dir and force the s6 branch."""
|
||||
def _arrange(monkeypatch, tmp_path, mgr, *, profile: str, seed_soul: bool):
|
||||
"""Force the s6 branch and make ``tmp_path`` the shared HERMES_HOME the slot maps back to."""
|
||||
from hermes_cli import gateway as gw
|
||||
from hermes_cli import service_manager as sm
|
||||
|
||||
monkeypatch.setattr(sm, "detect_service_manager", lambda: "s6")
|
||||
monkeypatch.setattr(sm, "get_service_manager", lambda: mgr)
|
||||
monkeypatch.setattr(sm, "_profile_dir_for_gateway_service", lambda name: tmp_path)
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
profile_dir = tmp_path / "profiles" / profile
|
||||
profile_dir.mkdir(parents=True)
|
||||
if seed_soul:
|
||||
(tmp_path / "SOUL.md").write_text("# soul\n", encoding="utf-8")
|
||||
(profile_dir / "SOUL.md").write_text("# soul\n", encoding="utf-8")
|
||||
return gw
|
||||
|
||||
|
||||
def test_start_registers_a_missing_slot_for_a_real_profile(monkeypatch, tmp_path, capsys):
|
||||
"""The bind-mounted-host-create shape: the directory exists (SOUL.md present) but no s6
|
||||
slot does. Starting must register and come up, not demand a container restart."""
|
||||
"""A profile created from the host against a bind-mounted home has a directory (SOUL.md) but
|
||||
no s6 slot. ``gateway start`` must register it and come up instead of demanding a container
|
||||
restart; the registration is ``down`` so the ordinary ``start`` owns the desired-state write."""
|
||||
mgr = _UnregisteredRecorder()
|
||||
gw = _arrange(monkeypatch, tmp_path, mgr, seed_soul=True)
|
||||
gw = _arrange(monkeypatch, tmp_path, mgr, profile="coder", seed_soul=True)
|
||||
|
||||
assert gw._dispatch_via_service_manager_if_s6("start", "coder") is True
|
||||
|
||||
# start_now=False + the ordinary start path keeps ONE owner for the desired-state write.
|
||||
assert mgr.registered == [("coder", False)]
|
||||
assert mgr.calls == [("start", "gateway-coder")]
|
||||
assert "registered the s6 gateway slot" in capsys.readouterr().out
|
||||
|
||||
|
||||
def test_start_refuses_to_mint_a_slot_without_the_real_profile_marker(monkeypatch, tmp_path, capsys):
|
||||
"""A mistyped ``-p`` name or a stray directory must keep the original error: SOUL.md is
|
||||
the boot reconciler's own marker for "this is a real profile"."""
|
||||
@pytest.mark.parametrize(
|
||||
("action", "seed_soul"),
|
||||
[
|
||||
pytest.param("stop", True, id="stop-never-registers"),
|
||||
pytest.param("start", False, id="no-soul-marker-mints-nothing"),
|
||||
],
|
||||
)
|
||||
def test_missing_slot_stays_an_error_outside_the_repair_case(
|
||||
monkeypatch, tmp_path, capsys, action, seed_soul
|
||||
):
|
||||
"""Only ``start`` on a real profile self-heals: stopping an unregistered profile and starting a
|
||||
mistyped/stray directory (no SOUL.md) keep the original ✗ + exit 1 and mint no slot."""
|
||||
mgr = _UnregisteredRecorder()
|
||||
gw = _arrange(monkeypatch, tmp_path, mgr, seed_soul=False)
|
||||
gw = _arrange(monkeypatch, tmp_path, mgr, profile="coder", seed_soul=seed_soul)
|
||||
|
||||
with pytest.raises(SystemExit) as excinfo:
|
||||
gw._dispatch_via_service_manager_if_s6("start", "codr")
|
||||
gw._dispatch_via_service_manager_if_s6(action, "coder")
|
||||
|
||||
assert excinfo.value.code == 1
|
||||
assert mgr.registered == []
|
||||
assert "✗" in capsys.readouterr().out
|
||||
|
||||
|
||||
def test_stop_never_registers_a_slot(monkeypatch, tmp_path, capsys):
|
||||
"""Only ``start`` self-heals. Stopping a profile that has no slot is still an error —
|
||||
registering one just to stop it would be absurd."""
|
||||
mgr = _UnregisteredRecorder()
|
||||
|
||||
def _stop(name: str) -> None:
|
||||
from hermes_cli.service_manager import GatewayNotRegisteredError
|
||||
raise GatewayNotRegisteredError(name.removeprefix("gateway-"))
|
||||
|
||||
mgr.stop = _stop # type: ignore[method-assign]
|
||||
gw = _arrange(monkeypatch, tmp_path, mgr, seed_soul=True)
|
||||
|
||||
with pytest.raises(SystemExit) as excinfo:
|
||||
gw._dispatch_via_service_manager_if_s6("stop", "coder")
|
||||
|
||||
assert excinfo.value.code == 1
|
||||
assert mgr.registered == []
|
||||
|
||||
|
||||
def test_registration_failure_is_an_actionable_error_not_a_traceback(monkeypatch, tmp_path, capsys):
|
||||
"""``register_profile_gateway`` raises RuntimeError when s6-svscanctl fails and ValueError
|
||||
on a slot that appeared underneath us. Both must surface as ✗ + exit 1."""
|
||||
mgr = _UnregisteredRecorder(register_error=RuntimeError("s6-svscanctl failed: no scandir"))
|
||||
gw = _arrange(monkeypatch, tmp_path, mgr, seed_soul=True)
|
||||
|
||||
with pytest.raises(SystemExit) as excinfo:
|
||||
gw._dispatch_via_service_manager_if_s6("start", "coder")
|
||||
|
||||
assert excinfo.value.code == 1
|
||||
out = capsys.readouterr().out
|
||||
assert "could not register the gateway slot" in out
|
||||
assert "s6-svscanctl failed" in out
|
||||
assert mgr.calls == []
|
||||
assert "no such gateway 'coder'" in capsys.readouterr().out
|
||||
|
||||
@@ -235,6 +235,8 @@ Each profile created with `hermes profile create <name>` gets:
|
||||
- Per-profile rotated logs at `${HERMES_HOME}/logs/gateways/<name>/current` (10 archives × 1 MB each).
|
||||
- State persistence across container restarts: the boot-time reconciler reads `gateway_state.json` from each profile directory and brings the slot back up only for profiles whose last recorded state was `running`. Only a gateway you explicitly stopped (`hermes gateway stop`) stays down across a restart — a container restart, image upgrade, or unexpected exit leaves the recorded state as `running`, so the gateway auto-starts on the next boot.
|
||||
|
||||
A profile created from the **host** against a bind-mounted `~/.hermes` gets its directory but no slot (the host process cannot reach the container's `/run/service`). Inside the container, `hermes -p <name> gateway start` registers the missing slot on demand and starts it — no `docker restart` needed. Only `start` does this, and only for a real profile directory (one carrying `SOUL.md`); `stop`/`restart` on an unregistered profile and a mistyped `-p` name still fail with `✗ no such gateway`.
|
||||
|
||||
The lifecycle commands you'd run on the host work the same way from inside the container:
|
||||
|
||||
```sh
|
||||
|
||||
Reference in New Issue
Block a user