diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 9f43d1ce3a..efb120ab27 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -1103,34 +1103,51 @@ def _load_global_auth_store() -> Dict[str, Any]: Returns an empty dict when no global fallback exists (classic mode, or the global auth.json is absent). Never raises on missing file. - Seat belt: under pytest, refuses to read the real user's - ``~/.hermes/auth.json`` even when HERMES_HOME is set to a profile - path. The hermetic conftest does not redirect ``HOME``, so - ``get_default_hermes_root()`` for a profile-shaped HERMES_HOME can - still resolve to the real user's home on a dev machine. That would - leak real credentials into tests. This guard uses the unmodified - ``HOME`` env var (what ``os.path.expanduser('~')`` would resolve to), - not ``Path.home()``, because ``Path.home`` is sometimes monkeypatched - by fixtures that want to relocate the global root to a tmp path. + Memoised keyed on the global auth file's path + mtime (same pattern as + ``_nous_auth_status_cache``): read_credential_pool() -> load_pool() runs + this once per provider row in the /model picker, and the path resolution + + JSON parse cost ~105us+ per call even when nothing changed. The global + store only changes when the user authenticates at global scope (writes + always go through _save_auth_store, which touches the file), so the mtime + key keeps the memo freshness-correct. Callers must treat the returned + store as read-only (all current callers do — .get / dict() / list() + copies only). """ + global _global_auth_store_cache global_path = _global_auth_file_path() if global_path is None or not global_path.exists(): + _global_auth_store_cache = None return {} + try: + resolved_path = str(global_path.resolve(strict=False)) + mtime_ns = global_path.stat().st_mtime_ns + cache_key: Optional[Tuple[str, int]] = (resolved_path, mtime_ns) + except Exception: + cache_key = None + if cache_key is not None and _global_auth_store_cache is not None: + cached_path, cached_mtime, cached_store = _global_auth_store_cache + if cached_path == cache_key[0] and cached_mtime == cache_key[1]: + return cached_store if os.environ.get("PYTEST_CURRENT_TEST"): real_home_env = os.environ.get("HOME", "") if real_home_env: real_root = Path(real_home_env) / ".hermes" / "auth.json" try: if global_path.resolve(strict=False) == real_root.resolve(strict=False): + _global_auth_store_cache = None return {} except Exception: pass try: - return _load_auth_store(global_path) + store = _load_auth_store(global_path) except Exception: # A malformed global store must not break profile reads. The # profile's own auth store is still authoritative. + _global_auth_store_cache = None return {} + if cache_key is not None: + _global_auth_store_cache = (cache_key[0], cache_key[1], store) + return store def _auth_lock_path() -> Path: @@ -6681,6 +6698,11 @@ def _snapshot_nous_pool_status() -> Dict[str, Any]: _NOUS_AUTH_STATUS_CACHE_TTL = 15.0 # seconds _nous_auth_status_cache: Optional[Tuple[float, str, Optional[float], Dict[str, Any]]] = None +# mtime-keyed memo for _load_global_auth_store(): (path, mtime_ns, store). +# Same invalidation contract as _nous_auth_status_cache — the global auth +# file changes only when a global-scope auth write touches it. +_global_auth_store_cache: Optional[Tuple[str, int, Dict[str, Any]]] = None + def _auth_file_cache_key() -> Tuple[str, Optional[float]]: auth_file = _auth_file_path() diff --git a/hermes_constants.py b/hermes_constants.py index 4250a1dac1..13602e20d6 100644 --- a/hermes_constants.py +++ b/hermes_constants.py @@ -170,6 +170,16 @@ def get_process_hermes_home() -> Path: return _hermes_home_from_env() +# Process-level memo for get_default_hermes_root(). The function resolves +# HERMES_HOME against the native home on every call (~80us of path +# resolution), and it is called at 31+ sites — every _load_global_auth_store() +# (per provider row in the /model picker), kanban, backup, gateway, update. +# Its result depends only on (HERMES_HOME, platform native home), which are +# compared for free on each call, so the memo is freshness-correct even if a +# test or plugin mutates HERMES_HOME mid-process. +_default_hermes_root_memo: "tuple[str, str, Path] | None" = None + + def get_default_hermes_root() -> Path: """Return the root Hermes directory for profile-level operations. @@ -187,27 +197,34 @@ def get_default_hermes_root() -> Path: Import-safe — no dependencies beyond stdlib. """ + global _default_hermes_root_memo native_home = _get_platform_default_hermes_home() env_home = os.environ.get("HERMES_HOME", "") + if _default_hermes_root_memo is not None: + memo_native, memo_env, memo_result = _default_hermes_root_memo + if memo_native == str(native_home) and memo_env == env_home: + return memo_result + if not env_home: - return native_home - env_path = Path(env_home) - try: - env_path.resolve().relative_to(native_home.resolve()) - # HERMES_HOME is under ~/.hermes (normal or profile mode) - return native_home - except ValueError: - pass - - # Docker / custom deployment. - # Check if this is a profile path: /profiles/ - # If the immediate parent dir is named "profiles", the root is - # the grandparent — this covers Docker profiles correctly. - if env_path.parent.name == "profiles": - return env_path.parent.parent - - # Not a profile path — HERMES_HOME itself is the root - return env_path + result = native_home + else: + env_path = Path(env_home) + try: + env_path.resolve().relative_to(native_home.resolve()) + # HERMES_HOME is under ~/.hermes (normal or profile mode) + result = native_home + except ValueError: + # Docker / custom deployment. + # Check if this is a profile path: /profiles/ + # If the immediate parent dir is named "profiles", the root is + # the grandparent — this covers Docker profiles correctly. + if env_path.parent.name == "profiles": + result = env_path.parent.parent + else: + # Not a profile path — HERMES_HOME itself is the root + result = env_path + _default_hermes_root_memo = (str(native_home), env_home, result) + return result def get_optional_skills_dir(default: Path | None = None) -> Path: diff --git a/tests/hermes_cli/test_global_auth_store_memo.py b/tests/hermes_cli/test_global_auth_store_memo.py new file mode 100644 index 0000000000..b49b00732d --- /dev/null +++ b/tests/hermes_cli/test_global_auth_store_memo.py @@ -0,0 +1,107 @@ +"""Measured-work pins for the _load_global_auth_store() memo. + +read_credential_pool() -> load_pool() runs _load_global_auth_store() once per +provider row in the /model picker, and the global-store JSON read + parse +cost ~60-100us+ per call even when nothing changed. The memo keyed on the +global auth file's path+mtime makes repeat reads a dict lookup. The store +only changes when the user authenticates at global scope (writes always go +through _save_auth_store, which touches the file), so the mtime key keeps +the memo freshness-correct. +""" + +from __future__ import annotations + +import json +import os + +import pytest + +import hermes_cli.auth as auth_mod + + +@pytest.fixture(autouse=True) +def _reset_cache(): + auth_mod._global_auth_store_cache = None + yield + auth_mod._global_auth_store_cache = None + + +def _make_global_store(tmp_path) -> "os.PathLike[str]": + """Write a realistic global auth.json and return its path.""" + path = tmp_path / "global-hermes" / "auth.json" + path.parent.mkdir(parents=True) + path.write_text( + json.dumps( + { + "version": 1, + "providers": { + "openai": {"api_key": "sk-x"}, + "anthropic": {"api_key": "an-x"}, + }, + "credential_pool": { + "openai": [{"id": "1", "access_token": "t"}], + "anthropic": [{"id": "2", "access_token": "u"}], + }, + } + ), + encoding="utf-8", + ) + return path + + +class TestLoadGlobalAuthStoreMemo: + def test_repeated_calls_read_store_once(self, tmp_path, monkeypatch): + """Repeated calls must not re-read/re-parse the global store.""" + global_path = _make_global_store(tmp_path) + monkeypatch.setattr( + auth_mod, "_global_auth_file_path", lambda: global_path + ) + reads = {"n": 0} + orig = auth_mod._load_auth_store + + def counting_load(store_path=None): + reads["n"] += 1 + return orig(store_path) + + monkeypatch.setattr(auth_mod, "_load_auth_store", counting_load) + + first = auth_mod._load_global_auth_store() + for _ in range(10): + auth_mod._load_global_auth_store() + assert reads["n"] == 1, ( + "repeated calls must be memo hits (store read once), " + f"got {reads['n']}" + ) + assert first.get("providers", {}).get("openai") == {"api_key": "sk-x"} + + def test_mtime_change_re_reads_once(self, tmp_path, monkeypatch): + """A store file change on disk invalidates the memo.""" + global_path = _make_global_store(tmp_path) + monkeypatch.setattr( + auth_mod, "_global_auth_file_path", lambda: global_path + ) + reads = {"n": 0} + orig = auth_mod._load_auth_store + + def counting_load(store_path=None): + reads["n"] += 1 + return orig(store_path) + + monkeypatch.setattr(auth_mod, "_load_auth_store", counting_load) + + auth_mod._load_global_auth_store() + assert reads["n"] == 1 + + # Bump the file mtime -> memo invalidates -> re-read once. + os.utime(global_path, (1_700_000_000, 1_700_000_000)) + auth_mod._load_global_auth_store() + assert reads["n"] == 2, "mtime change must force exactly one re-read" + + def test_absent_global_store_returns_empty_without_error(self, tmp_path, monkeypatch): + """No global fallback (classic mode) returns {} and stays cheap.""" + missing = tmp_path / "no-such" / "auth.json" + monkeypatch.setattr( + auth_mod, "_global_auth_file_path", lambda: missing + ) + assert auth_mod._load_global_auth_store() == {} + assert auth_mod._global_auth_store_cache is None diff --git a/tests/test_hermes_constants.py b/tests/test_hermes_constants.py index 158b5b4041..fa0c047996 100644 --- a/tests/test_hermes_constants.py +++ b/tests/test_hermes_constants.py @@ -64,6 +64,52 @@ class TestGetDefaultHermesRoot: assert get_default_hermes_root() == local_appdata / "hermes" + def test_result_memoised_until_env_or_home_changes(self, tmp_path, monkeypatch): + """Repeated calls reuse the memo; HERMES_HOME / home changes invalidate. + + get_default_hermes_root() resolves HERMES_HOME against the native + home (~80us of path resolution) and is called at 31+ sites — every + _load_global_auth_store() (per provider row in the /model picker), + kanban, backup, gateway, update. The memo is keyed on + (native home, HERMES_HOME) compared for free each call. + """ + monkeypatch.delenv("HERMES_HOME", raising=False) + monkeypatch.setattr(Path, "home", lambda: tmp_path) + + # Probe the expensive inner work: the memo check itself calls + # _get_platform_default_hermes_home() on every call (even hits), so + # count Path.resolve on the env path instead — only the actual + # resolution branch pays it. + resolve_calls = {"n": 0} + orig_resolve = Path.resolve + + def counting_resolve(self, *a, **k): + resolve_calls["n"] += 1 + return orig_resolve(self, *a, **k) + + monkeypatch.setattr(Path, "resolve", counting_resolve) + hermes_constants._default_hermes_root_memo = None + + first = get_default_hermes_root() + first_count = resolve_calls["n"] + for _ in range(10): + get_default_hermes_root() + assert resolve_calls["n"] == first_count, ( + "repeated calls must be memo hits (no path resolution on hits), " + f"resolve went {first_count} -> {resolve_calls['n']}" + ) + assert first == tmp_path / ".hermes" + + # HERMES_HOME change invalidates the memo (fresh resolution). + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "elsewhere")) + before = resolve_calls["n"] + assert get_default_hermes_root() == tmp_path / "elsewhere" + assert resolve_calls["n"] > before, ( + "HERMES_HOME change must force a fresh resolution" + ) + + + class TestGetHermesHome: