mirrors/hermes-agent
1
0
Fork 0
mirror of https://github.com/NousResearch/hermes-agent.git synced 2026-07-15 14:22:43 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

fix(config): retain last-known-good config when config.yaml fails to parse (#60591)

Browse source 
Port from openai/codex#31188: a parse failure in a policy-bearing config
file must not silently replace the effective policy with an empty/default
one. Codex's load_exec_policy_with_warning replaced the whole exec policy
with Policy::empty() when a .rules file failed to parse, silently dropping
managed prompt/forbidden rules; the fix preserves the managed policy while
still warning.

Hermes had the same bug shape in load_config(): a YAML parse error made
_load_config_impl() fall through to DEFAULT_CONFIG, dropping every user
override — including approvals.deny rules, which are documented to block
commands even under --yolo. In a long-running gateway, a user mid-editing
config.yaml into broken YAML silently disarmed their own deny rules on the
next load.

Now, when the process has a last successfully loaded config for that path
(_LAST_EXPANDED_CONFIG_BY_PATH), a parse failure keeps serving it (cached
under the corrupt file's signature so the broken file isn't re-parsed) and
the warning says edits are being ignored until the YAML is fixed. Fresh
processes with no last-known-good keep the existing DEFAULT_CONFIG
fallback and warning.

E2E-verified: deny rule 'curl*evil.com*' still blocks after mid-process
corruption; fixed file reloads normally; fresh-process fallback unchanged.
This commit is contained in:
Teknium 2026-07-09 16:36:03 -07:00 • committed by GitHub
parent 8e3f9537db
commit fe25806a6b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 153 additions and 8 deletions
Download patch file Download diff file Expand all files Collapse all files

68
hermes_cli/config.py
View file

@ -93,7 +93,9 @@ def _backup_corrupt_config(config_path: Path) -> Optional[Path]:
return None
def _warn_config_parse_failure(config_path: Path, exc: Exception) -> None:
def _warn_config_parse_failure(
config_path: Path, exc: Exception, *, fallback: str = "defaults"
) -> None:
"""Surface a config.yaml parse failure to user, log, and stderr.
A YAML parse error in ``~/.hermes/config.yaml`` causes ``load_config()``
@ -110,6 +112,11 @@ def _warn_config_parse_failure(config_path: Path, exc: Exception) -> None:
timestamped ``.bak`` (best-effort) so the user's recoverable content
survives any later rewrite of ``config.yaml`` by the setup wizard or
``hermes config set``.
``fallback`` selects the message wording: ``"defaults"`` (fresh process,
nothing else to serve) or ``"last-known-good"`` (in-process retention of
the previously loaded config — see the codex#31188 port in
``_load_config_impl``).
"""
try:
st = config_path.stat()
@ -122,12 +129,19 @@ def _warn_config_parse_failure(config_path: Path, exc: Exception) -> None:
backup_path = _backup_corrupt_config(config_path)
msg = (
f"Failed to parse {config_path}: {exc}. "
f"Falling back to default config — every user override "
f"(auxiliary providers, fallback chain, model settings) is being IGNORED. "
f"Fix the YAML and restart."
)
if fallback == "last-known-good":
msg = (
f"Failed to parse {config_path}: {exc}. "
f"Keeping the previously loaded config for this process — "
f"edits to config.yaml are being IGNORED until the YAML is fixed."
)
else:
msg = (
f"Failed to parse {config_path}: {exc}. "
f"Falling back to default config — every user override "
f"(auxiliary providers, fallback chain, model settings) is being IGNORED. "
f"Fix the YAML and restart."
)
if backup_path is not None:
msg += f" A copy of the corrupted file was saved to {backup_path}."
logger.warning(msg)
@ -6936,7 +6950,45 @@ def _load_config_impl(*, want_deepcopy: bool) -> Dict[str, Any]:
config = _deep_merge(config, user_config)
except Exception as e:
_warn_config_parse_failure(config_path, e)
# Last-known-good fallback (port of openai/codex#31188's
# invariant: a parse failure in a policy/config file must not
# silently replace the effective policy with an empty/default
# one). Falling through to DEFAULT_CONFIG here drops EVERY user
# override — including security-critical ``approvals.deny``
# rules, which are supposed to block commands even under yolo.
# A long-running gateway whose user mid-edits config.yaml into
# broken YAML would silently lose those rules on the next load.
# Within a running process we still have the last successfully
# loaded config — keep serving it until the file is fixed.
# Fresh processes with no last-known-good keep the existing
# DEFAULT_CONFIG fallback.
lkg = _LAST_EXPANDED_CONFIG_BY_PATH.get(path_key)
_warn_config_parse_failure(
config_path,
e,
fallback="last-known-good" if lkg is not None else "defaults",
)
if lkg is not None:
# save_config() stores the pre-expansion normalized dict
# (env-ref templates preserved); the load path stores the
# expanded one. Expand defensively — idempotent when the
# stored value is already expanded.
from typing import cast as _cast
lkg_copy: Dict[str, Any] = _cast(
Dict[str, Any], _expand_env_vars(copy.deepcopy(lkg))
)
if cache_sig is not None:
# Cache under the corrupt file's signature (empty env
# snapshot: always valid) so repeated loads don't
# re-parse the broken file; fixing the file changes the
# signature and triggers a normal reload.
_empty_env: Dict[str, Optional[str]] = {}
_LOAD_CONFIG_CACHE[path_key] = (
cache_sig[0], cache_sig[1],
cache_sig[2], cache_sig[3],
lkg_copy, _empty_env,
)
return copy.deepcopy(lkg_copy) if want_deepcopy else lkg_copy
normalized = _normalize_root_model_keys(_normalize_max_turns_config(config))
expanded = _expand_env_vars(normalized)

93
tests/hermes_cli/test_config.py
View file

@ -266,6 +266,99 @@ class TestLoadConfigParseFailure:
assert not list(tmp_path.glob("config.yaml.corrupt.*.bak"))
def test_last_known_good_retained_within_process(self, tmp_path, capsys):
"""Port of openai/codex#31188's invariant: a parse failure must not
silently replace the effective config (policy included) with
defaults when the process already loaded a good config.
Scenario: long-running gateway, user mid-edits config.yaml into
broken YAML. Before this fix the next load_config() dropped every
override — including ``approvals.deny`` security rules. Now the
last successfully loaded config keeps being served until the file
parses again.
"""
import time
from hermes_cli import config as cfg_mod
cfg_mod._CONFIG_PARSE_WARNED.clear()
with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}):
cfg = tmp_path / "config.yaml"
cfg.write_text(
"model:\n default: test/custom-model\n"
"approvals:\n deny:\n - 'curl*evil.com*'\n"
)
good = load_config()
assert good["model"]["default"] == "test/custom-model"
assert good["approvals"]["deny"] == ["curl*evil.com*"]
capsys.readouterr()
# Corrupt the file (mtime must change to bust the cache)
time.sleep(0.05)
cfg.write_text("approvals:\n deny: [unclosed\n :::bad {{{\n")
after = load_config()
# Last-known-good retained — NOT defaults
assert after["model"]["default"] == "test/custom-model"
assert after["approvals"]["deny"] == ["curl*evil.com*"]
# Warning says we kept the previous config, not defaults
err = capsys.readouterr().err
assert "previously loaded config" in err
def test_last_known_good_recovers_after_fix(self, tmp_path):
"""Fixing the YAML picks up the new content on the next load."""
import time
from hermes_cli import config as cfg_mod
cfg_mod._CONFIG_PARSE_WARNED.clear()
with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}):
cfg = tmp_path / "config.yaml"
cfg.write_text("model:\n default: test/first\n")
assert load_config()["model"]["default"] == "test/first"
time.sleep(0.05)
cfg.write_text("\tbroken:\n")
assert load_config()["model"]["default"] == "test/first"
time.sleep(0.05)
cfg.write_text("model:\n default: test/second\n")
assert load_config()["model"]["default"] == "test/second"
def test_fresh_process_still_falls_back_to_defaults(self, tmp_path):
"""With no last-known-good (fresh process for this path), a broken
config still falls back to DEFAULT_CONFIG as before."""
from hermes_cli import config as cfg_mod
cfg_mod._CONFIG_PARSE_WARNED.clear()
with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}):
(tmp_path / "config.yaml").write_text("\tbroken:\n")
# No prior good load for this path in _LAST_EXPANDED_CONFIG_BY_PATH
cfg_mod._LAST_EXPANDED_CONFIG_BY_PATH.pop(
str(tmp_path / "config.yaml"), None
)
config = load_config()
assert config["model"] == DEFAULT_CONFIG["model"]
def test_last_known_good_cached_no_rewarn_spam(self, tmp_path, capsys):
"""Repeated loads of the same broken file serve the cached LKG and
don't re-warn (dedup on mtime/size still applies)."""
import time
from hermes_cli import config as cfg_mod
cfg_mod._CONFIG_PARSE_WARNED.clear()
with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}):
cfg = tmp_path / "config.yaml"
cfg.write_text("model:\n default: test/custom\n")
load_config()
time.sleep(0.05)
cfg.write_text("\tbroken:\n")
load_config()
capsys.readouterr()
second = load_config()
assert second["model"]["default"] == "test/custom"
assert capsys.readouterr().err == ""
class TestSaveAndLoadRoundtrip:
@staticmethod