refactor: shrink profile-switch models cache diff
Reuse _get_models_cache_path(profile) for profile delete/create instead of a second path helper, derive the fingerprint home from the auth store path, fold the three switch test files into one parametrized file, and trim comments/docs. Behaviour, source axes and switch fast path are unchanged.
This commit is contained in:
carlotestor committed
1 parent
35bb952def
commit
7344c8df77
8 files changed
+215
-703
No files matched your search
@@ -316,7 +316,7 @@ def test_profile_switch_restarts_watcher_best_effort(monkeypatch):
|
||||
monkeypatch.setattr(routes, "read_body", lambda handler: {"name": "demo"})
|
||||
monkeypatch.setattr(profiles, "_validate_profile_name", lambda name: None)
|
||||
monkeypatch.setattr(profiles, "switch_profile", lambda name, process_wide=False: {"ok": True, "name": name})
|
||||
monkeypatch.setattr(config, "invalidate_models_cache", lambda *_a, **_kw: calls.append("cache"))
|
||||
monkeypatch.setattr(config, "invalidate_models_cache", lambda **kw: calls.append(("cache", kw)))
|
||||
monkeypatch.setattr(gateway_watcher, "restart_watcher_for_profile", lambda name: calls.append(("watcher", name)))
|
||||
|
||||
handler = _FakeHandler()
|
||||
@@ -324,7 +324,7 @@ def test_profile_switch_restarts_watcher_best_effort(monkeypatch):
|
||||
|
||||
assert handler.status == 200
|
||||
assert handler.get_json() == {"ok": True, "name": "demo"}
|
||||
assert calls == ["cache", ("watcher", "demo")]
|
||||
assert calls == [("cache", {"delete_disk": False}), ("watcher", "demo")]
|
||||
assert any(k == "Set-Cookie" for k, _v in handler.sent_headers)
|
||||
|
||||
|
||||
|
||||
@@ -1,227 +0,0 @@
|
||||
"""A retained per-profile models disk snapshot must be rejected after a switch
|
||||
when a catalog source changed in the target profile: its ``.env`` provider
|
||||
credentials, its installed model-provider plugins, or the profile itself
|
||||
(delete + recreate with the same name).
|
||||
|
||||
Each test saves a real snapshot for profile ``demo`` through
|
||||
``_save_models_cache_to_disk()``, changes one source while ``default`` is
|
||||
active, performs the per-client switch exactly as ``/api/profile/switch`` does
|
||||
(``invalidate_models_cache(delete_disk=False)``), and asserts the first
|
||||
``get_available_models()`` for ``demo`` serves the fresh rebuild.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import shutil
|
||||
import sys
|
||||
from types import SimpleNamespace
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
def _catalog(label: str) -> dict:
|
||||
return {
|
||||
"active_provider": "openai",
|
||||
"default_model": label,
|
||||
"configured_model_badges": {},
|
||||
"groups": [
|
||||
{
|
||||
"provider": "OpenAI",
|
||||
"provider_id": "openai",
|
||||
"models": [{"id": label, "label": label, "supports_fast_tier": False}],
|
||||
}
|
||||
],
|
||||
"aliases": {},
|
||||
}
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def two_profiles(tmp_path: Path, monkeypatch):
|
||||
import api.config as cfg
|
||||
import api.profiles as profiles
|
||||
|
||||
monkeypatch.delenv("HERMES_CONFIG_PATH", raising=False)
|
||||
default_home = tmp_path / ".hermes"
|
||||
default_home.mkdir()
|
||||
(default_home / "config.yaml").write_text("model:\n default: default-model\n", encoding="utf-8")
|
||||
demo_home = default_home / "profiles" / "demo"
|
||||
demo_home.mkdir(parents=True)
|
||||
(demo_home / "config.yaml").write_text("model:\n default: demo-model\n", encoding="utf-8")
|
||||
os.utime(demo_home / "config.yaml", (1_000_000, 1_000_000))
|
||||
|
||||
monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", default_home)
|
||||
monkeypatch.setattr(profiles, "_active_profile", "default")
|
||||
monkeypatch.setattr(profiles._tls, "profile", None, raising=False)
|
||||
monkeypatch.setattr(cfg, "_models_cache_path", tmp_path / "models_cache.json")
|
||||
monkeypatch.setattr(cfg, "_load_stale_models_cache_from_disk", lambda: None)
|
||||
monkeypatch.setattr(cfg, "_invoke_models_rebuild", lambda _builder: _catalog("fresh-model"))
|
||||
for attr in ("_cfg_mtime", "_cfg_path", "_cfg_fingerprint"):
|
||||
monkeypatch.setattr(cfg, attr, getattr(cfg, attr), raising=False)
|
||||
saved_cfg = dict(cfg._cfg_cache)
|
||||
cfg.invalidate_models_cache(delete_disk=False)
|
||||
cfg.reload_config()
|
||||
yield SimpleNamespace(cfg=cfg, profiles=profiles, demo_home=demo_home, cache=tmp_path / "models_cache.demo.json")
|
||||
profiles.clear_request_profile()
|
||||
cfg.invalidate_models_cache(delete_disk=False)
|
||||
cfg._cfg_cache.clear()
|
||||
cfg._cfg_cache.update(saved_cfg)
|
||||
|
||||
|
||||
def _save_demo_snapshot(env) -> None:
|
||||
env.profiles.set_request_profile("demo")
|
||||
try:
|
||||
env.cfg._save_models_cache_to_disk(_catalog("snapshot-model"))
|
||||
finally:
|
||||
env.profiles.clear_request_profile()
|
||||
assert env.cache.exists()
|
||||
|
||||
|
||||
def _switch_to_demo_and_fetch(env) -> dict:
|
||||
env.profiles.switch_profile("demo", process_wide=False)
|
||||
env.cfg.invalidate_models_cache(delete_disk=False)
|
||||
env.profiles.set_request_profile("demo")
|
||||
try:
|
||||
return env.cfg.get_available_models()
|
||||
finally:
|
||||
env.profiles.clear_request_profile()
|
||||
|
||||
|
||||
def _write_plugin(root: Path, name: str, version: str, *, flat: bool) -> Path:
|
||||
base = root / "plugins" if flat else root / "plugins" / "model-providers"
|
||||
d = base / name
|
||||
d.mkdir(parents=True, exist_ok=True)
|
||||
(d / "plugin.yaml").write_text(
|
||||
f"name: {name}\nkind: model-provider\nversion: {version}\n", encoding="utf-8"
|
||||
)
|
||||
return d
|
||||
|
||||
|
||||
def test_unchanged_sources_keep_serving_the_retained_snapshot(two_profiles):
|
||||
"""Control: with no source change the switch reuses the snapshot (the speed-up)."""
|
||||
_save_demo_snapshot(two_profiles)
|
||||
assert _switch_to_demo_and_fetch(two_profiles)["default_model"] == "snapshot-model"
|
||||
|
||||
|
||||
def test_switch_after_adding_env_key_rejects_snapshot(two_profiles):
|
||||
_save_demo_snapshot(two_profiles)
|
||||
(two_profiles.demo_home / ".env").write_text("DEEPSEEK_API_KEY=sk-new\n", encoding="utf-8")
|
||||
assert _switch_to_demo_and_fetch(two_profiles)["default_model"] == "fresh-model"
|
||||
|
||||
|
||||
def test_switch_after_removing_env_key_rejects_snapshot(two_profiles):
|
||||
env_file = two_profiles.demo_home / ".env"
|
||||
env_file.write_text("DEEPSEEK_API_KEY=sk-old\nOTHER=1\n", encoding="utf-8")
|
||||
_save_demo_snapshot(two_profiles)
|
||||
env_file.write_text("OTHER=1\n", encoding="utf-8")
|
||||
assert _switch_to_demo_and_fetch(two_profiles)["default_model"] == "fresh-model"
|
||||
|
||||
|
||||
def test_switch_after_export_prefix_rejects_snapshot(two_profiles):
|
||||
"""`export KEY=` is key `export KEY` to the profile .env loaders, so the provider key is gone."""
|
||||
env_file = two_profiles.demo_home / ".env"
|
||||
env_file.write_text("DEEPSEEK_API_KEY=sk-old\n", encoding="utf-8")
|
||||
_save_demo_snapshot(two_profiles)
|
||||
env_file.write_text("export DEEPSEEK_API_KEY=sk-old\n", encoding="utf-8")
|
||||
assert _switch_to_demo_and_fetch(two_profiles)["default_model"] == "fresh-model"
|
||||
|
||||
|
||||
def test_env_fingerprint_keys_match_provider_env_loader(two_profiles):
|
||||
from api.providers import _load_env_file
|
||||
|
||||
env_file = two_profiles.demo_home / ".env"
|
||||
env_file.write_text("export A=1\nB='x'\nC=\n# D=1\n", encoding="utf-8")
|
||||
loaded = sorted(k for k, v in _load_env_file(env_file).items() if v)
|
||||
assert two_profiles.cfg._models_cache_env_fingerprint(env_file)["present_keys"] == loaded
|
||||
|
||||
|
||||
def test_env_value_rotation_keeps_snapshot_and_fingerprint_has_no_secret(two_profiles):
|
||||
env_file = two_profiles.demo_home / ".env"
|
||||
env_file.write_text("DEEPSEEK_API_KEY=sk-secret-one\n", encoding="utf-8")
|
||||
_save_demo_snapshot(two_profiles)
|
||||
assert "sk-secret-one" not in two_profiles.cache.read_text(encoding="utf-8")
|
||||
env_file.write_text("DEEPSEEK_API_KEY=sk-secret-two\n", encoding="utf-8")
|
||||
assert _switch_to_demo_and_fetch(two_profiles)["default_model"] == "snapshot-model"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("flat", [False, True], ids=["model-providers-dir", "flat-install-dir"])
|
||||
def test_switch_after_installing_plugin_rejects_snapshot(two_profiles, flat):
|
||||
_save_demo_snapshot(two_profiles)
|
||||
_write_plugin(two_profiles.demo_home, "acme", "1.0.0", flat=flat)
|
||||
assert _switch_to_demo_and_fetch(two_profiles)["default_model"] == "fresh-model"
|
||||
|
||||
|
||||
def test_switch_after_removing_plugin_rejects_snapshot(two_profiles):
|
||||
plugin = _write_plugin(two_profiles.demo_home, "acme", "1.0.0", flat=False)
|
||||
_save_demo_snapshot(two_profiles)
|
||||
shutil.rmtree(plugin)
|
||||
assert _switch_to_demo_and_fetch(two_profiles)["default_model"] == "fresh-model"
|
||||
|
||||
|
||||
def test_switch_after_plugin_version_change_rejects_snapshot(two_profiles):
|
||||
_write_plugin(two_profiles.demo_home, "acme", "1.0.0", flat=True)
|
||||
_save_demo_snapshot(two_profiles)
|
||||
_write_plugin(two_profiles.demo_home, "acme", "2.0.0", flat=True)
|
||||
assert _switch_to_demo_and_fetch(two_profiles)["default_model"] == "fresh-model"
|
||||
|
||||
|
||||
def test_delete_profile_removes_its_models_cache(two_profiles, monkeypatch):
|
||||
import api.profiles as profiles
|
||||
|
||||
monkeypatch.setattr(profiles, "_is_isolated_profile_mode", lambda: False)
|
||||
monkeypatch.setitem(sys.modules, "hermes_cli.profiles", None)
|
||||
_save_demo_snapshot(two_profiles)
|
||||
profiles.delete_profile_api("demo")
|
||||
assert not two_profiles.demo_home.exists()
|
||||
assert not two_profiles.cache.exists()
|
||||
|
||||
|
||||
def test_recreate_same_name_profile_does_not_resurrect_old_catalog(two_profiles, monkeypatch):
|
||||
import api.profiles as profiles
|
||||
|
||||
monkeypatch.setattr(profiles, "_is_isolated_profile_mode", lambda: False)
|
||||
monkeypatch.setitem(sys.modules, "hermes_cli.profiles", None)
|
||||
monkeypatch.setattr(profiles, "list_profiles_api", lambda: [])
|
||||
_save_demo_snapshot(two_profiles)
|
||||
stale = two_profiles.cache.read_text(encoding="utf-8")
|
||||
profiles.delete_profile_api("demo")
|
||||
# A snapshot left behind by an older build or another process.
|
||||
two_profiles.cache.write_text(stale, encoding="utf-8")
|
||||
profiles.create_profile_api("demo")
|
||||
assert not two_profiles.cache.exists()
|
||||
(two_profiles.demo_home / "config.yaml").write_text("model:\n default: demo-model\n", encoding="utf-8")
|
||||
os.utime(two_profiles.demo_home / "config.yaml", (1_000_000, 1_000_000))
|
||||
assert _switch_to_demo_and_fetch(two_profiles)["default_model"] == "fresh-model"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("flat", [False, True], ids=["model-providers-dir", "flat-install-dir"])
|
||||
def test_switch_after_plugin_code_change_without_version_bump_rejects_snapshot(two_profiles, flat):
|
||||
"""fallback_models lives in the plugin's code, which the loader imports; the version may not move."""
|
||||
plugin = _write_plugin(two_profiles.demo_home, "acme", "1.0.0", flat=flat)
|
||||
init = plugin / "__init__.py"
|
||||
init.write_text("FALLBACK_MODELS = ('acme-1',)\n", encoding="utf-8")
|
||||
os.utime(init, (1_000_000, 1_000_000))
|
||||
_save_demo_snapshot(two_profiles)
|
||||
init.write_text("FALLBACK_MODELS = ('acme-1', 'acme-2')\n", encoding="utf-8")
|
||||
assert _switch_to_demo_and_fetch(two_profiles)["default_model"] == "fresh-model"
|
||||
|
||||
|
||||
def test_switch_after_plugin_submodule_change_rejects_snapshot(two_profiles):
|
||||
plugin = _write_plugin(two_profiles.demo_home, "acme", "1.0.0", flat=False)
|
||||
(plugin / "__init__.py").write_text("from .models import FALLBACK_MODELS\n", encoding="utf-8")
|
||||
sub = plugin / "models.py"
|
||||
sub.write_text("FALLBACK_MODELS = ('acme-1',)\n", encoding="utf-8")
|
||||
os.utime(sub, (1_000_000, 1_000_000))
|
||||
_save_demo_snapshot(two_profiles)
|
||||
sub.write_text("FALLBACK_MODELS = ('acme-1', 'acme-2')\n", encoding="utf-8")
|
||||
assert _switch_to_demo_and_fetch(two_profiles)["default_model"] == "fresh-model"
|
||||
|
||||
|
||||
def test_plugin_bytecode_cache_does_not_churn_fingerprint(two_profiles):
|
||||
plugin = _write_plugin(two_profiles.demo_home, "acme", "1.0.0", flat=False)
|
||||
(plugin / "__init__.py").write_text("X = 1\n", encoding="utf-8")
|
||||
before = two_profiles.cfg._models_cache_plugin_fingerprint(two_profiles.demo_home)
|
||||
(plugin / "__pycache__").mkdir()
|
||||
(plugin / "__pycache__" / "__init__.cpython-311.pyc").write_bytes(b"x")
|
||||
assert two_profiles.cfg._models_cache_plugin_fingerprint(two_profiles.demo_home) == before
|
||||
@@ -1,188 +1,174 @@
|
||||
"""Profile switch must not discard the per-profile models disk cache.
|
||||
|
||||
``POST /api/profile/switch`` calls ``invalidate_models_cache()`` so the next
|
||||
``/api/models`` re-resolves the new profile's catalog (#1200). That helper also
|
||||
deleted the on-disk ``models_cache.<profile>.json`` — a test-isolation
|
||||
behaviour — which forced a full cold rebuild (live provider ``fetch_models``
|
||||
HTTPS calls, ~490k deepcopy calls) on every switch even though no source had
|
||||
changed. The disk cache is keyed per profile and rejected on read when
|
||||
``_models_cache_source_fingerprint()`` differs; the source changes that must
|
||||
still force a rebuild are covered in test_profile_switch_models_cache_source_axes.py.
|
||||
|
||||
The switch route now uses ``invalidate_models_cache(delete_disk=False)``.
|
||||
"""
|
||||
"""/api/profile/switch keeps the target profile's models disk snapshot only while its sources are unchanged."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from urllib.parse import urlparse
|
||||
import os
|
||||
import shutil
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
import api.config as cfg
|
||||
import api.profiles as profiles
|
||||
|
||||
|
||||
def _catalog(label: str) -> dict:
|
||||
return {
|
||||
"active_provider": "openai",
|
||||
"default_model": label,
|
||||
"configured_model_badges": {},
|
||||
"groups": [
|
||||
{
|
||||
"provider": "OpenAI",
|
||||
"provider_id": "openai",
|
||||
"models": [{"id": label, "label": label, "supports_fast_tier": False}],
|
||||
}
|
||||
],
|
||||
"aliases": {},
|
||||
}
|
||||
group = {"provider": "OpenAI", "provider_id": "openai", "models": [{"id": label, "label": label, "supports_fast_tier": False}]}
|
||||
return {"active_provider": "openai", "default_model": label, "configured_model_badges": {}, "groups": [group], "aliases": {}}
|
||||
|
||||
|
||||
def _prime_memory_cache(monkeypatch, catalog: dict):
|
||||
import api.config as cfg
|
||||
|
||||
monkeypatch.setattr(cfg, "_available_models_cache", catalog, raising=False)
|
||||
monkeypatch.setattr(cfg, "_available_models_cache_ts", 1.0, raising=False)
|
||||
monkeypatch.setattr(cfg, "_available_models_cache_source_fingerprint", {"p": "x"}, raising=False)
|
||||
monkeypatch.setattr(cfg, "_cache_build_in_progress", False, raising=False)
|
||||
|
||||
|
||||
def test_invalidate_keeps_disk_cache_when_delete_disk_false(tmp_path, monkeypatch):
|
||||
import api.config as cfg
|
||||
|
||||
cache_path = tmp_path / "models_cache.profile.json"
|
||||
cache_path.write_text("{}", encoding="utf-8")
|
||||
monkeypatch.setattr(cfg, "_get_models_cache_path", lambda: cache_path)
|
||||
_prime_memory_cache(monkeypatch, _catalog("old-profile-model"))
|
||||
|
||||
cfg.invalidate_models_cache(delete_disk=False)
|
||||
|
||||
# In-memory snapshot dropped so the next request re-resolves the profile...
|
||||
assert cfg._available_models_cache is None
|
||||
assert cfg._available_models_cache_source_fingerprint is None
|
||||
assert cfg._models_cache_provenance is None
|
||||
# ...but the fingerprint-guarded disk cache survives.
|
||||
assert cache_path.exists(), "delete_disk=False must not unlink the per-profile disk cache"
|
||||
|
||||
|
||||
def test_invalidate_default_still_deletes_disk_cache(tmp_path, monkeypatch):
|
||||
"""Test-isolation contract is unchanged for every existing caller."""
|
||||
import api.config as cfg
|
||||
|
||||
cache_path = tmp_path / "models_cache.profile.json"
|
||||
cache_path.write_text("{}", encoding="utf-8")
|
||||
monkeypatch.setattr(cfg, "_get_models_cache_path", lambda: cache_path)
|
||||
_prime_memory_cache(monkeypatch, _catalog("old-profile-model"))
|
||||
|
||||
cfg.invalidate_models_cache()
|
||||
|
||||
assert cfg._available_models_cache is None
|
||||
assert not cache_path.exists()
|
||||
|
||||
|
||||
def test_memory_only_invalidate_serves_next_request_from_disk_without_rebuild(tmp_path, monkeypatch):
|
||||
"""After a memory-only drop, get_available_models() must reload the disk
|
||||
snapshot instead of running the live rebuild."""
|
||||
import api.config as cfg
|
||||
|
||||
cache_path = tmp_path / "models_cache.profile.json"
|
||||
cache_path.write_text("{}", encoding="utf-8")
|
||||
disk_catalog = _catalog("disk-model")
|
||||
monkeypatch.setattr(cfg, "_get_models_cache_path", lambda: cache_path)
|
||||
monkeypatch.setattr(cfg, "_load_models_cache_from_disk", lambda: disk_catalog if cache_path.exists() else None)
|
||||
@pytest.fixture
|
||||
def env(tmp_path: Path, monkeypatch):
|
||||
monkeypatch.delenv("HERMES_CONFIG_PATH", raising=False)
|
||||
default_home = tmp_path / ".hermes"
|
||||
demo_home = default_home / "profiles" / "demo"
|
||||
demo_home.mkdir(parents=True)
|
||||
(default_home / "config.yaml").write_text("model:\n default: default-model\n", encoding="utf-8")
|
||||
_write_demo_config(demo_home)
|
||||
monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", default_home)
|
||||
monkeypatch.setattr(profiles, "_active_profile", "default")
|
||||
monkeypatch.setattr(profiles, "_is_isolated_profile_mode", lambda: False)
|
||||
monkeypatch.setattr(profiles._tls, "profile", None, raising=False)
|
||||
monkeypatch.setattr(cfg, "_models_cache_path", tmp_path / "models_cache.json")
|
||||
monkeypatch.setattr(cfg, "_load_stale_models_cache_from_disk", lambda: None)
|
||||
monkeypatch.setattr(cfg, "_models_cache_source_fingerprint", lambda: {"profile": "demo"})
|
||||
# Pin config mtime tracking so an unrelated config change (or leaked
|
||||
# state from an earlier test) cannot force the reload branch.
|
||||
cfg_file = tmp_path / "config.yaml"
|
||||
cfg_file.write_text("model: {}\n", encoding="utf-8")
|
||||
monkeypatch.setattr(cfg, "_get_config_path", lambda: cfg_file)
|
||||
monkeypatch.setattr(cfg, "_cfg_path", cfg_file, raising=False)
|
||||
monkeypatch.setattr(cfg, "_cfg_mtime", cfg_file.stat().st_mtime, raising=False)
|
||||
|
||||
def _unexpected_rebuild(*_a, **_kw):
|
||||
raise AssertionError("profile switch must not trigger a live models rebuild")
|
||||
|
||||
monkeypatch.setattr(cfg, "_invoke_models_rebuild", _unexpected_rebuild)
|
||||
_prime_memory_cache(monkeypatch, _catalog("stale-memory-model"))
|
||||
|
||||
monkeypatch.setattr(cfg, "_invoke_models_rebuild", lambda _builder: _catalog("fresh-model"))
|
||||
for attr in ("_cfg_mtime", "_cfg_path", "_cfg_fingerprint"):
|
||||
monkeypatch.setattr(cfg, attr, getattr(cfg, attr), raising=False)
|
||||
saved_cfg = dict(cfg._cfg_cache)
|
||||
cfg.invalidate_models_cache(delete_disk=False)
|
||||
result = cfg.get_available_models()
|
||||
|
||||
assert result["default_model"] == "disk-model"
|
||||
cfg.reload_config() # the default profile's config is loaded, as at server boot
|
||||
yield SimpleNamespace(home=demo_home, cache=tmp_path / "models_cache.demo.json")
|
||||
profiles.clear_request_profile()
|
||||
cfg.invalidate_models_cache(delete_disk=False)
|
||||
cfg._cfg_cache.clear()
|
||||
cfg._cfg_cache.update(saved_cfg)
|
||||
|
||||
|
||||
class _FakeHandler:
|
||||
def __init__(self):
|
||||
self.status = None
|
||||
self.sent_headers: list[tuple[str, str]] = []
|
||||
self.body = bytearray()
|
||||
self.wfile = self
|
||||
self.headers = {}
|
||||
|
||||
def send_response(self, code):
|
||||
self.status = code
|
||||
|
||||
def send_header(self, key, value):
|
||||
self.sent_headers.append((key, value))
|
||||
|
||||
def end_headers(self):
|
||||
pass
|
||||
|
||||
def write(self, data):
|
||||
self.body.extend(data if isinstance(data, (bytes, bytearray)) else data.encode("utf-8"))
|
||||
|
||||
def get_json(self):
|
||||
return json.loads(self.body.decode("utf-8"))
|
||||
def _write_demo_config(home: Path, default: str = "demo-model", mtime: int = 1_000_000) -> None:
|
||||
(home / "config.yaml").write_text(f"model:\n default: {default}\n", encoding="utf-8")
|
||||
os.utime(home / "config.yaml", (mtime, mtime))
|
||||
|
||||
|
||||
def _drive_profile_switch(monkeypatch, name: str = "demo") -> _FakeHandler:
|
||||
"""Execute POST /api/profile/switch through routes.handle_post with the
|
||||
auth / CSRF / profile-fs / watcher collaborators stubbed out, leaving
|
||||
``config.invalidate_models_cache`` to whatever the caller installed."""
|
||||
from api import gateway_watcher, profiles, routes
|
||||
|
||||
for mod in ("api.auth", "api.helpers", "api.routes"):
|
||||
monkeypatch.setattr(f"{mod}.is_auth_enabled", lambda: False, raising=False)
|
||||
monkeypatch.setattr(routes, "_check_csrf", lambda handler: True)
|
||||
monkeypatch.setattr(routes, "read_body", lambda handler: {"name": name})
|
||||
monkeypatch.setattr(profiles, "_validate_profile_name", lambda _name: None)
|
||||
monkeypatch.setattr(profiles, "switch_profile", lambda _name, process_wide=False: {"ok": True, "name": _name})
|
||||
monkeypatch.setattr(gateway_watcher, "restart_watcher_for_profile", lambda _name: None)
|
||||
|
||||
handler = _FakeHandler()
|
||||
routes.handle_post(handler, urlparse("/api/profile/switch"))
|
||||
return handler
|
||||
def _save_demo_snapshot(env) -> None:
|
||||
profiles.set_request_profile("demo")
|
||||
try:
|
||||
cfg._save_models_cache_to_disk(_catalog("snapshot-model"))
|
||||
finally:
|
||||
profiles.clear_request_profile()
|
||||
assert env.cache.exists()
|
||||
|
||||
|
||||
def test_profile_switch_route_calls_invalidate_with_delete_disk_false(monkeypatch):
|
||||
"""Executed-route guard: the live switch handler must pass
|
||||
``delete_disk=False`` (not rely on the default, which unlinks the disk
|
||||
cache and forces a cold rebuild)."""
|
||||
from api import config
|
||||
|
||||
calls: list[tuple[tuple, dict]] = []
|
||||
monkeypatch.setattr(config, "invalidate_models_cache", lambda *a, **kw: calls.append((a, kw)))
|
||||
|
||||
handler = _drive_profile_switch(monkeypatch)
|
||||
|
||||
assert handler.status == 200, handler.body.decode("utf-8", "replace")
|
||||
assert handler.get_json() == {"ok": True, "name": "demo"}
|
||||
assert calls == [((), {"delete_disk": False})], (
|
||||
"/api/profile/switch must call invalidate_models_cache(delete_disk=False) exactly once; "
|
||||
f"got {calls!r}"
|
||||
)
|
||||
def _fetch_as_demo() -> dict:
|
||||
profiles.set_request_profile("demo")
|
||||
try:
|
||||
return cfg.get_available_models()
|
||||
finally:
|
||||
profiles.clear_request_profile()
|
||||
|
||||
|
||||
def test_profile_switch_route_leaves_disk_cache_file_in_place(tmp_path, monkeypatch):
|
||||
"""End-to-end through the real ``invalidate_models_cache``: after the
|
||||
executed switch route, the in-memory catalog is dropped but the
|
||||
per-profile disk snapshot still exists."""
|
||||
from api import config as cfg
|
||||
def _switch_to_demo_and_fetch() -> str:
|
||||
profiles.switch_profile("demo", process_wide=False)
|
||||
cfg.invalidate_models_cache(delete_disk=False) # as /api/profile/switch does
|
||||
return _fetch_as_demo()["default_model"]
|
||||
|
||||
cache_path = tmp_path / "models_cache.profile.json"
|
||||
cache_path.write_text("{}", encoding="utf-8")
|
||||
monkeypatch.setattr(cfg, "_get_models_cache_path", lambda: cache_path)
|
||||
_prime_memory_cache(monkeypatch, _catalog("old-profile-model"))
|
||||
|
||||
handler = _drive_profile_switch(monkeypatch)
|
||||
def _write_plugin(home: Path, version: str = "1.0.0", *, flat: bool = False, code: str | None = None, sub: str | None = None) -> Path:
|
||||
d = home / "plugins" / ("" if flat else "model-providers") / "acme"
|
||||
d.mkdir(parents=True, exist_ok=True)
|
||||
(d / "plugin.yaml").write_text(f"name: acme\nkind: model-provider\nversion: {version}\n", encoding="utf-8")
|
||||
for name, text in (("__init__.py", code), ("models.py", sub)):
|
||||
if text is not None:
|
||||
(d / name).write_text(text, encoding="utf-8")
|
||||
os.utime(d / name, (1_000_000, 1_000_000))
|
||||
return d
|
||||
|
||||
assert handler.status == 200, handler.body.decode("utf-8", "replace")
|
||||
assert cfg._available_models_cache is None, "switch must still drop the in-memory catalog (#1200)"
|
||||
assert cache_path.exists(), "switch must not unlink the per-profile disk cache"
|
||||
|
||||
def _env(text: str):
|
||||
return lambda home: (home / ".env").write_text(text, encoding="utf-8")
|
||||
|
||||
|
||||
# (id, source state before the snapshot is saved, change made after it, model served after the switch)
|
||||
_CASES = [
|
||||
("unchanged", None, None, "snapshot-model"),
|
||||
("env-value-rotated", _env("DEEPSEEK_API_KEY=sk-one\n"), _env("DEEPSEEK_API_KEY=sk-two\n"), "snapshot-model"),
|
||||
("env-key-added", None, _env("DEEPSEEK_API_KEY=sk-new\n"), "fresh-model"),
|
||||
("env-key-removed", _env("DEEPSEEK_API_KEY=sk-old\nOTHER=1\n"), _env("OTHER=1\n"), "fresh-model"),
|
||||
# provider detection reads `export KEY=` as key `export KEY`, so the credential is gone
|
||||
("env-export-prefix", _env("DEEPSEEK_API_KEY=sk-old\n"), _env("export DEEPSEEK_API_KEY=sk-old\n"), "fresh-model"),
|
||||
("plugin-installed", None, lambda h: _write_plugin(h), "fresh-model"),
|
||||
("plugin-installed-flat", None, lambda h: _write_plugin(h, flat=True), "fresh-model"),
|
||||
("plugin-removed", lambda h: _write_plugin(h), lambda h: shutil.rmtree(h / "plugins"), "fresh-model"),
|
||||
("plugin-version-bumped", lambda h: _write_plugin(h, flat=True), lambda h: _write_plugin(h, "2.0.0", flat=True), "fresh-model"),
|
||||
("plugin-code-edited", lambda h: _write_plugin(h, code="M = (1,)\n"), lambda h: (h / "plugins/model-providers/acme/__init__.py").write_text("M = (1, 2)\n"), "fresh-model"),
|
||||
("plugin-code-edited-flat", lambda h: _write_plugin(h, flat=True, code="M = (1,)\n"), lambda h: (h / "plugins/acme/__init__.py").write_text("M = (1, 2)\n"), "fresh-model"),
|
||||
("plugin-submodule-edited", lambda h: _write_plugin(h, code="from .models import M\n", sub="M = (1,)\n"), lambda h: (h / "plugins/model-providers/acme/models.py").write_text("M = (1, 2)\n"), "fresh-model"),
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("before, change, expected", [c[1:] for c in _CASES], ids=[c[0] for c in _CASES])
|
||||
def test_switch_serves_snapshot_only_while_sources_are_unchanged(env, before, change, expected):
|
||||
if before:
|
||||
before(env.home)
|
||||
_save_demo_snapshot(env)
|
||||
if change:
|
||||
change(env.home)
|
||||
assert _switch_to_demo_and_fetch() == expected
|
||||
assert "sk-" not in env.cache.read_text(encoding="utf-8") # the fingerprint never records a secret
|
||||
|
||||
|
||||
def test_same_profile_config_edit_still_deletes_disk_cache(env, monkeypatch):
|
||||
_save_demo_snapshot(env)
|
||||
assert _switch_to_demo_and_fetch() == "snapshot-model"
|
||||
_write_demo_config(env.home, "demo-model-2", 2_000_000)
|
||||
cfg.invalidate_models_cache(delete_disk=False)
|
||||
cache_at_rebuild = []
|
||||
monkeypatch.setattr(cfg, "_invoke_models_rebuild", lambda _b: cache_at_rebuild.append(env.cache.exists()) or _catalog("x"))
|
||||
_fetch_as_demo()
|
||||
assert cache_at_rebuild == [False]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("delete_disk", [True, False])
|
||||
def test_invalidate_drops_memory_and_deletes_disk_only_by_default(env, delete_disk):
|
||||
_save_demo_snapshot(env)
|
||||
_fetch_as_demo()
|
||||
profiles.set_request_profile("demo")
|
||||
try:
|
||||
cfg.invalidate_models_cache(delete_disk=delete_disk)
|
||||
finally:
|
||||
profiles.clear_request_profile()
|
||||
assert cfg._available_models_cache is None and cfg._models_cache_provenance is None
|
||||
assert env.cache.exists() is not delete_disk
|
||||
|
||||
|
||||
def test_env_fingerprint_keys_match_provider_env_loader(env):
|
||||
from api.providers import _load_env_file
|
||||
|
||||
_env("export A=1\nB='x'\nC=\n# D=1\n")(env.home)
|
||||
loaded = sorted(k for k, v in _load_env_file(env.home / ".env").items() if v)
|
||||
assert cfg._models_cache_env_fingerprint(env.home / ".env") == loaded
|
||||
|
||||
|
||||
def test_plugin_bytecode_cache_does_not_churn_fingerprint(env):
|
||||
plugin = _write_plugin(env.home, code="X = 1\n")
|
||||
before = cfg._models_cache_plugin_fingerprint(env.home)
|
||||
(plugin / "__pycache__").mkdir()
|
||||
(plugin / "__pycache__" / "__init__.cpython-311.pyc").write_bytes(b"x")
|
||||
assert cfg._models_cache_plugin_fingerprint(env.home) == before
|
||||
|
||||
|
||||
@pytest.mark.parametrize("recreate", [False, True])
|
||||
def test_delete_and_recreate_profile_drop_its_models_cache(env, monkeypatch, recreate):
|
||||
monkeypatch.setitem(sys.modules, "hermes_cli.profiles", None)
|
||||
monkeypatch.setattr(profiles, "list_profiles_api", lambda: [])
|
||||
_save_demo_snapshot(env)
|
||||
stale = env.cache.read_text(encoding="utf-8")
|
||||
profiles.delete_profile_api("demo")
|
||||
assert not env.home.exists() and not env.cache.exists()
|
||||
if recreate:
|
||||
env.cache.write_text(stale, encoding="utf-8") # left behind by an older build or another process
|
||||
profiles.create_profile_api("demo")
|
||||
assert not env.cache.exists()
|
||||
_write_demo_config(env.home)
|
||||
assert _switch_to_demo_and_fetch() == "fresh-model"
|
||||
@@ -1,156 +0,0 @@
|
||||
"""Greptile review thread on #7632 (docs/architecture/models-cache-invalidation.md):
|
||||
|
||||
A per-client ``POST /api/profile/switch`` (``process_wide=False``) skips
|
||||
``reload_config()``, so the process-global ``_cfg_mtime`` still holds the
|
||||
*previous* profile's nonzero mtime. The next ``/api/models`` for the new
|
||||
profile sees a different config mtime, takes the config-reload branch, and
|
||||
``_refresh_config_cache()`` used to treat ``_old_cfg_mtime != 0.0`` as "config
|
||||
edited" and unlink the disk snapshot — the *target* profile's
|
||||
``models_cache.<name>.json`` — defeating ``delete_disk=False`` on the very next
|
||||
request.
|
||||
|
||||
``_refresh_config_cache()`` now only deletes the disk snapshot when the same
|
||||
config.yaml path was already loaded (a real edit of the active profile's
|
||||
config). These tests drive real profile homes, real config files and real disk
|
||||
cache files through ``switch_profile()`` + ``get_available_models()``.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
def _catalog(label: str) -> dict:
|
||||
return {
|
||||
"active_provider": "openai",
|
||||
"default_model": label,
|
||||
"configured_model_badges": {},
|
||||
"groups": [
|
||||
{
|
||||
"provider": "OpenAI",
|
||||
"provider_id": "openai",
|
||||
"models": [{"id": label, "label": label, "supports_fast_tier": False}],
|
||||
}
|
||||
],
|
||||
"aliases": {},
|
||||
}
|
||||
|
||||
|
||||
def _setup_two_profiles(tmp_path: Path, monkeypatch):
|
||||
"""Real default + named profile homes, distinct config mtimes, one disk
|
||||
cache file per profile. Returns (default_cache, demo_cache, demo_config)."""
|
||||
import api.config as cfg
|
||||
import api.profiles as profiles
|
||||
|
||||
monkeypatch.delenv("HERMES_CONFIG_PATH", raising=False)
|
||||
default_home = tmp_path / ".hermes"
|
||||
default_home.mkdir()
|
||||
(default_home / "config.yaml").write_text("model:\n default: default-model\n", encoding="utf-8")
|
||||
demo_home = default_home / "profiles" / "demo"
|
||||
demo_home.mkdir(parents=True)
|
||||
demo_config = demo_home / "config.yaml"
|
||||
demo_config.write_text("model:\n default: demo-model\n", encoding="utf-8")
|
||||
os.utime(demo_config, (1_000_000, 1_000_000)) # guarantee a different mtime
|
||||
|
||||
monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", default_home)
|
||||
monkeypatch.setattr(profiles, "_active_profile", "default")
|
||||
monkeypatch.setattr(profiles._tls, "profile", None, raising=False)
|
||||
|
||||
default_cache = tmp_path / "models_cache.json"
|
||||
demo_cache = tmp_path / "models_cache.demo.json"
|
||||
default_cache.write_text("{}", encoding="utf-8")
|
||||
demo_cache.write_text("{}", encoding="utf-8")
|
||||
monkeypatch.setattr(cfg, "_models_cache_path", default_cache)
|
||||
|
||||
# Serve the disk snapshot; never allow a live rebuild.
|
||||
monkeypatch.setattr(cfg, "_load_models_cache_from_disk", lambda: _catalog("disk-model"))
|
||||
monkeypatch.setattr(cfg, "_load_stale_models_cache_from_disk", lambda: None)
|
||||
|
||||
def _unexpected_rebuild(*_a, **_kw):
|
||||
raise AssertionError("must not trigger a live models rebuild")
|
||||
|
||||
monkeypatch.setattr(cfg, "_invoke_models_rebuild", _unexpected_rebuild)
|
||||
|
||||
# Snapshot/restore the process-global config state touched by reload_config().
|
||||
for attr in ("_cfg_mtime", "_cfg_path", "_cfg_fingerprint"):
|
||||
monkeypatch.setattr(cfg, attr, getattr(cfg, attr), raising=False)
|
||||
saved_cache = dict(cfg._cfg_cache)
|
||||
|
||||
def _restore():
|
||||
cfg._cfg_cache.clear()
|
||||
cfg._cfg_cache.update(saved_cache)
|
||||
|
||||
monkeypatch.setattr(cfg, "_available_models_cache", None, raising=False)
|
||||
monkeypatch.setattr(cfg, "_available_models_cache_source_fingerprint", None, raising=False)
|
||||
monkeypatch.setattr(cfg, "_cache_build_in_progress", False, raising=False)
|
||||
|
||||
# Load the default profile's config as the server would have at boot.
|
||||
cfg.reload_config()
|
||||
assert cfg._cfg_path == default_home / "config.yaml"
|
||||
assert cfg._cfg_mtime != 0.0
|
||||
|
||||
return default_cache, demo_cache, demo_config, _restore
|
||||
|
||||
|
||||
def test_first_models_request_after_per_client_switch_keeps_target_disk_cache(tmp_path, monkeypatch):
|
||||
import api.config as cfg
|
||||
import api.profiles as profiles
|
||||
|
||||
default_cache, demo_cache, _demo_config, restore = _setup_two_profiles(tmp_path, monkeypatch)
|
||||
try:
|
||||
# Warm the default profile: /api/models with the default cookie.
|
||||
cfg.get_available_models()
|
||||
assert default_cache.exists()
|
||||
|
||||
# Per-client switch, exactly as the route does it.
|
||||
profiles.switch_profile("demo", process_wide=False)
|
||||
cfg.invalidate_models_cache(delete_disk=False)
|
||||
assert demo_cache.exists()
|
||||
prev_mtime = cfg._cfg_mtime
|
||||
assert prev_mtime != 0.0, "per-client switch leaves the previous profile's mtime in place"
|
||||
|
||||
# First /api/models from that client (cookie → thread-local profile).
|
||||
profiles.set_request_profile("demo")
|
||||
try:
|
||||
result = cfg.get_available_models()
|
||||
finally:
|
||||
profiles.clear_request_profile()
|
||||
|
||||
assert result["default_model"] == "disk-model"
|
||||
assert cfg._cfg_path == _demo_config, "config-reload branch must have re-pointed to the demo config"
|
||||
assert cfg._cfg_mtime == _demo_config.stat().st_mtime
|
||||
assert demo_cache.exists(), (
|
||||
"first /api/models after a per-client profile switch must not unlink the "
|
||||
"target profile's models_cache.demo.json"
|
||||
)
|
||||
assert default_cache.exists()
|
||||
finally:
|
||||
restore()
|
||||
|
||||
|
||||
def test_same_profile_config_edit_still_deletes_disk_cache(tmp_path, monkeypatch):
|
||||
"""The path guard must not weaken the real-edit contract: an mtime change
|
||||
on the *same* config.yaml still drops the disk snapshot."""
|
||||
import api.config as cfg
|
||||
import api.profiles as profiles
|
||||
|
||||
default_cache, demo_cache, demo_config, restore = _setup_two_profiles(tmp_path, monkeypatch)
|
||||
try:
|
||||
profiles.switch_profile("demo", process_wide=False)
|
||||
cfg.invalidate_models_cache(delete_disk=False)
|
||||
profiles.set_request_profile("demo")
|
||||
try:
|
||||
cfg.get_available_models()
|
||||
assert demo_cache.exists()
|
||||
# Edit demo's config.yaml → mtime moves on the same path.
|
||||
demo_config.write_text("model:\n default: demo-model-2\n", encoding="utf-8")
|
||||
os.utime(demo_config, (2_000_000, 2_000_000))
|
||||
cfg.invalidate_models_cache(delete_disk=False)
|
||||
cfg.get_available_models()
|
||||
finally:
|
||||
profiles.clear_request_profile()
|
||||
assert not demo_cache.exists(), "editing the active profile's config.yaml must still delete its disk cache"
|
||||
assert default_cache.exists()
|
||||
finally:
|
||||
restore()
|
||||
Reference in new issue
Block a user