fix: root-level provider in config.yaml no longer overrides model.provider
load_cli_config() had a priority inversion: a stale root-level 'provider' key in config.yaml would OVERRIDE the canonical 'model.provider' set by 'hermes model'. The gateway reads model.provider directly from YAML and worked correctly, but 'hermes chat -q' and the interactive CLI went through the merge logic and picked up the stale root-level key. Fix: root-level provider/base_url are now only used as a fallback when model.provider/model.base_url is not set (never as an override). Also added _normalize_root_model_keys() to config.py load_config() and save_config() — migrates root-level provider/base_url into the model section and removes the root-level keys permanently. Reported by (≧▽≦) in Discord: opencode-go provider persisted as a root-level key and overrode the correct model.provider=openrouter, causing 401 errors.
This commit is contained in:
25
cli.py
25
cli.py
@@ -263,17 +263,20 @@ def load_cli_config() -> Dict[str, Any]:
|
||||
# Old format: model is a dict with default/base_url
|
||||
defaults["model"].update(file_config["model"])
|
||||
|
||||
# Root-level provider and base_url override model config.
|
||||
# Users may write:
|
||||
# model: kimi-k2.5:cloud
|
||||
# provider: custom
|
||||
# base_url: http://localhost:11434/v1
|
||||
# These root-level keys must be merged into defaults["model"] so
|
||||
# they are picked up by CLI provider resolution.
|
||||
if "provider" in file_config and file_config["provider"]:
|
||||
defaults["model"]["provider"] = file_config["provider"]
|
||||
if "base_url" in file_config and file_config["base_url"]:
|
||||
defaults["model"]["base_url"] = file_config["base_url"]
|
||||
# Legacy root-level provider/base_url fallback.
|
||||
# Some users (or old code) put provider: / base_url: at the
|
||||
# config root instead of inside the model: section. These are
|
||||
# only used as a FALLBACK when model.provider / model.base_url
|
||||
# is not already set — never as an override. The canonical
|
||||
# location is model.provider (written by `hermes model`).
|
||||
if not defaults["model"].get("provider"):
|
||||
root_provider = file_config.get("provider")
|
||||
if root_provider:
|
||||
defaults["model"]["provider"] = root_provider
|
||||
if not defaults["model"].get("base_url"):
|
||||
root_base_url = file_config.get("base_url")
|
||||
if root_base_url:
|
||||
defaults["model"]["base_url"] = root_base_url
|
||||
|
||||
# Deep merge file_config into defaults.
|
||||
# First: merge keys that exist in both (deep-merge dicts, overwrite scalars)
|
||||
|
||||
@@ -1373,6 +1373,36 @@ def _expand_env_vars(obj):
|
||||
return obj
|
||||
|
||||
|
||||
def _normalize_root_model_keys(config: Dict[str, Any]) -> Dict[str, Any]:
|
||||
"""Move stale root-level provider/base_url into model section.
|
||||
|
||||
Some users (or older code) placed ``provider:`` and ``base_url:`` at the
|
||||
config root instead of inside ``model:``. These root-level keys are only
|
||||
used as a fallback when the corresponding ``model.*`` key is empty — they
|
||||
never override an existing ``model.provider`` or ``model.base_url``.
|
||||
After migration the root-level keys are removed so they can't cause
|
||||
confusion on subsequent loads.
|
||||
"""
|
||||
# Only act if there are root-level keys to migrate
|
||||
has_root = any(config.get(k) for k in ("provider", "base_url"))
|
||||
if not has_root:
|
||||
return config
|
||||
|
||||
config = dict(config)
|
||||
model = config.get("model")
|
||||
if not isinstance(model, dict):
|
||||
model = {"default": model} if model else {}
|
||||
config["model"] = model
|
||||
|
||||
for key in ("provider", "base_url"):
|
||||
root_val = config.get(key)
|
||||
if root_val and not model.get(key):
|
||||
model[key] = root_val
|
||||
config.pop(key, None)
|
||||
|
||||
return config
|
||||
|
||||
|
||||
def _normalize_max_turns_config(config: Dict[str, Any]) -> Dict[str, Any]:
|
||||
"""Normalize legacy root-level max_turns into agent.max_turns."""
|
||||
config = dict(config)
|
||||
@@ -1414,7 +1444,7 @@ def load_config() -> Dict[str, Any]:
|
||||
except Exception as e:
|
||||
print(f"Warning: Failed to load config: {e}")
|
||||
|
||||
return _expand_env_vars(_normalize_max_turns_config(config))
|
||||
return _expand_env_vars(_normalize_root_model_keys(_normalize_max_turns_config(config)))
|
||||
|
||||
|
||||
_SECURITY_COMMENT = """
|
||||
@@ -1521,7 +1551,7 @@ def save_config(config: Dict[str, Any]):
|
||||
|
||||
ensure_hermes_home()
|
||||
config_path = get_config_path()
|
||||
normalized = _normalize_max_turns_config(config)
|
||||
normalized = _normalize_root_model_keys(_normalize_max_turns_config(config))
|
||||
|
||||
# Build optional commented-out sections for features that are off by
|
||||
# default or only relevant when explicitly configured.
|
||||
|
||||
@@ -192,6 +192,91 @@ class TestHistoryDisplay:
|
||||
assert "A" * 250 + "..." not in output
|
||||
|
||||
|
||||
class TestRootLevelProviderOverride:
|
||||
"""Root-level provider/base_url in config.yaml must NOT override model.provider."""
|
||||
|
||||
def test_model_provider_wins_over_root_provider(self, tmp_path, monkeypatch):
|
||||
"""model.provider takes priority — root-level provider is only a fallback."""
|
||||
import yaml
|
||||
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
|
||||
config_path = hermes_home / "config.yaml"
|
||||
config_path.write_text(yaml.safe_dump({
|
||||
"provider": "opencode-go", # stale root-level key
|
||||
"model": {
|
||||
"default": "google/gemini-3-flash-preview",
|
||||
"provider": "openrouter", # correct canonical key
|
||||
},
|
||||
}))
|
||||
|
||||
import cli
|
||||
monkeypatch.setattr(cli, "_hermes_home", hermes_home)
|
||||
cfg = cli.load_cli_config()
|
||||
|
||||
assert cfg["model"]["provider"] == "openrouter"
|
||||
|
||||
def test_root_provider_ignored_when_default_model_provider_exists(self, tmp_path, monkeypatch):
|
||||
"""Even when model.provider is the default 'auto', root-level provider is ignored."""
|
||||
import yaml
|
||||
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
|
||||
config_path = hermes_home / "config.yaml"
|
||||
config_path.write_text(yaml.safe_dump({
|
||||
"provider": "opencode-go", # stale root key
|
||||
"model": {
|
||||
"default": "google/gemini-3-flash-preview",
|
||||
# no explicit model.provider — defaults provide "auto"
|
||||
},
|
||||
}))
|
||||
|
||||
import cli
|
||||
monkeypatch.setattr(cli, "_hermes_home", hermes_home)
|
||||
cfg = cli.load_cli_config()
|
||||
|
||||
# Root-level "opencode-go" must NOT leak through
|
||||
assert cfg["model"]["provider"] != "opencode-go"
|
||||
|
||||
def test_normalize_root_model_keys_moves_to_model(self):
|
||||
"""_normalize_root_model_keys migrates root keys into model section."""
|
||||
from hermes_cli.config import _normalize_root_model_keys
|
||||
|
||||
config = {
|
||||
"provider": "opencode-go",
|
||||
"base_url": "https://example.com/v1",
|
||||
"model": {
|
||||
"default": "some-model",
|
||||
},
|
||||
}
|
||||
result = _normalize_root_model_keys(config)
|
||||
# Root keys removed
|
||||
assert "provider" not in result
|
||||
assert "base_url" not in result
|
||||
# Migrated into model section
|
||||
assert result["model"]["provider"] == "opencode-go"
|
||||
assert result["model"]["base_url"] == "https://example.com/v1"
|
||||
|
||||
def test_normalize_root_model_keys_does_not_override_existing(self):
|
||||
"""Existing model.provider is never overridden by root-level key."""
|
||||
from hermes_cli.config import _normalize_root_model_keys
|
||||
|
||||
config = {
|
||||
"provider": "stale-provider",
|
||||
"model": {
|
||||
"default": "some-model",
|
||||
"provider": "correct-provider",
|
||||
},
|
||||
}
|
||||
result = _normalize_root_model_keys(config)
|
||||
assert result["model"]["provider"] == "correct-provider"
|
||||
assert "provider" not in result # root key still cleaned up
|
||||
|
||||
|
||||
class TestProviderResolution:
|
||||
def test_api_key_is_string_or_none(self):
|
||||
cli = _make_cli()
|
||||
|
||||
Reference in New Issue
Block a user