diff --git a/hermes_cli/config_backend.py b/hermes_cli/config_backend.py index 3142909565..76a142e88a 100644 --- a/hermes_cli/config_backend.py +++ b/hermes_cli/config_backend.py @@ -10,8 +10,9 @@ already keys on the config path, so a backend keeps per-home state. the version is the ``file_signature`` stat tuple, reads are ``open`` + ``fast_safe_load`` and raise exactly what those raise (``FileNotFoundError`` for an absent file, ``OSError``, YAML errors), and writes are the ruamel round-trip writers in ``utils``. Callers keep their own error handling, so a -backend whose layer always exists simply never takes the "no file" branches. The remote backend -(``HERMES_CONFIG_BACKEND=remote``) lives in ``plugins/config_backends/remote/``. +backend whose layer always exists simply never takes the "no file" branches. Every other kind +(``HERMES_CONFIG_BACKEND=``, e.g. ``remote``) is the package ``plugins/config_backends//``, +whose ``get_backend()`` returns the instance; this module names no backend beyond that. The managed scope (``/etc/hermes``) is NOT a backend concern: it stays an overlay applied on top of whatever user layer the backend returns. @@ -20,7 +21,9 @@ from __future__ import annotations import contextlib import contextvars +import importlib import os +import re import sys import threading from dataclasses import dataclass, field @@ -110,6 +113,21 @@ class ConfigBackend(Protocol): """Non-secret env names that select and address this backend (see :func:`deployment_env_names`).""" ... + def bootstrap_env_names(self) -> frozenset: + """Env names published from the launch home's and the managed ``.env`` before the first + config read (design §4.4 steps 1-2): what this backend needs to select, address and + authenticate itself before any config exists.""" + ... + + def preserve_read_base(self) -> contextlib.AbstractContextManager: + """A context in which a read does not become the base a later document save is diffed + against (see :func:`keeping_read_base`).""" + ... + + def doctor_warnings(self, home: Path) -> tuple: + """``(warning, hint, issue)`` triples ``hermes doctor`` reports after a successful read.""" + ... + def apply_in_memory(self, home: Path, doc: dict) -> None: """Replace the user layer in memory only (in-memory migration, D12). Only called for a backend without file tooling; nothing is sent or written.""" @@ -236,6 +254,15 @@ class FileBackend: def deployment_env_names(self) -> frozenset: return frozenset() + def bootstrap_env_names(self) -> frozenset: + return frozenset() + + def preserve_read_base(self) -> contextlib.AbstractContextManager: + return contextlib.nullcontext() + + def doctor_warnings(self, home: Path) -> tuple: + return () + def apply_in_memory(self, home: Path, doc: dict) -> None: raise NotImplementedError("the file backend persists migrations to config.yaml") @@ -254,10 +281,13 @@ _BOOTSTRAPPED = False _BOOTSTRAP_OWNER: Optional[int] = None -def _bootstrap_names() -> frozenset: - from plugins.config_backends.remote.backend import POLL_ENV - from plugins.config_backends.remote.credentials import PLANE_CREDENTIAL_ENV_NAMES - return PLANE_CREDENTIAL_ENV_NAMES | {POLL_ENV} +def _bootstrap_names(kind: str) -> frozenset: + """The env names backend *kind* (as the ``.env`` layers select it) needs before the first read; + none for a kind this build lacks (selection refuses it afterwards).""" + try: + return _backend_for(kind).bootstrap_env_names() + except ConfigBackendUnavailable: + return frozenset() def _ensure_bootstrap_env() -> None: @@ -285,7 +315,7 @@ def _bootstrap(home: Optional[Path], project_env: Optional[Path], *, once: bool) _BOOTSTRAP_OWNER = threading.get_ident() try: from hermes_cli.env_loader import apply_config_bootstrap_env - # names resolved only for a remote deployment + # names resolved only for the kind the .env layers select apply_config_bootstrap_env(_bootstrap_names, home, project_env) except ImportError: # no usable python-dotenv (a bare installer interpreter): no .env loads either pass @@ -303,15 +333,35 @@ def get_config_backend() -> ConfigBackend: select it (D11). Raises :class:`ConfigBackendUnavailable` for a backend this build lacks. """ _ensure_bootstrap_env() - kind = os.environ.get(BACKEND_ENV, "").strip().lower() or "file" + return _backend_for(os.environ.get(BACKEND_ENV, "").strip().lower() or "file") + + +# A kind is one package name under plugins/config_backends/: never a dotted or relative path. +_KIND_RE = re.compile(r"[a-z][a-z0-9_]{0,63}") +_BACKENDS_PACKAGE = "plugins.config_backends" + + +def _backend_for(kind: str) -> ConfigBackend: + """``file``, or the instance ``plugins.config_backends..get_backend()`` returns. The + package is imported directly, never through the general plugin loader (which reads config, D11).""" if kind == "file": return _FILE_BACKEND - if kind == "remote": - # Imported directly, never through the general plugin loader (which reads config, D11). - from plugins.config_backends.remote import get_remote_backend - return get_remote_backend() - raise ConfigBackendUnavailable( - f"{BACKEND_ENV}={kind!r} is not a config backend (expected 'file' or 'remote').") + unknown = f"{BACKEND_ENV}={kind!r} is not a config backend this build provides (expected 'file' or 'remote')." + if not _KIND_RE.fullmatch(kind): + raise ConfigBackendUnavailable(unknown) + name = f"{_BACKENDS_PACKAGE}.{kind}" + module = sys.modules.get(name) + if module is None: + try: + module = importlib.import_module(name) + except ImportError as exc: + if exc.name not in (name, _BACKENDS_PACKAGE, "plugins"): + raise # the backend exists but is broken: show that, not "unknown" + raise ConfigBackendUnavailable(unknown) from exc + get_backend = getattr(module, "get_backend", None) + if not callable(get_backend): + raise ConfigBackendUnavailable(unknown) + return get_backend() class _ExplicitFile(FileBackend): @@ -444,14 +494,9 @@ def write_config_keys(config_path: PathLike, updates: Mapping[str, Any]) -> None def keeping_read_base(read): """Run ``read()`` (a pre-write guard read) without making it the read a later document save is diffed against: the caller's document came from ITS earlier read (Remote Config). A - context that never read keeps the guard read. No-op unless the remote backend is loaded.""" - mod = sys.modules.get("plugins.config_backends.remote.backend") - held = mod._READ_BASE.get() if mod is not None else None - try: + context that never read keeps the guard read. No-op for the file backend.""" + with get_config_backend().preserve_read_base(): return read() - finally: - if held is not None: - mod._READ_BASE.set(held) # Dotted prefixes a document save in this context must write whole: a lock on a changed key under diff --git a/hermes_cli/doctor_config.py b/hermes_cli/doctor_config.py index ab79fd608f..0a4a7504fd 100644 --- a/hermes_cli/doctor_config.py +++ b/hermes_cli/doctor_config.py @@ -363,8 +363,8 @@ def _check_config_file(should_fix: bool, f: Finding) -> None: def _check_config_backend(backend, home, f: Finding) -> bool: - """Remote mode (§4.7): report where config comes from instead of the file checks, and flag a - managed /etc/hermes/config.yaml, which remote mode ignores (D18).""" + """A backend without a config file (§4.7): report where config comes from instead of the file + checks, plus the backend's own warnings (remote mode flags an ignored managed config.yaml, D18).""" try: backend.read_user_doc(home) check_ok(f"Config backend: {backend.describe(home)}") @@ -372,11 +372,9 @@ def _check_config_backend(backend, home, f: Finding) -> bool: check_fail(f"Config backend {backend.name!r} unavailable", str(exc.code or "")) f.issues.append(f"Config backend {backend.name!r} cannot serve config for {home}") return False - from plugins.config_backends.remote.backend import managed_config_file - managed = managed_config_file() if backend.name == "remote" else None - if managed is not None: - check_warn(f"{managed} exists but is ignored in remote config mode", "(remove it)") - f.issues.append(f"Remove {managed}: remote config mode takes config and locks only from Remote Config") + for warning, hint, issue in backend.doctor_warnings(home): + check_warn(warning, hint) + f.issues.append(issue) return True diff --git a/hermes_cli/env_loader.py b/hermes_cli/env_loader.py index 92c02ca8fa..cf8777bf90 100644 --- a/hermes_cli/env_loader.py +++ b/hermes_cli/env_loader.py @@ -524,8 +524,8 @@ def selected_config_backend_name(home: Path | None = None) -> str: return (_bootstrap_env(home).get(BACKEND_ENV) or "").strip().lower() or "file" -def apply_config_bootstrap_env(remote_names, home: Path | None = None, project_env: Path | None = None) -> None: - """Publish ``remote_names()`` (the remote backend's deployment and plane credential, D26/D32) from the +def apply_config_bootstrap_env(backend_names, home: Path | None = None, project_env: Path | None = None) -> None: + """Publish ``backend_names(kind)`` (the selected backend's deployment and credential, D26/D32) from the launch home's ``.env`` and the managed ``.env``, with the values ``load_hermes_dotenv`` gives them. Called before the first config read of a process, which can come before ``load_hermes_dotenv`` (``hermes_cli.config`` reads config at import time): the backend is @@ -540,10 +540,8 @@ def apply_config_bootstrap_env(remote_names, home: Path | None = None, project_e if worker_config_snapshot() is not None: return env = _bootstrap_env(home, project_env) - if (env.get(BACKEND_ENV) or "").strip().lower() == "remote": - names = remote_names() - else: - names = {BACKEND_ENV} # the selector itself, so a file deployment is not read as remote meanwhile + # The selector itself always, so a file deployment is not read as another kind meanwhile. + names = {BACKEND_ENV} | set(backend_names((env.get(BACKEND_ENV) or "").strip().lower() or "file")) for name in names: value = env.get(name) if value is not None and os.environ.get(name) != value: diff --git a/plugins/config_backends/remote/__init__.py b/plugins/config_backends/remote/__init__.py index 7ecf128cb0..471ae248b3 100644 --- a/plugins/config_backends/remote/__init__.py +++ b/plugins/config_backends/remote/__init__.py @@ -21,6 +21,11 @@ def get_remote_backend() -> RemoteBackend: return _BACKEND +def get_backend() -> RemoteBackend: + """The entry point ``hermes_cli.config_backend`` resolves ``HERMES_CONFIG_BACKEND=remote`` to.""" + return get_remote_backend() + + def _reset_for_tests() -> None: global _BACKEND with _LOCK: @@ -29,4 +34,4 @@ def _reset_for_tests() -> None: _BACKEND = None -__all__ = ["RemoteBackend", "get_remote_backend"] +__all__ = ["RemoteBackend", "get_backend", "get_remote_backend"] diff --git a/plugins/config_backends/remote/backend.py b/plugins/config_backends/remote/backend.py index 850b4708b9..8e96b7e64d 100644 --- a/plugins/config_backends/remote/backend.py +++ b/plugins/config_backends/remote/backend.py @@ -7,6 +7,7 @@ level with CAS (D10). Nothing is read from or written to a local ``config.yaml`` """ from __future__ import annotations +import contextlib import contextvars import copy import itertools @@ -737,6 +738,25 @@ class RemoteBackend: from hermes_cli.config_backend import BACKEND_ENV return frozenset({BACKEND_ENV, client.URL_ENV, client.INSTANCE_ENV, POLL_ENV}) + def bootstrap_env_names(self) -> frozenset: + return PLANE_CREDENTIAL_ENV_NAMES | {POLL_ENV} # the deployment (selector, plane, instance) and the credential + + @contextlib.contextmanager + def preserve_read_base(self): + held = _READ_BASE.get() + try: + yield + finally: + if held is not None: + _READ_BASE.set(held) + + def doctor_warnings(self, home: Path) -> tuple: + managed = managed_config_file() + if managed is None: + return () + return ((f"{managed} exists but is ignored in remote config mode", "(remove it)", + f"Remove {managed}: remote config mode takes config and locks only from Remote Config"),) + def boot(self, home: Path) -> None: """Design §4.4 steps 2-4: fetch this home's layer (fail closed) and flag a managed file.""" self._state(home) diff --git a/tests/hermes_cli/test_config_backend_kinds.py b/tests/hermes_cli/test_config_backend_kinds.py new file mode 100644 index 0000000000..3d37da8416 --- /dev/null +++ b/tests/hermes_cli/test_config_backend_kinds.py @@ -0,0 +1,126 @@ +"""``HERMES_CONFIG_BACKEND=`` resolves to the package ``plugins/config_backends//`` +through one entry point (``get_backend()``), and everything core needs from a backend is a protocol +method (teknium1 P2-1 on #122509). A backend core has never heard of is served end to end: the +fake package below lives in a temp directory, and nothing in core is patched or edited.""" +from __future__ import annotations + +import os +import sys +import textwrap + +import pytest + +import plugins.config_backends +from hermes_cli import config_backend as cb + +FAKE = "fakekind_p21" + +_FAKE_PACKAGE = ''' +import contextlib + +from hermes_cli.config_backend import FileBackend + +EVENTS = [] + + +class FakeBackend(FileBackend): + name = "{name}" + + def bootstrap_env_names(self): + return frozenset({{"FAKE_P21_PLANE"}}) + + @contextlib.contextmanager + def preserve_read_base(self): + EVENTS.append("enter") + try: + yield + finally: + EVENTS.append("exit") + + def doctor_warnings(self, home): + return (("fake warning for " + home.name, "(fake hint)", "fake issue"),) + + +_INSTANCE = FakeBackend() + + +def get_backend(): + return _INSTANCE +''' + + +@pytest.fixture +def fake_kind(tmp_path, monkeypatch): + """Registers ``plugins/config_backends//`` from a temp dir (the package path is extended, + the real import machinery resolves it) and starts a fresh process bootstrap.""" + root = tmp_path / "backends" + (root / FAKE).mkdir(parents=True) + (root / FAKE / "__init__.py").write_text(textwrap.dedent(_FAKE_PACKAGE.format(name=FAKE)), encoding="utf-8") + monkeypatch.setattr(plugins.config_backends, "__path__", [*plugins.config_backends.__path__, str(root)]) + home = tmp_path / "home" + home.mkdir() + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.delenv(cb.BACKEND_ENV, raising=False) + monkeypatch.delenv("FAKE_P21_PLANE", raising=False) + monkeypatch.setattr(cb, "_BOOTSTRAPPED", False) + yield home + for name in [m for m in sys.modules if m.startswith(f"plugins.config_backends.{FAKE}")]: + del sys.modules[name] + + +def test_unknown_kind_is_served_through_protocol_methods_only(fake_kind): + from hermes_cli.doctor_config import _check_config_backend + from hermes_cli.doctor_report import Finding + + home = fake_kind + (home / ".env").write_text(f"HERMES_CONFIG_BACKEND={FAKE}\nFAKE_P21_PLANE=https://plane.test\n", encoding="utf-8") + (home / "config.yaml").write_text("a: 1\n", encoding="utf-8") + + backend = cb.get_config_backend() + assert backend.name == FAKE + fake = sys.modules[f"plugins.config_backends.{FAKE}"] + assert backend is fake.get_backend() + # bootstrap_env_names(): published from the launch home's .env before the first read + assert os.environ["FAKE_P21_PLANE"] == "https://plane.test" + # preserve_read_base(): keeping_read_base runs the read inside the backend's context + assert cb.keeping_read_base(lambda: cb.read_config_doc(home / "config.yaml")) == {"a": 1} + assert fake.EVENTS == ["enter", "exit"] + # doctor hook + f = Finding() + assert _check_config_backend(backend, home, f) is True + assert f.issues == ["fake issue"] + + +def test_file_backend_protocol_defaults_are_no_ops(tmp_path): + fb = cb.FileBackend() + assert fb.bootstrap_env_names() == frozenset() + assert fb.doctor_warnings(tmp_path) == () + with fb.preserve_read_base(): + pass + + +@pytest.mark.parametrize("kind", ["../etc", "remote.backend", "remote x", "os;id", "_remote", "9lives", + "x" * 65, "remote/backend", "plugins.config_backends.remote"]) +def test_kind_is_validated_before_any_import(kind, monkeypatch): + def no_import(name, *a, **k): + raise AssertionError(f"imported {name!r} for an invalid kind") + monkeypatch.setattr(cb.importlib, "import_module", no_import) + monkeypatch.setattr(cb, "_BOOTSTRAPPED", True) + monkeypatch.setenv(cb.BACKEND_ENV, kind) + with pytest.raises(cb.ConfigBackendUnavailable, match="is not a config backend"): + cb.get_config_backend() + + +def test_absent_kind_fails_closed(monkeypatch): + monkeypatch.setattr(cb, "_BOOTSTRAPPED", True) + monkeypatch.setenv(cb.BACKEND_ENV, "nosuchbackend") + with pytest.raises(cb.ConfigBackendUnavailable, match="is not a config backend"): + cb.get_config_backend() + + +def test_broken_backend_package_shows_its_own_import_error(fake_kind): + pkg = plugins.config_backends.__path__[-1] + with open(f"{pkg}/{FAKE}/__init__.py", "w", encoding="utf-8") as fh: + fh.write("import hermes_missing_dependency_p21\n") + with pytest.raises(ModuleNotFoundError, match="hermes_missing_dependency_p21"): + cb._backend_for(FAKE)