mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
fix(config): importing hermes_cli.config or hermes_cli.auth reads no config
hermes_cli.config filled OPTIONAL_ENV_VARS from provider profiles at import, and hermes_cli.auth mirrored plugin providers into PROVIDER_REGISTRY at import. Both list providers, and provider discovery reads config (plugins.enabled, plugins.isolation). An import-time config read is the launch profile's config in a process that serves several profiles, and it deadlocks a config backend whose own load imports hermes_cli.auth (the remote backend's Nous credential). Both now run on first read: OPTIONAL_ENV_VARS is a dict that runs one armed fill before it is first read (CORE_DECLARED_ENV_NAMES follows it via module __getattr__), and PROVIDER_REGISTRY syncs before its first read. Contents and precedence are unchanged: core entries win, a provider entry replaces a platform manifest's, the first provider to declare a name wins. A fill that fails with ConfigBackendUnavailable propagates and re-arms.
This commit is contained in:
1 parent
7b62fb095d
commit
7e037e8b62
7 files changed
+255
-41
No files matched your search
+1
-1
@@ -257,7 +257,7 @@ from hermes_cli.auth_plugin_providers import (
|
||||
canonical_registry_rows as _canonical_registry_rows, get_plugin_oauth_auth_status,
|
||||
registry_lookup as _registry_lookup, sync_plugin_provider_registry)
|
||||
|
||||
sync_plugin_provider_registry()
|
||||
PROVIDER_REGISTRY.sync_before_first_read() # not at import: listing providers reads config
|
||||
|
||||
|
||||
def get_anthropic_key() -> str:
|
||||
|
||||
@@ -19,6 +19,7 @@ from __future__ import annotations
|
||||
|
||||
import copy
|
||||
import logging
|
||||
import threading
|
||||
from typing import Any, Callable, Optional
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
@@ -36,6 +37,9 @@ PLUGIN_AUTH_ACTIONS = ("add", "status", "logout", "refresh")
|
||||
PLUGIN_MIRRORED_PROVIDERS: set[str] = set()
|
||||
|
||||
|
||||
_SYNC_LOCK = threading.RLock()
|
||||
|
||||
|
||||
class HomeScopedRegistry(dict):
|
||||
"""``PROVIDER_REGISTRY``: process-wide storage, each row read as the bound profile home sees it.
|
||||
|
||||
@@ -63,13 +67,34 @@ class HomeScopedRegistry(dict):
|
||||
return row
|
||||
return _home_row(row, user, from_profile=mirrored and shared is None)
|
||||
|
||||
_sync_pending = False
|
||||
|
||||
def sync_before_first_read(self) -> None:
|
||||
"""Mirror plugin profiles (:func:`sync_plugin_provider_registry`) before the first read."""
|
||||
self._sync_pending = True
|
||||
|
||||
def _ready(self) -> None:
|
||||
if not self._sync_pending:
|
||||
return
|
||||
with _SYNC_LOCK: # re-entrant: the sync reads the registry
|
||||
if not self._sync_pending:
|
||||
return
|
||||
self._sync_pending = False
|
||||
try:
|
||||
sync_plugin_provider_registry()
|
||||
except BaseException:
|
||||
self._sync_pending = True
|
||||
raise
|
||||
|
||||
def __getitem__(self, key: str) -> Any:
|
||||
self._ready()
|
||||
row = self._view(key, dict.__getitem__(self, key), _resolver())
|
||||
if row is None:
|
||||
raise KeyError(key)
|
||||
return row
|
||||
|
||||
def get(self, key: str, default: Any = None) -> Any:
|
||||
self._ready()
|
||||
row = dict.get(self, key)
|
||||
row = None if row is None else self._view(key, row, _resolver())
|
||||
return default if row is None else row
|
||||
@@ -78,6 +103,7 @@ class HomeScopedRegistry(dict):
|
||||
return self.get(key) is not None
|
||||
|
||||
def items(self) -> list[tuple[str, Any]]:
|
||||
self._ready()
|
||||
resolve = _resolver()
|
||||
views = ((key, self._view(key, row, resolve)) for key, row in dict.items(self))
|
||||
return [(key, row) for key, row in views if row is not None]
|
||||
|
||||
+18
-35
@@ -586,6 +586,8 @@ def ensure_hermes_home():
|
||||
# ---- Config loading/saving ----
|
||||
|
||||
from hermes_cli.config_defaults import DEFAULT_CONFIG, OPTIONAL_ENV_VARS
|
||||
from hermes_cli import config_provider_env as _config_provider_env
|
||||
from hermes_cli.config_provider_env import core_declared_env_names
|
||||
from hermes_cli.config_providers import (
|
||||
_API_MODE_ALIASES, _CAMEL_ALIASES, _KNOWN_PROVIDER_KEYS, _PROVIDER_NORMALIZE_WARNED,
|
||||
_canonical_api_mode, _coerce_ssl_verify, _custom_provider_entry_to_provider_config,
|
||||
@@ -3734,34 +3736,7 @@ def config_command(args):
|
||||
sys.exit(1)
|
||||
|
||||
|
||||
# ---- OPTIONAL_ENV_VARS injection from provider profiles and platform plugins (once, at import) ----
|
||||
|
||||
def _inject_profile_env_vars() -> None:
|
||||
"""Expose env_vars of every ``auth_type="api_key"`` provider in providers/ via OPTIONAL_ENV_VARS
|
||||
without editing this file."""
|
||||
try:
|
||||
from providers import list_providers
|
||||
for _pp in list_providers():
|
||||
if _pp.auth_type != "api_key":
|
||||
continue
|
||||
for _var in _pp.env_vars:
|
||||
if _var in OPTIONAL_ENV_VARS:
|
||||
continue
|
||||
_is_key = not _var.endswith(("_BASE_URL", "_URL"))
|
||||
_label = _pp.display_name or _pp.name
|
||||
OPTIONAL_ENV_VARS[_var] = {
|
||||
"description": f"{_label} {'API key' if _is_key else 'base URL override'}",
|
||||
"prompt": f"{_label} {'API key' if _is_key else 'base URL (leave empty for default)'}",
|
||||
"url": _pp.signup_url or None,
|
||||
"password": _is_key,
|
||||
"category": "provider",
|
||||
"advanced": True}
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
|
||||
_inject_profile_env_vars()
|
||||
|
||||
# ---- OPTIONAL_ENV_VARS: platform plugin entries at import; provider entries on first read ----
|
||||
|
||||
PlatformManifestSource = Literal["all", "bundled", "user"]
|
||||
|
||||
@@ -3884,13 +3859,13 @@ def _platform_manifest_env_entries(manifest: dict, *, optional: bool = True):
|
||||
yield name, is_secret, meta
|
||||
|
||||
|
||||
def _manifest_secret_envs(manifests) -> frozenset[str]:
|
||||
def _manifest_secret_envs(manifests, core: Optional[frozenset[str]] = None) -> frozenset[str]:
|
||||
"""Upper-cased secret messaging env names the given manifests declare, minus core-declared
|
||||
names: a manifest never reclassifies a core variable such as OPENAI_API_KEY."""
|
||||
names = {name.upper() for _dir, manifest in manifests
|
||||
for name, is_secret, meta in _platform_manifest_env_entries(manifest)
|
||||
if is_secret and (meta.get("category") or "messaging") == "messaging"}
|
||||
return frozenset(names - {n.upper() for n in CORE_DECLARED_ENV_NAMES})
|
||||
return frozenset(names - {n.upper() for n in (core if core is not None else core_declared_env_names())})
|
||||
|
||||
|
||||
def platform_manifest_secret_envs(home: Optional[Path] = None, source: PlatformManifestSource = "user", *,
|
||||
@@ -3898,8 +3873,8 @@ def platform_manifest_secret_envs(home: Optional[Path] = None, source: PlatformM
|
||||
"""Secret env names declared by one source's platform plugin manifests: ``"user"`` reads only
|
||||
``home``'s user-installed plugins, which belong to that profile alone; ``"bundled"`` returns
|
||||
the set read once at import (re-read strictly if that read hit an I/O error)."""
|
||||
if source == "bundled" and BUNDLED_PLATFORM_SECRET_ENVS is not None:
|
||||
return BUNDLED_PLATFORM_SECRET_ENVS
|
||||
if source == "bundled" and BUNDLED_PLATFORM_SECRET_ENVS is not None: # minus provider names, known only now
|
||||
return frozenset(BUNDLED_PLATFORM_SECRET_ENVS - {n.upper() for n in core_declared_env_names()})
|
||||
return _manifest_secret_envs(_platform_plugin_manifests(home, source, strict=strict))
|
||||
|
||||
|
||||
@@ -3941,11 +3916,19 @@ def _inject_platform_plugin_env_vars() -> frozenset[str] | None:
|
||||
"url": meta.get("url") or None,
|
||||
"password": is_secret,
|
||||
"category": meta.get("category") or "messaging"}
|
||||
return _manifest_secret_envs(bundled) if bundled is not None else None
|
||||
return _manifest_secret_envs(bundled, _STATIC_CORE_ENV_NAMES) if bundled is not None else None
|
||||
|
||||
|
||||
# Names declared in core, before any platform manifest is read. A manifest never reclassifies
|
||||
# one: the config form keeps the core entry, and the child-env scrub keeps a plugin that lists
|
||||
# OPENAI_API_KEY from turning a provider key into an adapter secret.
|
||||
CORE_DECLARED_ENV_NAMES: frozenset[str] = frozenset(OPTIONAL_ENV_VARS)
|
||||
# OPENAI_API_KEY from turning a provider key into an adapter secret. CORE_DECLARED_ENV_NAMES adds
|
||||
# the provider profiles' names on first use (module __getattr__): listing them reads config.
|
||||
_STATIC_CORE_ENV_NAMES: frozenset[str] = frozenset(dict.keys(OPTIONAL_ENV_VARS))
|
||||
BUNDLED_PLATFORM_SECRET_ENVS: frozenset[str] | None = _inject_platform_plugin_env_vars()
|
||||
_config_provider_env.arm(OPTIONAL_ENV_VARS, _STATIC_CORE_ENV_NAMES)
|
||||
|
||||
|
||||
def __getattr__(name: str) -> Any:
|
||||
if name == "CORE_DECLARED_ENV_NAMES":
|
||||
return core_declared_env_names()
|
||||
raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
|
||||
@@ -2712,13 +2712,13 @@ DEFAULT_CONFIG = {
|
||||
}
|
||||
|
||||
|
||||
from hermes_cli.config_defaults_env import _base_url, _env, _msg, _prov, _setting, _skill, _tool
|
||||
from hermes_cli.config_defaults_env import OptionalEnvVars, _base_url, _env, _msg, _prov, _setting, _skill, _tool
|
||||
|
||||
|
||||
# Optional environment variables that enhance functionality. Feeds the dashboard keys page and setup
|
||||
# checklists; category: provider|tool|skill|messaging|setting, advanced=True hides from checklists,
|
||||
# tools=[...] lists the model tools the key unlocks.
|
||||
OPTIONAL_ENV_VARS = {
|
||||
# tools=[...] lists the model tools the key unlocks. Provider entries: config_provider_env (on first read).
|
||||
OPTIONAL_ENV_VARS = OptionalEnvVars({
|
||||
# ── Provider (handled in provider selection, not shown in checklists) ──
|
||||
"NOUS_BASE_URL": _base_url("Nous Portal"),
|
||||
"HERMES_ANON_API_SECRET": _env(
|
||||
@@ -3118,4 +3118,4 @@ OPTIONAL_ENV_VARS = {
|
||||
"HERMES_EPHEMERAL_SYSTEM_PROMPT": _setting(
|
||||
"Ephemeral system prompt injected at API-call time (never persisted to sessions)",
|
||||
"Ephemeral system prompt", None),
|
||||
}
|
||||
})
|
||||
@@ -1,4 +1,70 @@
|
||||
"""Entry factories for ``config_defaults.OPTIONAL_ENV_VARS`` (pure-data leaf, no imports)."""
|
||||
"""Entry factories for ``config_defaults.OPTIONAL_ENV_VARS`` (pure-data leaf, stdlib only)."""
|
||||
|
||||
import threading
|
||||
|
||||
_FILL_LOCK = threading.RLock() # re-entrant: the fill itself reads the table
|
||||
|
||||
|
||||
class OptionalEnvVars(dict):
|
||||
"""``OPTIONAL_ENV_VARS``: a dict that runs one armed ``fill(table)`` before it is first read.
|
||||
|
||||
Some entries (provider profiles') are only known after work that must not run at import
|
||||
(``hermes_cli.config_provider_env``); writes never trigger the fill. A fill that raises is
|
||||
re-armed, so the next read tries again."""
|
||||
|
||||
_fill = None
|
||||
|
||||
def arm(self, fill) -> None:
|
||||
self._fill = fill
|
||||
|
||||
def _ready(self) -> None:
|
||||
if self._fill is None:
|
||||
return
|
||||
with _FILL_LOCK:
|
||||
fill, self._fill = self._fill, None
|
||||
if fill is None:
|
||||
return
|
||||
try:
|
||||
fill(self)
|
||||
except BaseException:
|
||||
self._fill = fill
|
||||
raise
|
||||
|
||||
def __getitem__(self, key):
|
||||
self._ready()
|
||||
return dict.__getitem__(self, key)
|
||||
|
||||
def get(self, key, default=None):
|
||||
self._ready()
|
||||
return dict.get(self, key, default)
|
||||
|
||||
def __contains__(self, key) -> bool:
|
||||
self._ready()
|
||||
return dict.__contains__(self, key)
|
||||
|
||||
def __iter__(self):
|
||||
self._ready()
|
||||
return dict.__iter__(self)
|
||||
|
||||
def __len__(self) -> int:
|
||||
self._ready()
|
||||
return dict.__len__(self)
|
||||
|
||||
def keys(self):
|
||||
self._ready()
|
||||
return dict.keys(self)
|
||||
|
||||
def items(self):
|
||||
self._ready()
|
||||
return dict.items(self)
|
||||
|
||||
def values(self):
|
||||
self._ready()
|
||||
return dict.values(self)
|
||||
|
||||
def copy(self) -> dict:
|
||||
self._ready()
|
||||
return dict(dict.items(self))
|
||||
|
||||
|
||||
def _env(description, prompt, **keys):
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
"""The provider-profile entries of ``OPTIONAL_ENV_VARS``, added on first read instead of at import.
|
||||
|
||||
Listing provider profiles runs provider discovery, and discovery reads config: ``plugins.enabled``
|
||||
gates pip-installed provider plugins, ``plugins.isolation`` decides how ``$HERMES_HOME`` ones load.
|
||||
Done while ``hermes_cli.config`` imports, that read is the launch profile's config in a process that
|
||||
serves several profiles, and a cycle with a config backend that imports this code while it loads the
|
||||
config (the remote backend's Nous credential imports ``hermes_cli.auth``, which imports
|
||||
``hermes_cli.config``). So importing ``hermes_cli.config`` reads no config; the first read of the
|
||||
table (or of ``CORE_DECLARED_ENV_NAMES``) adds the entries.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
_STATIC_NAMES: frozenset[str] = frozenset() # declared in core (config_defaults), known at import
|
||||
_PROVIDER_NAMES: set[str] = set() # added from provider profiles by the fill
|
||||
|
||||
|
||||
def arm(table, static_names) -> None:
|
||||
"""Called once by ``hermes_cli.config`` at the end of its import."""
|
||||
global _STATIC_NAMES
|
||||
_STATIC_NAMES = frozenset(static_names)
|
||||
table.arm(_add_provider_entries)
|
||||
|
||||
|
||||
def _add_provider_entries(table) -> None:
|
||||
"""Expose the env vars of every ``auth_type="api_key"`` provider profile. A core entry wins;
|
||||
a provider entry replaces one a platform plugin manifest added (it was added first only
|
||||
because this runs late)."""
|
||||
try:
|
||||
from providers import list_providers
|
||||
profiles = list_providers()
|
||||
except Exception: # health: allow BLE001 -- as before: a broken provider layer must not break config readers
|
||||
# (ConfigBackendUnavailable is a SystemExit: it propagates, and the table re-arms)
|
||||
return
|
||||
for pp in profiles:
|
||||
if pp.auth_type != "api_key":
|
||||
continue
|
||||
for var in pp.env_vars:
|
||||
if var in _STATIC_NAMES or var in _PROVIDER_NAMES:
|
||||
continue
|
||||
is_key = not var.endswith(("_BASE_URL", "_URL"))
|
||||
label = pp.display_name or pp.name
|
||||
dict.__setitem__(table, var, {
|
||||
"description": f"{label} {'API key' if is_key else 'base URL override'}",
|
||||
"prompt": f"{label} {'API key' if is_key else 'base URL (leave empty for default)'}",
|
||||
"url": pp.signup_url or None,
|
||||
"password": is_key,
|
||||
"category": "provider",
|
||||
"advanced": True})
|
||||
_PROVIDER_NAMES.add(var)
|
||||
|
||||
|
||||
def core_declared_env_names() -> frozenset[str]:
|
||||
"""Names declared in core: config_defaults' plus the provider profiles'. A platform manifest
|
||||
never reclassifies one."""
|
||||
from hermes_cli.config_defaults import OPTIONAL_ENV_VARS
|
||||
OPTIONAL_ENV_VARS.get("") # runs the fill once
|
||||
return _STATIC_NAMES | frozenset(_PROVIDER_NAMES)
|
||||
@@ -0,0 +1,82 @@
|
||||
"""Provider-profile entries of OPTIONAL_ENV_VARS: added on first read, same contents as before."""
|
||||
from __future__ import annotations
|
||||
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
from hermes_cli import config_provider_env as cpe
|
||||
from hermes_cli.config_backend import ConfigBackendUnavailable
|
||||
from hermes_cli.config_defaults_env import OptionalEnvVars
|
||||
|
||||
|
||||
def _profile(name, *env_vars, auth_type="api_key"):
|
||||
return SimpleNamespace(name=name, display_name=name.title(), env_vars=env_vars, auth_type=auth_type,
|
||||
signup_url=f"https://{name}.test/keys")
|
||||
|
||||
|
||||
def test_every_api_key_provider_var_is_declared_in_core():
|
||||
import hermes_cli.config as cfg
|
||||
from providers import list_providers
|
||||
|
||||
expected = {v for p in list_providers() if p.auth_type == "api_key" for v in p.env_vars}
|
||||
assert expected, "no api_key provider profiles registered"
|
||||
assert expected <= set(cfg.OPTIONAL_ENV_VARS)
|
||||
assert expected <= cfg.CORE_DECLARED_ENV_NAMES
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def table(monkeypatch):
|
||||
monkeypatch.setattr(cpe, "_STATIC_NAMES", frozenset({"CORE_KEY"}))
|
||||
monkeypatch.setattr(cpe, "_PROVIDER_NAMES", set())
|
||||
t = OptionalEnvVars({"CORE_KEY": {"description": "core"}, "PLUG_KEY": {"description": "platform"}})
|
||||
t.arm(cpe._add_provider_entries)
|
||||
return t
|
||||
|
||||
|
||||
def test_fill_runs_on_first_read_core_wins_provider_replaces_platform_first_provider_wins(table, monkeypatch):
|
||||
import providers
|
||||
|
||||
calls = []
|
||||
profiles = [_profile("alpha", "CORE_KEY", "PLUG_KEY", "ALPHA_API_KEY", "ALPHA_BASE_URL"),
|
||||
_profile("beta", "ALPHA_API_KEY"), _profile("oauth", "OAUTH_TOKEN", auth_type="oauth")]
|
||||
monkeypatch.setattr(providers, "list_providers", lambda: calls.append(1) or profiles)
|
||||
dict.__setitem__(table, "LATE", {}) # writes and raw dict access never fill
|
||||
assert calls == []
|
||||
assert table["CORE_KEY"] == {"description": "core"} and calls == [1]
|
||||
assert table["PLUG_KEY"]["description"] == "Alpha API key"
|
||||
assert table["ALPHA_API_KEY"]["description"] == "Alpha API key" and table["ALPHA_API_KEY"]["password"]
|
||||
assert table["ALPHA_BASE_URL"]["password"] is False and "OAUTH_TOKEN" not in table
|
||||
list(table.items())
|
||||
assert calls == [1] # once
|
||||
|
||||
|
||||
def test_unavailable_config_fails_the_read_loudly_and_the_next_read_retries(table, monkeypatch):
|
||||
import providers
|
||||
|
||||
answers = [ConfigBackendUnavailable("boot in flight"), [_profile("alpha", "ALPHA_API_KEY")]]
|
||||
|
||||
def list_providers():
|
||||
answer = answers.pop(0)
|
||||
if isinstance(answer, BaseException):
|
||||
raise answer
|
||||
return answer
|
||||
|
||||
monkeypatch.setattr(providers, "list_providers", list_providers)
|
||||
with pytest.raises(ConfigBackendUnavailable): # fail closed, never a silently partial table
|
||||
table.get("ALPHA_API_KEY")
|
||||
assert "ALPHA_API_KEY" in table and answers == []
|
||||
|
||||
|
||||
def test_a_broken_provider_layer_is_not_retried(table, monkeypatch):
|
||||
import providers
|
||||
|
||||
calls = []
|
||||
|
||||
def boom():
|
||||
calls.append(1)
|
||||
raise RuntimeError("broken")
|
||||
|
||||
monkeypatch.setattr(providers, "list_providers", boom)
|
||||
assert "CORE_KEY" in table and "CORE_KEY" in table
|
||||
assert calls == [1]
|
||||
Reference in new issue
Block a user