diff --git a/CHANGELOG.md b/CHANGELOG.md index fa8707a0..8236ccb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,17 @@ # Hermes Web UI -- Changelog +## [v0.50.282] — 2026-05-03 + +### Fixed (1 PR — closes #1538, #1539) + +- **Nous Portal full live catalog + dropdown cache invalidation on provider remove** (#1544; closes #1538, #1539) — two related dropdown-staleness bugs reported by Deor (Discord, May 03 2026, relayed by AvidFuturist). Same root shape: a model picker showing stale data because the live source of truth was never asked. + + **#1538 — Nous Portal picker stuck at 4 hardcoded models.** `_PROVIDER_MODELS["nous"]` had four hardcoded entries (Claude Opus 4.6 / Sonnet 4.6, GPT-5.4 Mini, Gemini 3.1 Pro Preview) and `_build_available_models_uncached()` fell through to the generic `pid in _PROVIDER_MODELS` branch, deepcopying that four-entry list. The actual live Nous catalog has 30 models — Claude Opus 4.7, GPT-5.5, Kimi K2.6, MiniMax M2.7, Gemini 3.1 Pro/Flash, several Xiaomi/Tencent/StepFun entries, and more. Two parallel surfaces showed the stale four: `/api/models` (composer picker, Settings → Default Model, /model slash) and `/api/providers` (Settings → Providers card). **Fix:** new `_format_nous_label()` helper in `api/config.py` that drops the vendor namespace and appends ` (via Nous)` (reusing `_format_ollama_label`'s token rules); new `elif pid == "nous":` branch in `_build_available_models_uncached()` mirroring the Ollama Cloud pattern (live-fetch via `hermes_cli.models.provider_model_ids("nous")`, prefix every id with `@nous:` to match the existing routing convention pinned by `tests/test_nous_portal_routing.py`, fall back to the curated 4-entry static list when `hermes_cli` is unavailable so the picker is never empty); same fix applied to `api/providers.py:get_providers()` for the parallel card-list path. + + **#1539 — Removed provider lingered in dropdowns until restart.** Server-side cache was correctly flushed (`set_provider_key()` calls `invalidate_models_cache()` on both add and remove), but three JS-side caches were never dropped after `/api/providers/delete`: `_slashModelCache`/`_slashModelCachePromise` (commands.js — feeds /model slash suggestions) and `_dynamicModelLabels`/`window._configuredModelBadges` (ui.js — populated by `populateModelDropdown`). Pre-fix, `_removeProviderKey()` only refreshed the providers card list and never asked any consumer to re-fetch /api/models. **Fix:** new `_invalidateSlashModelCache()` helper in `static/commands.js` (typeof-window-guarded so the module remains importable in headless `vm.runInContext` test contexts used by `tests/test_cli_only_slash_commands.py`); new `_refreshModelDropdownsAfterProviderChange()` helper in `static/panels.js` that calls the invalidator + `populateModelDropdown()`, wrapped in try/catch with a fire-and-forget `Promise.resolve(...).catch(()=>{})` so a slow `/api/models` doesn't block the providers panel refresh. Both `_saveProviderKey` and `_removeProviderKey` invoke the helper — defense-in-depth, the same staleness shape applies to the add path too. + + Verified live on port 8789: `/api/models` Nous group returns 30 models (was 4); browser `document.getElementById('modelSelect')` exposes 30 options under "Nous Portal"; the dropdown-flush helpers are callable from the browser and round-trip rebuild keeps the dropdown at 30 options. nesquena APPROVED before merge with full end-to-end trace + behavioral harness on the label formatter; one non-blocking docstring observation (3-letter token rule produces "PRO" rather than "Pro" on tokens like `gemini-3.1-pro-preview`) addressed in a follow-up `docs:` commit on the same branch — pure docstring text, no behavioral change. 23 new regression tests (12 on `tests/test_issue1538_nous_live_catalog.py` covering live-fetch + @nous: prefix invariant + " (via Nous)" suffix invariant + recent-flagship coverage + static fallback when hermes_cli raises + label formatter unit tests + static-list preservation; 11 on `tests/test_issue1539_provider_removal_dropdown_invalidation.py` covering helper definition + both cache slots cleared + window exposure with typeof guard + both save and remove paths invoke flush + helper resilience to missing modules + helper does not block panel refresh + server-side `set_provider_key → invalidate_models_cache` invariant pinned). 4013 tests pass (was 3990 → 4013, +23 from this PR). + ## [v0.50.281] — 2026-05-03 ### Fixed (1 PR by external contributor — closes #1527, #1530) diff --git a/ROADMAP.md b/ROADMAP.md index e6037424..e3b862d6 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,7 +2,7 @@ > Web companion to the Hermes Agent CLI. Same workflows, browser-native. > -> Last updated: v0.50.281 (May 03, 2026) — 3995 tests collected +> Last updated: v0.50.282 (May 03, 2026) — 4018 tests collected > Test source: `pytest tests/ --collect-only -q` > Per-version detail: see [CHANGELOG.md](./CHANGELOG.md) diff --git a/TESTING.md b/TESTING.md index 532502fa..37eaaabd 100644 --- a/TESTING.md +++ b/TESTING.md @@ -1835,8 +1835,8 @@ Bridged CLI sessions: --- -*Last updated: v0.50.281, May 03, 2026* -*Total automated tests collected: 3995* +*Last updated: v0.50.282, May 03, 2026* +*Total automated tests collected: 4018* *Regression gate: tests/test_regressions.py* *Run: pytest tests/ -v --timeout=60* *Source: /* diff --git a/api/config.py b/api/config.py index f6bd774f..694867b9 100644 --- a/api/config.py +++ b/api/config.py @@ -860,6 +860,37 @@ def _format_ollama_label(mid: str) -> str: return label +def _format_nous_label(mid: str) -> str: + """Turn a Nous Portal model id into a readable display label. + + Nous IDs are ``/[:]`` (e.g. ``anthropic/claude-opus-4.7``); + drop the vendor namespace, prettify the model name with the same token + rules as :func:`_format_ollama_label` (short acronyms uppercase, size + suffixes uppercase, capitalize the rest), then append ``" (via Nous)"`` + so the entry is visually distinct from same-named models in other + provider groups (e.g. direct Anthropic). + + Examples (matches the helper's actual output — labels are produced by + :func:`_format_ollama_label`'s token rules, so 3-letter tokens like + ``GPT`` and ``PRO`` render uppercase):: + + anthropic/claude-opus-4.7 -> Claude Opus 4.7 (via Nous) + openai/gpt-5.4-mini -> GPT 5.4 Mini (via Nous) + google/gemini-3.1-pro-preview -> Gemini 3.1 PRO Preview (via Nous) + moonshotai/kimi-k2.6 -> Kimi K2.6 (via Nous) + qwen/qwen3.5-plus-02-15 -> Qwen3.5 Plus 02 15 (via Nous) + nvidia/nemotron-3-super-120b-a12b -> Nemotron 3 Super 120B A12b (via Nous) + minimax/minimax-m2.5:free -> MiniMax M2.5 (Free) (via Nous) + """ + name_part = mid.split("/", 1)[-1] if "/" in mid else mid + # MiniMax-CN ids come back lowercase on the live wire (`minimax-m2.5`) but + # the curated label convention is mixed-case "MiniMax M2.5" — match that. + if name_part.lower().startswith("minimax"): + name_part = "MiniMax" + name_part[len("minimax"):] + base = _format_ollama_label(name_part) + return f"{base} (via Nous)" + + def _apply_provider_prefix( raw_models: list[dict], provider_id: str, @@ -2100,6 +2131,44 @@ def get_available_models() -> dict: except Exception: logger.warning("Failed to load Ollama Cloud models from hermes_cli") + if raw_models: + models = _apply_provider_prefix(raw_models, pid, active_provider) + groups.append( + { + "provider": provider_name, + "provider_id": pid, + "models": models, + } + ) + elif pid == "nous": + # Nous Portal exposes a curated catalog (~30 models, currently) + # via inference-api.nousresearch.com. Like ollama-cloud, we + # live-fetch through hermes_cli.models.provider_model_ids() + # rather than relying on the static four-entry list, which + # chronically drifts out of date (#1538). Fall back to the + # static list when hermes_cli is unavailable (test envs, + # package mismatches) so the picker is never empty. + raw_models = [] + try: + from hermes_cli.models import provider_model_ids as _provider_model_ids + + live_ids = _provider_model_ids("nous") or [] + raw_models = [ + # Prefix every live id with "@nous:" so routing matches + # the explicit-provider-hint branch of resolve_model_provider + # (same convention as the curated static list — see + # tests/test_nous_portal_routing.py for the invariant). + {"id": f"@nous:{mid}", "label": _format_nous_label(mid)} + for mid in live_ids + ] + except Exception: + logger.warning("Failed to load Nous Portal models from hermes_cli") + + if not raw_models: + # Static fallback: deepcopy so dedup/prefix mutation + # below does not bleed into the module-level catalog. + raw_models = copy.deepcopy(_PROVIDER_MODELS.get("nous", [])) + if raw_models: models = _apply_provider_prefix(raw_models, pid, active_provider) groups.append( diff --git a/api/providers.py b/api/providers.py index 4226aa1f..74b41354 100644 --- a/api/providers.py +++ b/api/providers.py @@ -391,7 +391,26 @@ def get_providers() -> dict[str, Any]: except Exception: pass - models = _PROVIDER_MODELS.get(pid, []) + models = list(_PROVIDER_MODELS.get(pid, [])) + # Nous Portal: prefer the live catalog so the providers card matches + # the dropdown picker (#1538). Same fallback shape as the static-only + # case below — when hermes_cli is unavailable or its lookup raises, + # we keep the four-entry curated list. + if pid == "nous": + try: + from hermes_cli.models import provider_model_ids as _provider_model_ids + + live_ids = _provider_model_ids("nous") or [] + if live_ids: + # Lazy-import to avoid circular dep with api.config. + from api.config import _format_nous_label + + models = [ + {"id": f"@nous:{mid}", "label": _format_nous_label(mid)} + for mid in live_ids + ] + except Exception: + logger.debug("Failed to load Nous Portal models from hermes_cli") # Also include models from config.yaml providers section if isinstance(providers_cfg, dict): provider_cfg = providers_cfg.get(pid, {}) diff --git a/static/commands.js b/static/commands.js index dc806f19..375a9d67 100644 --- a/static/commands.js +++ b/static/commands.js @@ -88,6 +88,22 @@ let _slashPersonalityCachePromise=null; let _agentCommandCache=null; let _agentCommandCachePromise=null; +// Invalidate the /api/models slash-suggestion cache. Called by panels.js +// after a provider is added or removed so the next /model autocomplete +// rebuilds from a fresh /api/models response (#1539). Returning a function +// rather than letting callers poke the module-local lets/promises directly +// keeps the cache shape encapsulated to this module. +function _invalidateSlashModelCache(){ + _slashModelCache=null; + _slashModelCachePromise=null; +} +// Expose on window when available. Guarded by typeof so the module is +// importable in headless test contexts (vm.runInContext) that don't +// define a window global — see tests/test_cli_only_slash_commands.py. +if(typeof window!=='undefined'){ + window._invalidateSlashModelCache=_invalidateSlashModelCache; +} + function _normalizeSlashSubArg(value){ return String(value||'').trim(); } diff --git a/static/panels.js b/static/panels.js index 5934a5c9..f633a7f4 100644 --- a/static/panels.js +++ b/static/panels.js @@ -3364,6 +3364,11 @@ async function _saveProviderKey(providerId){ if(res.ok){ showToast(res.provider+' key '+res.action); els.input.value=''; + // Invalidate every dropdown surface that caches /api/models so the + // newly-configured provider's models show up without a server restart + // or page reload (#1539). Server-side invalidate_models_cache() is + // already called by api/providers.py:set_provider_key. + _refreshModelDropdownsAfterProviderChange(); await loadProvidersPanel(); // refresh list }else{ showToast(res.error||'Failed to save key'); @@ -3385,6 +3390,12 @@ async function _removeProviderKey(providerId){ const res=await api('/api/providers/delete',{method:'POST',body:JSON.stringify({provider:providerId})}); if(res.ok){ showToast(res.provider+' key '+t('providers_key_removed').toLowerCase()); + // Drop the removed provider from every cached dropdown surface so it + // disappears immediately — composer picker, /model slash command, + // Settings → Default Model, configured-model badges (#1539). + // Without this, a stale list from before the delete keeps offering + // the now-removed provider's models until the page is reloaded. + _refreshModelDropdownsAfterProviderChange(); await loadProvidersPanel(); // refresh list }else{ showToast(res.error||'Failed to remove key'); @@ -3396,6 +3407,28 @@ async function _removeProviderKey(providerId){ } } +// Shared dropdown-cache flush invoked after a provider add/remove. The +// server-side TTL cache is already invalidated by /api/providers and +// /api/providers/delete (via api/providers.py:set_provider_key); this +// flushes the JS-side caches so the next render rebuilds from a fresh +// /api/models response. Wrapped in a try/catch so a UI module that hasn't +// loaded yet (e.g. during early Settings open) cannot break the save flow. +function _refreshModelDropdownsAfterProviderChange(){ + try{ + if(typeof window._invalidateSlashModelCache==='function'){ + window._invalidateSlashModelCache(); + } + if(typeof populateModelDropdown==='function'){ + // Fire-and-forget: don't block the providers panel refresh on a + // dropdown rebuild. The composer/Settings dropdowns will catch up + // on the very next paint frame. + Promise.resolve(populateModelDropdown()).catch(()=>{}); + } + }catch(_e){ + // Swallow — dropdown refresh is best-effort, providers panel must still update. + } +} + async function _refreshProviderModels(providerId, btn){ btn.disabled=true; const orig=btn.innerHTML; diff --git a/tests/test_issue1538_nous_live_catalog.py b/tests/test_issue1538_nous_live_catalog.py new file mode 100644 index 00000000..4a9ddb03 --- /dev/null +++ b/tests/test_issue1538_nous_live_catalog.py @@ -0,0 +1,313 @@ +"""Regression tests for #1538 — Nous Portal model picker should live-fetch +the full catalog (~30 models) instead of returning the four-entry static list. + +Background +---------- +Settings → Default Model showed only four Nous models (Claude Opus 4.6, Claude +Sonnet 4.6, GPT-5.4 Mini, Gemini 3.1 Pro Preview) because +``_build_available_models_uncached()`` fell through to the generic +``pid in _PROVIDER_MODELS`` branch and returned ``copy.deepcopy(_PROVIDER_MODELS["nous"])``. +The actual Nous Portal catalog has ~30 models live — including the latest +Anthropic 4.7 family, GPT-5.5, Gemini 3.1 Pro/Flash, Kimi K2.6, MiniMax M2.7, +several Xiaomi/Tencent/StepFun entries. + +Fix +--- +A dedicated ``elif pid == "nous":`` branch in ``_build_available_models_uncached()`` +mirroring the Ollama Cloud pattern: live-fetch via +``hermes_cli.models.provider_model_ids("nous")``, prefix every id with ``@nous:`` +to match the existing routing convention, fall back to the curated static +list when ``hermes_cli`` is unavailable. +""" + +from __future__ import annotations + +import sys +import types + +import api.config as config +import api.profiles as profiles + + +# Sample Nous catalog used in the live-fetch test. Mirrors the shape returned +# by hermes_cli.models.provider_model_ids("nous") (see #1538 issue body). +SAMPLE_NOUS_LIVE_IDS = [ + "moonshotai/kimi-k2.6", + "xiaomi/mimo-v2.5-pro", + "anthropic/claude-opus-4.7", + "anthropic/claude-opus-4.6", + "anthropic/claude-sonnet-4.6", + "anthropic/claude-haiku-4.5", + "openai/gpt-5.5", + "openai/gpt-5.4-mini", + "openai/gpt-5.3-codex", + "google/gemini-3-pro-preview", + "google/gemini-3.1-pro-preview", + "google/gemini-3.1-flash-lite-preview", + "qwen/qwen3.5-plus-02-15", + "minimax/minimax-m2.7", + "z-ai/glm-5.1", + "x-ai/grok-4.20-beta", + "tencent/hy3-preview", + "stepfun/step-3.5-flash", + "nvidia/nemotron-3-super-120b-a12b", + "arcee-ai/trinity-large-thinking", +] + + +def _install_fake_hermes_cli(monkeypatch, *, nous_ids=None, raise_on_lookup=False): + """Install fake ``hermes_cli`` modules so detection sees Nous as authenticated + and ``provider_model_ids("nous")`` returns the desired catalog. + + Mirrors :func:`tests.test_issue1420_lmstudio_provider_env_var._install_fake_hermes_cli` + but specialised for Nous detection (Nous is OAuth so the env-var path + is not used — we drive detection via ``hermes_cli.auth.list_auth_providers``). + """ + fake_pkg = types.ModuleType("hermes_cli") + fake_pkg.__path__ = [] + + fake_models = types.ModuleType("hermes_cli.models") + fake_models.list_available_providers = lambda: [] + if raise_on_lookup: + def _raise(_pid): + raise RuntimeError("simulated hermes_cli failure") + fake_models.provider_model_ids = _raise + else: + ids = list(nous_ids) if nous_ids is not None else [] + fake_models.provider_model_ids = lambda pid: ids if pid == "nous" else [] + + fake_auth = types.ModuleType("hermes_cli.auth") + + def _list_auth_providers(): + return [{"id": "nous", "authenticated": True}] + + def _get_auth_status(pid): + return {"logged_in": True, "key_source": ""} if pid == "nous" else {} + + fake_auth.list_auth_providers = _list_auth_providers + fake_auth.get_auth_status = _get_auth_status + + monkeypatch.setitem(sys.modules, "hermes_cli", fake_pkg) + monkeypatch.setitem(sys.modules, "hermes_cli.models", fake_models) + monkeypatch.setitem(sys.modules, "hermes_cli.auth", fake_auth) + monkeypatch.delitem(sys.modules, "agent.credential_pool", raising=False) + monkeypatch.delitem(sys.modules, "agent", raising=False) + + config.invalidate_models_cache() + + +def _swap_in_test_config(extra_cfg): + """Snapshot config.cfg, replace with a minimal test config; return restore-fn.""" + old_cfg = dict(config.cfg) + old_mtime = config._cfg_mtime + config.cfg.clear() + config.cfg["model"] = {} + config.cfg.update(extra_cfg) + try: + config._cfg_mtime = config.Path(config._get_config_path()).stat().st_mtime + except Exception: + config._cfg_mtime = 0.0 + + def _restore(): + config.cfg.clear() + config.cfg.update(old_cfg) + config._cfg_mtime = old_mtime + + return _restore + + +def _scrub_provider_env(monkeypatch): + """Drop every provider env var so detection only sees what we install + via the fake hermes_cli stubs (not unrelated keys leaked from the runner).""" + for var in ( + "ANTHROPIC_API_KEY", "OPENAI_API_KEY", "GOOGLE_API_KEY", "GEMINI_API_KEY", + "DEEPSEEK_API_KEY", "XAI_API_KEY", "GROQ_API_KEY", + "MISTRAL_API_KEY", "OPENROUTER_API_KEY", + "OLLAMA_CLOUD_API_KEY", "OLLAMA_API_KEY", + "GLM_API_KEY", "KIMI_API_KEY", "MOONSHOT_API_KEY", + "MINIMAX_API_KEY", "MINIMAX_CN_API_KEY", + "OPENCODE_ZEN_API_KEY", "OPENCODE_GO_API_KEY", + "NOUS_API_KEY", "NVIDIA_API_KEY", "LM_API_KEY", "LMSTUDIO_API_KEY", + ): + monkeypatch.delenv(var, raising=False) + + +class TestNousLiveCatalog: + """When the Nous live catalog is available, the dropdown must surface it + in full (>=20 entries) — not the four-entry static fallback (#1538).""" + + def test_nous_models_live_fetch_when_hermes_cli_available(self, monkeypatch, tmp_path): + _scrub_provider_env(monkeypatch) + _install_fake_hermes_cli(monkeypatch, nous_ids=SAMPLE_NOUS_LIVE_IDS) + monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path) + + restore = _swap_in_test_config({"model": {"provider": "nous"}}) + try: + data = config.get_available_models() + nous_groups = [g for g in data.get("groups", []) if g.get("provider_id") == "nous"] + assert len(nous_groups) == 1, ( + f"Expected exactly one Nous group, got {len(nous_groups)}: " + f"{[g.get('provider_id') for g in data.get('groups', [])]}" + ) + models = nous_groups[0]["models"] + assert len(models) >= 20, ( + f"Live-fetched Nous catalog should expose >=20 entries, got " + f"{len(models)}. The dispatch branch fell through to the four-entry " + f"static list — pre-#1538 behaviour." + ) + finally: + restore() + + def test_nous_model_ids_carry_at_nous_prefix(self, monkeypatch, tmp_path): + _scrub_provider_env(monkeypatch) + _install_fake_hermes_cli(monkeypatch, nous_ids=SAMPLE_NOUS_LIVE_IDS) + monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path) + + restore = _swap_in_test_config({"model": {"provider": "nous"}}) + try: + data = config.get_available_models() + nous_group = next(g for g in data["groups"] if g["provider_id"] == "nous") + for m in nous_group["models"]: + assert m["id"].startswith("@nous:"), ( + f"Every Nous model id must start with '@nous:' so " + f"resolve_model_provider routes through the explicit-provider-hint " + f"branch (matches the static-list invariant from " + f"tests/test_nous_portal_routing.py). Got: {m['id']!r}" + ) + finally: + restore() + + def test_nous_labels_carry_via_nous_suffix(self, monkeypatch, tmp_path): + _scrub_provider_env(monkeypatch) + _install_fake_hermes_cli(monkeypatch, nous_ids=SAMPLE_NOUS_LIVE_IDS) + monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path) + + restore = _swap_in_test_config({"model": {"provider": "nous"}}) + try: + data = config.get_available_models() + nous_group = next(g for g in data["groups"] if g["provider_id"] == "nous") + for m in nous_group["models"]: + assert m["label"].endswith(" (via Nous)"), ( + f"Every Nous live-fetched label must end with ' (via Nous)' so " + f"the user can distinguish them from same-named direct-provider " + f"entries (e.g. 'Claude Opus 4.7' via direct Anthropic). " + f"Got: {m['label']!r}" + ) + finally: + restore() + + def test_nous_live_catalog_includes_recent_models(self, monkeypatch, tmp_path): + """Sanity: the recent-flagship models from the user's bug report + (Claude Opus 4.7, GPT-5.5, Kimi K2.6) must reach the dropdown.""" + _scrub_provider_env(monkeypatch) + _install_fake_hermes_cli(monkeypatch, nous_ids=SAMPLE_NOUS_LIVE_IDS) + monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path) + + restore = _swap_in_test_config({"model": {"provider": "nous"}}) + try: + data = config.get_available_models() + nous_group = next(g for g in data["groups"] if g["provider_id"] == "nous") + ids = {m["id"] for m in nous_group["models"]} + for required in ( + "@nous:anthropic/claude-opus-4.7", + "@nous:openai/gpt-5.5", + "@nous:moonshotai/kimi-k2.6", + "@nous:google/gemini-3.1-pro-preview", + "@nous:minimax/minimax-m2.7", + ): + assert required in ids, ( + f"{required} missing from live-fetched Nous catalog. Either " + f"the hermes_cli dispatch is broken or the @nous: prefix is " + f"missing." + ) + finally: + restore() + + +class TestNousStaticFallback: + """When ``hermes_cli`` is not importable or its lookup raises, we fall back + to the curated four-entry static list — never empty.""" + + def test_static_fallback_when_hermes_cli_raises(self, monkeypatch, tmp_path): + _scrub_provider_env(monkeypatch) + _install_fake_hermes_cli(monkeypatch, raise_on_lookup=True) + monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path) + + restore = _swap_in_test_config({"model": {"provider": "nous"}}) + try: + data = config.get_available_models() + nous_groups = [g for g in data.get("groups", []) if g.get("provider_id") == "nous"] + assert nous_groups, ( + "Nous group must still appear when hermes_cli fails — the " + "branch should fall back to the curated static list." + ) + models = nous_groups[0]["models"] + assert len(models) == 4, ( + f"Static fallback should expose exactly the four curated entries " + f"in _PROVIDER_MODELS['nous']. Got {len(models)}: " + f"{[m['id'] for m in models]}" + ) + for m in models: + assert m["id"].startswith("@nous:"), m["id"] + finally: + restore() + + +class TestFormatNousLabel: + """Unit tests for the label formatter helper.""" + + def test_strips_vendor_namespace(self): + from api.config import _format_nous_label + assert _format_nous_label("anthropic/claude-opus-4.7") == "Claude Opus 4.7 (via Nous)" + assert _format_nous_label("openai/gpt-5.4-mini") == "GPT 5.4 Mini (via Nous)" + + def test_handles_missing_vendor(self): + from api.config import _format_nous_label + # Defensive: id without slash should still render a sane label. + assert _format_nous_label("kimi-k2.6") == "Kimi K2.6 (via Nous)" + + def test_handles_variant_after_colon(self): + from api.config import _format_nous_label + # Variant rendered in parentheses, mirroring _format_ollama_label. + out = _format_nous_label("minimax/minimax-m2.5:free") + assert out.endswith(" (via Nous)") + assert "Free" in out + assert "MiniMax M2.5" in out + + def test_minimax_renders_mixed_case(self): + from api.config import _format_nous_label + # Live wire returns lowercase 'minimax/minimax-...' but the curated + # convention is mixed-case 'MiniMax'. + assert _format_nous_label("minimax/minimax-m2.7").startswith("MiniMax M2.7") + + def test_label_always_ends_with_via_nous_suffix(self): + from api.config import _format_nous_label + for sample in [ + "anthropic/claude-opus-4.7", + "openai/gpt-5.5", + "google/gemini-3.1-pro-preview", + "moonshotai/kimi-k2.6", + "z-ai/glm-5.1", + "stepfun/step-3.5-flash", + ]: + assert _format_nous_label(sample).endswith(" (via Nous)"), sample + + +class TestStaticListPreservedAsFallback: + """The curated ``_PROVIDER_MODELS['nous']`` entry stays as the static + fallback; existing routing invariants from + :mod:`tests.test_nous_portal_routing` must remain valid.""" + + def test_static_list_present(self): + from api.config import _PROVIDER_MODELS + assert _PROVIDER_MODELS.get("nous"), ( + "The curated static Nous list must remain in _PROVIDER_MODELS as " + "a fallback for environments where hermes_cli is unavailable." + ) + + def test_static_list_keeps_at_nous_prefix(self): + # Keep parity with tests/test_nous_portal_routing.py — ensures the + # static fallback path produces correctly-routable ids when used. + from api.config import _PROVIDER_MODELS + for m in _PROVIDER_MODELS["nous"]: + assert m["id"].startswith("@nous:"), m["id"] diff --git a/tests/test_issue1539_provider_removal_dropdown_invalidation.py b/tests/test_issue1539_provider_removal_dropdown_invalidation.py new file mode 100644 index 00000000..59ada50d --- /dev/null +++ b/tests/test_issue1539_provider_removal_dropdown_invalidation.py @@ -0,0 +1,225 @@ +"""Regression tests for #1539 — removing a provider in Settings must invalidate +every dropdown surface that caches /api/models, so the removed provider +disappears immediately without a server restart or page reload. + +The bug +------- +Pre-fix, ``_removeProviderKey()`` in ``static/panels.js`` only called +``loadProvidersPanel()`` after deletion. That refreshed the providers card +list but left these JS-side caches stale: + + * ``_slashModelCache`` / ``_slashModelCachePromise`` (``static/commands.js``) — + cache for the ``/model`` slash-command suggestions. + * ``_dynamicModelLabels`` / ``window._configuredModelBadges`` (``static/ui.js``) — + populated by ``populateModelDropdown()`` on boot and on profile switch. + +Layered server-side cache via ``api/config.invalidate_models_cache`` was +already flushed (``set_provider_key`` calls it on both add + remove), so the +next ``/api/models`` request would return the correct list — but no consumer +was triggering one. + +The fix +------- +``static/commands.js`` exposes an ``_invalidateSlashModelCache()`` helper on +``window``. ``static/panels.js`` calls it from a shared +``_refreshModelDropdownsAfterProviderChange()`` helper after both the save +and the remove paths, plus invokes ``populateModelDropdown()`` to rebuild +the composer / Settings dropdowns and ``_configuredModelBadges`` map. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest + + +REPO = Path(__file__).resolve().parent.parent + + +def _read_static(name: str) -> str: + return (REPO / "static" / name).read_text(encoding="utf-8") + + +def _extract_function_body(src: str, signature: str) -> str: + """Return the source of a top-level ``async function NAME(...)`` / + ``function NAME(...)`` declaration via brace-balance — robust to nested + blocks (try/catch/await) and not dependent on indentation. + """ + idx = src.find(signature) + if idx == -1: + raise AssertionError(f"signature {signature!r} not found in source") + open_idx = src.find("{", idx) + if open_idx == -1: + raise AssertionError(f"could not find opening brace after {signature!r}") + depth = 0 + for i in range(open_idx, len(src)): + c = src[i] + if c == "{": + depth += 1 + elif c == "}": + depth -= 1 + if depth == 0: + return src[idx : i + 1] + raise AssertionError(f"unbalanced braces in {signature!r}") + + +class TestSlashModelCacheInvalidator: + """``static/commands.js`` must export the helper to ``window`` so + ``static/panels.js`` can drop the slash-command cache without poking + module-local lets across module boundaries.""" + + def test_invalidator_helper_defined(self): + src = _read_static("commands.js") + assert "function _invalidateSlashModelCache(" in src, ( + "_invalidateSlashModelCache helper missing from static/commands.js. " + "Without it static/panels.js cannot drop the /model slash-command " + "cache when a provider is added/removed (#1539)." + ) + + def test_invalidator_clears_both_cache_slots(self): + src = _read_static("commands.js") + body = _extract_function_body(src, "function _invalidateSlashModelCache(") + # Cache slots from static/commands.js:84-85 — keep both null'd. + assert "_slashModelCache=null" in body, ( + "_invalidateSlashModelCache must null _slashModelCache so the next " + "/model autocomplete refetches /api/models." + ) + assert "_slashModelCachePromise=null" in body, ( + "_invalidateSlashModelCache must null _slashModelCachePromise so an " + "in-flight load doesn't resolve into the stale cache slot after " + "invalidation." + ) + + def test_invalidator_exposed_on_window(self): + src = _read_static("commands.js") + # Exposed on window via a typeof-guarded assignment so the module is + # also importable in headless test contexts (vm.runInContext) that + # don't define a window global. + assert "window._invalidateSlashModelCache=_invalidateSlashModelCache" in src, ( + "_invalidateSlashModelCache must be exposed on window so static/panels.js " + "can invoke it across module boundaries." + ) + assert "typeof window!=='undefined'" in src, ( + "The window-export assignment must be guarded by `typeof window!=='undefined'` " + "so static/commands.js stays importable in headless vm contexts (the " + "tests/test_cli_only_slash_commands.py harness has no window global)." + ) + + +class TestProviderRemoveInvalidatesDropdowns: + """The remove path in ``static/panels.js`` must trigger the dropdown-cache + flush and rebuild — otherwise the dropped provider lingers in every + /model dropdown until the page reloads (#1539).""" + + def test_remove_path_invokes_dropdown_flush(self): + src = _read_static("panels.js") + body = _extract_function_body(src, "async function _removeProviderKey(") + assert "_refreshModelDropdownsAfterProviderChange()" in body, ( + "_removeProviderKey must call _refreshModelDropdownsAfterProviderChange() " + "after a successful delete. Without this, the JS-side caches " + "(_slashModelCache, _dynamicModelLabels, _configuredModelBadges) " + "still offer the deleted provider's models until reload (#1539)." + ) + + def test_save_path_invokes_dropdown_flush(self): + """Defense-in-depth: adding a key has the same staleness shape — the + new provider's models won't show up until reload without this call. + Bundled in #1539.""" + src = _read_static("panels.js") + body = _extract_function_body(src, "async function _saveProviderKey(") + assert "_refreshModelDropdownsAfterProviderChange()" in body, ( + "_saveProviderKey must also call _refreshModelDropdownsAfterProviderChange() " + "so a newly-configured provider's models appear in every dropdown " + "without a reload. Same staleness shape as the remove path (#1539)." + ) + + def test_dropdown_flush_helper_defined(self): + src = _read_static("panels.js") + assert "function _refreshModelDropdownsAfterProviderChange(" in src, ( + "_refreshModelDropdownsAfterProviderChange must be defined in " + "static/panels.js (single helper used by both save + remove paths)." + ) + + def test_dropdown_flush_calls_slash_cache_invalidator(self): + src = _read_static("panels.js") + body = _extract_function_body(src, "function _refreshModelDropdownsAfterProviderChange(") + # Must invoke the commands.js helper — directly poking module-local + # lets across module boundaries is brittle. + assert "_invalidateSlashModelCache" in body, ( + "_refreshModelDropdownsAfterProviderChange must call " + "window._invalidateSlashModelCache() so the /model slash-command " + "cache is dropped (covers the slash-command surface from #1539)." + ) + + def test_dropdown_flush_calls_populate_model_dropdown(self): + src = _read_static("panels.js") + body = _extract_function_body(src, "function _refreshModelDropdownsAfterProviderChange(") + assert "populateModelDropdown" in body, ( + "_refreshModelDropdownsAfterProviderChange must call " + "populateModelDropdown() so the composer model picker, Settings → " + "Default Model dropdown, _dynamicModelLabels, and " + "_configuredModelBadges all rebuild from a fresh /api/models " + "response (covers the dropdown + badge surfaces from #1539)." + ) + + def test_dropdown_flush_is_resilient_to_missing_modules(self): + """If commands.js or ui.js failed to load, the providers panel must + still update — the dropdown flush is best-effort (#1539).""" + src = _read_static("panels.js") + body = _extract_function_body(src, "function _refreshModelDropdownsAfterProviderChange(") + # Outer try/catch wraps the whole helper so a runtime error inside + # populateModelDropdown / cache flush cannot surface as an unhandled + # rejection that breaks the surrounding save/remove flow. + assert re.search(r"\btry\s*\{", body), ( + "_refreshModelDropdownsAfterProviderChange must wrap its work in " + "try/catch — if commands.js or ui.js failed to load, a missing " + "function should not break the providers panel update (#1539)." + ) + # And the populateModelDropdown call must be guarded by typeof — the + # dropdown rebuild is best-effort. + assert "typeof populateModelDropdown" in body, ( + "populateModelDropdown lookup must use typeof so it gracefully " + "skips when ui.js hasn't loaded yet." + ) + + def test_dropdown_flush_does_not_block_panel_refresh(self): + """populateModelDropdown is async; its result must not be awaited + synchronously inside the helper — otherwise a slow /api/models would + delay the providers panel re-render (#1539).""" + src = _read_static("panels.js") + body = _extract_function_body(src, "function _refreshModelDropdownsAfterProviderChange(") + # The helper itself is non-async (signature checked indirectly: the + # source begins with 'function _refresh...', not 'async function'). + # Anything async is fired with Promise.resolve(...).catch(...) so the + # provider panel re-render is not blocked. + assert body.startswith("function _refreshModelDropdownsAfterProviderChange"), ( + "_refreshModelDropdownsAfterProviderChange should be a sync helper " + "that fires-and-forgets populateModelDropdown — not an async one " + "the save/remove paths await." + ) + + +class TestServerSideInvariantPreserved: + """Server-side ``invalidate_models_cache()`` is the load-bearing invariant + that lets the next /api/models request return correct data; #1539 was a + pure frontend bug, but pin the server-side wiring so a refactor of + ``set_provider_key`` cannot silently regress it.""" + + def test_set_provider_key_invalidates_cache(self): + src = (REPO / "api" / "providers.py").read_text(encoding="utf-8") + # set_provider_key is the canonical write path — both add and remove + # flow through it (remove_provider_key calls set_provider_key(pid, None)). + m = re.search( + r"def set_provider_key\([^)]*\).*?(?=\ndef |\Z)", + src, + re.DOTALL, + ) + assert m, "set_provider_key not found in api/providers.py" + body = m.group(0) + assert "invalidate_models_cache()" in body, ( + "set_provider_key must call invalidate_models_cache() so the " + "server-side TTL cache is flushed on every add/remove. Without " + "this, even a perfectly-cached frontend would receive stale data." + )