TUI model switching corrupts config persistence

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #51043 fix(tui): preserve config on model switch — atomic writes + custom-provider guard (#48305)

  merged to main

  files: hermes_cli/model_switch.py, hermes_cli/models.py, scripts/release.py, tests/hermes_cli/test_models.py, tests/test_tui_gateway_server.py, tui_gateway/server.py

  Show the diff

  • hermes_cli/model_switch.py

    @@ -1057,8 +1057,10 @@ def switch_model(
     
             # --- Step e: detect_provider_for_model() as last resort ---
             _base = current_base_url or ""
    -        is_custom = current_provider in {"custom", "local"} or (
    -            "localhost" in _base or "127.0.0.1" in _base
    +        is_custom = (
    +            current_provider in {"custom", "local"}
    +            or current_provider.startswith("custom:")
    +            or ("localhost" in _base or "127.0.0.1" in _base)
             )
     
             if (

  • hermes_cli/models.py

    @@ -1882,6 +1882,14 @@ def detect_static_provider_for_model(
             return None
     
         # --- Step 1: check static provider catalogs for a direct match ---
    +    # If the current provider is a custom endpoint (custom or custom:*), never
    +    # auto-switch away from it based on a static catalog match — the user
    +    # explicitly configured their own endpoint and the same model name may be
    +    # served there (#48305).
    +    _is_custom_current = (
    +        current_provider == "custom"
    +        or current_provider.startswith("custom:")
    +    )
         for pid, models in _PROVIDER_MODELS.items():
             if (
                 pid in current_keys
    @@ -1889,6 +1897,8 @@ def detect_static_provider_for_model(
                 or pid in _BORROWED_MODEL_PROVIDERS
             ):
                 continue
    +        if _is_custom_current:
    +            continue
             if any(name_lower == m.lower() for m in models):
                 return (pid, name)
     

  • scripts/release.py

    @@ -1673,6 +1673,7 @@
         "[redacted:email]": "AMIK-coorporations",  # PR #40578 (Urdu README) co-author
         "[redacted:email]": "AMIK-coorporations",  # PR #40578 (Urdu README)
         "[redacted:email]": "kyssta-exe",  # PR #44282 (Windows dashboard re-exec)
    +    "[email protected]": "Elshayib",  # PR #48351 (custom-provider misattribution guard; #48305)
         "[redacted:email]": "loongfay",  # PR #43508 (Yuanbao wechat forward msg)
         "[redacted:email]": "BROCCOLO1D",  # PR #42733 (lazy-parse docker env config)
         "[redacted:email]": "underthestars-zhy",  # PR #46907 co-author (Photon Spectrum project ids)

  • tests/hermes_cli/test_models.py

    @@ -301,6 +301,26 @@ def test_aggregator_not_suggested(self):
             assert result is not None
             assert result[0] not in {"nous",}  # nous has claude models but shouldn't be suggested
     
    +    def test_custom_provider_not_overridden_by_static_catalog(self):
    +        """When current provider is custom:*, a static-catalog match must NOT
    +        override it — otherwise a model served by the user's own endpoint gets
    +        misattributed to a native provider, rewriting model.provider (#48305).
    +
    +        `gpt-5.4` is in the static openai catalog; with current=custom:foo,
    +        detection must return None instead of switching to openai.
    +        """
    +        assert detect_provider_for_model("gpt-5.4", "custom:foo") is None
    +
    +    def test_bare_custom_provider_not_overridden_by_static_catalog(self):
    +        """Same protection for the bare 'custom' provider."""
    +        assert detect_provider_for_model("gpt-5.4", "custom") is None
    +
    +    def test_non_custom_provider_detection_unaffected(self):
    +        """The custom-provider guard must NOT change detection for non-custom
    +        current providers — a static-catalog model still routes normally."""
    +        result = detect_provider_for_model("gpt-5.4", "openrouter")
    +        assert result is not None and result[0] == "openai"
    +
     
     class TestIsNousFreeTier:
         """Tests for is_nous_free_tier — account tier detection."""

  • tests/test_tui_gateway_server.py

    @@ -3266,7 +3266,7 @@ def switch_model(self, **kwargs):
             warning_message="",
         )
         seen = {}
    -    saved = {}
    +    saved_values = {}
     
         def _switch_model(**kwargs):
             seen.update(kwargs)
    @@ -3276,7 +3276,9 @@ def _switch_model(**kwargs):
         monkeypatch.setattr("hermes_cli.model_switch.switch_model", _switch_model)
         monkeypatch.setattr(server, "_restart_slash_worker", lambda sid, session: None)
         monkeypatch.setattr(server, "_emit", lambda *args, **kwargs: None)
    -    monkeypatch.setattr("hermes_cli.config.save_config", lambda cfg: saved.update(cfg))
    +    # _persist_model_switch uses targeted save_config_value writes (#48305) so it
    +    # preserves sibling model.* keys instead of rewriting the whole block.
    +    monkeypatch.setattr("cli.save_config_value", lambda key, value: saved_values.__setitem__(key, value) or True)
     
         resp = server.handle_request(
             {
    @@ -3292,9 +3294,9 @@ def _switch_model(**kwargs):
     
         assert resp["result"]["value"] == "anthropic/claude-sonnet-4.6"
         assert seen["is_global"] is True
    -    assert saved["model"]["default"] == "anthropic/claude-sonnet-4.6"
    -    assert saved["model"]["provider"] == "anthropic"
    -    assert saved["model"]["base_url"] == "https://api.anthropic.com"
    +    assert saved_values["model.default"] == "anthropic/claude-sonnet-4.6"
    +    assert saved_values["model.provider"] == "anthropic"
    +    assert saved_values["model.base_url"] == "https://api.anthropic.com"
     
     
     def test_config_set_model_explicit_provider_skips_broken_default_init(monkeypatch):
    @@ -7988,3 +7990,73 @@ def _boom():
         # Field omitted, but the rest of the payload is intact.
         assert "active_subagents" not in usage
         assert usage["model"] == "x"
    +
    +
    +def test_persist_model_switch_preserves_sibling_model_keys(tmp_path, monkeypatch):
    +    """#48305: switching models from the TUI must NOT destroy sibling keys under
    +    `model:` (model_slots, model_fallback, etc.). _persist_model_switch now uses
    +    targeted save_config_value writes instead of rewriting the whole block."""
    +    import types
    +    import yaml
    +    import cli
    +
    +    cfg_path = tmp_path / "config.yaml"
    +    cfg_path.write_text(
    +        "model:\n"
    +        "  default: old-model\n"
    +        "  provider: openai\n"
    +        "  model_slots:\n"
    +        "    fast: gpt-5-mini\n"
    +        "  model_fallback:\n"
    +        "    - claude-haiku\n"
    +        "agent:\n"
    +        "  system_prompt: keepme\n"
    +    )
    +    # save_config_value() resolves the config path from cli._hermes_home, which
    +    # is captured at import time — patch it directly (set_hermes_home_override
    +    # does NOT affect this snapshot).
    +    monkeypatch.setattr(cli, "_hermes_home", tmp_path)
    +
    +    result = types.SimpleNamespace(
    +        new_model="new-model", target_provider="anthropic", base_url=None
    +    )
    +    server._persist_model_switch(result)
    +    saved = yaml.safe_load(cfg_path.read_text())
    +
    +    # The switched fields updated...
    +    assert saved["model"]["default"] == "new-model"
    +    assert saved["model"]["provider"] == "anthropic"
    +    # ...and the sibling keys SURVIVED (the bug was that they got wiped).
    +    assert saved["model"]["model_slots"] == {"fast": "gpt-5-mini"}
    +    assert saved["model"]["model_fallback"] == ["claude-haiku"]
    +    assert saved["agent"]["system_prompt"] == "keepme"
    +
    +
    +def test_persist_model_switch_clears_stale_base_url(tmp_path, monkeypatch):
    +    """#48305: switching from a custom endpoint (which set model.base_url) to a
    +    provider with no base_url must CLEAR the stale base_url, not leave it
    +    pointing at the old host."""
    +    import types
    +    import yaml
    +    import cli
    +
    +    cfg_path = tmp_path / "config.yaml"
    +    cfg_path.write_text(
    +        "model:\n"
    +        "  default: local-model\n"
    +        "  provider: custom:mylocal\n"
    +        "  base_url: http://localhost:1234/v1\n"
    +    )
    +    monkeypatch.setattr(cli, "_hermes_home", tmp_path)
    +
    +    # Switch to a native provider with no base_url.
    +    result = types.SimpleNamespace(
    +        new_model="claude-haiku", target_provider="anthropic", base_url=None
    +    )
    +    server._persist_model_switch(result)
    +    saved = yaml.safe_load(cfg_path.read_text())
    +
    +    assert saved["model"]["default"] == "claude-haiku"
    +    assert saved["model"]["provider"] == "anthropic"
    +    # Stale custom base_url must be cleared (null coalesces to absent on read).
    +    assert not saved["model"].get("base_url"), saved["model"].get("base_url")

  • tui_gateway/server.py

    @@ -2301,21 +2301,23 @@ def _restart_slash_worker(sid: str, session: dict):
     
     
     def _persist_model_switch(result) -> None:
    -    from hermes_cli.config import save_config
    -
    -    cfg = _load_cfg()
    -    model_cfg = cfg.get("model")
    -    if not isinstance(model_cfg, dict):
    -        model_cfg = {}
    -        cfg["model"] = model_cfg
    -
    -    model_cfg["default"] = result.new_model
    -    model_cfg["provider"] = result.target_provider
    +    # Use targeted, atomic key writes (comment/ordering-preserving) instead of
    +    # rewriting the whole `model:` block. A full-block rewrite via save_config()
    +    # destroys sibling keys the user set under `model:` — `model_slots`,
    +    # `model_fallback`, etc. — when switching models from the TUI (#48305).
    +    from cli import save_config_value
    +
    +    save_config_value("model.default", result.new_model)
    +    save_config_value("model.provider", result.target_provider)
         if result.base_url:
    -        model_cfg["base_url"] = result.base_url
    +        save_config_value("model.base_url", result.base_url)
         else:
    -        model_cfg.pop("base_url", None)
    -    save_config(cfg)
    +        # Clear any stale base_url when switching to a provider that doesn't use
    +        # one (e.g. custom endpoint -> native provider). Reads coalesce null to
    +        # absent (`model_cfg.get("base_url") or ""`), so a null is equivalent to
    +        # removal without needing a key-delete. Leaving the old value would
    +        # route the new model at the previous custom host (#48305).
    +        save_config_value("model.base_url", None)
     
     
     def _apply_model_switch(