Review — end-to-end ✅ (clean approve, three-layer fix verified against upstream)

Self-authored fix for #1384 — the "Provider 'local' is set in config.yaml but no API key was found" mid-conversation crash for local-model users. Implements the issue's recommended Fix C (WebUI defensive) plus two heal-existing-state layers. +33 LOC in api/config.py, 9 regression tests.

Single commit

c221d33  fix(#1384): heal 'provider: local' mid-conversation crash for local-model users

What this ships — three-layer fix

The bug: WebUI's auto-detect block at api/config.py:1745-1782 writes provider = "local" to config.yaml for unknown loopback hosts (e.g. 192.168.1.10:8080, llama.cpp on 127.0.0.1:8080, vLLM, TabbyAPI, custom proxies). But "local" is not registered in hermes_cli.auth.PROVIDER_REGISTRY. The main agent works because it reads model.base_url + model.api_key directly. But once auto-compression / vision / web-extract fires and routes through the auxiliary client, /tmp/hermes-agent-fresh/agent/auxiliary_client.py:3336-3342 raises:

_explicit = (resolved_provider or "").strip().lower()
if _explicit and _explicit not in ("auto", "openrouter", "custom"):
    raise RuntimeError(
        f"Provider '{_explicit}' is set in config.yaml but no API key was found. "
        f"Set the {_explicit.upper()}_API_KEY environment variable, ..."
    )

"local" is not in the safe-pass list ("auto", "openrouter", "custom") → raise → user perceives "the chat suddenly broke after a few messages."

The fix routes through "custom" instead, which:

1. IS in the safe-pass list at three sites: agent/auxiliary_client.py:3337, auxiliary_client.py:3647, run_agent.py:1466.
2. Has an explicit handler at agent/auxiliary_client.py:2115-2147 that takes the "no-key-required" OpenAI-compat path for explicit base_url. Verified the handler's exact behavior at line 2118-2122: custom_key = explicit_api_key or OPENAI_API_KEY env or "no-key-required".

Three layers so users already in the broken state heal automatically:

Layer 1 — Stop creating new broken state at api/config.py:1758,1770-1780: the auto-detect block now starts with provider = "custom" as the default at line 1758, AND the explicit else branch at line 1780 reaffirms provider = "custom". The else is technically a no-op (since the default is already "custom"), but it's explicit for documentation purposes — clearly says "unknown loopback host → custom".

Layer 2 — Heal existing broken configs at read time at api/config.py:980-988:

# Heal legacy ``provider: local`` entries (written by WebUI < v0.50.252)
# at read time. ``local`` is not a registered provider...
if isinstance(config_provider, str) and config_provider.strip().lower() == "local":
    config_provider = "custom"

This fires on every resolve_model_provider() call, used by streaming.py:1704 on every chat turn. Existing broken configs heal without the user editing config.yaml. Case-insensitive (Local, LOCAL both healed).

Layer 3 — Refuse to persist "local" on save at api/config.py:1209-1210:

if persisted_provider.lower() == "local":
    persisted_provider = "custom"

Backstop that fires when previous_provider == "local" falls through (because resolved_provider returned None). Plus an alias entry at api/config.py:622: "local": "custom" in _PROVIDER_ALIASES.

Traced against upstream hermes-agent

Verified:

• Custom branch IS the no-key-required path: agent/auxiliary_client.py:2115-2147 — custom_key = ... or "no-key-required" at line 2121.
• All three LOCAL_API_KEY raise sites whitelist "custom": lines 3337, 3647 in auxiliary_client.py, line 1466 in run_agent.py.
• Upstream _PROVIDER_ALIASES has no "local" entry: /tmp/hermes-agent-fresh/hermes_cli/models.py:813. The webui's local entry is the only thing that maps local → custom for the _resolve_provider_alias function. Verified the lookup chain at api/config.py:640-645: tries agent's table first (no hit for local), then falls through to webui's local table (local → custom). ✅

End-to-end trace — both directions

Forward (preventing new state):

1. User sets model.base_url: http://192.168.1.10:8080/v1 (custom non-loopback proxy or unfamiliar local URL).
2. _build_configured_model_badges() runs → urlparse → checks if addr.is_private/loopback/link_local.
3. Hostname matching: "ollama" not in host, "127.0.0.1" not in host, "localhost" not in host, "lmstudio" not in host → falls to else.
4. provider = "custom" (line 1780). Persisted. ✅

Reverse (healing existing state):

1. User has pre-existing config.yaml with provider: local.
2. User sends a chat → streaming.py:1704 calls resolve_model_provider(model).
3. Line 987-988: config_provider = "local" → healed to "custom".
4. Returns (model, "custom", base_url).
5. Line 1711: resolve_runtime_provider(requested="custom") — agent's resolver takes the safe path.
6. Line 1771: AIAgent(provider="custom", ...) — main agent uses explicit base_url.
7. Auto-compression fires at some point → auxiliary client → resolve_provider_client("custom", ...) → no-key-required path. ✅

Save round-trip:

1. User picks a model in dropdown → set_hermes_default_model("qwen").
2. Reads YAML directly: previous_provider = "local".
3. Calls resolve_model_provider("qwen") → returns resolved_provider = "custom" (Layer 2 heal).
4. persisted_provider = "custom" (resolved_provider takes precedence).
5. Layer 3 backstop at line 1209-1210 fires only if persisted_provider somehow ended up as "local" (defense in depth).
6. Writes provider: custom to YAML. ✅

Cross-tool consistency

• ✅ provider: custom is honored by both webui and agent CLI — it's a canonical value in PROVIDER_REGISTRY and has explicit handlers.
• ✅ base_url and api_key round-trip cleanly — webui preserves them; agent reads them via resolve_runtime_provider.
• ✅ Frontend dropdown still works — _resolve_provider_alias("local") returns "custom", so any badge/dropdown logic that normalizes through the alias correctly maps to the custom group.
• ✅ CLI users with provider: local in their YAML — the issue notes the WebUI fix doesn't address the underlying agent gap (Fix A/B). CLI users still hit the bug, but this PR is correctly scoped to webui-only relief. CLI fix belongs in a separate hermes-agent PR.

Security audit

• ✅ No new endpoints, no new env vars, no new file-serving surface.
• ✅ No SSRF surface added — the auto-detect block already handled URL parsing; this PR only changes the result string.
• ✅ Path traversal: "local" and "custom" are static strings; no user input flows through.
• ✅ Whitespace + case handling — config_provider.strip().lower() == "local" correctly tolerates "Local", "LOCAL", "local ", etc.
• ✅ isinstance(config_provider, str) guard at line 987 prevents AttributeError when provider is None/int/etc.

Edge-case matrix

Tests

• test_issue1384_local_provider.py — 9/9 pass:
  • 2 source-code invariants (no provider = "local" literal; auto-detect else assigns "custom").
  • 3 read-time healing (lowercase, mixed-case, pass-through for non-local providers).
  • 1 round-trip save invariant (set_hermes_default_model with previous_provider="local" persists "custom").
  • 3 alias-table tests (resolution, case-insensitive, raw entry presence).
• Full suite: 3443 passed, 54 skipped, 3 xpassed, 0 failed in 16.82s on c221d33.
• CI: No checks attached (branch was just pushed). Mirrors what the PR description claims (3495 passing on the bot's machine — counting drift consistent with prior PRs).

Other audit — confirmed correct

• ✅ Backwards compat — any existing provider: custom configs still work exactly as before.
• ✅ Forward compat — if upstream agent ever adds "local" as a real provider with its own credentials, the webui's alias-rewrite would mask it. Worth removing the alias if/when that happens. Not a current concern.
• ✅ No mutation of cfg global — resolve_model_provider mutates only the local config_provider variable, not the in-memory _cfg_cache. So subsequent calls re-heal on every read; no torn state.
• ✅ set_hermes_default_model reads YAML directly at line 1184 (_load_yaml_config_file(config_path)) — bypasses _cfg_cache. So even if cache had stale "local", the save path sees fresh disk content. Then layer 3 backstop covers the edge case.

Minor observations (non-blocking)

• Layer 1's else: provider = "custom" is a no-op redundancy — the default at line 1758 already sets provider = "custom". The else branch is purely documentation. Worth keeping for future readers; not worth removing.
• The auto-detect block isn't directly tested behaviorally — only via source-level regex. A test that constructs a config with base_url: http://192.168.1.10:8080/v1, runs _build_configured_model_badges, and asserts the resulting badge has provider: "custom" would lock the auto-detect path end-to-end. Out of scope; the source-level regex catches the most likely regression (someone reverting to provider = "local").
• Upstream PR for Fix A/B — the issue notes that CLI users (without the WebUI in front) still hit the bug. A complementary hermes-agent PR adding local/lmstudio/ollama handlers in resolve_provider_client() would close the underlying gap. Out of scope here; flagged in the PR description.
• _PROVIDER_ALIASES table grows — the local copy now has 30+ entries. Most are 1:1 cosmetic aliases that should ideally live upstream. Future cleanup, not blocking.
• Comment refers to "WebUI < v0.50.252" at line 980 — assumes the next release will be 0.50.252. The release agent will pick the actual number. Worth updating after merge if it lands as something else.

Recommendation

✅ Approved. Three-layer fix correctly heals both new and existing broken state. Cross-checked against upstream that "custom" is the canonical safe-pass value in all three LOCAL_API_KEY raise sites and takes the no-key-required OpenAI-compat path in resolve_provider_client. The auto-detect block now never produces "local", the read path heals it, the save path refuses to persist it, and the alias table catches any other normalizer.

Parked at approval — ready for the release agent's merge/tag pipeline.