mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-30 06:41:51 +00:00
fix(gateway): coerce scalar model: to dict before /model --global persist (#32272)
Reported via AskClaw. When config.yaml has `model: <name>` (flat string)
instead of the nested `model: {default: ..., provider: ...}` form, every
gateway `/model X --global` crashed silently with
TypeError: 'str' object does not support item assignment
The persist block did:
model_cfg = cfg.setdefault("model", {})
model_cfg["default"] = result.new_model
`setdefault` returns the existing scalar, and the next assignment blows
up. The 'switch failed' warning was logged at WARNING level and the user
never saw why their persist didn't stick.
Coerce scalar/None `model:` into a dict before mutation, in both the
gateway path (`gateway/run.py`) and the sister site in
`hermes_cli/doctor.py --fix` (same setdefault-on-string flaw). The CLI
`/model` path is unaffected because it goes through `_set_nested` which
already replaces scalar leaves with dicts.
Regression test `tests/gateway/test_model_command_flat_string_config.py`
covers the flat-string, missing, and proper-dict cases. Without the fix,
the flat-string case fails with the exact original TypeError.
This commit is contained in:
parent
de76f4dbcf
commit
2c6bbaf352
3 changed files with 185 additions and 2 deletions
|
|
@ -10436,7 +10436,21 @@ class GatewayRunner:
|
|||
cfg = yaml.safe_load(f) or {}
|
||||
else:
|
||||
cfg = {}
|
||||
model_cfg = cfg.setdefault("model", {})
|
||||
# Coerce scalar/None ``model:`` into a dict before mutation —
|
||||
# otherwise ``cfg.setdefault("model", {})`` returns the existing
|
||||
# scalar and the next assignment raises
|
||||
# ``TypeError: 'str' object does not support item assignment``.
|
||||
# Reproduces when ``config.yaml`` has ``model: <name>`` (flat
|
||||
# string) instead of the proper nested ``model: {default: ...}``.
|
||||
raw_model = cfg.get("model")
|
||||
if isinstance(raw_model, dict):
|
||||
model_cfg = raw_model
|
||||
elif isinstance(raw_model, str) and raw_model.strip():
|
||||
model_cfg = {"default": raw_model.strip()}
|
||||
cfg["model"] = model_cfg
|
||||
else:
|
||||
model_cfg = {}
|
||||
cfg["model"] = model_cfg
|
||||
model_cfg["default"] = result.new_model
|
||||
model_cfg["provider"] = result.target_provider
|
||||
if result.base_url:
|
||||
|
|
|
|||
|
|
@ -812,7 +812,18 @@ def run_doctor(args):
|
|||
"(should be under 'model:' section)"
|
||||
)
|
||||
if should_fix:
|
||||
model_section = raw_config.setdefault("model", {})
|
||||
# Coerce scalar/None ``model:`` into a dict before mutation —
|
||||
# ``setdefault("model", {})`` would return an existing scalar
|
||||
# and then ``model_section[k] = ...`` would raise TypeError.
|
||||
raw_model = raw_config.get("model")
|
||||
if isinstance(raw_model, dict):
|
||||
model_section = raw_model
|
||||
elif isinstance(raw_model, str) and raw_model.strip():
|
||||
model_section = {"default": raw_model.strip()}
|
||||
raw_config["model"] = model_section
|
||||
else:
|
||||
model_section = {}
|
||||
raw_config["model"] = model_section
|
||||
for k in stale_root_keys:
|
||||
if not model_section.get(k):
|
||||
model_section[k] = raw_config.pop(k)
|
||||
|
|
|
|||
158
tests/gateway/test_model_command_flat_string_config.py
Normal file
158
tests/gateway/test_model_command_flat_string_config.py
Normal file
|
|
@ -0,0 +1,158 @@
|
|||
"""Regression tests for gateway /model --global persistence when config.yaml
|
||||
has a flat-string ``model:`` value instead of a nested dict.
|
||||
|
||||
Before fix: ``cfg.setdefault("model", {})`` returned the existing string and
|
||||
the next assignment raised ``TypeError: 'str' object does not support item
|
||||
assignment``, so every ``/model X --global`` from Telegram/Discord crashed
|
||||
silently and the user-visible result was "switch failed" with no persist.
|
||||
|
||||
After fix: the persist block coerces a scalar ``model:`` into a nested dict
|
||||
before mutation, so ``--global`` succeeds and the config is rewritten in
|
||||
the proper ``model: {default: ..., provider: ...}`` form.
|
||||
"""
|
||||
|
||||
import yaml
|
||||
import pytest
|
||||
|
||||
from gateway.config import Platform
|
||||
from gateway.platforms.base import MessageEvent, MessageType
|
||||
from gateway.run import GatewayRunner
|
||||
from gateway.session import SessionSource
|
||||
|
||||
|
||||
def _make_runner():
|
||||
runner = object.__new__(GatewayRunner)
|
||||
runner.adapters = {}
|
||||
runner._voice_mode = {}
|
||||
runner._session_model_overrides = {}
|
||||
runner._running_agents = {}
|
||||
return runner
|
||||
|
||||
|
||||
def _make_event(text):
|
||||
return MessageEvent(
|
||||
text=text,
|
||||
message_type=MessageType.TEXT,
|
||||
source=SessionSource(platform=Platform.TELEGRAM, chat_id="12345", chat_type="dm"),
|
||||
)
|
||||
|
||||
|
||||
def _fake_switch_result():
|
||||
"""Build a successful ModelSwitchResult that bypasses real provider resolution."""
|
||||
from hermes_cli.model_switch import ModelSwitchResult
|
||||
|
||||
return ModelSwitchResult(
|
||||
success=True,
|
||||
new_model="gpt-5.5",
|
||||
target_provider="openrouter",
|
||||
provider_changed=True,
|
||||
api_key="sk-test",
|
||||
base_url="https://openrouter.ai/api/v1",
|
||||
api_mode="chat_completions",
|
||||
provider_label="OpenRouter",
|
||||
is_global=True,
|
||||
)
|
||||
|
||||
|
||||
def _setup_isolated_home(tmp_path, monkeypatch, model_yaml_value):
|
||||
"""Write a config.yaml with the given ``model:`` value and stub the heavy bits."""
|
||||
import gateway.run as gateway_run
|
||||
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
cfg_path = hermes_home / "config.yaml"
|
||||
cfg_path.write_text(
|
||||
yaml.safe_dump({"model": model_yaml_value, "providers": {}}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
monkeypatch.setattr(gateway_run, "_hermes_home", hermes_home)
|
||||
monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {})
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.model_switch.switch_model",
|
||||
lambda **kw: _fake_switch_result(),
|
||||
)
|
||||
# save_config writes to ``get_hermes_home() / config.yaml`` — point it here.
|
||||
monkeypatch.setattr("hermes_constants.get_hermes_home", lambda: hermes_home)
|
||||
monkeypatch.setattr("hermes_cli.config.get_hermes_home", lambda: hermes_home)
|
||||
return cfg_path
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_model_global_persists_when_config_has_flat_string_model(tmp_path, monkeypatch):
|
||||
"""Regression: ``model: deepseek-v4-flash`` (flat string) used to crash
|
||||
the gateway ``/model X --global`` persist branch with TypeError. After
|
||||
the fix, the flat string is coerced to ``{"default": ...}`` and the new
|
||||
model+provider are persisted on top.
|
||||
"""
|
||||
cfg_path = _setup_isolated_home(tmp_path, monkeypatch, "deepseek-v4-flash")
|
||||
|
||||
result = await _make_runner()._handle_model_command(
|
||||
_make_event("/model gpt-5.5 --global")
|
||||
)
|
||||
|
||||
# Sanity: the handler returned a success-looking message (not a crash log).
|
||||
assert result is not None
|
||||
assert "gpt-5.5" in result
|
||||
|
||||
# The persist block must have rewritten config.yaml as a nested dict.
|
||||
written = yaml.safe_load(cfg_path.read_text(encoding="utf-8"))
|
||||
assert isinstance(written["model"], dict), (
|
||||
"model: should be coerced to a dict, got %r" % (written["model"],)
|
||||
)
|
||||
assert written["model"]["default"] == "gpt-5.5"
|
||||
assert written["model"]["provider"] == "openrouter"
|
||||
assert written["model"]["base_url"] == "https://openrouter.ai/api/v1"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_model_global_persists_when_config_has_missing_model(tmp_path, monkeypatch):
|
||||
"""Companion case: ``model:`` key absent entirely. setdefault would have
|
||||
worked here, but the coercion branch also has to handle this cleanly.
|
||||
"""
|
||||
import gateway.run as gateway_run
|
||||
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
cfg_path = hermes_home / "config.yaml"
|
||||
cfg_path.write_text(yaml.safe_dump({"providers": {}}), encoding="utf-8")
|
||||
|
||||
monkeypatch.setattr(gateway_run, "_hermes_home", hermes_home)
|
||||
monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {})
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.model_switch.switch_model",
|
||||
lambda **kw: _fake_switch_result(),
|
||||
)
|
||||
monkeypatch.setattr("hermes_constants.get_hermes_home", lambda: hermes_home)
|
||||
monkeypatch.setattr("hermes_cli.config.get_hermes_home", lambda: hermes_home)
|
||||
|
||||
result = await _make_runner()._handle_model_command(
|
||||
_make_event("/model gpt-5.5 --global")
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
written = yaml.safe_load(cfg_path.read_text(encoding="utf-8"))
|
||||
assert isinstance(written["model"], dict)
|
||||
assert written["model"]["default"] == "gpt-5.5"
|
||||
assert written["model"]["provider"] == "openrouter"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_model_global_persists_when_config_has_proper_dict_model(tmp_path, monkeypatch):
|
||||
"""Already-correct nested dict must still work — no regression on the
|
||||
common case.
|
||||
"""
|
||||
cfg_path = _setup_isolated_home(
|
||||
tmp_path,
|
||||
monkeypatch,
|
||||
{"default": "old-model", "provider": "openai-codex"},
|
||||
)
|
||||
|
||||
result = await _make_runner()._handle_model_command(
|
||||
_make_event("/model gpt-5.5 --global")
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
written = yaml.safe_load(cfg_path.read_text(encoding="utf-8"))
|
||||
assert written["model"]["default"] == "gpt-5.5"
|
||||
assert written["model"]["provider"] == "openrouter"
|
||||
Loading…
Add table
Add a link
Reference in a new issue