diff --git a/gateway/run.py b/gateway/run.py index b1d44e4e98d..9568eab2ceb 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -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: `` (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: diff --git a/hermes_cli/doctor.py b/hermes_cli/doctor.py index 9bdbc77e7ef..dbc486e87b1 100644 --- a/hermes_cli/doctor.py +++ b/hermes_cli/doctor.py @@ -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) diff --git a/tests/gateway/test_model_command_flat_string_config.py b/tests/gateway/test_model_command_flat_string_config.py new file mode 100644 index 00000000000..38d6ea11dae --- /dev/null +++ b/tests/gateway/test_model_command_flat_string_config.py @@ -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"