From 3858cf43075edc7a7d530ed18a4934eb79c81ce4 Mon Sep 17 00:00:00 2001 From: Frowtek Date: Thu, 4 Jun 2026 01:40:28 +0300 Subject: [PATCH] fix(cli): honor global-root active_provider fallback for named profiles --- hermes_cli/auth.py | 40 ++++++- .../hermes_cli/test_auth_profile_fallback.py | 106 +++++++++++++++++- 2 files changed, 141 insertions(+), 5 deletions(-) diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 021905c3e..165410bcd 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -1322,10 +1322,38 @@ def get_provider_auth_state(provider_id: str) -> Optional[Dict[str, Any]]: return _load_provider_state(auth_store, provider_id) +def _active_provider_from_store(auth_store: Dict[str, Any]) -> Optional[str]: + """Return the active provider for a loaded auth store. + + In profile mode, falls back to the global-root ``auth.json`` when the + profile store has no ``active_provider`` set. This mirrors the per-provider + shadowing already used by ``_load_provider_state`` and + ``read_credential_pool``: a named profile that never selected its own + provider still resolves the provider the user authenticated at the global + root (e.g. a Nous OAuth login), so ``model.provider: auto`` works under a + profile. A profile that has its own ``active_provider`` always wins; the + fallback only fires when the profile has none. Returns ``None`` when + neither scope has one. In classic mode ``_load_global_auth_store`` returns + an empty dict, so this is a no-op. See issue #18594 follow-up. + """ + active = auth_store.get("active_provider") + if active: + return active + global_store = _load_global_auth_store() + if global_store: + return global_store.get("active_provider") + return None + + def get_active_provider() -> Optional[str]: - """Return the currently active provider ID from auth store.""" + """Return the currently active provider ID from auth store. + + In profile mode this falls back to the global-root ``active_provider`` + when the profile has not selected one of its own — see + ``_active_provider_from_store``. + """ auth_store = _load_auth_store() - return auth_store.get("active_provider") + return _active_provider_from_store(auth_store) def is_provider_explicitly_configured(provider_id: str) -> bool: @@ -1547,10 +1575,14 @@ def resolve_provider( if explicit_api_key or explicit_base_url: return "openrouter" - # Check auth store for an active OAuth provider + # Check auth store for an active OAuth provider. In profile mode this + # honors the global-root active_provider when the profile has none of its + # own, mirroring the credential-pool / provider-state fallbacks so a + # named profile running model.provider: auto can use a globally + # authenticated provider. See issue #18594 follow-up. try: auth_store = _load_auth_store() - active = auth_store.get("active_provider") + active = _active_provider_from_store(auth_store) if active and active in PROVIDER_REGISTRY: status = get_auth_status(active) if status.get("logged_in"): diff --git a/tests/hermes_cli/test_auth_profile_fallback.py b/tests/hermes_cli/test_auth_profile_fallback.py index 5210404c4..d041b4efa 100644 --- a/tests/hermes_cli/test_auth_profile_fallback.py +++ b/tests/hermes_cli/test_auth_profile_fallback.py @@ -17,12 +17,18 @@ from pathlib import Path import pytest -def _make_auth_store(pool: dict | None = None, providers: dict | None = None) -> dict: +def _make_auth_store( + pool: dict | None = None, + providers: dict | None = None, + active_provider: str | None = None, +) -> dict: store: dict = {"version": 1} if pool is not None: store["credential_pool"] = pool if providers is not None: store["providers"] = providers + if active_provider is not None: + store["active_provider"] = active_provider return store @@ -450,3 +456,101 @@ def test_write_credential_pool_targets_profile_not_global(profile_env): # Subsequent read returns profile (shadows global). assert [e["id"] for e in read_credential_pool("openrouter")] == ["prof-new"] + + +# --------------------------------------------------------------------------- +# get_active_provider — global active_provider fallback (issue #18594 follow-up) +# +# The per-provider state/pool fallbacks let a profile *read* a provider that +# was only authenticated at the global root, but ``resolve_provider()`` picks +# the ``auto`` provider from ``active_provider`` — which only ever read the +# profile store. A named profile running ``model.provider: auto`` could see +# the global Nous login (``get_provider_auth_state('nous')`` succeeds) yet +# still fail to select it. These pin the active_provider shadowing so the +# selection mirrors the state/pool fallbacks: profile wins when present, fall +# back to global when the profile never chose its own provider. +# --------------------------------------------------------------------------- + + +def test_active_provider_falls_back_to_global(profile_env): + """An empty profile inherits the global-root active_provider selection.""" + from hermes_cli.auth import get_active_provider + + _write(profile_env["global"] / "auth.json", _make_auth_store( + providers={"nous": {"access_token": "nous-global"}}, + active_provider="nous", + )) + _write(profile_env["profile"] / "auth.json", _make_auth_store(providers={})) + + assert get_active_provider() == "nous" + + +def test_active_provider_profile_wins_over_global(profile_env): + """A profile that selected its own provider shadows the global selection.""" + from hermes_cli.auth import get_active_provider + + _write(profile_env["global"] / "auth.json", _make_auth_store( + providers={"nous": {"access_token": "nous-global"}}, + active_provider="nous", + )) + _write(profile_env["profile"] / "auth.json", _make_auth_store( + providers={"anthropic": {"access_token": "ant-profile"}}, + active_provider="anthropic", + )) + + assert get_active_provider() == "anthropic" + + +def test_active_provider_none_when_neither_has_it(profile_env): + """No selection anywhere stays None — the fallback must not invent one.""" + from hermes_cli.auth import get_active_provider + + _write(profile_env["global"] / "auth.json", _make_auth_store(providers={})) + _write(profile_env["profile"] / "auth.json", _make_auth_store(providers={})) + + assert get_active_provider() is None + + +def test_active_provider_classic_mode_reads_profile(tmp_path, monkeypatch): + """In classic mode there is no global to fall back to; behavior is unchanged.""" + fake_home = tmp_path / "home" + fake_home.mkdir() + monkeypatch.setattr(Path, "home", lambda: fake_home) + hermes_home = tmp_path / "classic" + hermes_home.mkdir() + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + + _write(hermes_home / "auth.json", _make_auth_store( + providers={"nous": {"access_token": "classic-token"}}, + active_provider="nous", + )) + + from hermes_cli.auth import get_active_provider + + assert get_active_provider() == "nous" + + +def test_resolve_provider_uses_global_active_provider(profile_env, monkeypatch): + """resolve_provider('auto') honors the global-root active_provider. + + This is the user-visible contract: a named profile with no provider entry + of its own, started with ``model.provider: auto`` while a valid login + exists at the global root, resolves that provider instead of raising + ``No inference provider configured``. ``get_auth_status`` is stubbed so the + login check stays offline (no Nous token refresh / network). + """ + import hermes_cli.auth as auth + + _write(profile_env["global"] / "auth.json", _make_auth_store( + providers={"nous": {"access_token": "nous-global"}}, + active_provider="nous", + )) + _write(profile_env["profile"] / "auth.json", _make_auth_store(providers={})) + + monkeypatch.setattr( + auth, + "get_auth_status", + lambda provider=None: {"logged_in": True, "provider": provider}, + ) + + assert auth.resolve_provider("auto") == "nous"