fix: setup wizard overwrites custom endpoint config (#4172)
_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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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):
|
||||
|
||||
Reference in New Issue
Block a user