From 5bccc4e2381448ffbb15640f8ad3b14de2fab7f2 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Mon, 14 Sep 2026 21:00:10 -0700 Subject: [PATCH] fix(model_switch): clear key_env only when the route changes; drop it on custom activation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `model.key_env` is not custom-only: the Desktop settings UI stores REGISTRY provider keys there (e.g. HERMES_CUSTOM_LMSTUDIO_API_KEY with provider lmstudio, #106336) and auth._model_level_key_env honours it. The previous predicate (`not custom or route_changed`) therefore wiped that pointer on a same-provider same-base_url model re-pick and silently broke the user's credential. The pointer now clears ONLY when provider or base_url changed; the inline api_key/api rule is unchanged. Custom-endpoint activation (model_setup_flows_custom) popped base_url / api_key but never key_env, so a stale pointer from a previous endpoint outranked the credential it had just written — pop it alongside. Tests: the same-route re-pick case now covers a registry provider (red on the old predicate), and the two clear_model_endpoint_credentials tests are folded into one invariant. --- hermes_cli/model_setup_flows_custom.py | 7 ++++ hermes_cli/model_switch.py | 21 +++++++----- .../test_model_persist_one_shape.py | 12 +++++++ ...test_update_config_clears_custom_fields.py | 33 +++++++------------ 4 files changed, 43 insertions(+), 30 deletions(-) diff --git a/hermes_cli/model_setup_flows_custom.py b/hermes_cli/model_setup_flows_custom.py index 7d296d97d4..fb250f6e3e 100644 --- a/hermes_cli/model_setup_flows_custom.py +++ b/hermes_cli/model_setup_flows_custom.py @@ -177,6 +177,9 @@ def _model_flow_custom(config): def _apply_endpoint(model: dict) -> None: model["provider"] = "custom" model["base_url"] = effective_url + # A previous endpoint's key_env pointer would outrank the credential written below. + model.pop("key_env", None) + model.pop("api_key_env", None) if custom_key_env: model["api_key"] = f"${{{custom_key_env}}}" if api_mode: @@ -384,6 +387,10 @@ def _model_flow_named_custom(config, provider_info): # Activate and save the model to the custom_providers entry _save_model_choice(model_name) cfg, model = _load_config_model_section() + # The endpoint being activated owns the credential: drop the previous endpoint's pointer + # (key_env outranks the provider entry's own key at resolution time). + model.pop("key_env", None) + model.pop("api_key_env", None) if provider_key: model["provider"] = custom_provider_slug(name, provider_key) model.pop("base_url", None) diff --git a/hermes_cli/model_switch.py b/hermes_cli/model_switch.py index 9976efb6a2..8d873ff765 100644 --- a/hermes_cli/model_switch.py +++ b/hermes_cli/model_switch.py @@ -1543,10 +1543,12 @@ def model_selection_config_updates(result: ModelSwitchResult, current_model_cfg: ``model.api_key`` is a leftover that would contaminate later custom resolution. For custom targets the inline key belongs to ONE endpoint: it survives only a same-route re-pick (same provider and base_url) — ``custom:a`` -> ``custom:b`` must not hand endpoint A's secret to B. - The ``key_env`` / ``api_key_env`` credential POINTER (written by custom-endpoint activation, - resolved by runtime_provider / auxiliary_client) clears under the same rule: left behind, it - routes the NEW provider's requests to the OLD endpoint's env var. The dashboard re-adds an - explicitly submitted key / the target provider's own pointer after this + The ``key_env`` / ``api_key_env`` credential POINTER (written by custom-endpoint activation + and, for REGISTRY providers too, by the Desktop settings UI (#106336); resolved by + runtime_provider / auxiliary_client / ``auth._model_level_key_env``) clears only when the + route changed: left behind it routes the NEW provider's requests to the OLD endpoint's env + var, but a same-provider same-base_url model re-pick keeps it whatever the provider is. The + dashboard re-adds an explicitly submitted key / the target provider's own pointer after this (``_apply_main_model_assignment`` / ``_resolve_assignment_credentials``).""" model_cfg = current_model_cfg if isinstance(current_model_cfg, dict) else {} updates: dict[str, Any] = { @@ -1560,10 +1562,13 @@ def model_selection_config_updates(result: ModelSwitchResult, current_model_cfg: model_cfg.get("base_url"), result.base_url, model_cfg.get("provider"), result.target_provider): updates["context_length"] = None target = str(result.target_provider or "").strip().lower() - if not target.startswith("custom") or _route_changed(model_cfg, result): - for key in ("api_key", "api", "key_env", "api_key_env"): - if key in model_cfg: - updates[key] = None + route_changed = _route_changed(model_cfg, result) + stale = ["api_key", "api"] if (not target.startswith("custom") or route_changed) else [] + if route_changed: + stale += ["key_env", "api_key_env"] + for key in stale: + if key in model_cfg: + updates[key] = None return updates diff --git a/tests/hermes_cli/test_model_persist_one_shape.py b/tests/hermes_cli/test_model_persist_one_shape.py index 6b9f32e93d..5e2bf0712c 100644 --- a/tests/hermes_cli/test_model_persist_one_shape.py +++ b/tests/hermes_cli/test_model_persist_one_shape.py @@ -149,6 +149,18 @@ def test_provider_switch_drops_the_key_env_pointer_but_a_same_route_repick_keeps persist_model_selection(same_route) assert _model_block(seeded_home)["key_env"] == "CUSTOM_BOX_API_KEY" + # Registry providers get the pointer too (Desktop stores e.g. HERMES_CUSTOM_LMSTUDIO_API_KEY + # as model.key_env with provider lmstudio, #106336): a same-route model re-pick keeps it. + (seeded_home / "config.yaml").write_text( + seed.replace("provider: custom\n", "provider: lmstudio\n") + .replace("CUSTOM_BOX_API_KEY", "HERMES_CUSTOM_LMSTUDIO_API_KEY"), encoding="utf-8") + persist_model_selection(ModelSwitchResult( + success=True, new_model="qwen3-8b", target_provider="lmstudio", + base_url="http://localhost:1234/v1", api_mode="openai_chat", is_global=True)) + block = _model_block(seeded_home) + assert block["default"] == "qwen3-8b" + assert block["key_env"] == "HERMES_CUSTOM_LMSTUDIO_API_KEY" + def test_gateway_persists_to_the_profile_config_it_was_given(tmp_path, monkeypatch): """Multiplexed gateway: the write lands in the routed profile's config.yaml, never the diff --git a/tests/hermes_cli/test_update_config_clears_custom_fields.py b/tests/hermes_cli/test_update_config_clears_custom_fields.py index d385ca58d9..b63f143ff5 100644 --- a/tests/hermes_cli/test_update_config_clears_custom_fields.py +++ b/tests/hermes_cli/test_update_config_clears_custom_fields.py @@ -66,33 +66,22 @@ class TestUpdateConfigForProviderClearsStaleCustomFields: assert "api_mode" not in model_cfg assert model_cfg["provider"] == "openrouter" - def test_clear_model_endpoint_credentials_removes_key_env_pointer(self): - # key_env is a live credential pointer (runtime_provider / - # auxiliary_client resolve it), written by custom-endpoint activation. - # Surviving a provider switch it routes the NEW provider's requests to - # the OLD endpoint's env var — it clears with the inline key. - model_cfg = { - "provider": "custom_myendpoint", - "default": "some-model", - "key_env": "CUSTOM_MYENDPOINT_API_KEY", - "api_key_env": "LEGACY_PTR", - } - - clear_model_endpoint_credentials(model_cfg) - - assert "key_env" not in model_cfg - assert "api_key_env" not in model_cfg - - def test_clear_api_key_false_preserves_key_env(self): - # The clear_api_key=False flavor (same-provider re-pick) must keep the - # pointer exactly as it keeps the inline key. - model_cfg = {"provider": "custom_x", "key_env": "CUSTOM_X_API_KEY", "api_mode": "openai"} + def test_clear_model_endpoint_credentials_treats_key_env_like_the_inline_key(self): + # key_env is a live credential pointer (runtime_provider / auxiliary_client resolve it). + # Surviving a provider switch it routes the NEW provider's requests to the OLD + # endpoint's env var, so it clears with the inline key — and, like the inline key, + # survives the clear_api_key=False flavor (same-provider re-pick). + model_cfg = {"provider": "custom_x", "key_env": "CUSTOM_X_API_KEY", + "api_key_env": "LEGACY_PTR", "api_mode": "openai"} clear_model_endpoint_credentials(model_cfg, clear_api_key=False) - assert model_cfg["key_env"] == "CUSTOM_X_API_KEY" assert "api_mode" not in model_cfg + clear_model_endpoint_credentials(model_cfg) + assert "key_env" not in model_cfg + assert "api_key_env" not in model_cfg + def test_switching_to_openrouter_clears_api_key_and_api_mode(self): _seed_custom_provider_config()