mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
refactor(config-remote): core resolves a backend kind generically and asks it through the protocol
HERMES_CONFIG_BACKEND=<kind> now resolves to plugins.config_backends.<kind>.get_backend() (a direct import, never the general plugin loader, D11), with the kind checked against a package-name pattern before any import. What core read from the remote plugin's internals is a protocol method with a no-op FileBackend version: bootstrap_env_names() (env_loader's .env bootstrap), preserve_read_base() (keeping_read_base) and doctor_warnings() (hermes doctor). Addresses teknium1's P2-1 on #122509.
This commit is contained in:
1 parent
a8c44982a3
commit
3f903abeed
6 files changed
+227
-35
No files matched your search
@@ -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=<kind>``, e.g. ``remote``) is the package ``plugins/config_backends/<kind>/``,
|
||||
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.<kind>.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
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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"]
|
||||
@@ -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)
|
||||
|
||||
@@ -0,0 +1,126 @@
|
||||
"""``HERMES_CONFIG_BACKEND=<kind>`` resolves to the package ``plugins/config_backends/<kind>/``
|
||||
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/<FAKE>/`` 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)
|
||||
Reference in new issue
Block a user