From fea47bd986dfad2079ef34030183046188d1873d Mon Sep 17 00:00:00 2001 From: Hermes Agent Date: Fri, 1 May 2026 05:29:42 +0000 Subject: [PATCH] Heal 'provider: local' mid-conversation crash for local-model users (#1388, fixes #1384) --- CHANGELOG.md | 4 +- api/config.py | 34 ++++- tests/test_issue1384_local_provider.py | 194 +++++++++++++++++++++++++ 3 files changed, 230 insertions(+), 2 deletions(-) create mode 100644 tests/test_issue1384_local_provider.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 64d784ad..0ccd33a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## [Unreleased] +### Fixed +- **Local model setup no longer fails mid-conversation with `LOCAL_API_KEY` error** — when `model.base_url` pointed at an OpenAI-compatible loopback endpoint that didn't match the `ollama`/`localhost`/`lmstudio` keyword classifier (e.g. `http://192.168.1.10:8080/v1`, llama.cpp on `127.0.0.1:8080`, vLLM, TabbyAPI, custom proxies), `_build_available_models_uncached` auto-detected the provider as `"local"` and persisted that into `config.yaml`. Inference worked initially because the main agent has its own direct path that uses the explicit `base_url + api_key`. But once the conversation grew enough to trip auto-compression — or when vision / web extraction / skills-hub fired — the agent's auxiliary client routed through `resolve_provider_client("local", …)`, fell through every branch, and raised `Provider 'local' is set in config.yaml but no API key was found. Set the LOCAL_API_KEY environment variable, or switch to a different provider with hermes model.` Three-layer fix: (1) the auto-detect block now writes `provider: "custom"` instead of `provider: "local"` for unknown loopback hosts — `custom` is the canonical OpenAI-compat fall-through and the agent's auxiliary client takes the `no-key-required` path for it; (2) `resolve_model_provider()` rewrites legacy `"local"` to `"custom"` at read time so existing broken configs heal automatically without requiring the user to edit `config.yaml` by hand; (3) `set_hermes_default_model()` refuses to persist `"local"` going forward, and `_PROVIDER_ALIASES` gets a `"local" → "custom"` entry for any consumer that normalises through the alias table. 9 regression tests in `test_issue1384_local_provider.py` covering the source-code invariant, both YAML cases, the migration path, and the alias table. (`api/config.py`, `tests/test_issue1384_local_provider.py`) Closes #1384 + ## [v0.50.252] — 2026-05-01 ### Fixed @@ -17,7 +20,6 @@ ### Changed - **API credential redaction now uses `force=True`** — `_combined_redact` (introduced by #1379) now passes `force=True` to `redact_sensitive_text` so the agent's broader patterns (Stripe `sk_live_`, Google `AIza…`, JWT `eyJ…`, DB connection strings, Telegram bot tokens) run regardless of the user's `HERMES_REDACT_SECRETS` opt-in. The local fallback then handles the short-prefix shapes the agent omits (`ghp_`, `sk-`, `hf_`, `AKIA`). WebUI API responses are a hard safety boundary — no opt-in should be required. (`api/helpers.py`) — Opus pre-release follow-up - **`_active_profile_for_live_models_cache` logs the fallback path** — when `get_active_profile_name()` raises (transient state, mid-switch, etc.) the live-models cache (#1378) falls back to `"default"`, mis-scoping the cache for up to 60s. Now logs at debug so we can detect this in production logs without changing the blast radius (TTL still caps the bad-cache window). (`api/routes.py`) — Opus pre-release follow-up - ## [v0.50.251] — 2026-04-30 ### Fixed diff --git a/api/config.py b/api/config.py index 45694f40..90b788a6 100644 --- a/api/config.py +++ b/api/config.py @@ -614,6 +614,12 @@ _PROVIDER_ALIASES = { "nvidia-nim": "nvidia", "build-nvidia": "nvidia", "nemotron": "nvidia", + # Legacy alias — earlier WebUI builds wrote ``provider: local`` for unknown + # loopback endpoints, but ``local`` is not registered in + # ``hermes_cli.auth.PROVIDER_REGISTRY``. Routing it through ``custom`` + # lets the agent's auxiliary client take the ``no-key-required`` + # OpenAI-compat path. See #1384. + "local": "custom", } @@ -971,6 +977,16 @@ def resolve_model_provider(model_id: str) -> tuple: config_provider = model_cfg.get("provider") config_base_url = model_cfg.get("base_url") + # Heal legacy ``provider: local`` entries (written by WebUI < v0.50.252) + # at read time. ``local`` is not a registered provider, so passing it + # downstream raises a ``LOCAL_API_KEY`` error from the auxiliary client + # mid-conversation when compression/vision/web-extract fires. Route + # through ``custom`` instead — it takes the ``no-key-required`` + # OpenAI-compat path that local servers (Ollama, LM Studio, llama.cpp, + # vLLM, TabbyAPI) actually use. See #1384. + if isinstance(config_provider, str) and config_provider.strip().lower() == "local": + config_provider = "custom" + model_id = (model_id or "").strip() if not model_id: return model_id, config_provider, config_base_url @@ -1185,6 +1201,13 @@ def set_hermes_default_model(model_id: str) -> dict: # matcher — see `static/panels.js` (#895). persisted_model = str(resolved_model or selected_model).strip() persisted_provider = str(resolved_provider or previous_provider or "").strip() + # Never persist the bogus ``local`` value — see #1384. The auto-detect + # block in ``_build_available_models_uncached`` was rewriting unknown + # loopback hosts to ``provider: "local"``, which is not registered and + # broke compression/vision mid-conversation. Route through ``custom`` + # so the agent's auxiliary client uses the ``no-key-required`` path. + if persisted_provider.lower() == "local": + persisted_provider = "custom" model_cfg["default"] = persisted_model if persisted_provider: @@ -1745,7 +1768,16 @@ def get_available_models() -> dict: elif "lmstudio" in host or "lm-studio" in host: provider = "lmstudio" else: - provider = "local" + # Unknown loopback/private endpoint: route through + # the generic ``custom`` provider so the agent's + # auxiliary client (compression, vision, web + # extraction) takes the OpenAI-compat custom path + # with ``no-key-required`` semantics. Writing + # ``provider: local`` here used to break + # compression mid-conversation because ``local`` + # is not a registered provider in + # ``hermes_cli.auth.PROVIDER_REGISTRY`` — see #1384. + provider = "custom" except ValueError: pass diff --git a/tests/test_issue1384_local_provider.py b/tests/test_issue1384_local_provider.py new file mode 100644 index 00000000..86e60bb1 --- /dev/null +++ b/tests/test_issue1384_local_provider.py @@ -0,0 +1,194 @@ +"""Regression tests for #1384 — ``provider: "local"`` mid-conversation crash. + +Earlier WebUI builds auto-detected unknown loopback hosts and persisted +``provider: "local"`` to ``config.yaml``. That value is not a registered +provider in ``hermes_cli.auth.PROVIDER_REGISTRY``, so the agent's auxiliary +client (compression, vision, web extraction) raised +``"Provider 'local' is set in config.yaml but no API key was found"`` +mid-conversation when the context-compression threshold was hit. + +The fix is three layers deep so a user with an already-broken config gets +healed automatically: + +1. The auto-detect block now writes ``provider: "custom"`` for unknown + loopback hosts (``custom`` is the canonical generic-OpenAI-compat + provider, and the agent's auxiliary client takes the + ``no-key-required`` path for it). +2. ``resolve_model_provider()`` rewrites legacy ``"local"`` to ``"custom"`` + at read time, so existing configs route correctly without requiring the + user to edit ``config.yaml`` by hand. +3. ``set_hermes_default_model()`` refuses to persist ``"local"`` going + forward, so any other code path that tries to write it is also healed. +4. The local alias table has ``"local" → "custom"`` for any consumer that + normalises through ``_resolve_provider_alias``. +""" + +import re +from pathlib import Path + +import pytest + +import api.config as cfg + + +# ── 1. Auto-detect block writes ``custom``, not ``local`` ──────────────── + + +class TestAutoDetectWritesCustom: + """The auto-detect branch in ``_build_available_models_uncached`` must + never write ``provider = "local"`` because that value breaks the + auxiliary client mid-conversation.""" + + def test_source_code_no_local_assignment(self): + """The string ``provider = "local"`` must not appear in api/config.py.""" + src = Path(cfg.__file__).read_text(encoding="utf-8") + assert 'provider = "local"' not in src, ( + 'api/config.py must not assign provider = "local" — see #1384. ' + "Use ``custom`` instead so the agent's auxiliary client takes the " + "``no-key-required`` OpenAI-compat path." + ) + + def test_auto_detect_branch_uses_custom(self): + """The else-branch in the auto-detect block resolves to ``custom``.""" + src = Path(cfg.__file__).read_text(encoding="utf-8") + # Find the auto-detect block (host-keyword classifier). + m = re.search( + r'if "ollama" in host or "127\.0\.0\.1" in host or "localhost" in host:\s*\n' + r'\s*provider = "ollama"\s*\n' + r'\s*elif "lmstudio" in host or "lm-studio" in host:\s*\n' + r'\s*provider = "lmstudio"\s*\n' + r'\s*else:', + src, + ) + assert m, "Auto-detect host-classifier block not found in api/config.py" + # Find the next provider assignment after the else. + tail = src[m.end() : m.end() + 1500] + provider_assign = re.search(r'provider = "([a-z-]+)"', tail) + assert provider_assign, "No provider assignment found after auto-detect else" + assert provider_assign.group(1) == "custom", ( + f"Auto-detect else branch must assign provider = \"custom\", " + f"got {provider_assign.group(1)!r}" + ) + + +# ── 2. resolve_model_provider() heals legacy ``local`` configs ─────────── + + +class TestResolveModelProviderHealsLegacyLocal: + """Existing ``config.yaml`` files with ``provider: local`` (written by + earlier WebUI builds) must be normalised to ``custom`` at read time so + downstream agent calls take the working path.""" + + def test_provider_local_normalised_to_custom(self, tmp_path, monkeypatch): + cfgfile = tmp_path / "config.yaml" + cfgfile.write_text( + "model:\n" + " default: qwen2.5-coder:14b\n" + " provider: local\n" + " base_url: http://127.0.0.1:11434/v1\n", + encoding="utf-8", + ) + monkeypatch.setattr(cfg, "_get_config_path", lambda: cfgfile) + cfg.reload_config() + try: + model_id, provider, base_url = cfg.resolve_model_provider("qwen2.5-coder:14b") + assert provider == "custom", ( + f"resolve_model_provider must rewrite legacy 'local' to 'custom', " + f"got {provider!r}" + ) + assert base_url == "http://127.0.0.1:11434/v1" + assert model_id == "qwen2.5-coder:14b" + finally: + cfg.reload_config() + + def test_provider_local_uppercase_also_normalised(self, tmp_path, monkeypatch): + """Case-insensitive match — YAML may have ``Local`` or ``LOCAL``.""" + cfgfile = tmp_path / "config.yaml" + cfgfile.write_text( + "model:\n" + " default: my-model\n" + " provider: Local\n" + " base_url: http://192.168.1.10:8000/v1\n", + encoding="utf-8", + ) + monkeypatch.setattr(cfg, "_get_config_path", lambda: cfgfile) + cfg.reload_config() + try: + _, provider, _ = cfg.resolve_model_provider("my-model") + assert provider == "custom" + finally: + cfg.reload_config() + + def test_other_providers_pass_through_unchanged(self, tmp_path, monkeypatch): + """The migration must not touch any other provider name.""" + cfgfile = tmp_path / "config.yaml" + cfgfile.write_text( + "model:\n" + " default: claude-sonnet-4.6\n" + " provider: anthropic\n", + encoding="utf-8", + ) + monkeypatch.setattr(cfg, "_get_config_path", lambda: cfgfile) + cfg.reload_config() + try: + _, provider, _ = cfg.resolve_model_provider("claude-sonnet-4.6") + assert provider == "anthropic" + finally: + cfg.reload_config() + + +# ── 3. set_hermes_default_model never persists 'local' ─────────────────── + + +class TestSetHermesDefaultModelNeverPersistsLocal: + """Even if a caller (or a stale resolver) hands us ``provider='local'``, + we must not write that value back to ``config.yaml``.""" + + def test_existing_local_provider_is_replaced_on_save(self, tmp_path, monkeypatch): + """Writing a new default model when previous_provider is 'local' + must persist 'custom' instead, because previous_provider falls + through into persisted_provider when resolve_model_provider doesn't + return a new provider hint.""" + import yaml + + cfgfile = tmp_path / "config.yaml" + cfgfile.write_text( + "model:\n" + " default: old-model\n" + " provider: local\n" + " base_url: http://127.0.0.1:11434/v1\n", + encoding="utf-8", + ) + monkeypatch.setattr(cfg, "_get_config_path", lambda: cfgfile) + cfg.reload_config() + try: + cfg.set_hermes_default_model("qwen2.5-coder:14b") + saved = yaml.safe_load(cfgfile.read_text(encoding="utf-8")) + persisted = saved.get("model", {}).get("provider", "") + assert persisted != "local", ( + f"set_hermes_default_model must rewrite 'local' on save — " + f"got {persisted!r}" + ) + assert persisted == "custom" + finally: + cfg.reload_config() + + +# ── 4. Alias table has the entry ───────────────────────────────────────── + + +class TestAliasTableHasLocalEntry: + """``_resolve_provider_alias`` must rewrite ``local`` → ``custom`` for + any other code path that normalises through the alias table.""" + + def test_local_alias_resolves_to_custom(self): + assert cfg._resolve_provider_alias("local") == "custom" + + def test_local_alias_case_insensitive(self): + assert cfg._resolve_provider_alias("LOCAL") == "custom" + assert cfg._resolve_provider_alias("Local") == "custom" + + def test_alias_table_contains_local_entry(self): + assert cfg._PROVIDER_ALIASES.get("local") == "custom", ( + "_PROVIDER_ALIASES must map 'local' → 'custom' for #1384" + )