diff --git a/api/oauth.py b/api/oauth.py index b9f6495be..13d9a6937 100644 --- a/api/oauth.py +++ b/api/oauth.py @@ -218,6 +218,23 @@ def _now_iso() -> str: return datetime.now(timezone.utc).isoformat().replace("+00:00", "Z") +def _invalidate_provider_state_caches(provider: str) -> None: + """Invalidate caches derived from provider credential state.""" + try: + from api.config import invalidate_credential_pool_cache + + invalidate_credential_pool_cache(provider) + except Exception: + logger.debug("Failed to invalidate %s credential cache", provider, exc_info=True) + + try: + from api.providers import invalidate_providers_cache + + invalidate_providers_cache() + except Exception: + logger.debug("Failed to invalidate providers cache after %s credential change", provider, exc_info=True) + + def _persist_codex_credentials(hermes_home: Path, token_data: dict[str, Any]) -> Path: """Persist Codex OAuth credentials to active-profile auth.json.""" access_token = str(token_data.get("access_token") or "").strip() @@ -277,12 +294,7 @@ def _persist_codex_credentials(hermes_home: Path, token_data: dict[str, Any]) -> auth["updated_at"] = now path = _write_auth_json(auth, auth_path) - try: - from api.config import invalidate_credential_pool_cache - - invalidate_credential_pool_cache("openai-codex") - except Exception: - logger.debug("Failed to invalidate openai-codex credential cache", exc_info=True) + _invalidate_provider_state_caches("openai-codex") return path @@ -389,12 +401,7 @@ def _link_anthropic_credentials(hermes_home: Path) -> None: }) auth["updated_at"] = now _write_auth_json(auth, auth_path) - - try: - from api.config import invalidate_credential_pool_cache - invalidate_credential_pool_cache("anthropic") - except Exception: - logger.debug("Failed to invalidate anthropic credential cache", exc_info=True) + _invalidate_provider_state_caches("anthropic") def _anthropic_public_start_payload(flow_id: str, flow: dict[str, Any]) -> dict[str, Any]: @@ -515,11 +522,7 @@ def _remove_anthropic_link_marker(hermes_home: Path) -> None: pool.pop("anthropic", None) auth["updated_at"] = _now_iso() _write_auth_json(auth, auth_path) - try: - from api.config import invalidate_credential_pool_cache - invalidate_credential_pool_cache("anthropic") - except Exception: - logger.debug("Failed to invalidate anthropic credential cache", exc_info=True) + _invalidate_provider_state_caches("anthropic") # ── Codex protocol ────────────────────────────────────────────────────────── diff --git a/api/providers.py b/api/providers.py index 8dfe63148..98bb2960b 100644 --- a/api/providers.py +++ b/api/providers.py @@ -9,6 +9,7 @@ from __future__ import annotations import atexit import base64 +import copy import hashlib import json import logging @@ -78,6 +79,7 @@ _OPENROUTER_KEY_URL = "https://openrouter.ai/api/v1/key" _PROVIDER_QUOTA_TIMEOUT_SECONDS = 3.0 _ACCOUNT_USAGE_SUBPROCESS_TIMEOUT_SECONDS = 35.0 _ACCOUNT_USAGE_CACHE_TTL_SECONDS = 45.0 +_PROVIDERS_CACHE_TTL_SECONDS = 30.0 _ACCOUNT_USAGE_CACHE_MAX_ENTRIES = 64 _ACCOUNT_USAGE_WORKER_IDLE_SECONDS = 5 * 60 _ACCOUNT_USAGE_PROVIDERS = frozenset({"openai-codex", "anthropic"}) @@ -134,6 +136,8 @@ _account_usage_probe_semaphore: threading.BoundedSemaphore | None = None # represented as non-None snapshots and remain cacheable. _account_usage_status_cache: dict[tuple[str, str, str], tuple[float, Any]] = {} _account_usage_status_cache_lock = threading.Lock() +_providers_cache: dict[tuple[Any, ...], tuple[float, dict[str, Any]]] = {} +_providers_cache_lock = threading.Lock() _account_usage_worker_pool: dict[str, list["_AccountUsageProbeWorker"]] = {} _account_usage_worker_pool_lock = threading.Lock() @@ -967,6 +971,74 @@ def _get_hermes_home() -> Path: return Path.home() / ".hermes" +def _providers_file_mtime_ns(path: Path) -> int: + """Best-effort file mtime for providers-cache invalidation.""" + try: + return path.stat().st_mtime_ns + except OSError: + return 0 + + +def _providers_config_fingerprint(cfg: Any) -> str: + """Stable fingerprint for config fields that shape the Providers response.""" + try: + return hashlib.sha256( + json.dumps(cfg, sort_keys=True, default=str, separators=(",", ":")).encode("utf-8") + ).hexdigest() + except Exception: + return repr(cfg) + + +def _providers_cache_key(cfg: Any) -> tuple[Any, ...]: + """Return a profile-scoped cache key for ``get_providers()`` (#6010). + + The endpoint reads provider state from the active Hermes home plus the + current config. Include the home path and the two files users commonly + mutate from Settings so a short TTL never crosses profile boundaries or + masks immediate credential/config changes. + """ + home = _get_hermes_home() + try: + home_key = str(home.resolve()) + except OSError: + home_key = str(home) + return ( + home_key, + _providers_file_mtime_ns(home / ".env"), + _providers_file_mtime_ns(home / "config.yaml"), + _providers_config_fingerprint(cfg), + ) + + +def _get_cached_providers(cache_key: tuple[Any, ...]) -> dict[str, Any] | None: + now = time.monotonic() + with _providers_cache_lock: + cached = _providers_cache.get(cache_key) + if cached is None: + return None + ts, payload = cached + if now - ts >= _PROVIDERS_CACHE_TTL_SECONDS: + _providers_cache.pop(cache_key, None) + return None + return copy.deepcopy(payload) + + +def _store_cached_providers(cache_key: tuple[Any, ...], payload: dict[str, Any]) -> dict[str, Any]: + with _providers_cache_lock: + # Single-entry by design: /api/providers is cacheable only for the + # active profile/config snapshot, so clear older snapshots to avoid + # retaining unbounded provider metadata across profile switches. + _providers_cache.clear() + _providers_cache[cache_key] = (time.monotonic(), copy.deepcopy(payload)) + return payload + + +def invalidate_providers_cache() -> None: + """Clear cached ``GET /api/providers`` responses.""" + with _providers_cache_lock: + _providers_cache.clear() + + def _load_env_file(env_path: Path) -> dict[str, str]: """Read key=value pairs from a .env file.""" values: dict[str, str] = {} @@ -2459,14 +2531,18 @@ def get_providers() -> dict[str, Any]: ``config_yaml``, ``oauth``, ``none``) - ``models``: list of known model IDs for this provider """ - providers = [] - # Collect all known provider IDs from multiple sources known_ids = set(_PROVIDER_DISPLAY.keys()) | set(_PROVIDER_MODELS.keys()) known_ids.update(plugin_model_provider_ids()) # Also detect providers from config.yaml providers section cfg = get_config() + cache_key = _providers_cache_key(cfg) + cached = _get_cached_providers(cache_key) + if cached is not None: + return cached + + providers = [] providers_cfg = cfg.get("providers") or {} if isinstance(providers_cfg, dict): known_ids.update(providers_cfg.keys()) @@ -2783,10 +2859,11 @@ def get_providers() -> dict[str, Any]: return (3, pid) providers.sort(key=_provider_sort_key) - return { + result = { "providers": providers, "active_provider": active_provider, } + return _store_cached_providers(cache_key, result) def set_provider_key(provider_id: str, api_key: str | None) -> dict[str, Any]: @@ -2839,6 +2916,7 @@ def set_provider_key(provider_id: str, api_key: str | None) -> dict[str, Any]: # disrupting active streaming sessions that may be reading config.cfg. invalidate_models_cache() invalidate_account_usage_status_cache(provider_id) + invalidate_providers_cache() return { "ok": True, @@ -2941,5 +3019,6 @@ def _clean_provider_key_from_config(provider_id: str) -> None: # reload_config() also acquires _cfg_lock internally. if changed: reload_config() + invalidate_providers_cache() except Exception: logger.exception("Failed to clean provider key from config.yaml for %s", provider_id) diff --git a/tests/conftest.py b/tests/conftest.py index bee665fca..d0602e278 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -216,6 +216,28 @@ def _reset_password_hash_cache(): _invalidate_password_hash_cache() +@pytest.fixture(autouse=True) +def _invalidate_providers_cache(): + """Clear the /api/providers TTL cache around every test (#6010). + + get_providers() caches its response for 30s keyed on profile home + + .env/config.yaml mtimes + config fingerprint. Tests that monkeypatch auth + state (e.g. test_issue1202_oauth_provider_status) without changing those + key inputs would receive a stale cached result from a sibling test, + breaking test hermeticity in the full sharded suite. Clear before AND after + each test so no cache entry leaks across the isolation boundary. + """ + try: + from api.providers import invalidate_providers_cache + except Exception: + invalidate_providers_cache = None + if invalidate_providers_cache: + invalidate_providers_cache() + yield + if invalidate_providers_cache: + invalidate_providers_cache() + + _MISSING = object() # sentinel: api.profiles module not loaded pre-test diff --git a/tests/test_provider_management.py b/tests/test_provider_management.py index 4c1b5491f..013a8aad4 100644 --- a/tests/test_provider_management.py +++ b/tests/test_provider_management.py @@ -73,6 +73,121 @@ def _install_fake_hermes_cli(monkeypatch): class TestGetProviders: """Unit tests for get_providers() function.""" + def test_reuses_short_ttl_cache_for_same_profile_home(self, monkeypatch, tmp_path): + """Back-to-back provider reads should not re-run expensive probes (#6010).""" + _install_fake_hermes_cli(monkeypatch) + monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path) + + from api import providers as prov + + calls = [] + monkeypatch.setattr(prov, "_PROVIDER_DISPLAY", {"openai": "OpenAI"}) + monkeypatch.setattr(prov, "_PROVIDER_MODELS", {"openai": []}) + monkeypatch.setattr(prov, "_OAUTH_PROVIDERS", frozenset()) + monkeypatch.setattr(prov, "plugin_model_provider_ids", lambda: set()) + monkeypatch.setattr(prov, "get_config", lambda: {"model": {}, "providers": {}}) + + def _counting_has_key(pid): + calls.append(pid) + return False + + monkeypatch.setattr(prov, "_provider_has_key", _counting_has_key) + + try: + first = prov.get_providers() + second = prov.get_providers() + assert first == second + assert calls == ["openai"] + finally: + if hasattr(prov, "invalidate_providers_cache"): + prov.invalidate_providers_cache() + + def test_provider_cache_is_scoped_by_profile_home(self, monkeypatch, tmp_path): + """Provider cache entries must not leak across profile homes (#3957/#6010).""" + _install_fake_hermes_cli(monkeypatch) + home_a = tmp_path / "a" + home_b = tmp_path / "b" + home_a.mkdir() + home_b.mkdir() + active_home = {"path": home_a} + monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: active_home["path"]) + + from api import providers as prov + + monkeypatch.setattr(prov, "_PROVIDER_DISPLAY", {"openai": "OpenAI"}) + monkeypatch.setattr(prov, "_PROVIDER_MODELS", {"openai": []}) + monkeypatch.setattr(prov, "_OAUTH_PROVIDERS", frozenset()) + monkeypatch.setattr(prov, "plugin_model_provider_ids", lambda: set()) + monkeypatch.setattr(prov, "get_config", lambda: {"model": {}, "providers": {}}) + monkeypatch.setattr(prov, "_provider_has_key", lambda _pid: active_home["path"] == home_b) + + try: + first = prov.get_providers() + active_home["path"] = home_b + second = prov.get_providers() + first_openai = next(p for p in first["providers"] if p["id"] == "openai") + second_openai = next(p for p in second["providers"] if p["id"] == "openai") + assert first_openai["has_key"] is False + assert second_openai["has_key"] is True + finally: + if hasattr(prov, "invalidate_providers_cache"): + prov.invalidate_providers_cache() + + def test_set_provider_key_invalidates_providers_cache(self, monkeypatch, tmp_path): + """Saving a key should invalidate the cached Providers response (#6010).""" + _install_fake_hermes_cli(monkeypatch) + monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path) + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + + from api import providers as prov + + key_present = {"value": False} + monkeypatch.setattr(prov, "_PROVIDER_DISPLAY", {"anthropic": "Anthropic"}) + monkeypatch.setattr(prov, "_PROVIDER_MODELS", {"anthropic": []}) + monkeypatch.setattr(prov, "_OAUTH_PROVIDERS", frozenset()) + monkeypatch.setattr(prov, "plugin_model_provider_ids", lambda: set()) + monkeypatch.setattr(prov, "get_config", lambda: {"model": {}, "providers": {}}) + monkeypatch.setattr(prov, "_provider_has_key", lambda _pid: key_present["value"]) + + def _fake_write_env_file(_path, values): + key_present["value"] = bool(values.get("ANTHROPIC_API_KEY")) + + monkeypatch.setattr(prov, "_write_env_file", _fake_write_env_file) + monkeypatch.setattr(prov, "invalidate_models_cache", lambda: None) + monkeypatch.setattr(prov, "invalidate_account_usage_status_cache", lambda _provider_id=None: None) + + try: + before = prov.get_providers() + result = prov.set_provider_key("anthropic", "sk-test-12345678") + after = prov.get_providers() + + before_anthropic = next(p for p in before["providers"] if p["id"] == "anthropic") + after_anthropic = next(p for p in after["providers"] if p["id"] == "anthropic") + assert result["ok"] is True + assert before_anthropic["has_key"] is False + assert after_anthropic["has_key"] is True + finally: + if hasattr(prov, "invalidate_providers_cache"): + prov.invalidate_providers_cache() + + def test_oauth_credential_updates_invalidate_providers_cache(self, monkeypatch, tmp_path): + """OAuth credential updates should invalidate cached Providers responses (#6010).""" + from api import oauth + from api import providers as prov + + invalidated_credentials = [] + providers_invalidated = [] + monkeypatch.setattr(config, "invalidate_credential_pool_cache", invalidated_credentials.append) + monkeypatch.setattr(prov, "invalidate_providers_cache", lambda: providers_invalidated.append(True)) + + oauth._persist_codex_credentials( + tmp_path, + {"access_token": "access-token", "refresh_token": "refresh-token"}, + ) + + assert invalidated_credentials == ["openai-codex"] + assert providers_invalidated == [True] + def test_returns_list_of_known_providers(self, monkeypatch, tmp_path): """GET /api/providers should return a list of all known providers.""" _install_fake_hermes_cli(monkeypatch)