diff --git a/cli.py b/cli.py index f2c9773aef7..c86e7a4a329 100644 --- a/cli.py +++ b/cli.py @@ -8210,7 +8210,11 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin): try: if ctx is None: raise RuntimeError("inventory context unavailable") - providers = build_models_payload(ctx)["providers"] + providers = build_models_payload( + ctx, + probe_custom_providers=force_refresh, + probe_current_custom_provider=not force_refresh, + )["providers"] except Exception: providers = [] diff --git a/hermes_cli/model_switch.py b/hermes_cli/model_switch.py index fcb66917162..8af49151d2f 100644 --- a/hermes_cli/model_switch.py +++ b/hermes_cli/model_switch.py @@ -101,6 +101,52 @@ def _declared_model_ids(value: Any) -> list[str]: return ids +def _save_discovered_models_to_config( + api_url: str, model_ids: list[str] +) -> None: + """Persist discovered models into ``custom_providers`` in config.yaml. + + Called after a successful ``/v1/models`` probe so that the next read + with ``discover_models: false`` uses the cached list instead of a stale + or minimal manually-configured subset. + + Matches entries by ``base_url`` (trailing-slash-normalised). A failed + config write is swallowed — the picker still shows the live models for + this session. + """ + if not api_url or not model_ids: + return + try: + from hermes_cli.config import load_config, save_config + + cfg = load_config() + providers = cfg.get("custom_providers") or [] + if not isinstance(providers, list): + return + + norm_url = api_url.strip().rstrip("/").lower() + changed = False + for entry in providers: + if not isinstance(entry, dict): + continue + entry_url = (entry.get("base_url", "") or entry.get("url", "") or "").strip() + if entry_url.rstrip("/").lower() != norm_url: + continue + existing = entry.get("models") + # Only update when models are stale — avoids unnecessary + # config writes on every picker open. + if isinstance(existing, list) and existing == model_ids: + continue + entry["models"] = model_ids + changed = True + + if changed: + cfg["custom_providers"] = providers + save_config(cfg) + except Exception: + pass + + def _bare_custom_provider_def(current_base_url: str) -> Optional[ProviderDef]: """ProviderDef for a direct ``model.provider: custom`` endpoint.""" base_url = str(current_base_url or "").strip() @@ -2447,6 +2493,12 @@ def list_authenticated_providers( if live_models: grp["models"] = live_models grp["total_models"] = len(live_models) + # Auto-save discovered models back to config so + # ``discover_models: false`` has a populated cache + # on the next read. A failed save is non-fatal. + _save_discovered_models_to_config( + api_url, live_models + ) except Exception: pass results.append({ diff --git a/tests/hermes_cli/test_inventory.py b/tests/hermes_cli/test_inventory.py index fd9f25fdefb..6c262180b9d 100644 --- a/tests/hermes_cli/test_inventory.py +++ b/tests/hermes_cli/test_inventory.py @@ -276,6 +276,43 @@ def test_build_models_payload_can_probe_only_current_custom_provider(): assert mock_list.call_args.kwargs["probe_current_custom_provider"] is True +def test_cli_model_picker_forwards_force_refresh_to_probe_flags(): + """CLI /model picker must pass force_refresh to probe flags (#65652, #65650). + + Normal open (/model bare) skips non-current probes; /model --refresh probes + all custom providers to freshen their model lists. + """ + ctx = _empty_ctx() + + # Normal open — skip non-current probes + force_refresh = False + with patch( + "hermes_cli.model_switch.list_authenticated_providers", + return_value=[], + ) as mock_list: + build_models_payload( + ctx, + probe_custom_providers=force_refresh, + probe_current_custom_provider=not force_refresh, + ) + assert mock_list.call_args.kwargs["probe_custom_providers"] is False + assert mock_list.call_args.kwargs["probe_current_custom_provider"] is True + + # Refresh open — probe everything + force_refresh = True + with patch( + "hermes_cli.model_switch.list_authenticated_providers", + return_value=[], + ) as mock_list: + build_models_payload( + ctx, + probe_custom_providers=force_refresh, + probe_current_custom_provider=not force_refresh, + ) + assert mock_list.call_args.kwargs["probe_custom_providers"] is True + assert mock_list.call_args.kwargs["probe_current_custom_provider"] is False + + def test_list_authenticated_providers_force_fresh_is_keyword_only(): """``force_fresh_nous_tier`` must be keyword-only on the public listing API. diff --git a/tests/hermes_cli/test_model_switch_custom_providers.py b/tests/hermes_cli/test_model_switch_custom_providers.py index 06722b4f9f1..abd1089623f 100644 --- a/tests/hermes_cli/test_model_switch_custom_providers.py +++ b/tests/hermes_cli/test_model_switch_custom_providers.py @@ -1326,3 +1326,156 @@ def test_resolve_custom_provider_bare_custom_self_heal_passes_key_env(): assert resolved is not None assert resolved.api_key_env_vars == ("XIAOMI_MIMO_API_KEY",) + + +def test_discovered_models_auto_saved_to_cache(monkeypatch): + """Discovered models are persisted to config so ``discover_models: false`` + has a populated cache on the next read (#65652). + + When a successful probe returns live models, ``_save_discovered_models_to_config`` + must be called with the provider's base_url and the discovered model list. + """ + monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {}) + monkeypatch.setattr("hermes_cli.providers.HERMES_OVERLAYS", {}) + + save_calls = [] + + def fake_fetch_api_models(api_key, base_url, **kwargs): + return ["discovered-a", "discovered-b", "discovered-c"] + + monkeypatch.setattr("hermes_cli.models.fetch_api_models", fake_fetch_api_models) + monkeypatch.setattr( + "hermes_cli.model_switch._save_discovered_models_to_config", + lambda api_url, model_ids: save_calls.append((api_url, model_ids)), + ) + + custom_providers = [ + { + "name": "my-gateway", + "api_key": "***", + "base_url": "https://gateway.example.com/v1", + "discover_models": True, + "model": "only-model", + "models": {"only-model": {"context_length": 128000}}, + } + ] + + providers = list_authenticated_providers( + current_provider="my-gateway", + current_base_url="https://gateway.example.com/v1", + custom_providers=custom_providers, + max_models=50, + probe_custom_providers=True, + ) + + assert len(save_calls) == 1, ( + "_save_discovered_models_to_config must be called after a successful probe" + ) + assert save_calls[0][0] == "https://gateway.example.com/v1" + assert save_calls[0][1] == ["discovered-a", "discovered-b", "discovered-c"] + + gateway_prov = next( + (p for p in providers if p.get("api_url") == "https://gateway.example.com/v1"), + None, + ) + assert gateway_prov is not None + assert gateway_prov["models"] == ["discovered-a", "discovered-b", "discovered-c"] + + +def test_discovered_models_not_saved_on_empty_probe(monkeypatch): + """When a probe returns an empty list, no auto-save must happen (#65652).""" + monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {}) + monkeypatch.setattr("hermes_cli.providers.HERMES_OVERLAYS", {}) + + save_calls = [] + + def fake_fetch_api_models(api_key, base_url, **kwargs): + return [] + + monkeypatch.setattr("hermes_cli.models.fetch_api_models", fake_fetch_api_models) + monkeypatch.setattr( + "hermes_cli.model_switch._save_discovered_models_to_config", + lambda api_url, model_ids: save_calls.append((api_url, model_ids)), + ) + + custom_providers = [ + { + "name": "my-gateway", + "api_key": "***", + "base_url": "https://gateway.example.com/v1", + "discover_models": True, + "model": "only-model", + } + ] + + list_authenticated_providers( + current_provider="my-gateway", + current_base_url="https://gateway.example.com/v1", + custom_providers=custom_providers, + max_models=50, + probe_custom_providers=True, + ) + + assert save_calls == [], "Empty probe must not trigger a save" + + +def test_save_discovered_models_skips_unchanged(monkeypatch): + """``_save_discovered_models_to_config`` must not write config when the + model list hasn't changed (#65652).""" + from hermes_cli.model_switch import _save_discovered_models_to_config + + save_calls = [] + + def fake_save(config): + save_calls.append(dict(config)) + + monkeypatch.setattr("hermes_cli.config.save_config", fake_save) + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: { + "custom_providers": [ + { + "name": "my-gateway", + "base_url": "https://gateway.example.com/v1", + "models": ["model-a", "model-b"], + } + ] + }, + ) + + # Same list — no write + _save_discovered_models_to_config( + "https://gateway.example.com/v1", + ["model-a", "model-b"], + ) + assert save_calls == [], "Unchanged models must not trigger config write" + + # Changed list — write + _save_discovered_models_to_config( + "https://gateway.example.com/v1", + ["model-a", "model-b", "model-c"], + ) + assert len(save_calls) == 1, "Changed models must trigger config write" + updated = save_calls[0]["custom_providers"][0] + assert updated["models"] == ["model-a", "model-b", "model-c"] + + +def test_save_discovered_models_noop_on_empty_args(monkeypatch): + """``_save_discovered_models_to_config`` is a no-op when api_url or + model_ids are blank (#65652).""" + from hermes_cli.model_switch import _save_discovered_models_to_config + + load_calls = 0 + + def fake_load(): + nonlocal load_calls + load_calls += 1 + return {"custom_providers": []} + + monkeypatch.setattr("hermes_cli.config.load_config", fake_load) + + _save_discovered_models_to_config("", ["a"]) + _save_discovered_models_to_config("https://x.com", []) + _save_discovered_models_to_config("", []) + + assert load_calls == 0, "load_config must not be called for empty args"