fix(dashboard): a rejected model switch is a 400, never a flattened model: block
_denormalize_config_from_web ran the new switch_model validation inside the pre-existing 'except Exception: pass' disk-read fallback. Any rejection (offline models.dev, no OpenRouter key, unlisted model) left config['model'] a flat string, and PUT /api/config's deep-merge wrote that string over the on-disk model: dict, destroying provider/base_url/api_mode/context_length/model_slots. Only the load_config() read keeps its fallback; validation propagates as the HTTPException(400) the caller's http_failure passes through. Invariant test PUTs a rejected model against a real config.yaml and asserts 400 + byte-identical file.
This commit is contained in:
@@ -822,34 +822,38 @@ def _denormalize_config_from_web(config: Dict[str, Any]) -> Dict[str, Any]:
|
||||
if not (has_model or ctx_sent):
|
||||
return config
|
||||
try:
|
||||
disk_model = load_config().get("model")
|
||||
if isinstance(disk_model, dict):
|
||||
if has_model:
|
||||
prev_default = str(disk_model.get("default") or "").strip()
|
||||
prev_provider = str(disk_model.get("provider") or "").strip()
|
||||
if model_val != prev_default and prev_provider:
|
||||
new_provider, resolved_model = _infer_provider_on_model_change(model_val, prev_provider)
|
||||
if new_provider and new_provider.strip().lower() != prev_provider.lower():
|
||||
norm_provider, norm_model = _normalize_main_model_assignment(new_provider, resolved_model)
|
||||
result = _validated_main_model_selection(load_config(), norm_provider, norm_model)
|
||||
disk_model = _apply_main_model_assignment(disk_model, result)
|
||||
model_val = result.new_model
|
||||
disk_model["default"] = model_val
|
||||
if ctx_sent:
|
||||
if ctx_override > 0:
|
||||
disk_model["context_length"] = ctx_override
|
||||
else:
|
||||
disk_model.pop("context_length", None)
|
||||
config["model"] = disk_model
|
||||
elif ctx_sent and ctx_override > 0:
|
||||
# Model was a bare string (or absent) — upgrade to a dict for the override.
|
||||
if has_model:
|
||||
default = model_val
|
||||
elif isinstance(disk_model, str) and disk_model:
|
||||
default = disk_model
|
||||
else:
|
||||
default = ""
|
||||
config["model"] = {"default": default, "context_length": ctx_override}
|
||||
disk_cfg = load_config()
|
||||
except Exception:
|
||||
pass # can't read disk config — just use the string form
|
||||
return config # can't read disk config — just use the string form
|
||||
# Only the disk READ has a fallback. A validation rejection below must propagate as its
|
||||
# HTTPException(400): swallowing it here left ``model`` a flat string, and the caller's
|
||||
# deep-merge then overwrote the whole on-disk ``model:`` dict (provider, base_url, slots).
|
||||
disk_model = disk_cfg.get("model")
|
||||
if isinstance(disk_model, dict):
|
||||
if has_model:
|
||||
prev_default = str(disk_model.get("default") or "").strip()
|
||||
prev_provider = str(disk_model.get("provider") or "").strip()
|
||||
if model_val != prev_default and prev_provider:
|
||||
new_provider, resolved_model = _infer_provider_on_model_change(model_val, prev_provider)
|
||||
if new_provider and new_provider.strip().lower() != prev_provider.lower():
|
||||
norm_provider, norm_model = _normalize_main_model_assignment(new_provider, resolved_model)
|
||||
result = _validated_main_model_selection(disk_cfg, norm_provider, norm_model)
|
||||
disk_model = _apply_main_model_assignment(disk_model, result)
|
||||
model_val = result.new_model
|
||||
disk_model["default"] = model_val
|
||||
if ctx_sent:
|
||||
if ctx_override > 0:
|
||||
disk_model["context_length"] = ctx_override
|
||||
else:
|
||||
disk_model.pop("context_length", None)
|
||||
config["model"] = disk_model
|
||||
elif ctx_sent and ctx_override > 0:
|
||||
# Model was a bare string (or absent) — upgrade to a dict for the override.
|
||||
if has_model:
|
||||
default = model_val
|
||||
elif isinstance(disk_model, str) and disk_model:
|
||||
default = disk_model
|
||||
else:
|
||||
default = ""
|
||||
config["model"] = {"default": default, "context_length": ctx_override}
|
||||
return config
|
||||
|
||||
@@ -3308,6 +3308,38 @@ class TestDenormalizeProviderSwitch:
|
||||
assert model["provider"] == "openrouter"
|
||||
assert model["context_length"] == 128000
|
||||
|
||||
def test_rejected_switch_is_400_and_leaves_the_model_block_byte_identical(self, monkeypatch):
|
||||
"""``switch_model`` rejecting the inferred provider must surface as 400 from
|
||||
``PUT /api/config`` — not fall back to the flat string, which the deep-merge would
|
||||
write OVER the on-disk ``model:`` dict (provider/base_url/api_mode/slots destroyed)."""
|
||||
from starlette.testclient import TestClient
|
||||
from hermes_constants import get_hermes_home
|
||||
from hermes_cli.model_switch import ModelSwitchResult
|
||||
from hermes_cli.web_server import app, _SESSION_HEADER_NAME, _SESSION_TOKEN
|
||||
|
||||
cfg_path = get_hermes_home() / "config.yaml"
|
||||
cfg_path.write_text(
|
||||
"model:\n"
|
||||
" default: llama3.2\n"
|
||||
" provider: ollama-local\n"
|
||||
" base_url: http://localhost:11434/v1\n"
|
||||
" api_mode: chat_completions\n"
|
||||
" context_length: 32000\n"
|
||||
" model_slots:\n"
|
||||
" fast: qwen3\n",
|
||||
encoding="utf-8")
|
||||
before = cfg_path.read_bytes()
|
||||
monkeypatch.setattr("hermes_cli.models_detect.provider_has_credentials", lambda p: p == "openrouter")
|
||||
monkeypatch.setattr("hermes_cli.model_switch.switch_model",
|
||||
lambda **_kw: ModelSwitchResult(success=False, error_message="models.dev offline"))
|
||||
|
||||
client = TestClient(app)
|
||||
client.headers[_SESSION_HEADER_NAME] = _SESSION_TOKEN
|
||||
resp = client.put("/api/config", json={"config": {"model": "openai/gpt-5.5-zzz"}})
|
||||
|
||||
assert resp.status_code == 400 and "models.dev offline" in resp.json()["detail"]
|
||||
assert cfg_path.read_bytes() == before
|
||||
|
||||
|
||||
class TestModelContextLengthSchema:
|
||||
"""Tests for model_context_length placement in CONFIG_SCHEMA."""
|
||||
|
||||
Reference in New Issue
Block a user