mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-21 10:22:18 +00:00
fix(model): clear stale endpoint credentials across switches
This commit is contained in:
parent
95a3affc2e
commit
c253b07380
9 changed files with 187 additions and 17 deletions
|
|
@ -34,7 +34,7 @@ from agent.i18n import t
|
|||
from gateway.config import HomeChannel, Platform, PlatformConfig
|
||||
from gateway.platforms.base import EphemeralReply, MessageEvent, MessageType
|
||||
from gateway.session import SessionSource, build_session_key
|
||||
from hermes_cli.config import cfg_get
|
||||
from hermes_cli.config import cfg_get, clear_model_endpoint_credentials
|
||||
from utils import (
|
||||
atomic_json_write,
|
||||
atomic_yaml_write,
|
||||
|
|
@ -1239,6 +1239,8 @@ class GatewaySlashCommandsMixin:
|
|||
_persist_model_cfg["provider"] = result.target_provider
|
||||
if result.base_url:
|
||||
_persist_model_cfg["base_url"] = result.base_url
|
||||
if str(result.target_provider or "").strip().lower() != "custom":
|
||||
clear_model_endpoint_credentials(_persist_model_cfg)
|
||||
from hermes_cli.config import save_config
|
||||
save_config(_persist_cfg)
|
||||
except Exception as e:
|
||||
|
|
@ -1429,6 +1431,8 @@ class GatewaySlashCommandsMixin:
|
|||
model_cfg["provider"] = result.target_provider
|
||||
if result.base_url:
|
||||
model_cfg["base_url"] = result.base_url
|
||||
if str(result.target_provider or "").strip().lower() != "custom":
|
||||
clear_model_endpoint_credentials(model_cfg)
|
||||
from hermes_cli.config import save_config
|
||||
save_config(cfg)
|
||||
except Exception as e:
|
||||
|
|
|
|||
|
|
@ -6386,16 +6386,12 @@ def _update_config_for_provider(
|
|||
# Clear stale base_url to prevent contamination when switching providers
|
||||
model_cfg.pop("base_url", None)
|
||||
|
||||
# Clear stale api_key/api_mode left over from a previous custom provider.
|
||||
# When the user switches from e.g. a MiniMax custom endpoint
|
||||
# (api_mode=anthropic_messages, api_key=mxp-...) to a built-in provider
|
||||
# (e.g. OpenRouter), the stale api_key/api_mode would override the new
|
||||
# provider's credentials and transport choice. Built-in providers that
|
||||
# need a specific api_mode (copilot, xai) set it at request-resolution
|
||||
# time via `_copilot_runtime_api_mode` / `_detect_api_mode_for_url`, so
|
||||
# removing the persisted value here is safe.
|
||||
model_cfg.pop("api_key", None)
|
||||
model_cfg.pop("api_mode", None)
|
||||
# Clear stale endpoint credentials left over from a previous custom provider.
|
||||
# Built-in providers resolve credentials from env/auth state, not inline
|
||||
# model.api_key.
|
||||
from hermes_cli.config import clear_model_endpoint_credentials
|
||||
|
||||
clear_model_endpoint_credentials(model_cfg)
|
||||
|
||||
# When switching to a non-OpenRouter provider, ensure model.default is
|
||||
# valid for the new provider. An OpenRouter-formatted name like
|
||||
|
|
|
|||
|
|
@ -3915,6 +3915,30 @@ def _set_nested(config, dotted_key: str, value):
|
|||
current[last] = value
|
||||
|
||||
|
||||
def clear_model_endpoint_credentials(
|
||||
model_cfg: Dict[str, Any],
|
||||
*,
|
||||
clear_api_key: bool = True,
|
||||
clear_api_mode: bool = True,
|
||||
) -> Dict[str, Any]:
|
||||
"""Remove stale inline endpoint credentials from a model config.
|
||||
|
||||
``model.api_key`` is valid only for explicit custom endpoint assignments.
|
||||
Built-in providers resolve credentials from env vars, auth.json, or the
|
||||
credential pool. When switching away from a custom endpoint, leaving these
|
||||
fields behind keeps secrets in config.yaml and can contaminate later custom
|
||||
resolution paths.
|
||||
"""
|
||||
if not isinstance(model_cfg, dict):
|
||||
return model_cfg
|
||||
if clear_api_key:
|
||||
model_cfg.pop("api_key", None)
|
||||
model_cfg.pop("api", None)
|
||||
if clear_api_mode:
|
||||
model_cfg.pop("api_mode", None)
|
||||
return model_cfg
|
||||
|
||||
|
||||
def get_missing_config_fields() -> List[Dict[str, Any]]:
|
||||
"""
|
||||
Check which config fields are missing or outdated (recursive).
|
||||
|
|
|
|||
|
|
@ -24,6 +24,8 @@ import argparse
|
|||
import os
|
||||
import subprocess
|
||||
|
||||
from hermes_cli.config import clear_model_endpoint_credentials
|
||||
|
||||
|
||||
def _prompt_auth_credentials_choice(title: str) -> str:
|
||||
"""Prompt for reuse / reauthenticate / cancel with the standard radio UI.
|
||||
|
|
@ -123,6 +125,7 @@ def _model_flow_openrouter(config, current_model=""):
|
|||
model["provider"] = "openrouter"
|
||||
model["base_url"] = OPENROUTER_BASE_URL
|
||||
model["api_mode"] = "chat_completions"
|
||||
clear_model_endpoint_credentials(model, clear_api_mode=False)
|
||||
save_config(cfg)
|
||||
deactivate_provider()
|
||||
print(f"Default model set to: {selected} (via OpenRouter)")
|
||||
|
|
@ -341,6 +344,7 @@ def _model_flow_nous(config, current_model="", args=None):
|
|||
model_cfg["base_url"] = inference_url.rstrip("/")
|
||||
else:
|
||||
model_cfg.pop("base_url", None)
|
||||
clear_model_endpoint_credentials(model_cfg)
|
||||
config["model"] = model_cfg
|
||||
# Clear any custom endpoint that might conflict
|
||||
if get_env_value("OPENAI_BASE_URL"):
|
||||
|
|
@ -1249,6 +1253,7 @@ def _model_flow_azure_foundry(config, current_model=""):
|
|||
model["api_mode"] = api_mode
|
||||
model["default"] = effective_model
|
||||
model["auth_mode"] = auth_mode_label
|
||||
clear_model_endpoint_credentials(model, clear_api_mode=False)
|
||||
if use_entra:
|
||||
# Persist only the non-default Entra scope so config.yaml stays tidy.
|
||||
# Azure identity selection stays in standard AZURE_* env vars.
|
||||
|
|
@ -1670,6 +1675,7 @@ def _model_flow_copilot(config, current_model=""):
|
|||
catalog=catalog,
|
||||
api_key=api_key,
|
||||
)
|
||||
clear_model_endpoint_credentials(model, clear_api_mode=False)
|
||||
if selected_effort is not None:
|
||||
_set_reasoning_effort(cfg, selected_effort)
|
||||
save_config(cfg)
|
||||
|
|
@ -1795,6 +1801,7 @@ def _model_flow_copilot_acp(config, current_model=""):
|
|||
model["provider"] = provider_id
|
||||
model["base_url"] = effective_base
|
||||
model["api_mode"] = "chat_completions"
|
||||
clear_model_endpoint_credentials(model, clear_api_mode=False)
|
||||
save_config(cfg)
|
||||
deactivate_provider()
|
||||
|
||||
|
|
@ -1884,6 +1891,7 @@ def _model_flow_kimi(config, current_model=""):
|
|||
model["provider"] = provider_id
|
||||
model["base_url"] = effective_base
|
||||
model.pop("api_mode", None) # let runtime auto-detect from URL
|
||||
clear_model_endpoint_credentials(model, clear_api_mode=False)
|
||||
save_config(cfg)
|
||||
deactivate_provider()
|
||||
|
||||
|
|
@ -1997,6 +2005,7 @@ def _model_flow_stepfun(config, current_model=""):
|
|||
model["provider"] = provider_id
|
||||
model["base_url"] = effective_base
|
||||
model.pop("api_mode", None)
|
||||
clear_model_endpoint_credentials(model, clear_api_mode=False)
|
||||
save_config(cfg)
|
||||
deactivate_provider()
|
||||
|
||||
|
|
@ -2080,6 +2089,7 @@ def _model_flow_bedrock_api_key(config, region, current_model=""):
|
|||
model["provider"] = "custom"
|
||||
model["base_url"] = mantle_base_url
|
||||
model.pop("api_mode", None) # chat_completions is the default
|
||||
clear_model_endpoint_credentials(model, clear_api_mode=False)
|
||||
|
||||
# Also save region in bedrock config for reference
|
||||
bedrock_cfg = cfg.get("bedrock", {})
|
||||
|
|
@ -2273,6 +2283,7 @@ def _model_flow_bedrock(config, current_model=""):
|
|||
model["provider"] = "bedrock"
|
||||
model["base_url"] = f"https://bedrock-runtime.{region}.amazonaws.com"
|
||||
model.pop("api_mode", None) # bedrock_converse is auto-detected
|
||||
clear_model_endpoint_credentials(model, clear_api_mode=False)
|
||||
|
||||
bedrock_cfg = cfg.get("bedrock", {})
|
||||
if not isinstance(bedrock_cfg, dict):
|
||||
|
|
@ -2566,6 +2577,7 @@ def _model_flow_api_key_provider(config, provider_id, current_model=""):
|
|||
cfg["model"] = model
|
||||
model["provider"] = provider_id
|
||||
model["base_url"] = effective_base
|
||||
clear_model_endpoint_credentials(model, clear_api_mode=False)
|
||||
if provider_id in {"opencode-zen", "opencode-go"}:
|
||||
model["api_mode"] = opencode_model_api_mode(provider_id, selected)
|
||||
else:
|
||||
|
|
@ -2720,6 +2732,7 @@ def _model_flow_anthropic(config, current_model=""):
|
|||
cfg["model"] = model
|
||||
model["provider"] = "anthropic"
|
||||
model.pop("base_url", None)
|
||||
clear_model_endpoint_credentials(model)
|
||||
save_config(cfg)
|
||||
deactivate_provider()
|
||||
|
||||
|
|
|
|||
|
|
@ -48,6 +48,7 @@ from hermes_cli.config import (
|
|||
cfg_get,
|
||||
DEFAULT_CONFIG,
|
||||
OPTIONAL_ENV_VARS,
|
||||
clear_model_endpoint_credentials,
|
||||
get_config_path,
|
||||
get_env_path,
|
||||
get_hermes_home,
|
||||
|
|
@ -901,8 +902,11 @@ def _apply_main_model_assignment(
|
|||
# same-provider re-pick so re-selecting a model doesn't wipe the key.
|
||||
if api_key.strip():
|
||||
model_cfg["api_key"] = api_key.strip()
|
||||
model_cfg.pop("api", None)
|
||||
elif model_cfg.get("api_key") and new_provider != prev_provider:
|
||||
model_cfg["api_key"] = ""
|
||||
clear_model_endpoint_credentials(model_cfg, clear_api_mode=False)
|
||||
if new_provider != prev_provider:
|
||||
clear_model_endpoint_credentials(model_cfg, clear_api_key=False)
|
||||
model_cfg.pop("context_length", None)
|
||||
return model_cfg
|
||||
|
||||
|
|
@ -3871,6 +3875,8 @@ def _apply_model_assignment_sync(
|
|||
slot_cfg = {}
|
||||
slot_cfg["provider"] = "auto"
|
||||
slot_cfg["model"] = ""
|
||||
slot_cfg.pop("base_url", None)
|
||||
clear_model_endpoint_credentials(slot_cfg)
|
||||
aux[slot] = slot_cfg
|
||||
cfg["auxiliary"] = aux
|
||||
save_config(cfg)
|
||||
|
|
@ -3886,8 +3892,13 @@ def _apply_model_assignment_sync(
|
|||
slot_cfg = aux.get(slot)
|
||||
if not isinstance(slot_cfg, dict):
|
||||
slot_cfg = {}
|
||||
prev_provider = str(slot_cfg.get("provider") or "").strip().lower()
|
||||
new_provider = provider.strip().lower()
|
||||
slot_cfg["provider"] = provider
|
||||
slot_cfg["model"] = model
|
||||
if new_provider != prev_provider and new_provider != "custom":
|
||||
slot_cfg.pop("base_url", None)
|
||||
clear_model_endpoint_credentials(slot_cfg)
|
||||
aux[slot] = slot_cfg
|
||||
|
||||
cfg["auxiliary"] = aux
|
||||
|
|
|
|||
|
|
@ -378,6 +378,99 @@ def test_model_flow_nous_does_not_restore_stale_custom_api_key(tmp_path, monkeyp
|
|||
assert "api_mode" not in model
|
||||
|
||||
|
||||
def _seed_stale_custom_model(tmp_path, monkeypatch):
|
||||
import yaml
|
||||
|
||||
config_home = tmp_path / "hermes"
|
||||
config_home.mkdir()
|
||||
monkeypatch.setenv("HERMES_HOME", str(config_home))
|
||||
config_path = config_home / "config.yaml"
|
||||
config_path.write_text(
|
||||
yaml.safe_dump(
|
||||
{
|
||||
"model": {
|
||||
"provider": "custom",
|
||||
"default": "glm-5.2",
|
||||
"base_url": "https://api.neuralwatt.com/v1",
|
||||
"api_key": "${NEURALWATT_API_KEY}",
|
||||
"api": "legacy-stale-key",
|
||||
"api_mode": "anthropic_messages",
|
||||
}
|
||||
},
|
||||
sort_keys=False,
|
||||
)
|
||||
)
|
||||
(config_home / ".env").write_text("")
|
||||
return config_path
|
||||
|
||||
|
||||
def test_model_flow_openrouter_clears_stale_custom_key(tmp_path, monkeypatch):
|
||||
import yaml
|
||||
|
||||
config_path = _seed_stale_custom_model(tmp_path, monkeypatch)
|
||||
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.main._prompt_api_key",
|
||||
lambda *args, **kwargs: ("sk-openrouter", False),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.models.model_ids",
|
||||
lambda **kwargs: ["anthropic/claude-sonnet-4.6"],
|
||||
)
|
||||
monkeypatch.setattr("hermes_cli.models.get_pricing_for_provider", lambda *a, **k: {})
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.auth._prompt_model_selection",
|
||||
lambda *args, **kwargs: "anthropic/claude-sonnet-4.6",
|
||||
)
|
||||
monkeypatch.setattr("hermes_cli.auth.deactivate_provider", lambda: None)
|
||||
|
||||
hermes_main._model_flow_openrouter({}, current_model="glm-5.2")
|
||||
|
||||
config = yaml.safe_load(config_path.read_text()) or {}
|
||||
model = config["model"]
|
||||
assert model["provider"] == "openrouter"
|
||||
assert model["default"] == "anthropic/claude-sonnet-4.6"
|
||||
assert model["api_mode"] == "chat_completions"
|
||||
assert "api_key" not in model
|
||||
assert "api" not in model
|
||||
|
||||
|
||||
def test_model_flow_anthropic_clears_stale_custom_key_and_mode(tmp_path, monkeypatch):
|
||||
import yaml
|
||||
|
||||
config_path = _seed_stale_custom_model(tmp_path, monkeypatch)
|
||||
|
||||
monkeypatch.setattr("hermes_cli.auth.get_anthropic_key", lambda: "sk-ant-api03-test")
|
||||
monkeypatch.setattr(
|
||||
"agent.anthropic_adapter.read_claude_code_credentials",
|
||||
lambda: None,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"agent.anthropic_adapter.is_claude_code_token_valid",
|
||||
lambda creds: False,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.model_setup_flows._prompt_auth_credentials_choice",
|
||||
lambda title: "use",
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.auth._prompt_model_selection",
|
||||
lambda *args, **kwargs: "claude-sonnet-4-6",
|
||||
)
|
||||
monkeypatch.setattr("hermes_cli.auth.deactivate_provider", lambda: None)
|
||||
|
||||
hermes_main._model_flow_anthropic({}, current_model="glm-5.2")
|
||||
|
||||
config = yaml.safe_load(config_path.read_text()) or {}
|
||||
model = config["model"]
|
||||
assert model["provider"] == "anthropic"
|
||||
assert model["default"] == "claude-sonnet-4-6"
|
||||
assert "base_url" not in model
|
||||
assert "api_key" not in model
|
||||
assert "api" not in model
|
||||
assert "api_mode" not in model
|
||||
|
||||
|
||||
def test_model_flow_nous_offers_tool_gateway_prompt_when_unconfigured(monkeypatch, capsys):
|
||||
from hermes_cli.nous_account import NousPortalAccountInfo
|
||||
|
||||
|
|
|
|||
|
|
@ -140,7 +140,13 @@ async def _drive_picker(runner, event):
|
|||
"seed_model",
|
||||
[
|
||||
# Already-nested dict (common case).
|
||||
{"default": "old-model", "provider": "openai-codex"},
|
||||
{
|
||||
"default": "old-model",
|
||||
"provider": "custom",
|
||||
"base_url": "https://api.custom.example/v1",
|
||||
"api_key": "sk-stale",
|
||||
"api_mode": "anthropic_messages",
|
||||
},
|
||||
# Flat-string model: must be coerced to a nested dict on a tap (same
|
||||
# scalar-``model:`` guard the text path has) instead of raising
|
||||
# ``TypeError`` on assignment.
|
||||
|
|
@ -166,6 +172,8 @@ async def test_picker_tap_persists_by_default(tmp_path, monkeypatch, seed_model)
|
|||
assert written["model"]["default"] == "gpt-5.5"
|
||||
assert written["model"]["provider"] == "openrouter"
|
||||
assert written["model"]["base_url"] == "https://openrouter.ai/api/v1"
|
||||
assert "api_key" not in written["model"]
|
||||
assert "api_mode" not in written["model"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
|
|||
|
|
@ -16,7 +16,7 @@ from __future__ import annotations
|
|||
import yaml
|
||||
|
||||
from hermes_cli.auth import _update_config_for_provider
|
||||
from hermes_cli.config import get_config_path
|
||||
from hermes_cli.config import clear_model_endpoint_credentials, get_config_path
|
||||
|
||||
|
||||
def _read_model_cfg() -> dict:
|
||||
|
|
@ -49,6 +49,23 @@ def _seed_custom_provider_config(api_mode: str = "anthropic_messages") -> None:
|
|||
|
||||
|
||||
class TestUpdateConfigForProviderClearsStaleCustomFields:
|
||||
def test_clear_model_endpoint_credentials_removes_key_alias_and_mode(self):
|
||||
model_cfg = {
|
||||
"provider": "openrouter",
|
||||
"default": "anthropic/claude-sonnet-4.6",
|
||||
"api_key": "sk-stale",
|
||||
"api": "sk-legacy-stale",
|
||||
"api_mode": "anthropic_messages",
|
||||
}
|
||||
|
||||
returned = clear_model_endpoint_credentials(model_cfg)
|
||||
|
||||
assert returned is model_cfg
|
||||
assert "api_key" not in model_cfg
|
||||
assert "api" not in model_cfg
|
||||
assert "api_mode" not in model_cfg
|
||||
assert model_cfg["provider"] == "openrouter"
|
||||
|
||||
def test_switching_to_openrouter_clears_api_key_and_api_mode(self):
|
||||
_seed_custom_provider_config()
|
||||
|
||||
|
|
|
|||
|
|
@ -2327,9 +2327,10 @@ class TestWebServerEndpoints:
|
|||
# api_key follows the same lifecycle as base_url:
|
||||
# supplied → persisted.
|
||||
out = _apply_main_model_assignment(
|
||||
{}, "custom", "m", "http://x/v1", "sk-secret"
|
||||
{"api": "sk-legacy-old"}, "custom", "m", "http://x/v1", "sk-secret"
|
||||
)
|
||||
assert out["api_key"] == "sk-secret"
|
||||
assert "api" not in out
|
||||
|
||||
# same provider, no new key → existing key preserved (re-picking a model
|
||||
# on the same custom endpoint must not wipe the saved key).
|
||||
|
|
@ -2342,9 +2343,12 @@ class TestWebServerEndpoints:
|
|||
|
||||
# switching providers without a new key → stale key cleared.
|
||||
out = _apply_main_model_assignment(
|
||||
{"provider": "custom", "api_key": "sk-old"}, "openrouter", "m"
|
||||
{"provider": "custom", "api_key": "sk-old", "api_mode": "anthropic_messages"},
|
||||
"openrouter",
|
||||
"m",
|
||||
)
|
||||
assert out["api_key"] == ""
|
||||
assert "api_key" not in out
|
||||
assert "api_mode" not in out
|
||||
|
||||
def test_parse_model_ids_handles_openai_and_bare_shapes(self):
|
||||
"""Model discovery must tolerate the common /v1/models shapes and
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue