mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
refactor: canonical config loaders for behavioral reads + guarded raw-read primitive (kills the managed-scope/env-expansion drift class)
The disease: ~15 scattered raw yaml.safe_load(config.yaml) reads that
silently miss managed-scope overlay, ${ENV_VAR} expansion, profile-aware
pathing, and root-model normalization. Every new config feature needed an
N-site sweep (incident chain 9cbcc0c9c8 → 732293cf87 → b0e47a98f9 →
1928aa0443). This commit assigns every raw read to an owner and adds a
lint-guard test so the class cannot regrow.
New primitive (additive-only change to hermes_cli/config.py):
read_user_config_raw(path=None) — reads the user file EXACTLY as
written; docstring states it is ONLY legal for write-back round-trips
and raw-file diagnostics. Behavioral reads must use
load_config()/load_config_readonly().
BEHAVIOR FIXES (class-a sites migrated to a canonical loader — these
previously read values that could DIFFER from the effective config):
gateway/run.py _try_resolve_fallback_provider → _load_gateway_runtime_config
keys: fallback_providers/fallback_model (provider, model, base_url,
api_key). Drift fixed: a managed-pinned fallback chain was ignored;
an api_key of "${OPENROUTER_API_KEY}" reached the resolver unexpanded.
gateway/run.py GatewayRunner._load_provider_routing → same loader
key: provider_routing. Drift fixed: managed-pinned routing prefs and
${VAR} templates were ignored.
gateway/run.py GatewayRunner._load_fallback_model → same loader
keys: fallback chain. Same drift as above.
gateway/run.py GatewayRunner._refresh_fallback_model
keeps the raw primitive (its last-known-good-on-parse-failure contract
forbids the fail-open loader, which returns {} on a torn write) but now
applies managed overlay + env expansion inline. Drift fixed: chain
edits under managed scope / env templates were previously frozen out.
tui_gateway/server.py _load_cfg (72 behavioral call sites)
now = raw read + managed overlay (pre-existing) + NEW ${VAR} expansion,
split from a new _load_cfg_raw() write-back primitive. Drift fixed:
e.g. custom_prompt: "hello ${VAR}", agent.system_prompt, model,
api_key/base_url templates reached sessions unexpanded. DEFAULT_CONFIG
is deliberately NOT merged (callers treat missing keys as unset;
`_load_cfg() == {}` sentinels and _save_cfg round-trips depend on it).
tui_gateway/server.py _profile_configured_cwd
keys: terminal.cwd of a NON-launch profile. Drift fixed: managed
overlay + ${VAR} expansion now apply (load_config() would resolve the
wrong profile's home, so the raw primitive + inline pipeline is used).
plugins/platforms/telegram/adapter.py _reload_dm_topics_from_config
→ load_config_readonly(). keys: platforms.telegram.extra.dm_topics.
Drift fixed: managed overlay + profile-aware pathing + expansion.
plugins/memory/holographic _load_plugin_config → load_config_readonly().
keys: plugins.hermes-memory-store.*. Same drift class.
WRITE-BACK ROUND-TRIPS (class-b: stay raw BY DESIGN via read_user_config_raw;
merging defaults/overlay would pollute the saved user file):
gateway/slash_commands.py: model persist x2, _save_gateway_config_key,
memory/skills write_approval toggles
gateway/platforms/yuanbao.py auto-sethome
tui_gateway/server.py _write_config_key + all cfg→_save_cfg blocks
(reasoning show/hide/full/clamp, details_mode[.section], prompt)
→ new _load_cfg_raw()
plugins/memory/holographic save_config
RAW-FILE DIAGNOSTICS + presence-sensitive bridges (class-c: stay raw,
now via the shared primitive with an explanatory comment):
hermes_cli/doctor.py x5 (model validation, stale-root-keys, .env drift,
deprecation sweep, memory-provider probe — the latter two keep their
inline managed overlay where they had one)
gateway/run.py _bridge_max_turns_from_config and the module-level
TERMINAL_*/HERMES_* env bridge (bridging merged defaults would export
all of DEFAULT_CONFIG into the environment; both keep their inline
overlay + expansion)
hermes_cli/send_cmd.py env bridge (same presence-sensitivity)
hermes_cli/gateway.py multiplex-conflict probe (reads the DEFAULT root's
config, not the active profile's — load_config is the wrong owner)
hermes_cli/profiles.py / hermes_cli/web_server.py / tools/wake_word.py
multi-profile reads (load_config targets only the ACTIVE profile home)
cron/jobs.py _resolve_default_model_snapshot and cron/scheduler.py
run_job config read keep their existing inline overlay+expansion but
now share the primitive (their fail-open + last-value semantics and
the deliberate no-defaults merge are preserved exactly).
Failure-semantics audit: every migrated site preserves its exact previous
behavior on missing file ({} / early return) and parse failure (raise into
the caller's existing except, warn, last-known-good, or fail-open) —
read_user_config_raw intentionally mirrors bare open()+safe_load semantics
(raises on parse errors, {} only on FileNotFoundError/non-dict root).
Guard: tests/hermes_cli/test_config_read_guard.py scans the tree for
yaml.safe_load within 6 lines of a 'config.yaml' reference outside an
explicit ALLOWLIST (hermes_cli/config.py, gateway/config.py, gateway/run.py
fallback path, hermes_cli/managed_scope.py which reads the MANAGED file,
gateway/readiness.py parse-health probe) and fails on new offenders.
E2E: tests/hermes_cli/test_config_loader_e2e.py runs a subprocess with a
temp HERMES_HOME (config.yaml containing ${E2E_PROMPT_SUFFIX}) plus a
HERMES_MANAGED_DIR overlay pinning agent.reasoning_effort, asserting
tui _load_cfg resolves "hello world"/"high" while _load_cfg_raw +
_save_cfg round-trip the template and user value verbatim with no
managed/default leakage.
This commit is contained in:
parent
c92e2c0fbf
commit
ed33ebca1d
17 changed files with 500 additions and 142 deletions
|
|
@ -2888,6 +2888,55 @@ def read_raw_config() -> Dict[str, Any]:
|
|||
return data
|
||||
|
||||
|
||||
def read_user_config_raw(config_path: Optional[Path] = None) -> Dict[str, Any]:
|
||||
"""Read a user ``config.yaml`` EXACTLY as written on disk.
|
||||
|
||||
No DEFAULT_CONFIG merge, no managed-scope overlay, no ``${ENV_VAR}``
|
||||
expansion, no migration, no root-model normalization, no caching.
|
||||
|
||||
ONLY legal for write-back round-trips and raw-file diagnostics —
|
||||
behavioral reads must use load_config()/load_config_readonly().
|
||||
|
||||
Legal call sites, exhaustively:
|
||||
|
||||
* WRITE-BACK ROUND-TRIPS (read → mutate one key → save): merging
|
||||
defaults or the managed overlay here would persist hundreds of
|
||||
default keys (or administrator-pinned values) into the user's file
|
||||
on the next save. Raw is *correct*, not an optimization.
|
||||
* RAW-FILE DIAGNOSTICS (doctor, deprecation sweeps): these inspect
|
||||
what the user actually wrote — stale root keys, drift against .env —
|
||||
and merged defaults would produce false positives.
|
||||
* PRESENCE-SENSITIVE ENV BRIDGES (gateway/send bridges that only
|
||||
export a key when the user explicitly set it): a defaults merge
|
||||
would make every key "present" and bridge the entire DEFAULT_CONFIG
|
||||
into the environment. These sites must still apply
|
||||
``managed_scope.apply_managed_overlay`` + ``_expand_env_vars``
|
||||
inline, which they do.
|
||||
|
||||
Semantics (deliberately mirrors the bare ``open()+yaml.safe_load()``
|
||||
pattern this replaces, so migrated sites keep their exact failure
|
||||
behavior):
|
||||
|
||||
* missing file → ``{}``
|
||||
* unparseable YAML / other I/O errors → raises (callers that want
|
||||
fail-open already wrap in try/except; callers with last-known-good
|
||||
or warn semantics rely on the exception)
|
||||
* non-dict YAML root → ``{}``
|
||||
|
||||
``config_path`` defaults to :func:`get_config_path` (profile-aware).
|
||||
Pass an explicit path when the caller resolves its own home (gateway
|
||||
``_hermes_home``, tui profile override, multi-profile probes).
|
||||
"""
|
||||
if config_path is None:
|
||||
config_path = get_config_path()
|
||||
try:
|
||||
with open(config_path, encoding="utf-8") as f:
|
||||
data = fast_safe_load(f) or {}
|
||||
except FileNotFoundError:
|
||||
return {}
|
||||
return data if isinstance(data, dict) else {}
|
||||
|
||||
|
||||
def require_readable_config_before_write(config_path: Optional[Path] = None) -> None:
|
||||
"""Refuse to replace an existing config.yaml that cannot be read."""
|
||||
if config_path is None:
|
||||
|
|
|
|||
|
|
@ -951,8 +951,9 @@ def run_doctor(args):
|
|||
|
||||
# Validate model.provider and model.default values
|
||||
try:
|
||||
import yaml as _yaml
|
||||
cfg = _yaml.safe_load(config_path.read_text(encoding="utf-8")) or {}
|
||||
# Raw-file diagnostic: inspects what the user actually wrote.
|
||||
from hermes_cli.config import read_user_config_raw
|
||||
cfg = read_user_config_raw(config_path)
|
||||
model_section = cfg.get("model") or {}
|
||||
provider_raw = (model_section.get("provider") or "").strip()
|
||||
provider = provider_raw.lower()
|
||||
|
|
@ -1184,9 +1185,9 @@ def run_doctor(args):
|
|||
|
||||
# Detect stale root-level model keys (known bug source — PR #4329)
|
||||
try:
|
||||
import yaml
|
||||
with open(config_path, encoding="utf-8") as f:
|
||||
raw_config = yaml.safe_load(f) or {}
|
||||
# Raw-file diagnostic: stale-key detection must see the raw file.
|
||||
from hermes_cli.config import read_user_config_raw
|
||||
raw_config = read_user_config_raw(config_path)
|
||||
stale_root_keys = [k for k in ("provider", "base_url") if k in raw_config and isinstance(raw_config[k], str)]
|
||||
if stale_root_keys:
|
||||
check_warn(
|
||||
|
|
@ -1231,10 +1232,9 @@ def run_doctor(args):
|
|||
# Read the .env FILE directly (load_env), not get_env_value/os.environ,
|
||||
# which the startup bridge may already have overridden.
|
||||
try:
|
||||
import yaml
|
||||
from hermes_cli.config import load_env, remove_env_value
|
||||
with open(config_path, encoding="utf-8") as f:
|
||||
raw_config = yaml.safe_load(f) or {}
|
||||
from hermes_cli.config import load_env, read_user_config_raw, remove_env_value
|
||||
# Raw-file diagnostic: drift check against the raw file.
|
||||
raw_config = read_user_config_raw(config_path)
|
||||
agent_cfg = raw_config.get("agent")
|
||||
cfg_max_turns = (
|
||||
agent_cfg.get("max_turns")
|
||||
|
|
@ -1281,11 +1281,11 @@ def run_doctor(args):
|
|||
# Migrations may still live in config.py version steps; doctor does
|
||||
# not auto-delete here — only tells the user the modern replacement.
|
||||
try:
|
||||
import yaml as _yaml_depr
|
||||
from hermes_cli.config import load_env as _load_env_depr
|
||||
from hermes_cli.config import read_user_config_raw as _read_raw_depr
|
||||
|
||||
with open(config_path, encoding="utf-8") as _f_depr:
|
||||
_raw_for_depr = _yaml_depr.safe_load(_f_depr) or {}
|
||||
# Raw-file diagnostic: deprecation sweep inspects the raw file.
|
||||
_raw_for_depr = _read_raw_depr(config_path)
|
||||
# Prefer the on-disk .env so bridged process env (e.g. TERMINAL_CWD
|
||||
# from terminal.cwd) does not false-positive.
|
||||
try:
|
||||
|
|
@ -2528,11 +2528,11 @@ def run_doctor(args):
|
|||
_section("Memory Provider")
|
||||
_active_memory_provider = ""
|
||||
try:
|
||||
import yaml as _yaml
|
||||
from hermes_cli.config import read_user_config_raw as _read_raw_mem
|
||||
_mem_cfg_path = HERMES_HOME / "config.yaml"
|
||||
if _mem_cfg_path.exists():
|
||||
with open(_mem_cfg_path, encoding="utf-8") as _f:
|
||||
_raw_cfg = _yaml.safe_load(_f) or {}
|
||||
# Raw-file diagnostic (+ managed overlay below, unchanged).
|
||||
_raw_cfg = _read_raw_mem(_mem_cfg_path)
|
||||
try:
|
||||
from hermes_cli import managed_scope
|
||||
_raw_cfg = managed_scope.apply_managed_overlay(_raw_cfg)
|
||||
|
|
|
|||
|
|
@ -4683,8 +4683,11 @@ def _guard_named_profile_under_multiplexer(force: bool = False) -> None:
|
|||
cfg_path = default_root / "config.yaml"
|
||||
if not cfg_path.exists():
|
||||
return
|
||||
with open(cfg_path, encoding="utf-8") as f:
|
||||
cfg = _yaml.safe_load(f) or {}
|
||||
# Raw read of the DEFAULT root's config (not the active profile
|
||||
# home, so load_config() is the wrong owner here); whole probe is
|
||||
# fail-open via the enclosing except.
|
||||
from hermes_cli.config import read_user_config_raw
|
||||
cfg = read_user_config_raw(cfg_path)
|
||||
multiplex = bool(
|
||||
cfg.get("multiplex_profiles")
|
||||
or (cfg.get("gateway", {}) or {}).get("multiplex_profiles")
|
||||
|
|
|
|||
|
|
@ -683,9 +683,10 @@ def _read_config_model(profile_dir: Path) -> tuple:
|
|||
if not config_path.exists():
|
||||
return None, None
|
||||
try:
|
||||
import yaml
|
||||
with open(config_path, "r", encoding="utf-8") as f:
|
||||
cfg = yaml.safe_load(f) or {}
|
||||
# Multi-profile display read: load_config() targets the ACTIVE
|
||||
# profile's home, so read THIS profile's file via the raw primitive.
|
||||
from hermes_cli.config import read_user_config_raw
|
||||
cfg = read_user_config_raw(config_path)
|
||||
model_cfg = cfg.get("model", {})
|
||||
if isinstance(model_cfg, str):
|
||||
return model_cfg, None
|
||||
|
|
|
|||
|
|
@ -260,13 +260,10 @@ def _load_hermes_env() -> None:
|
|||
return
|
||||
|
||||
try:
|
||||
import yaml # type: ignore[import-not-found]
|
||||
except Exception:
|
||||
return
|
||||
|
||||
try:
|
||||
with open(config_path, "r", encoding="utf-8") as fh:
|
||||
raw = yaml.safe_load(fh) or {}
|
||||
# Presence-sensitive env bridge: raw read is deliberate — only keys
|
||||
# the user actually wrote get bridged. Overlay + expansion below.
|
||||
from hermes_cli.config import read_user_config_raw
|
||||
raw = read_user_config_raw(config_path)
|
||||
except Exception:
|
||||
return
|
||||
|
||||
|
|
|
|||
|
|
@ -2858,8 +2858,10 @@ def _profile_platform_ports(profile_home: Path, runtime: Optional[dict]) -> Dict
|
|||
|
||||
blocks: Dict[str, dict] = {}
|
||||
try:
|
||||
with open(profile_home / "config.yaml", encoding="utf-8") as f:
|
||||
cfg = yaml.safe_load(f) or {}
|
||||
# Multi-profile probe: load_config() targets the ACTIVE profile's
|
||||
# home, so read the probed profile's file via the raw primitive.
|
||||
from hermes_cli.config import read_user_config_raw
|
||||
cfg = read_user_config_raw(profile_home / "config.yaml")
|
||||
gateway_cfg = cfg.get("gateway") if isinstance(cfg.get("gateway"), dict) else {}
|
||||
# gateway.platforms first, top-level platforms second — later wins,
|
||||
# matching the precedence in gateway.config.load_gateway_config().
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue