diff --git a/cli.py b/cli.py index e6718e935b..1287d90ce0 100644 --- a/cli.py +++ b/cli.py @@ -3661,10 +3661,7 @@ class HermesCLI(CLIProcessNotificationsMixin, CLIAgentSetupMixin, CLICommandsMix # a chat that spins ~30s and fails with a provider-specific error. TTY only. A # configured profile whose credential is benched or signed out gets the reason instead. try: - if sys.stdin.isatty(): - ready, error = self._probe_runtime_credentials() - if not ready and not self._explain_unusable_credentials(error): - self._offer_first_run_setup() + self._maybe_offer_first_run_setup() except Exception: logger.debug("first-run setup offer failed", exc_info=True) diff --git a/hermes_cli/cli_agent_setup_mixin.py b/hermes_cli/cli_agent_setup_mixin.py index beb829c675..c7d5f69683 100644 --- a/hermes_cli/cli_agent_setup_mixin.py +++ b/hermes_cli/cli_agent_setup_mixin.py @@ -394,8 +394,9 @@ class CLIAgentSetupMixin: return self._probe_runtime_credentials()[0] def _probe_runtime_credentials(self) -> tuple: - """``(ready, error)``: *error* is the exception that stopped resolution, ``None`` when a - provider resolved (usable or merely keyless). Never prints or mutates CLI state.""" + """``(ready, error)``: *error* is the exception that stopped resolution — raised, or + swallowed by the "auto" ladder and stamped on a keyless fallback — ``None`` when a provider + resolved (usable or merely keyless). Never prints or mutates CLI state.""" from hermes_cli.runtime_provider import resolve_runtime_provider try: runtime = resolve_runtime_provider( @@ -409,7 +410,16 @@ class CLIAgentSetupMixin: base_url = runtime.get("base_url") if callable(api_key) or (isinstance(api_key, str) and api_key): return bool(base_url), None - return _keyless_custom_base(base_url), None + return _keyless_custom_base(base_url), runtime.get("auth_error") + + def _maybe_offer_first_run_setup(self) -> None: + """Interactive startup gate: a blank install goes to the provider wizard; a configured + profile whose credential is benched or signed out gets the reason instead (#113720).""" + if not sys.stdin.isatty(): + return + ready, error = self._probe_runtime_credentials() + if not ready and not self._explain_unusable_credentials(error): + self._offer_first_run_setup() def _explain_unusable_credentials(self, error) -> bool: """A configured profile whose credential is benched, quarantined or signed out is not a diff --git a/hermes_cli/runtime_provider.py b/hermes_cli/runtime_provider.py index bfb0a0e464..9a39758e65 100644 --- a/hermes_cli/runtime_provider.py +++ b/hermes_cli/runtime_provider.py @@ -680,17 +680,10 @@ _OAUTH_RUNTIME_PROVIDERS: Dict[str, _OAuthRuntimeSpec] = { def _resolve_oauth_runtime(provider, requested_provider, model_cfg, target_model) -> Optional[Dict[str, Any]]: - """Runtime from an ``_OAUTH_RUNTIME_PROVIDERS`` spec. On AuthError: re-raise for an explicit - request; for "auto" (auto-detected but credentials stale/revoked) log and return None so the - ladder falls through to env-var providers (e.g. OpenRouter).""" + """Runtime from an ``_OAUTH_RUNTIME_PROVIDERS`` spec; raises AuthError when the credential is + stale/revoked/benched (``_ladder_rungs`` decides whether an "auto" request falls through).""" spec = _OAUTH_RUNTIME_PROVIDERS[provider] - try: - creds = spec.resolve() - except AuthError: - if requested_provider != "auto": - raise - logger.info("%s; falling through to next provider.", spec.failure_msg) - return None + creds = spec.resolve() api_mode = spec.api_mode(_effective_model(model_cfg, target_model)) if callable(spec.api_mode) else spec.api_mode return _runtime(provider, api_mode, (creds.get("base_url") or "").rstrip("/") or spec.default_base_url, creds.get("api_key", ""), source=creds.get("source", spec.default_source), @@ -879,7 +872,8 @@ def resolve_runtime_provider(*, requested: Optional[str] = None, explicit_api_ke 4. local-endpoint bypass (no explicit creds, config base_url at a non-cloud host) 5. ``auth.resolve_provider`` → explicit --api-key/--base-url path 6. credential pool (OpenRouter pool only without custom endpoint/override) - 7. OAuth specs (nous/codex/xai/qwen; "auto" swallows AuthError and logs) → minimax-oauth + 7. OAuth specs (nous/codex/xai/qwen; "auto" swallows AuthError, logs, and stamps it on a + keyless fallback as ``auth_error``) → minimax-oauth → external-process → anthropic env → bedrock → registry api_key providers 8. OpenRouter / bare-custom fallback target_model overrides model_cfg["default"] when computing provider-specific api_mode (e.g. @@ -930,8 +924,17 @@ def _ladder_rungs(requested_provider, explicit_api_key, explicit_base_url, targe explicit_api_key=explicit_api_key, explicit_base_url=explicit_base_url, target_model=target_model) yield _resolve_from_pool(provider, requested_provider, model_cfg, explicit_api_key, explicit_base_url, target_model) + swallowed_auth_error = None if provider in _OAUTH_RUNTIME_PROVIDERS: - yield _resolve_oauth_runtime(provider, requested_provider, model_cfg, target_model) + try: + yield _resolve_oauth_runtime(provider, requested_provider, model_cfg, target_model) + except AuthError as exc: + # Auto-detected login with stale/revoked/benched credentials: fall through to the env-var + # providers, but keep the error so a keyless fallback can still say what is wrong. + if requested_provider != "auto": + raise + logger.info("%s; falling through to next provider.", _OAUTH_RUNTIME_PROVIDERS[provider].failure_msg) + swallowed_auth_error = exc if provider == "minimax-oauth": yield _minimax_oauth_runtime(provider, requested_provider) if _is_external_process_provider(provider): @@ -943,7 +946,10 @@ def _ladder_rungs(requested_provider, explicit_api_key, explicit_base_url, targe pconfig = PROVIDER_REGISTRY.get(provider) if pconfig and pconfig.auth_type == "api_key": yield _api_key_provider_runtime(provider, pconfig, requested_provider, model_cfg, target_model) - yield _openrouter_fallback(requested_provider, explicit_api_key, explicit_base_url) + fallback = _openrouter_fallback(requested_provider, explicit_api_key, explicit_base_url) + if swallowed_auth_error is not None and not fallback.get("api_key"): + fallback["auth_error"] = swallowed_auth_error + yield fallback def format_runtime_provider_error(error: Exception) -> str: diff --git a/tests/hermes_cli/test_cli_first_run_setup.py b/tests/hermes_cli/test_cli_first_run_setup.py index d4738d6148..4912b3e59e 100644 --- a/tests/hermes_cli/test_cli_first_run_setup.py +++ b/tests/hermes_cli/test_cli_first_run_setup.py @@ -12,6 +12,7 @@ Covers: """ import importlib +import os import sys import types @@ -277,12 +278,33 @@ def test_empty_key_error_names_actual_provider(monkeypatch, capsys): # --------------------------------------------------------------------------- -def test_benched_credential_prints_cooldown_instead_of_wizard(monkeypatch, capsys): - """A profile whose only credential is cooling down is not a blank install: the startup notice - names the failure and the remaining cooldown, and the first-run wizard is not offered.""" +def _bench_nous_pool(monkeypatch, **entry_fields): import time from agent.credential_pool import STATUS_EXHAUSTED, CredentialPool, PooledCredential + benched = PooledCredential(id="e1", provider="nous", auth_type="oauth", access_token="x", + refresh_token="r", label="portal", source="manual:device_code", + priority=0, last_status=STATUS_EXHAUSTED, last_status_at=time.time() - 5, + **entry_fields) + pool = CredentialPool.__new__(CredentialPool) + monkeypatch.setattr(pool, "has_credentials", lambda: True, raising=False) + monkeypatch.setattr(pool, "has_available", lambda **kw: False, raising=False) + monkeypatch.setattr(pool, "next_available_at", lambda **kw: time.time() + 55, raising=False) + monkeypatch.setattr(pool, "entries", lambda: [benched], raising=False) + monkeypatch.setattr("agent.credential_pool.load_pool", lambda provider: pool) + + +def _forbid_wizard(monkeypatch, shell): + monkeypatch.setattr("hermes_cli.main.select_provider_and_model", + lambda: (_ for _ in ()).throw(AssertionError("wizard must not run"))) + monkeypatch.setattr(shell, "_offer_first_run_setup", + lambda: (_ for _ in ()).throw(AssertionError("wizard must not be offered"))) + + +def test_benched_credential_prints_cooldown_instead_of_wizard(monkeypatch, capsys): + """A profile whose only credential is cooling down is not a blank install: the interactive + startup gate prints the cooldown (with why and how long) as the headline, without telling the + user to re-authenticate, and never offers the first-run wizard.""" cli = _import_cli() shell = _make_shell(cli, monkeypatch) shell.requested_provider = "nous" @@ -292,37 +314,60 @@ def test_benched_credential_prints_cooldown_instead_of_wizard(monkeypatch, capsy code="nous_auth_missing", relogin_required=True) monkeypatch.setattr("hermes_cli.runtime_provider.resolve_runtime_provider", _raise) - benched = PooledCredential(id="e1", provider="nous", auth_type="oauth", access_token="x", - refresh_token="r", label="portal", source="manual:device_code", - priority=0, last_status=STATUS_EXHAUSTED, last_status_at=time.time() - 5) - pool = CredentialPool.__new__(CredentialPool) - monkeypatch.setattr(pool, "has_credentials", lambda: True, raising=False) - monkeypatch.setattr(pool, "has_available", lambda **kw: False, raising=False) - monkeypatch.setattr(pool, "next_available_at", lambda **kw: time.time() + 55, raising=False) - monkeypatch.setattr(pool, "entries", lambda: [benched], raising=False) - monkeypatch.setattr("agent.credential_pool.load_pool", lambda provider: pool) - monkeypatch.setattr("hermes_cli.main.select_provider_and_model", - lambda: (_ for _ in ()).throw(AssertionError("wizard must not run"))) + _bench_nous_pool(monkeypatch, last_error_code=429, last_error_reason="rate_limited") + _forbid_wizard(monkeypatch, shell) + monkeypatch.setattr(sys.stdin, "isatty", lambda: True) + + shell._maybe_offer_first_run_setup() - ready, error = shell._probe_runtime_credentials() - assert ready is False - assert shell._explain_unusable_credentials(error) is True out = capsys.readouterr().out assert "No inference provider is configured yet" not in out + headline = next(line for line in out.splitlines() if line.strip()) + assert "cooling down after a rate-limit or quota response" in headline and "about 1m" in headline + assert "failed token refresh" not in out assert "not logged into Nous Portal" in out - assert "cooling down" in out and "about 1m" in out + assert "re-authenticate" not in out and "hermes model" not in out -def test_nothing_configured_still_reaches_wizard(monkeypatch): - """Control: the resolver's ``no_provider_configured`` is the blank-install signal — nothing - to explain, so the first-run wizard is offered as before.""" +def test_auth_json_only_login_explains_instead_of_wizard(monkeypatch, capsys, tmp_path): + """auth.json-only shape: logged into Nous but no ``model.provider`` (requested "auto"). The + ladder swallows the AuthError and falls through to a keyless OpenRouter fallback; the gate + must still explain the real failure rather than treat the profile as a blank install. + Control: the resolver's ``no_provider_configured`` still reaches the wizard.""" + import dataclasses + + import hermes_cli.runtime_provider as rp + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + (tmp_path / "config.yaml").write_text("model:\n default: some-model\n", encoding="utf-8") + for key in [k for k in os.environ if k.endswith("_API_KEY")]: + monkeypatch.delenv(key, raising=False) + + def _nous_fail(): + raise AuthError("Hermes is not logged into Nous Portal.", provider="nous", + code="nous_auth_missing", relogin_required=True) + + monkeypatch.setattr(rp, "resolve_provider", lambda *a, **kw: "nous") + monkeypatch.setitem(rp._OAUTH_RUNTIME_PROVIDERS, "nous", + dataclasses.replace(rp._OAUTH_RUNTIME_PROVIDERS["nous"], resolve=_nous_fail)) + cli = _import_cli() shell = _make_shell(cli, monkeypatch) + shell.requested_provider = "auto" + shell._explicit_api_key = None + shell._explicit_base_url = None + _forbid_wizard(monkeypatch, shell) + monkeypatch.setattr(sys.stdin, "isatty", lambda: True) - def _raise(**kwargs): - raise AuthError("Hermes is not connected to any AI provider yet.", code="no_provider_configured") + shell._maybe_offer_first_run_setup() + out = capsys.readouterr().out + assert "not logged into Nous Portal" in out + assert "No inference provider is configured yet" not in out - monkeypatch.setattr("hermes_cli.runtime_provider.resolve_runtime_provider", _raise) - ready, error = shell._probe_runtime_credentials() - assert ready is False - assert shell._explain_unusable_credentials(error) is False + offered = [] + monkeypatch.setattr(shell, "_offer_first_run_setup", lambda: offered.append(True) or True) + monkeypatch.setattr("hermes_cli.runtime_provider.resolve_runtime_provider", lambda **kw: (_ for _ in ()).throw( + AuthError("Hermes is not connected to any AI provider yet.", code="no_provider_configured"))) + shell._maybe_offer_first_run_setup() + assert offered == [True] + assert "not logged into" not in capsys.readouterr().out