perf(cli): memoise default-hermes-root resolution and global auth-store read
get_default_hermes_root() resolves HERMES_HOME against the platform native home (~80us of path resolution) on EVERY call and 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, native home), so memoise it keyed on those two inputs, compared for free on each call (freshness-correct even if a test or plugin mutates HERMES_HOME mid-process). _load_global_auth_store() re-read + re-parsed the global auth.json on every call; read_credential_pool() -> load_pool() runs it once per provider row in the /model picker even when the profile has entries and the global fallback never fires. Memoise keyed on the global auth file's path+mtime (same pattern as _nous_auth_status_cache); the store only changes when a global-scope auth write touches the file. Measured (profile mode, 30-provider global store): get_default_hermes_root 81us -> 10us; _load_global_auth_store 128us -> 66us; load_pool 165us -> 137us per call — ~2ms saved per /model picker render (20 provider rows). Regression tests: hermes_constants memo pin (no path resolution on repeat calls, HERMES_HOME change forces a fresh resolution); global-store memo pins (store read once across repeats, mtime bump re-reads once, absent store stays cheap). (cherry picked from commit be348f32e5bd7479c26fabb652de549fb9c8a1e1)
This commit is contained in:
@@ -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()
|
||||
|
||||
@@ -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: <root>/profiles/<name>
|
||||
# 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: <root>/profiles/<name>
|
||||
# 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:
|
||||
|
||||
107
tests/hermes_cli/test_global_auth_store_memo.py
Normal file
107
tests/hermes_cli/test_global_auth_store_memo.py
Normal file
@@ -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
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user