Fix setup wizard overwriting saved config choices

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #28882 fix(cli): preserve wizard config + honour custom-provider aliases (#22074, #27132)

  merged to main

  files: hermes_cli/runtime_provider.py, hermes_cli/setup.py, scripts/release.py, tests/hermes_cli/test_runtime_provider_resolution.py, tests/hermes_cli/test_setup_model_provider.py

  Show the diff

  • hermes_cli/runtime_provider.py

    @@ -47,7 +47,8 @@ def _config_base_url_trustworthy_for_bare_custom(cfg_base_url: str, cfg_provider
         """Decide whether ``model.base_url`` may back bare ``custom`` runtime resolution.
     
         GitHub #14676: the model picker can select Custom while ``model.provider`` still reflects a
    -    previous provider. Reject non-loopback URLs unless the YAML provider is already ``custom``,
    +    previous provider. Reject non-loopback URLs unless the YAML provider is already ``custom``
    +    (or one of the local-server aliases that resolve to ``custom`` — ollama, vllm, llamacpp, …),
         so a stale OpenRouter/Z.ai base_url cannot hijack local ``custom`` sessions.
         """
         cfg_provider_norm = (cfg_provider or "").strip().lower()
    @@ -56,6 +57,17 @@ def _config_base_url_trustworthy_for_bare_custom(cfg_base_url: str, cfg_provider
             return False
         if cfg_provider_norm == "custom":
             return True
    +    # GitHub #27132: provider aliases that resolve to "custom" at runtime
    +    # (ollama, vllm, llamacpp, …) should be trusted the same way "custom"
    +    # is, otherwise a legit LAN/WireGuard ollama endpoint silently falls
    +    # through to OpenRouter.
    +    try:
    +        from hermes_cli.auth import resolve_provider as _resolve_provider
    +
    +        if _resolve_provider(cfg_provider_norm) == "custom":
    +            return True
    +    except Exception:
    +        pass
         if base_url_host_matches(bu, "openrouter.ai"):
             return False
         return _loopback_hostname(base_url_hostname(bu))
    @@ -547,7 +559,20 @@ def _resolve_named_custom_runtime(
         # Bare `provider="custom"` with an explicit base_url (e.g. propagated
         # from a `model_aliases:` direct-alias resolution) — build a runtime
         # directly so the alias's base_url actually takes effect.
    +    #
    +    # GitHub #27132: provider aliases that resolve to "custom" at runtime
    +    # (ollama, vllm, llamacpp, …) are treated identically here, so a YAML
    +    # `provider: ollama` with a LAN/WireGuard `base_url` doesn't silently
    +    # fall through to OpenRouter.
         requested_norm = (requested_provider or "").strip().lower()
    +    if requested_norm and requested_norm != "custom":
    +        try:
    +            from hermes_cli.auth import resolve_provider as _resolve_provider
    +
    +            if _resolve_provider(requested_norm) == "custom":
    +                requested_norm = "custom"
    +        except Exception:
    +            pass
         if requested_norm == "custom" and explicit_base_url:
             base_url = explicit_base_url.strip().rstrip("/")
             # Check credential pool first — mirrors the named-custom-provider path
    @@ -638,6 +663,19 @@ def _resolve_openrouter_runtime(
                 break
         requested_norm = (requested_provider or "").strip().lower()
         cfg_provider = cfg_provider.strip().lower()
    +    # GitHub #27132: provider aliases that resolve to "custom" (ollama,
    +    # vllm, llamacpp, …) follow the same base_url trust + routing rules
    +    # as a bare `provider: custom`. Normalising here keeps every check
    +    # below — `requested_norm == "custom"`, the trust check, the pool
    +    # gate up the stack — alias-aware without duplicating the alias map.
    +    if requested_norm and requested_norm != "custom":
    +        try:
    +            from hermes_cli.auth import resolve_provider as _resolve_provider
    +
    +            if _resolve_provider(requested_norm) == "custom":
    +                requested_norm = "custom"
    +        except Exception:
    +            pass
     
         env_openrouter_base_url = os.getenv("OPENROUTER_BASE_URL", "").strip()
         env_custom_base_url = os.getenv("CUSTOM_BASE_URL", "").strip()

  • hermes_cli/setup.py

    @@ -820,13 +820,12 @@ def setup_model_provider(config: dict, *, quick: bool = False):
         # Re-sync the wizard's config dict from what cmd_model saved to disk.
         # This is critical: cmd_model writes to disk via its own load/save cycle,
         # and the wizard's final save_config(config) must not overwrite those
    -    # changes with stale values (#4172).
    +    # changes with stale values (#4172). Refresh the dict in place so callers
    +    # that keep the same object see every section the shared model picker may
    +    # have changed (model, custom_providers, auxiliary, provider metadata, etc.).
         _refreshed = load_config()
    -    config["model"] = _refreshed.get("model", config.get("model"))
    -    if "custom_providers" in _refreshed:
    -        config["custom_providers"] = _refreshed["custom_providers"]
    -    else:
    -        config.pop("custom_providers", None)
    +    config.clear()
    +    config.update(_refreshed)
     
         # Derive the selected provider for downstream steps (vision setup).
         selected_provider = None

  • scripts/release.py

    @@ -1145,6 +1145,7 @@
         "[email protected]": "LifeJiggy",  # stale salvage commit alias (PR #28315)
         "[redacted:email]": "nekwo",  # PR #26481 (PS5.1 UTF-8 BOM)
         "[email protected]": "nekwo",
    +    "[email protected]": "stepanov1975",  # PR #22074 (setup config picker writes)
         "[email protected]": "flooryyyy",  # PR #26374 (tool_trace error detection)
         "[email protected]": "dgians",  # PR #26034 (.ts/.py/.sh docs types)
         "[redacted:email]": "dgians",  # PR #26034 (bot-committed by zealy-tzco under dgians' PR)

  • tests/hermes_cli/test_runtime_provider_resolution.py

    @@ -2321,3 +2321,74 @@ def select(self):
         assert resolved["provider"] == "minimax-oauth"
         assert resolved["api_mode"] == "anthropic_messages"
         assert resolved["base_url"] == "https://api.minimax.io/anthropic"
    +
    +
    +# ----------------------------------------------------------------------
    +# GitHub #27132 — provider aliases (ollama/vllm/llamacpp/llama-cpp) must
    +# follow the same base_url trust + routing rules as bare `provider: custom`.
    +# Without this, a YAML `provider: ollama` with a LAN/WireGuard `base_url`
    +# silently falls through to OpenRouter (HTTP 401).
    +# ----------------------------------------------------------------------
    +
    +
    [redacted:email](
    +    "alias,base_url",
    +    [
    +        ("ollama", "http://192.168.0.103:11434/v1"),
    +        ("vllm", "http://192.168.0.103:8000/v1"),
    +        ("llamacpp", "http://192.168.0.103:8080/v1"),
    +        ("llama-cpp", "http://192.168.0.103:8080/v1"),
    +    ],
    +)
    +def test_custom_aliases_with_lan_base_url_route_to_custom_not_openrouter(
    +    monkeypatch, alias, base_url
    +):
    +    """provider: ollama|vllm|llamacpp + LAN IP must NOT fall through to OpenRouter."""
    +    monkeypatch.setattr(
    +        rp,
    +        "_get_model_config",
    +        lambda: {"provider": alias, "base_url": base_url},
    +    )
    +    # Pretend OPENROUTER_API_KEY is set so the openrouter fallback would
    +    # otherwise succeed — we want to prove the alias short-circuits before
    +    # reaching it.
    +    monkeypatch.setenv("OPENROUTER_API_KEY", "sk-or-fake-test")
    +    # No custom credential pool — exercise the bare-alias path.
    +    monkeypatch.setattr(rp, "load_pool", lambda provider: None)
    +
    +    resolved = rp.resolve_runtime_provider()
    +
    +    assert resolved["provider"] == "custom", (
    +        f"alias {alias!r} with LAN base_url should resolve to provider=custom, "
    +        f"got {resolved['provider']!r}"
    +    )
    +    assert resolved["base_url"] == base_url.rstrip("/"), (
    +        f"base_url should be the configured LAN endpoint, got {resolved['base_url']!r}"
    +    )
    +
    +
    +def test_custom_alias_with_loopback_base_url_routes_to_custom(monkeypatch):
    +    """provider: ollama + loopback should also route to custom (regression guard)."""
    +    monkeypatch.setattr(
    +        rp,
    +        "_get_model_config",
    +        lambda: {"provider": "ollama", "base_url": "http://localhost:11434/v1"},
    +    )
    +    monkeypatch.setenv("OPENROUTER_API_KEY", "sk-or-fake-test")
    +    monkeypatch.setattr(rp, "load_pool", lambda provider: None)
    +
    +    resolved = rp.resolve_runtime_provider()
    +
    +    assert resolved["provider"] == "custom"
    +    assert resolved["base_url"] == "http://localhost:11434/v1"
    +
    +
    +def test_trustworthy_check_accepts_custom_aliases():
    +    """_config_base_url_trustworthy_for_bare_custom() must accept aliases for custom."""
    +    fn = rp._config_base_url_trustworthy_for_bare_custom
    +    for alias in ("ollama", "vllm", "llamacpp", "llama-cpp", "llama.cpp"):
    +        assert fn("http://192.168.0.103:11434/v1", alias) is True, (
    +            f"alias {alias!r} should be trusted with non-loopback base_url"
    +        )
    +    # Unrelated provider name should still be rejected with non-loopback URL.
    +    assert fn("http://192.168.0.103:11434/v1", "openrouter") is False

  • tests/hermes_cli/test_setup_model_provider.py

    @@ -63,6 +63,38 @@ def _write_model_config(provider, base_url="", model_name="test-model"):
         save_config(cfg)
     
     
    +def _write_aux_config(task="compression", provider="gemini", model_name="gemini-2.5-flash"):
    +    """Simulate the aux picker writing a task override to disk."""
    +    cfg = load_config()
    +    aux = cfg.setdefault("auxiliary", {})
    +    entry = aux.setdefault(task, {})
    +    entry["provider"] = provider
    +    entry["model"] = model_name
    +    save_config(cfg)
    +
    +
    +def test_setup_model_provider_preserves_auxiliary_choices_written_by_picker(tmp_path, monkeypatch):
    +    """Aux choices made inside hermes setup must survive the wizard's final save."""
    +    monkeypatch.setenv("HERMES_HOME", str(tmp_path))
    +    _clear_provider_env(monkeypatch)
    +
    +    config = load_config()
    +    assert config["auxiliary"]["compression"]["provider"] == "auto"
    +
    +    def fake_select():
    +        _write_aux_config("compression", "gemini", "gemini-2.5-flash")
    +
    +    monkeypatch.setattr("hermes_cli.main.select_provider_and_model", fake_select)
    +
    +    setup_model_provider(config, quick=True)
    +    save_config(config)  # mirrors run_setup_wizard(section="model") final save
    +
    +    reloaded = load_config()
    +    compression = reloaded["auxiliary"]["compression"]
    +    assert compression["provider"] == "gemini"
    +    assert compression["model"] == "gemini-2.5-flash"
    +
    +
     def test_setup_keep_current_custom_from_config_does_not_fall_through(tmp_path, monkeypatch):
         """Keep-current custom should not fall through to the generic model menu."""
         monkeypatch.setenv("HERMES_HOME", str(tmp_path))