From 9c36cafcec8efe116c8103e1b330cd3e2987d113 Mon Sep 17 00:00:00 2001 From: John Paul Soliva Date: Sun, 6 Sep 2026 15:41:43 +0900 Subject: [PATCH] fix(gateway): in-container gateway start registers a missing s6 slot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 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. --- hermes_cli/gateway.py | 43 +++++++- tests/hermes_cli/test_gateway_s6_dispatch.py | 102 +++++++++++++++++++ 2 files changed, 144 insertions(+), 1 deletion(-) diff --git a/hermes_cli/gateway.py b/hermes_cli/gateway.py index 6fdeea4bbc..0f9b150bbd 100644 --- a/hermes_cli/gateway.py +++ b/hermes_cli/gateway.py @@ -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 diff --git a/tests/hermes_cli/test_gateway_s6_dispatch.py b/tests/hermes_cli/test_gateway_s6_dispatch.py index a4fa7b37c6..7755e8fc8b 100644 --- a/tests/hermes_cli/test_gateway_s6_dispatch.py +++ b/tests/hermes_cli/test_gateway_s6_dispatch.py @@ -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