fix(model_switch): clear key_env only when the route changes; drop it on custom activation
`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.
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user