fix(gateway): in-container gateway start registers a missing s6 slot
`profile create` registers an s6 gateway slot only when the creating process is itself inside the container: `detect_service_manager()` reads `/proc/1/comm` in the CALLER's PID namespace, so on a host whose `~/.hermes` is bind-mounted into the container the hook is a silent no-op. The profile directory lands exactly where the container reads it, but no slot exists, and `hermes -p <name> gateway start` inside the container fails with "not registered" until the operator restarts the container so the boot reconciler notices. Same symptom as #54174 reaches `profile install` by the same route. Register the slot on demand instead. When `start` hits `GatewayNotRegisteredError` and the profile directory carries `SOUL.md` — the boot reconciler's own "real profile" marker — create the slot and start it. That makes `_maybe_register_gateway_service`'s promise true without a restart. Deliberately narrow: - Only `start` self-heals. `stop` and `restart` on an unregistered profile keep the original error; registering a slot in order to stop it would be absurd. - `SOUL.md` gates it, 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 after a container restart keeps a single owner. - `ValueError` (slot appeared underneath us) and `RuntimeError` (s6-svscanctl failed) surface as the existing actionable error, never a traceback. Refs #54174. PR #54182 fixes the narrower `profile install` case by adding the same registration call; this closes the symptom for any existing profile directory and without a container restart.
This commit is contained in:
committed by
Teknium
parent
49b9bbb6fc
commit
9c36cafcec
+42
-1
@@ -5873,12 +5873,53 @@ def _dispatch_via_service_manager_if_s6(action: str, profile: str | None = None)
|
||||
return False
|
||||
try:
|
||||
getattr(mgr, action)(f"gateway-{profile}")
|
||||
except (GatewayNotRegisteredError, S6CommandError) as exc:
|
||||
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:
|
||||
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
|
||||
|
||||
@@ -154,3 +154,105 @@ def test_redirect_falls_back_when_sleep_missing(
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Lazy slot registration — a profile dir that exists but was never registered
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class _UnregisteredRecorder(_CallRecorder):
|
||||
"""Recorder whose slot is missing until ``register_profile_gateway`` runs."""
|
||||
|
||||
def __init__(self, *, register_error: Exception | None = None) -> 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:
|
||||
from hermes_cli.service_manager import GatewayNotRegisteredError
|
||||
if name not in self._slots:
|
||||
raise GatewayNotRegisteredError(name.removeprefix("gateway-"))
|
||||
self.calls.append(("start", 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."""
|
||||
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)
|
||||
if seed_soul:
|
||||
(tmp_path / "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."""
|
||||
mgr = _UnregisteredRecorder()
|
||||
gw = _arrange(monkeypatch, tmp_path, mgr, 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"."""
|
||||
mgr = _UnregisteredRecorder()
|
||||
gw = _arrange(monkeypatch, tmp_path, mgr, seed_soul=False)
|
||||
|
||||
with pytest.raises(SystemExit) as excinfo:
|
||||
gw._dispatch_via_service_manager_if_s6("start", "codr")
|
||||
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user