ouyangbo/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 9 Packages Projects Releases Wiki Activity

fix: widen the remaining config.yaml stat caches to file_signature

Browse Source 
Three caches that read config.yaml still keyed change detection on
st_mtime (or st_mtime_ns + st_size), so a same-size replacement that
keeps the old timestamp (cp -p, rsync -t, a timestamp-pinning writer)
was never noticed:

- model_tools._tool_defs_cache_key: get_tool_definitions kept serving
  stale dynamic tool schemas / mcp_servers for the process lifetime.
- CLI mcp_servers auto-reload watcher (cli_tui_mixin seed +
  cli_info_mixin._check_config_mcp_changes): the replaced mcp_servers
  section was never reloaded. The seed is now _config_sig.
- tui_gateway/server._load_cfg_raw / _save_cfg (_cfg_mtime -> _cfg_sig):
  the raw-config cache served the stale document and the next _save_cfg
  would write it back over the on-disk file.

All three now use utils.file_signature like the rest of the PR. Tests that
reset the renamed module/instance attributes follow the rename; one
pinned-mtime replacement test per cache, red on the previous head.
Also corrects two stale type/comment annotations in hermes_cli/config.py
(_env_cache key shape, _RAW_CONFIG_CACHE record shape).

Review finding: three sibling config.yaml caches (tool-defs memo, CLI mcp watcher, TUI-gateway raw cfg) still compared mtime/size only.
This commit is contained in:
teknium1
2026-09-14 21:28:14 -07:00
committed by Teknium
parent a1e7f74e64
commit 182ec5c28d
16 changed files with 105 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files
hermes_cli/cli_info_mixin.py
+4 -4
View File
@@ -16,7 +16,7 @@ import time
from hermes_constants import is_termux as _is_termux_environment
from rich.markup import escape as _escape
from utils import base_url_hostname
from utils import base_url_hostname, file_signature
from hermes_cli.cli_modal_mixin import _gated_confirm
from hermes_cli.colors import Colors as _Colors
@@ -818,13 +818,13 @@ class CLIInfoMixin:
if not cfg_path.exists():
return
try:
mtime = cfg_path.stat().st_mtime
sig = file_signature(cfg_path.stat())
except OSError:
return
if mtime == self._config_mtime:
if sig == self._config_sig:
return # unchanged — fast path
self._config_mtime = mtime
self._config_sig = sig
try:
with open(cfg_path, encoding="utf-8") as f:
new_cfg = _yaml.safe_load(f) or {}
hermes_cli/cli_tui_mixin.py
+1 -1
View File
@@ -1819,7 +1819,7 @@ class CLITuiMixin:
# Config file watcher — detect mcp_servers changes and auto-reload.
from hermes_cli.config import get_config_path as _get_config_path
_cfg_path = _get_config_path()
self._config_mtime: float = _cfg_path.stat().st_mtime if _cfg_path.exists() else 0.0
self._config_sig: tuple | None = file_signature(_cfg_path.stat()) if _cfg_path.exists() else None
self._config_mcp_servers: dict = self.config.get("mcp_servers") or {}
self._last_config_check: float = 0.0 # monotonic time of last check
hermes_cli/config.py
+2 -2
View File
@@ -204,7 +204,7 @@ _LAST_EXPANDED_CONFIG_BY_PATH: Dict[str, Any] = {}
# atomic_yaml_write which produces a fresh inode, so stat() sees a new signature and the next load
# repopulates automatically — no explicit invalidation hook. See #58514.
_LOAD_CONFIG_CACHE: Dict[str, Tuple[int, ...]] = {}
# path -> (mtime_ns, size, raw yaml dict) for read_raw_config() (no defaults merged in).
# path -> (mtime_ns, size, ino, ctime_ns, raw yaml dict) for read_raw_config() (no defaults merged in).
_RAW_CONFIG_CACHE: Dict[str, Tuple[int, ...]] = {}
# Env var names written to .env that aren't in OPTIONAL_ENV_VARS (managed by setup/provider
@@ -2392,7 +2392,7 @@ def save_config(
# load_env() memo keyed on (path, *file_signature). Editing .env bumps mtime/inode -> rebuild;
# invalidate_env_cache() is the explicit knob for writers on coarse-mtime filesystems.
_env_cache: Optional[Tuple[Tuple[str, Optional[float], Optional[int]], Dict[str, str]]] = None
_env_cache: Optional[Tuple[Tuple[str, Optional[Tuple[int, int, int, int]]], Dict[str, str]]] = None
def load_env() -> Dict[str, str]:
model_tools.py
+3 -2
View File
@@ -22,6 +22,7 @@ from tools.registry import CHECK_FN_CACHE_BYPASS, check_fn_cache_scope, discover
from tools.registry import _MAX_TOOL_ERROR_CHARS as _TOOL_ERROR_MAX_LEN
from toolsets import resolve_toolset, validate_toolset
from tools.arg_coercion import coerce_tool_args
from utils import file_signature
logger = logging.getLogger(__name__)
@@ -257,7 +258,7 @@ def _tool_defs_cache_key(
"""Memo key for get_tool_definitions, or None when caching must be bypassed.
Covers every argument plus everything that changes the result without one:
registry generation, config.yaml mtime/size (dynamic schemas), kanban
registry generation, config.yaml stat signature (dynamic schemas), kanban
context, profile scope. check_fn results are TTL-cached in the registry.
"""
profile_scope = check_fn_cache_scope()
@@ -266,7 +267,7 @@ def _tool_defs_cache_key(
try:
from hermes_cli.config import get_config_path
cfg_stat = get_config_path().stat()
cfg_fp = (cfg_stat.st_mtime_ns, cfg_stat.st_size)
cfg_fp = file_signature(cfg_stat)
except (FileNotFoundError, OSError, ImportError):
cfg_fp = None
return (
tests/conftest.py
+3 -3
View File
@@ -920,7 +920,7 @@ def _reset_tui_gateway_server_state():
if mod is not None:
snapshot = {
"methods": dict(mod._methods),
"cfg": (mod._cfg_cache, mod._cfg_mtime, mod._cfg_path),
"cfg": (mod._cfg_cache, mod._cfg_sig, mod._cfg_path),
"db": (mod._db, mod._db_error),
"real_stdout": mod._real_stdout,
}
@@ -951,7 +951,7 @@ def _reset_tui_gateway_server_state():
if snapshot is not None:
mod._methods.clear()
mod._methods.update(snapshot["methods"])
mod._cfg_cache, mod._cfg_mtime, mod._cfg_path = snapshot["cfg"]
mod._cfg_cache, mod._cfg_sig, mod._cfg_path = snapshot["cfg"]
mod._db, mod._db_error = snapshot["db"]
mod._real_stdout = snapshot["real_stdout"]
else:
@@ -959,7 +959,7 @@ def _reset_tui_gateway_server_state():
# for the globals we could not snapshot (``_methods`` is left to
# the importing file's fixture, see block comment above).
mod._cfg_cache = None
mod._cfg_mtime = None
mod._cfg_sig = None
mod._cfg_path = None
mod._db = None
mod._db_error = None
tests/hermes_cli/test_cli_mcp_config_watch.py
+30 -8
View File
@@ -3,6 +3,8 @@ import time
from pathlib import Path
from unittest.mock import MagicMock, patch
from utils import file_signature
def _make_cli(tmp_path, mcp_servers=None, extra_config=None):
"""Create a minimal HermesCLI instance with mocked config."""
@@ -18,7 +20,7 @@ def _make_cli(tmp_path, mcp_servers=None, extra_config=None):
cfg_file = tmp_path / "config.yaml"
cfg_file.write_text("mcp_servers: {}\n")
obj._config_mtime = cfg_file.stat().st_mtime
obj._config_sig = file_signature(cfg_file.stat())
obj._reload_mcp = MagicMock()
obj._busy_command = MagicMock()
@@ -40,7 +42,7 @@ class TestMCPConfigWatch:
# Simulate user adding a new MCP server to config.yaml
cfg_file.write_text(yaml.dump({"mcp_servers": {"github": {"url": "https://mcp.github.com"}}}))
obj._config_mtime = 0.0 # force stale mtime
obj._config_sig = None # force stale mtime
with patch("hermes_cli.config.get_config_path", return_value=cfg_file):
obj._check_config_mcp_changes()
@@ -54,7 +56,7 @@ class TestMCPConfigWatch:
# Simulate user removing the server
cfg_file.write_text(yaml.dump({"mcp_servers": {}}))
obj._config_mtime = 0.0
obj._config_sig = None
with patch("hermes_cli.config.get_config_path", return_value=cfg_file):
obj._check_config_mcp_changes()
@@ -87,7 +89,7 @@ class TestMCPConfigWatch:
"mcp": {"auto_reload_on_config_change": False},
"mcp_servers": {"github": {"url": "https://mcp.github.com"}},
}))
obj._config_mtime = 0.0 # force stale mtime
obj._config_sig = None # force stale mtime
with patch("hermes_cli.config.get_config_path", return_value=cfg_file):
obj._check_config_mcp_changes()
@@ -110,13 +112,13 @@ class TestMCPConfigWatch:
"mcp": {"auto_reload_on_config_change": False},
"mcp_servers": {"github": {"url": "https://mcp.github.com"}},
}))
obj._config_mtime = 0.0
obj._config_sig = None
with patch("hermes_cli.config.get_config_path", return_value=cfg_file):
obj._check_config_mcp_changes()
# Second pass: same file content, new mtime — no reload, no change.
obj._last_config_check = 0.0
obj._config_mtime = 0.0
obj._config_sig = None
obj._check_config_mcp_changes()
obj._reload_mcp.assert_not_called()
@@ -139,7 +141,7 @@ class TestMCPConfigWatch:
"auxiliary": {"mcp": {"auto_reload_on_config_change": False}},
"mcp_servers": {"github": {"url": "https://mcp.github.com"}},
}))
obj._config_mtime = 0.0
obj._config_sig = None
with patch("hermes_cli.config.get_config_path", return_value=cfg_file):
obj._check_config_mcp_changes()
@@ -184,10 +186,30 @@ class TestMCPConfigWatch:
"agent": {"reasoning_effort": "high"},
"mcp_servers": raw_servers,
}))
obj._config_mtime = 0.0
obj._config_sig = None
with patch("hermes_cli.config.get_config_path", return_value=cfg_file):
obj._check_config_mcp_changes()
obj._reload_mcp.assert_not_called()
assert "MCP server config changed" not in capsys.readouterr().out
def test_pinned_mtime_same_size_replacement_triggers_reload(tmp_path):
"""#111105: cp -p / rsync -t style replacement (same mtime, same size) must still reload."""
import os
import shutil
obj, cfg_file = _make_cli(tmp_path, mcp_servers={"bb": {"command": "b"}})
cfg_file.write_text("mcp_servers:\n bb: {command: b}\n")
obj._config_sig = file_signature(cfg_file.stat())
other = tmp_path / "other.yaml"
other.write_text("mcp_servers:\n aa: {command: a}\n")
shutil.copy2(other, cfg_file)
os.utime(cfg_file, ns=(obj._config_sig[0], obj._config_sig[0]))
with patch("hermes_cli.config.get_config_path", return_value=cfg_file):
obj._check_config_mcp_changes()
obj._reload_mcp.assert_called_once()
assert obj._config_mcp_servers == {"aa": {"command": "a"}}
tests/tools/test_mcp_stability.py
+1 -1
View File
@@ -443,7 +443,7 @@ class TestMCPReloadTimeout:
# Create a mock HermesCLI-like object with the needed attributes
class FakeCLI:
_config_mtime = 0.0
_config_sig = None
_config_mcp_servers = {}
_last_config_check = 0.0
_command_running = False
tests/tools/test_model_tools.py
+22 -1
View File
@@ -570,7 +570,9 @@ class TestBridgeDispatch:
assert disp.call_args.args[0] == "mcp_x" and disp.call_args.args[1] == {"a": 1}
# =========================================================================
# ==================================================================
# Browser schema retrieval hints
# =========================================================================
@@ -600,3 +602,22 @@ class TestBrowserRetrievalHints:
rendered = " ".join(d["function"]["description"] for d in _apply_dynamic_schemas(defs + self._defs("terminal")))
assert "web_search" not in rendered
assert "web_extract" not in rendered
def test_tool_defs_cache_key_sees_config_replacement_with_pinned_mtime(tmp_path):
"""#111105: a same-size config.yaml swapped in with the old mtime must change the memo key."""
import os
import shutil
from model_tools import _tool_defs_cache_key
cfg = tmp_path / "config.yaml"
cfg.write_text("mcp_servers:\n aa: {command: a}\n", encoding="utf-8")
with patch("hermes_cli.config.get_config_path", return_value=cfg):
before = _tool_defs_cache_key(None, None, False)
st = cfg.stat()
other = tmp_path / "other.yaml"
other.write_text("mcp_servers:\n bb: {command: b}\n", encoding="utf-8")
shutil.copy2(other, cfg)
os.utime(cfg, ns=(st.st_atime_ns, st.st_mtime_ns))
assert _tool_defs_cache_key(None, None, False) != before
tests/tui_gateway/test_config_profile_scope.py
+1 -1
View File
@@ -45,7 +45,7 @@ def _homes(tmp_path: Path) -> tuple[Path, Path]:
def _reset_cfg_cache() -> None:
server._cfg_cache = None
server._cfg_mtime = None
server._cfg_sig = None
server._cfg_path = None
tests/tui_gateway/test_config_set_display_toggles.py
+2 -2
View File
@@ -16,9 +16,9 @@ from tui_gateway import server
def config_home(tmp_path, monkeypatch):
"""Point the server's config read/write at a temp file."""
monkeypatch.setattr(server, "_hermes_home", tmp_path)
server._cfg_cache = server._cfg_mtime = server._cfg_path = None
server._cfg_cache = server._cfg_sig = server._cfg_path = None
yield tmp_path / "config.yaml"
server._cfg_cache = server._cfg_mtime = server._cfg_path = None
server._cfg_cache = server._cfg_sig = server._cfg_path = None
def _set(key, value):
tests/tui_gateway/test_config_set_voice_chat_mode.py
+2 -2
View File
@@ -14,9 +14,9 @@ from tui_gateway import server
@pytest.fixture
def config_home(tmp_path, monkeypatch):
monkeypatch.setattr(server, "_hermes_home", tmp_path)
server._cfg_cache = server._cfg_mtime = server._cfg_path = None
server._cfg_cache = server._cfg_sig = server._cfg_path = None
yield tmp_path / "config.yaml"
server._cfg_cache = server._cfg_mtime = server._cfg_path = None
server._cfg_cache = server._cfg_sig = server._cfg_path = None
def _set(value):
tests/tui_gateway/test_goal_command.py
+1 -1
View File
@@ -60,7 +60,7 @@ def server(hermes_home, monkeypatch):
# originals on teardown so nothing leaks to later tests either.
monkeypatch.setattr(mod, "_hermes_home", hermes_home)
monkeypatch.setattr(mod, "_cfg_cache", None)
monkeypatch.setattr(mod, "_cfg_mtime", None)
monkeypatch.setattr(mod, "_cfg_sig", None)
monkeypatch.setattr(mod, "_cfg_path", None)
yield mod
# Reset module-level session state without re-importing. importlib.reload
tests/tui_gateway/test_protocol.py
+1 -1
View File
@@ -1357,7 +1357,7 @@ def test_skin_live_switch_end_to_end(server, tmp_path, monkeypatch):
monkeypatch.setattr(skin_engine, "get_hermes_home", lambda: tmp_path)
monkeypatch.setattr(server, "_hermes_home", tmp_path)
monkeypatch.setattr(server, "_last_skin_sig", None, raising=False)
server._cfg_cache = server._cfg_mtime = server._cfg_path = None
server._cfg_cache = server._cfg_sig = server._cfg_path = None
emitted = []
monkeypatch.setattr(server, "_emit", lambda ev, sid, payload=None: emitted.append((ev, payload)))
tests/tui_gateway/test_session_control.py
+1 -1
View File
@@ -39,7 +39,7 @@ def server(hermes_home, monkeypatch):
mod = importlib.import_module("tui_gateway.server")
monkeypatch.setattr(mod, "_hermes_home", hermes_home)
monkeypatch.setattr(mod, "_cfg_cache", None)
monkeypatch.setattr(mod, "_cfg_mtime", None)
monkeypatch.setattr(mod, "_cfg_sig", None)
monkeypatch.setattr(mod, "_cfg_path", None)
yield mod
mod._sessions.clear()
tests/tui_gateway/test_tui_gateway_server.py
+22 -2
View File
@@ -105,7 +105,7 @@ def test_session_slot_is_claimed_on_first_turn_not_on_create(monkeypatch, tmp_pa
try:
server._cfg_cache = None
server._cfg_mtime = None
server._cfg_sig = None
server._cfg_path = None
_clear_server_sessions()
monkeypatch.setattr(server, "_start_agent_build", lambda *args, **kwargs: None)
@@ -139,7 +139,7 @@ def test_session_slot_is_claimed_on_first_turn_not_on_create(monkeypatch, tmp_pa
finally:
_clear_server_sessions()
server._cfg_cache = None
server._cfg_mtime = None
server._cfg_sig = None
server._cfg_path = None
reset_hermes_home_override(token)
@@ -22496,3 +22496,23 @@ def test_workspace_move_rehomes_running_session(monkeypatch, tmp_path):
assert captured["row_update"] == (target, str(new_cwd))
assert live["cwd"] == str(new_cwd)
assert live.get("explicit_cwd") is True
def test_load_cfg_raw_sees_replacement_with_pinned_mtime_and_size(monkeypatch, tmp_path):
"""#111105: the raw-config cache must not serve (and later write back) a stale document after a
same-size replacement that keeps the old mtime."""
import shutil
cfg = tmp_path / "config.yaml"
cfg.write_text("model:\n default: bbbb-route\n", encoding="utf-8")
monkeypatch.setattr(server, "_active_config_path", lambda: cfg)
monkeypatch.setattr(server, "_cfg_cache", None)
monkeypatch.setattr(server, "_cfg_sig", None)
monkeypatch.setattr(server, "_cfg_path", None)
assert server._load_cfg_raw()["model"]["default"] == "bbbb-route"
st = cfg.stat()
other = tmp_path / "other.yaml"
other.write_text("model:\n default: aaaa-route\n", encoding="utf-8")
shutil.copy2(other, cfg)
os.utime(cfg, ns=(st.st_atime_ns, st.st_mtime_ns))
assert server._load_cfg_raw()["model"]["default"] == "aaaa-route"
tui_gateway/server.py
+9 -9
View File
@@ -26,7 +26,7 @@ from hermes_constants import (
get_hermes_home, get_hermes_home_override, profile_name_for_home,
reset_hermes_home_override, set_hermes_home_override)
from hermes_cli.env_loader import load_hermes_dotenv
from utils import is_truthy_value
from utils import file_signature, is_truthy_value
from hermes_state_ids import new_session_id
from tools.environments.local import hermes_subprocess_env
from agent.replay_cleanup import canonicalize_replay_history
@@ -96,7 +96,7 @@ _cfg_lock = threading.Lock()
_profile_ui_meta_lock = threading.Lock()
_sessions_lock = threading.RLock() # reentrant: _close_session_by_id may run under callers that already hold it
_cfg_cache: dict | None = None
_cfg_mtime: float | None = None
_cfg_sig: tuple | None = None
_cfg_path = None
_session_resume_lock = threading.Lock()
_SLASH_WORKER_TIMEOUT_S = max(5.0, env_float("HERMES_TUI_SLASH_TIMEOUT_S", 45.0))
@@ -1227,17 +1227,17 @@ def _load_cfg_raw() -> dict:
read→mutate→``_save_cfg`` round-trips and raw inspection (defaults / managed overlay / ``${VAR}``
expansion applied here would be persisted on the next save). Behavioral reads use :func:`_load_cfg`.
Cache keyed on the resolved path so profiles don't clobber."""
global _cfg_cache, _cfg_mtime, _cfg_path
global _cfg_cache, _cfg_sig, _cfg_path
with contextlib.suppress(Exception):
p = _active_config_path()
mtime = p.stat().st_mtime if p.exists() else None
sig = file_signature(p.stat()) if p.exists() else None
with _cfg_lock:
if _cfg_cache is not None and _cfg_mtime == mtime and _cfg_path == p:
if _cfg_cache is not None and _cfg_sig == sig and _cfg_path == p:
return copy.deepcopy(_cfg_cache)
from hermes_cli.config import read_user_config_raw
data = read_user_config_raw(p) if p.exists() else {}
with _cfg_lock: # cache the RAW config: _save_cfg writes _cfg_cache back to disk
_cfg_cache, _cfg_mtime, _cfg_path = copy.deepcopy(data), mtime, p
_cfg_cache, _cfg_sig, _cfg_path = copy.deepcopy(data), sig, p
return data
return {}
@@ -1254,7 +1254,7 @@ def _load_cfg() -> dict:
def _save_cfg(cfg: dict):
global _cfg_cache, _cfg_mtime, _cfg_path
global _cfg_cache, _cfg_sig, _cfg_path
from utils import atomic_roundtrip_yaml_save
path = _active_config_path()
# Comment-, ordering- and Unicode-preserving write (a plain safe_dump clobbered hand-written configs);
@@ -1263,9 +1263,9 @@ def _save_cfg(cfg: dict):
with _cfg_lock:
_cfg_cache, _cfg_path = copy.deepcopy(cfg), path
try:
_cfg_mtime = path.stat().st_mtime
_cfg_sig = file_signature(path.stat())
except Exception:
_cfg_mtime = None
_cfg_sig = None
def _session_for_key(session_key: str) -> dict | None: