From 89d8127772b7e0710159a876e741ae7bfe502a46 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Mon, 30 Mar 2026 23:17:26 -0700 Subject: [PATCH] fix: setup wizard overwrites custom endpoint config (#4172) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _model_flow_custom() saved model.provider and model.base_url to disk via its own load_config/save_config cycle, but never updated the setup wizard's in-memory config dict. The wizard's final save_config(config) then overwrote the custom settings with the stale default string model value. Fix: after saving to disk, also mutate the caller's config dict so the wizard's final save preserves model.provider='custom' and the base_url. Both the model_name and no-model_name branches are covered. Added regression tests that simulate the full wizard flow including the final save_config(config) call — the step that was previously untested. --- hermes_cli/main.py | 15 ++++ tests/hermes_cli/test_setup.py | 122 +++++++++++++++++++++++++++++++-- 2 files changed, 133 insertions(+), 4 deletions(-) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 3bd6afa5..aad6e7f1 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -1278,10 +1278,25 @@ def _model_flow_custom(config): save_config(cfg) deactivate_provider() + # Sync the caller's config dict so the setup wizard's final + # save_config(config) preserves our model settings. Without + # this, the wizard overwrites model.provider/base_url with + # the stale values from its own config dict (#4172). + config["model"] = dict(model) + print(f"Default model set to: {model_name} (via {effective_url})") else: if base_url or api_key: deactivate_provider() + # Even without a model name, persist the custom endpoint on the + # caller's config dict so the setup wizard doesn't lose it. + _caller_model = config.get("model") + if not isinstance(_caller_model, dict): + _caller_model = {"default": _caller_model} if _caller_model else {} + _caller_model["provider"] = "custom" + _caller_model["base_url"] = effective_url + _caller_model.pop("api_mode", None) + config["model"] = _caller_model print("Endpoint saved. Use `/model` in chat or `hermes model` to set a model.") # Auto-save to custom_providers so it appears in the menu next time diff --git a/tests/hermes_cli/test_setup.py b/tests/hermes_cli/test_setup.py index a4c85ba2..c5a19f06 100644 --- a/tests/hermes_cli/test_setup.py +++ b/tests/hermes_cli/test_setup.py @@ -118,11 +118,125 @@ def test_custom_setup_clears_active_oauth_provider(tmp_path, monkeypatch): # Core assertion: switching to custom endpoint clears OAuth provider assert get_active_provider() is None - # _model_flow_custom writes config via its own load/save cycle + # Simulate what the real setup wizard does: save_config(config) AFTER + # setup_model_provider returns. This is the step that previously + # overwrote model.provider/base_url (#4172). + save_config(config) + reloaded = load_config() - if isinstance(reloaded.get("model"), dict): - assert reloaded["model"].get("provider") == "custom" - assert reloaded["model"].get("default") == "custom/model" + assert isinstance(reloaded.get("model"), dict), ( + "model should be a dict after custom setup, not " + + repr(type(reloaded.get("model"))) + ) + assert reloaded["model"].get("provider") == "custom" + assert reloaded["model"].get("default") == "custom/model" + assert "custom.example" in reloaded["model"].get("base_url", "") + + +def test_custom_setup_preserves_provider_after_wizard_save_config( + tmp_path, monkeypatch +): + """Regression test for #4172: the setup wizard's final save_config(config) + must not overwrite model.provider/base_url that _model_flow_custom set. + + Simulates the full flow: + 1. load config (fresh install — model is a string) + 2. setup_model_provider picks custom + 3. wizard calls save_config(config) afterward + 4. verify resolve_requested_provider returns "custom" + """ + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + _clear_provider_env(monkeypatch) + + config = load_config() + # Sanity: fresh install has model as a string + assert isinstance(config.get("model"), str) or config.get("model") is None + + def fake_prompt_choice(question, choices, default=0): + if question == "Select your inference provider:": + return 3 # Custom endpoint + tts_idx = _maybe_keep_current_tts(question, choices) + if tts_idx is not None: + return tts_idx + raise AssertionError(f"Unexpected prompt_choice call: {question}") + + monkeypatch.setattr("hermes_cli.setup.prompt_choice", fake_prompt_choice) + + input_values = iter([ + "http://localhost:11434/v1", # Ollama URL + "", # no API key (local) + "qwen3.5:32b", # model name + "", # context length (auto-detect) + ]) + monkeypatch.setattr("builtins.input", lambda _prompt="": next(input_values)) + monkeypatch.setattr("hermes_cli.setup.prompt_yes_no", lambda *a, **kw: False) + monkeypatch.setattr("hermes_cli.auth.detect_external_credentials", lambda: []) + monkeypatch.setattr("hermes_cli.main._save_custom_provider", lambda *a, **kw: None) + monkeypatch.setattr( + "hermes_cli.models.probe_api_models", + lambda api_key, base_url: {"models": ["qwen3.5:32b"], "probed_url": base_url + "/models"}, + ) + + # Full wizard cycle + setup_model_provider(config) + save_config(config) # ← this is what the real wizard does + + # Verify config on disk + reloaded = load_config() + assert isinstance(reloaded["model"], dict) + assert reloaded["model"]["provider"] == "custom" + assert reloaded["model"]["base_url"] == "http://localhost:11434/v1" + assert reloaded["model"]["default"] == "qwen3.5:32b" + assert "api_mode" not in reloaded["model"] + + # Verify the runtime resolver sees "custom", not "auto" + from hermes_cli.runtime_provider import resolve_requested_provider + assert resolve_requested_provider() == "custom" + + +def test_custom_setup_no_model_name_still_preserves_endpoint( + tmp_path, monkeypatch +): + """When the user enters a URL and key but skips the model name, + model.provider and model.base_url must still survive the wizard's + final save_config(config).""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + _clear_provider_env(monkeypatch) + + config = load_config() + + def fake_prompt_choice(question, choices, default=0): + if question == "Select your inference provider:": + return 3 + tts_idx = _maybe_keep_current_tts(question, choices) + if tts_idx is not None: + return tts_idx + raise AssertionError(f"Unexpected prompt_choice call: {question}") + + monkeypatch.setattr("hermes_cli.setup.prompt_choice", fake_prompt_choice) + + input_values = iter([ + "http://192.168.1.50:8080/v1", # URL + "my-key", # API key + "", # no model name + "", # context length + ]) + monkeypatch.setattr("builtins.input", lambda _prompt="": next(input_values)) + monkeypatch.setattr("hermes_cli.setup.prompt_yes_no", lambda *a, **kw: False) + monkeypatch.setattr("hermes_cli.auth.detect_external_credentials", lambda: []) + monkeypatch.setattr("hermes_cli.main._save_custom_provider", lambda *a, **kw: None) + monkeypatch.setattr( + "hermes_cli.models.probe_api_models", + lambda api_key, base_url: {"models": None, "probed_url": base_url + "/models"}, + ) + + setup_model_provider(config) + save_config(config) + + reloaded = load_config() + assert isinstance(reloaded["model"], dict) + assert reloaded["model"]["provider"] == "custom" + assert reloaded["model"]["base_url"] == "http://192.168.1.50:8080/v1" def test_codex_setup_uses_runtime_access_token_for_live_model_list(tmp_path, monkeypatch):