fix(cli): route keyless first run into provider onboarding instead of a broken chat
A completely unconfigured install previously booted into a working-looking chat (banner showed model 'unknown'), accepted a message, spun ~30s, then failed with 'Set OPENROUTER_API_KEY' — a provider the user never chose — and never offered setup. - HermesCLI.run() now probes provider readiness at startup (TTY only) and offers the shared provider picker (hermes model flow, which fronts Quick Setup / Nous Portal OAuth) when nothing is configured. Decline is respected; picker state re-syncs into the live CLI so the next turn works without a restart. - New silent probe _runtime_credentials_ready(): no printing, no state mutation; handles keyless local endpoints and callable bearer providers. - The empty-api-key error is provider-aware: names the actual resolved provider and points at 'hermes model' / 'hermes setup' instead of hardcoding OPENROUTER_API_KEY. - Banner: unconfigured installs render 'no model configured — run /model' in red instead of the silent 'unknown' model slug. Consumer-onboarding audit finding #2 (sev 5), Aug 2026.
This commit is contained in:
Teknium
parent
cc0af6b9e8
commit
d7522118ef
12
cli.py
12
cli.py
|
|
@ -14846,6 +14846,18 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin):
|
|||
# Surface any active supply-chain security advisories right after the
|
||||
# welcome banner. Quiet/single-query paths call this themselves.
|
||||
self._show_security_advisories()
|
||||
|
||||
# First-run: a completely unconfigured install must route into
|
||||
# provider onboarding, not a chat that cannot work. Previously a
|
||||
# keyless `hermes` accepted a message, spun for ~30s, then failed
|
||||
# with a provider-specific error the user never chose. Only fires
|
||||
# on a real TTY; quiet/single-query paths keep their own handling.
|
||||
try:
|
||||
if sys.stdin.isatty() and not self._runtime_credentials_ready():
|
||||
self._offer_first_run_setup()
|
||||
except Exception:
|
||||
logger.debug("first-run setup offer failed", exc_info=True)
|
||||
|
||||
# If resuming a session, load history and display it immediately
|
||||
# so the user has context before typing their first message.
|
||||
if self._resumed:
|
||||
|
|
|
|||
|
|
@ -659,13 +659,22 @@ def build_welcome_banner(console: "Console", model: str, cwd: str,
|
|||
ctx_str = f" [dim {dim}]·[/] [dim {dim}]{_format_context_length(context_length)} context[/]" if context_length else ""
|
||||
left_lines.append(f"[{accent}]MoA: {preset_name}[/]{agg_str}{ctx_str} [dim {dim}]·[/] [dim {dim}]Nous Research[/]")
|
||||
else:
|
||||
model_short = model.split("/")[-1] if "/" in model else model
|
||||
if model_short.endswith(".gguf"):
|
||||
model_short = model_short[:-5]
|
||||
if len(model_short) > 28:
|
||||
model_short = model_short[:25] + "..."
|
||||
ctx_str = f" [dim {dim}]·[/] [dim {dim}]{_format_context_length(context_length)} context[/]" if context_length else ""
|
||||
left_lines.append(f"[{accent}]{model_short}[/]{ctx_str} [dim {dim}]·[/] [dim {dim}]Nous Research[/]")
|
||||
if not (model or "").strip() or (model or "").strip().lower() == "unknown":
|
||||
# Unconfigured install: say so in red instead of a blank/"unknown"
|
||||
# slug — this is the single clearest place to tell the user what
|
||||
# is wrong and how to fix it.
|
||||
left_lines.append(
|
||||
f"[bold red]no model configured[/] "
|
||||
f"[dim {dim}]— run /model or hermes setup[/]"
|
||||
)
|
||||
else:
|
||||
model_short = model.split("/")[-1] if "/" in model else model
|
||||
if model_short.endswith(".gguf"):
|
||||
model_short = model_short[:-5]
|
||||
if len(model_short) > 28:
|
||||
model_short = model_short[:25] + "..."
|
||||
ctx_str = f" [dim {dim}]·[/] [dim {dim}]{_format_context_length(context_length)} context[/]" if context_length else ""
|
||||
left_lines.append(f"[{accent}]{model_short}[/]{ctx_str} [dim {dim}]·[/] [dim {dim}]Nous Research[/]")
|
||||
|
||||
if os.getenv("HERMES_YOLO_MODE"):
|
||||
left_lines.append(f"[bold red]⚠ YOLO mode[/] [dim {dim}]— all approval prompts bypassed[/]")
|
||||
|
|
|
|||
|
|
@ -111,8 +111,13 @@ class CLIAgentSetupMixin:
|
|||
base_url, _source,
|
||||
)
|
||||
else:
|
||||
print("\n⚠️ Provider resolver returned an empty API key. "
|
||||
"Set OPENROUTER_API_KEY or run: hermes setup")
|
||||
_prov = (resolved_provider or self.requested_provider or "").strip()
|
||||
if _prov and _prov != "auto":
|
||||
print(f"\n⚠️ No API key found for provider '{_prov}'.")
|
||||
else:
|
||||
print("\n⚠️ No inference provider is configured.")
|
||||
print(" Run 'hermes model' to choose a provider, or "
|
||||
"'hermes setup' for first-time setup.")
|
||||
return False
|
||||
if not isinstance(base_url, str) or not base_url:
|
||||
print("\n⚠️ Provider resolver returned an empty base URL. "
|
||||
|
|
@ -179,6 +184,104 @@ class CLIAgentSetupMixin:
|
|||
|
||||
return True
|
||||
|
||||
def _runtime_credentials_ready(self) -> bool:
|
||||
"""Silently probe whether any inference provider can be resolved.
|
||||
|
||||
Unlike ``_ensure_runtime_credentials`` this never prints and never
|
||||
mutates CLI state — it exists so the interactive first-run path can
|
||||
detect a completely unconfigured install *before* the user types a
|
||||
message into a chat that cannot work (#62935-adjacent UX class:
|
||||
keyless first run must route into onboarding, not a broken chat).
|
||||
"""
|
||||
from hermes_cli.runtime_provider import resolve_runtime_provider
|
||||
|
||||
try:
|
||||
runtime = resolve_runtime_provider(
|
||||
requested=self.requested_provider,
|
||||
explicit_api_key=self._explicit_api_key,
|
||||
explicit_base_url=self._explicit_base_url,
|
||||
)
|
||||
except Exception:
|
||||
return False
|
||||
if not isinstance(runtime, dict):
|
||||
return False
|
||||
api_key = runtime.get("api_key")
|
||||
base_url = runtime.get("base_url")
|
||||
if callable(api_key) and not isinstance(api_key, str):
|
||||
return bool(base_url)
|
||||
if isinstance(api_key, str) and api_key:
|
||||
return bool(base_url)
|
||||
# Keyless custom/local endpoints (ollama, llama.cpp, vLLM…) are fine.
|
||||
return bool(
|
||||
isinstance(base_url, str)
|
||||
and base_url
|
||||
and "openrouter.ai" not in base_url
|
||||
)
|
||||
|
||||
def _offer_first_run_setup(self) -> bool:
|
||||
"""Offer the provider picker when no provider is configured at all.
|
||||
|
||||
Called from the interactive startup path when
|
||||
``_runtime_credentials_ready()`` is False and stdin is a TTY. Runs the
|
||||
exact same flow as ``hermes model`` (which fronts Quick Setup / Nous
|
||||
Portal OAuth as the first, recommended option) so there is a single
|
||||
source of truth for provider onboarding. Returns True when a provider
|
||||
was configured.
|
||||
"""
|
||||
from cli import _cprint, logger
|
||||
|
||||
_cprint("")
|
||||
_cprint("⚕ No inference provider is configured yet — let's fix that.")
|
||||
_cprint(" You'll pick a provider (Nous Portal OAuth is the fastest; "
|
||||
"no API key needed) and a model.")
|
||||
try:
|
||||
answer = input(" Set up a provider now? [Y/n]: ").strip().lower()
|
||||
except (KeyboardInterrupt, EOFError):
|
||||
print()
|
||||
answer = "n"
|
||||
if answer in {"n", "no"}:
|
||||
_cprint(" Skipped. Run 'hermes model' or 'hermes setup' any time.")
|
||||
return False
|
||||
|
||||
try:
|
||||
from hermes_cli.main import select_provider_and_model
|
||||
select_provider_and_model()
|
||||
except (KeyboardInterrupt, EOFError, SystemExit):
|
||||
print()
|
||||
_cprint(" Setup cancelled. Run 'hermes model' any time.")
|
||||
return False
|
||||
except Exception as exc:
|
||||
logger.debug("first-run provider setup failed: %s", exc)
|
||||
_cprint(f" ⚠️ Provider setup failed: {exc}")
|
||||
_cprint(" Run 'hermes model' to try again.")
|
||||
return False
|
||||
|
||||
# Re-sync CLI state from what the picker persisted so the very next
|
||||
# turn uses the new provider without a restart.
|
||||
try:
|
||||
from hermes_cli.config import load_config
|
||||
_model_cfg = (load_config().get("model") or {})
|
||||
if isinstance(_model_cfg, dict):
|
||||
_new_provider = (_model_cfg.get("provider") or "").strip()
|
||||
if _new_provider:
|
||||
self.requested_provider = _new_provider
|
||||
_new_model = (
|
||||
_model_cfg.get("default") or _model_cfg.get("model") or ""
|
||||
).strip()
|
||||
if _new_model:
|
||||
self.model = _new_model
|
||||
except Exception as exc:
|
||||
logger.debug("first-run config re-sync failed: %s", exc)
|
||||
# Force credential re-resolution + agent rebuild on next use.
|
||||
self.agent = None
|
||||
self._active_agent_route_signature = None
|
||||
|
||||
if self._runtime_credentials_ready():
|
||||
_cprint(" ✓ Provider configured — you're ready to chat.")
|
||||
return True
|
||||
_cprint(" Provider setup didn't complete. Run 'hermes model' to retry.")
|
||||
return False
|
||||
|
||||
def _resolve_turn_agent_config(self, user_message: str) -> dict:
|
||||
"""Build the effective model/runtime config for a single user turn.
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,252 @@
|
|||
"""First-run onboarding routing for a completely unconfigured install.
|
||||
|
||||
Regression tests for the "keyless first run boots into a broken chat" bug:
|
||||
a fresh install with zero providers accepted a message, spun for ~30s, then
|
||||
failed with a provider-specific error ("Set OPENROUTER_API_KEY") the user
|
||||
never chose, and never offered setup.
|
||||
|
||||
Covers:
|
||||
- ``_runtime_credentials_ready()`` silent probe semantics
|
||||
- ``_offer_first_run_setup()`` routing into the shared provider picker
|
||||
- the provider-aware (non-OpenRouter-specific) empty-key error message
|
||||
"""
|
||||
|
||||
import importlib
|
||||
import sys
|
||||
import types
|
||||
|
||||
import pytest
|
||||
|
||||
from hermes_cli.auth import AuthError
|
||||
|
||||
|
||||
def _reset_modules(prefixes: tuple[str, ...]):
|
||||
for name in list(sys.modules):
|
||||
if any(name == p or name.startswith(p + ".") for p in prefixes):
|
||||
sys.modules.pop(name, None)
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _restore_cli_and_tool_modules():
|
||||
prefixes = ("tools", "cli", "run_agent")
|
||||
original_modules = {
|
||||
name: module
|
||||
for name, module in sys.modules.items()
|
||||
if any(name == p or name.startswith(p + ".") for p in prefixes)
|
||||
}
|
||||
try:
|
||||
yield
|
||||
finally:
|
||||
_reset_modules(prefixes)
|
||||
sys.modules.update(original_modules)
|
||||
|
||||
|
||||
def _import_cli():
|
||||
for name in list(sys.modules):
|
||||
if name == "cli" or name == "run_agent" or name == "tools" or name.startswith("tools."):
|
||||
sys.modules.pop(name, None)
|
||||
if "firecrawl" not in sys.modules:
|
||||
sys.modules["firecrawl"] = types.SimpleNamespace(Firecrawl=object)
|
||||
return importlib.import_module("cli")
|
||||
|
||||
|
||||
def _make_shell(cli, monkeypatch):
|
||||
shell = cli.HermesCLI(compact=True, max_turns=1)
|
||||
return shell
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _runtime_credentials_ready
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_credentials_ready_false_when_no_provider(monkeypatch):
|
||||
cli = _import_cli()
|
||||
|
||||
def _raise(**kwargs):
|
||||
raise AuthError("No inference provider configured.", code="no_provider_configured")
|
||||
|
||||
monkeypatch.setattr("hermes_cli.runtime_provider.resolve_runtime_provider", _raise)
|
||||
shell = _make_shell(cli, monkeypatch)
|
||||
assert shell._runtime_credentials_ready() is False
|
||||
|
||||
|
||||
def test_credentials_ready_false_on_empty_openrouter_key(monkeypatch):
|
||||
"""The exact broken-chat state: provider resolves but api_key is empty."""
|
||||
cli = _import_cli()
|
||||
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.runtime_provider.resolve_runtime_provider",
|
||||
lambda **kw: {
|
||||
"provider": "openrouter",
|
||||
"api_key": "",
|
||||
"base_url": "https://openrouter.ai/api/v1",
|
||||
"source": "env/config",
|
||||
},
|
||||
)
|
||||
shell = _make_shell(cli, monkeypatch)
|
||||
assert shell._runtime_credentials_ready() is False
|
||||
|
||||
|
||||
def test_credentials_ready_true_with_key(monkeypatch):
|
||||
cli = _import_cli()
|
||||
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.runtime_provider.resolve_runtime_provider",
|
||||
lambda **kw: {
|
||||
"provider": "openrouter",
|
||||
"api_key": "sk-test",
|
||||
"base_url": "https://openrouter.ai/api/v1",
|
||||
"source": "env/config",
|
||||
},
|
||||
)
|
||||
shell = _make_shell(cli, monkeypatch)
|
||||
assert shell._runtime_credentials_ready() is True
|
||||
|
||||
|
||||
def test_credentials_ready_true_for_keyless_local_endpoint(monkeypatch):
|
||||
"""ollama/llama.cpp-style custom endpoints need no key."""
|
||||
cli = _import_cli()
|
||||
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.runtime_provider.resolve_runtime_provider",
|
||||
lambda **kw: {
|
||||
"provider": "custom",
|
||||
"api_key": "",
|
||||
"base_url": "http://localhost:11434/v1",
|
||||
"source": "custom_provider",
|
||||
},
|
||||
)
|
||||
shell = _make_shell(cli, monkeypatch)
|
||||
assert shell._runtime_credentials_ready() is True
|
||||
|
||||
|
||||
def test_credentials_ready_true_for_callable_bearer_provider(monkeypatch):
|
||||
cli = _import_cli()
|
||||
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.runtime_provider.resolve_runtime_provider",
|
||||
lambda **kw: {
|
||||
"provider": "azure-foundry",
|
||||
"api_key": lambda: "tok",
|
||||
"base_url": "https://foundry.example/v1",
|
||||
"source": "entra",
|
||||
},
|
||||
)
|
||||
shell = _make_shell(cli, monkeypatch)
|
||||
assert shell._runtime_credentials_ready() is True
|
||||
|
||||
|
||||
def test_credentials_ready_never_prints(monkeypatch, capsys):
|
||||
cli = _import_cli()
|
||||
|
||||
def _raise(**kwargs):
|
||||
raise AuthError("No inference provider configured.", code="no_provider_configured")
|
||||
|
||||
monkeypatch.setattr("hermes_cli.runtime_provider.resolve_runtime_provider", _raise)
|
||||
shell = _make_shell(cli, monkeypatch)
|
||||
capsys.readouterr() # drain construction output
|
||||
shell._runtime_credentials_ready()
|
||||
out = capsys.readouterr()
|
||||
assert out.out == ""
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _offer_first_run_setup
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_offer_first_run_setup_routes_into_shared_picker(monkeypatch):
|
||||
cli = _import_cli()
|
||||
shell = _make_shell(cli, monkeypatch)
|
||||
|
||||
picker_calls = {"count": 0}
|
||||
|
||||
def _fake_picker():
|
||||
picker_calls["count"] += 1
|
||||
|
||||
monkeypatch.setattr("hermes_cli.main.select_provider_and_model", _fake_picker)
|
||||
monkeypatch.setattr("builtins.input", lambda *a, **k: "y")
|
||||
# After the picker "runs", config has a provider and creds resolve.
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.config.load_config",
|
||||
lambda: {"model": {"provider": "nous", "default": "hermes-4-405b"}},
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.runtime_provider.resolve_runtime_provider",
|
||||
lambda **kw: {
|
||||
"provider": "nous",
|
||||
"api_key": "portal-token",
|
||||
"base_url": "https://inference-api.nousresearch.com/v1",
|
||||
"source": "oauth",
|
||||
},
|
||||
)
|
||||
|
||||
assert shell._offer_first_run_setup() is True
|
||||
assert picker_calls["count"] == 1
|
||||
assert shell.requested_provider == "nous"
|
||||
assert shell.model == "hermes-4-405b"
|
||||
# Agent must be rebuilt with the new credentials on next use.
|
||||
assert shell.agent is None
|
||||
|
||||
|
||||
def test_offer_first_run_setup_declined(monkeypatch):
|
||||
cli = _import_cli()
|
||||
shell = _make_shell(cli, monkeypatch)
|
||||
|
||||
def _fail_picker():
|
||||
raise AssertionError("picker must not run when declined")
|
||||
|
||||
monkeypatch.setattr("hermes_cli.main.select_provider_and_model", _fail_picker)
|
||||
monkeypatch.setattr("builtins.input", lambda *a, **k: "n")
|
||||
assert shell._offer_first_run_setup() is False
|
||||
|
||||
|
||||
def test_offer_first_run_setup_picker_cancel_is_graceful(monkeypatch):
|
||||
cli = _import_cli()
|
||||
shell = _make_shell(cli, monkeypatch)
|
||||
|
||||
def _cancel_picker():
|
||||
raise KeyboardInterrupt()
|
||||
|
||||
monkeypatch.setattr("hermes_cli.main.select_provider_and_model", _cancel_picker)
|
||||
monkeypatch.setattr("builtins.input", lambda *a, **k: "")
|
||||
# Empty answer defaults to yes -> picker runs -> cancels -> False, no raise.
|
||||
assert shell._offer_first_run_setup() is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Provider-aware empty-key error (replaces the OpenRouter-specific one)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_empty_key_error_names_actual_provider(monkeypatch, capsys):
|
||||
cli = _import_cli()
|
||||
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.runtime_provider.resolve_runtime_provider",
|
||||
lambda **kw: {
|
||||
"provider": "fireworks",
|
||||
"api_key": "",
|
||||
"base_url": "https://api.fireworks.ai/inference/v1/extra",
|
||||
"source": "env/config",
|
||||
},
|
||||
)
|
||||
shell = _make_shell(cli, monkeypatch)
|
||||
# A custom base_url would get the no-key placeholder; force the
|
||||
# openrouter-shaped branch by pointing base_url at openrouter.
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.runtime_provider.resolve_runtime_provider",
|
||||
lambda **kw: {
|
||||
"provider": "fireworks",
|
||||
"api_key": "",
|
||||
"base_url": "https://openrouter.ai/api/v1",
|
||||
"source": "env/config",
|
||||
},
|
||||
)
|
||||
capsys.readouterr()
|
||||
assert shell._ensure_runtime_credentials() is False
|
||||
out = capsys.readouterr().out
|
||||
assert "fireworks" in out
|
||||
assert "OPENROUTER_API_KEY" not in out
|
||||
assert "hermes model" in out or "hermes setup" in out
|
||||
Loading…
Reference in New Issue