mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-05-25 11:10:18 +00:00
Merge pull request #1577 from nesquena/stage-288
v0.50.288 — picker symmetry + cron profile isolation (3 PRs)
This commit is contained in:
@@ -1,5 +1,34 @@
|
||||
# Hermes Web UI -- Changelog
|
||||
|
||||
## [v0.50.288] — 2026-05-03
|
||||
|
||||
### Fixed (3 PRs — picker symmetry + cron profile isolation — closes #1567, #1568, #1573)
|
||||
|
||||
- **Nous Portal endpoint disagreement + featured-set cap** (#1569; closes #1567) — reporter (Deor, Discord, relayed by @AvidFuturist) saw Settings → Providers card showing `"Nous Portal — 396 models · OAuth"` while the in-conversation model picker dropdown listed only the 4 hardcoded curated entries (Claude Opus 4.6, Claude Sonnet 4.6, GPT-5.4 Mini, Gemini 3.1 Pro Preview). Two related root-shape bugs bundled. **(1)** Asymmetric auth detection — `api/providers.py:get_providers` iterates ALL OAuth providers regardless of authentication state and unconditionally live-fetches the catalog, while `api/config.py:_build_available_models_uncached` only iterates providers in `detected_providers`, gated on `hermes_cli.models.list_available_providers().authenticated`. That flag can disagree with `hermes_cli.auth.get_auth_status(<id>).logged_in`, so when the disagreement happens for Nous, the picker silently falls through to the curated 4-entry static list while the providers card keeps showing the live catalog. **Fix:** added explicit `get_auth_status("nous").logged_in` check after the existing `list_available_providers()` loop — picker now includes Nous whenever the providers card would. **(2)** UX cap — even with the disagreement fixed, dumping a 397-model catalog into a flat dropdown is unusable. New `_build_nous_featured_set()` helper at `api/config.py:965` runs the same algorithm in both `/api/models` and `/api/models/live` so background enrichment doesn't undo the trim. Selection rules (deterministic): sticky-selection always pinned, every curated flagship preserved, vendor round-robin via `_NOUS_VENDOR_PRIORITY` for top-up to 15. Disclosure pattern: optgroup label `"Nous Portal (15 of 397)"`, new `extra_models` field on the API surface, slash command + `_dynamicModelLabels` map hydrated from both halves so a model selected outside the featured slice still renders with its proper label, providers card uses `models_total` for the header count + small `+N more` disclosure pill at the end of the rendered pill list. **(3)** Stale-fallback poisoning — when authenticated AND live-fetch returns `[]` (transient hermes_cli failure, OAuth refresh in flight, cache miss), omit the Nous group entirely rather than falling back to stale-4 (which actively contradicts the providers card instead of self-healing). Static fallback only when `hermes_cli` is unavailable or raises (test envs, package mismatches). 20 new tests in `tests/test_issue1567_nous_picker_capacity_and_symmetry.py` covering selection helper invariants, large-catalog cap behavior, detection symmetry, live-fetch-empty handling, providers/picker symmetry, frontend extras contract.
|
||||
|
||||
- **Cron Scheduled Jobs panel respects per-request active profile** (#1571 by @kowenhaoai; closes #1573) — `/api/crons*` endpoints called into `cron.jobs` (from `hermes-agent`), whose path resolver reads `HERMES_HOME` from `os.environ` at call time. The WebUI's per-request profile isolation (#798) is thread-local — set per-request from the `hermes_profile` cookie in `server.py`, cleared after the request — so those two mechanisms didn't talk to each other and `cron.jobs` always saw the process-default `HERMES_HOME` no matter which profile the request belonged to. CRUD operations silently wrote to the wrong `jobs.json`. **Fix:** two new context managers in `api/profiles.py:139-260`, both holding a module-level `_cron_env_lock`. `cron_profile_context()` is the HTTP-side variant (resolves home via `get_active_hermes_home()` which honors the TLS cookie, swaps `os.environ['HERMES_HOME']`, re-patches the cached `cron.jobs.HERMES_DIR/CRON_DIR/JOBS_FILE/OUTPUT_DIR` module constants, restores everything on exit). `cron_profile_context_for_home(home)` is the thread-side variant (worker threads have no TLS context, so the HTTP handler captures the active home at dispatch time and passes it explicitly). All 12 cron endpoints wrapped (6 GET + 6 POST). `_handle_cron_run` additionally captures the TLS-active home at dispatch and forwards it into `_run_cron_tracked(job, profile_home)` so cron output files land in the correct profile directory. Pre-release reviewer pushed test-skip-on-missing-agent fix so machines without `~/hermes-agent` run the suite cleanly. Post-review tightening: removed an over-broad `except Exception` around `get_active_hermes_home()` in `_handle_cron_run` (silent fallback to `_profile_home=None` would have re-introduced the exact bug the PR fixes — let any unexpected exception 500 the request rather than risk silent cross-profile state corruption); added thread-safety note on `os.environ` mutation explaining why `_cron_env_lock` is sufficient given CPython GIL semantics + `subprocess.Popen` env inheritance at fork time. 4 regression tests in `tests/test_scheduled_jobs_profile_isolation.py`. Two follow-up issues filed for architectural concerns (#1574 lock granularity, #1575 in-process scheduler bypass) — both deferred as out of scope. **Verified end-to-end via real browser test on isolated environment** (12 sessions, 3 projects, 6 default crons + 1 work-only-cron, 2 profiles): UI profile switch → cron tab auto-refreshes to show only target profile's jobs, both directions; on-disk verification confirmed perfect isolation in `~/.hermes/cron/jobs.json` (default profile) vs `~/.hermes/profiles/work/cron/jobs.json`.
|
||||
|
||||
- **Collapse duplicate provider groups + guard provider-id-as-model.default** (#1572; closes #1568) — reporter (Deor, Discord, relayed by @AvidFuturist) saw the Settings → Default Model dropdown rendering OpenCode Go provider as TWO separate optgroups: `"OpenCode Go"` (canonical, with all 14 catalog models) and `"Opencode_Go"` (phantom group containing one self-referential entry). Three structural causes (all in `api/config.py:_build_available_models_uncached`). **(1)** Detection-path id leakage — `cfg["providers"]` keys are read verbatim, so a config with `providers.opencode_go.api_key` (underscore variant) AND another path adding the canonical `opencode-go` (e.g. via `active_provider`) end up with both in `detected_providers`, creating two distinct provider groups with the second labelled via `pid.title()` fallback as `"Opencode_Go"`. **(2)** Injection-block rogue model — the default-model injection block puts ANY `model.default` string into the picker as a fake option, so a stray `model.default: opencode_go` (provider id mistakenly used as a model id) surfaces as a phantom model labelled `"Opencode GO"`. **(3)** Empty-group bleed — when a non-canonical provider id makes it into `detected_providers` but has no entry in `_PROVIDER_MODELS`, the build loop creates an optgroup with zero models. **Fix:** new `_canonicalise_provider_id()` helper folds underscores to hyphens, lowercases, applies alias resolution only when the alias target is itself canonical in `_PROVIDER_DISPLAY` (the constraint that prevents `x-ai` from round-tripping through the alias table to `xai`). Detection-path canonicalises before adding to `detected_providers`; same treatment in the `only_show_configured` intersection. Post-collection dedup pass re-canonicalises every entry (belt-and-braces against future regressions in any of the ~25 `detected_providers.add(...)` callsites). Provider-id guard on the model.default injection block — when the injected value matches a known provider display name or alias (after underscore/case normalization), skip the injection and emit a `logger.warning`. Real unknown model IDs (newly released models, custom endpoints) still get injected — only provider-shaped values are rejected. Empty-group filter at end of build (drops optgroups with zero models, with `custom:` exemption since users may want an empty card visible as a reminder). 17 new tests in `tests/test_issue1568_duplicate_provider_groups.py` covering the helper unit, dedup E2E, model.default guard, empty-group filter. Plus one structural test fix in `tests/test_issue604_all_providers_model_picker.py:test_cfg_providers_only_adds_known` — widened the regex window from 500 → 1500 chars so the new documentation comment block doesn't push `_PROVIDER_MODELS` past the substring slice (pre-existing brittle-window pattern, not a new issue).
|
||||
|
||||
### Tests
|
||||
|
||||
4053 → **4094 passing** (+41 net: +20 from #1569 Nous featured-set, +17 from #1572 dedup, +4 from #1571 cron isolation). 0 regressions. Full suite in 108s.
|
||||
|
||||
### Pre-release verification
|
||||
|
||||
- All 41 PR-related tests pass standalone.
|
||||
- All 4094 tests pass in the full suite (clean state, no pre-existing flakes triggered).
|
||||
- Browser sanity (HTTP API checks against port 8789): 11/11 endpoints verified.
|
||||
- All modified JS files (`static/commands.js`, `static/panels.js`, `static/ui.js`) pass `node -c`.
|
||||
- **Real-world browser testing** on isolated test environment (12 sessions, 3 projects, 6 default crons + 1 work cron, 4 skills, 2 profiles): profile switch via UI updates the chip, sidebar re-renders, **cron tab auto-refreshes to show only target profile's jobs**. On-disk verification confirms perfect isolation. Profile chip + cron tab UI confirmed by vision-model.
|
||||
- Pre-release Opus advisor: SHIP AS-IS — no MUST-FIX. All 5 verification questions check out (conflict-free merge, no deadlock between `_cron_env_lock` and `_available_models_cache_lock`, subprocess env inheritance under lock verified, `_canonicalise_provider_id` dedup-pass idempotent, stale-fallback handling correct under partial network failure). One non-blocking symmetry nit on `_run_cron_tracked` worker-side broad-except flagged as a follow-up issue.
|
||||
|
||||
### Maintainer in-stage actions
|
||||
|
||||
- **PR rebase verified clean** (REBASE-DEFAULT rule applied). All 3 PR branches were on or near current master; rebase was no-op.
|
||||
- **#1571 post-review fix combination**: contributor's `df03055` (post-review tightening) was on `pull/1571/head` while reviewer's `d83e1d8` (test-skip-on-missing-agent) was on `origin/fix/scheduled-jobs-profile-isolation`. Cherry-picked the test-skip commit onto the contributor branch to combine both fixes before merging into stage.
|
||||
|
||||
|
||||
## [v0.50.287] — 2026-05-03
|
||||
|
||||
### Fixed (1 PR — closes another vector for the pending-message-loss class)
|
||||
|
||||
+1
-1
@@ -2,7 +2,7 @@
|
||||
|
||||
> Web companion to the Hermes Agent CLI. Same workflows, browser-native.
|
||||
>
|
||||
> Last updated: v0.50.287 (May 03, 2026) — 4053 tests collected
|
||||
> Last updated: v0.50.288 (May 03, 2026) — 4094 tests collected
|
||||
> Test source: `pytest tests/ --collect-only -q`
|
||||
> Per-version detail: see [CHANGELOG.md](./CHANGELOG.md)
|
||||
|
||||
|
||||
+2
-2
@@ -1835,8 +1835,8 @@ Bridged CLI sessions:
|
||||
|
||||
---
|
||||
|
||||
*Last updated: v0.50.287, May 03, 2026*
|
||||
*Total automated tests collected: 4053*
|
||||
*Last updated: v0.50.288, May 03, 2026*
|
||||
*Total automated tests collected: 4094*
|
||||
*Regression gate: tests/test_regressions.py*
|
||||
*Run: pytest tests/ -v --timeout=60*
|
||||
*Source: <repo>/*
|
||||
|
||||
+344
-46
@@ -653,6 +653,48 @@ def _resolve_provider_alias(name: str) -> str:
|
||||
return _PROVIDER_ALIASES.get(raw, name)
|
||||
|
||||
|
||||
def _canonicalise_provider_id(name: object) -> str:
|
||||
"""Normalise a provider id slug into a stable lowercase-hyphenated form.
|
||||
|
||||
Folds underscores to hyphens and lowercases the result, so a user with
|
||||
``providers.opencode_go.api_key`` in ``config.yaml`` and
|
||||
``model.provider: opencode-go`` sees ONE provider group, not two
|
||||
(#1568). Then attempts alias resolution but only if the alias target
|
||||
is itself a known canonical id in ``_PROVIDER_DISPLAY`` — this avoids
|
||||
converting ``x-ai`` (canonical in WebUI's data structures) to ``xai``
|
||||
(the hermes_cli alias target which the WebUI doesn't index by).
|
||||
|
||||
Examples::
|
||||
|
||||
opencode-go -> opencode-go (canonical, no change)
|
||||
opencode_go -> opencode-go (underscore folded)
|
||||
OpenCode-Go -> opencode-go (case folded)
|
||||
OPENCODE_GO -> opencode-go (both folded)
|
||||
z_ai -> zai (alias-resolved — zai is canonical)
|
||||
x-ai -> x-ai (preserved — x-ai is canonical)
|
||||
|
||||
Empty input passes through as the empty string. Unknown ids preserve
|
||||
their normalised form.
|
||||
"""
|
||||
if not name:
|
||||
return ""
|
||||
raw = str(name).strip().lower().replace("_", "-")
|
||||
if not raw:
|
||||
return ""
|
||||
# Already a canonical id known to _PROVIDER_DISPLAY/_PROVIDER_MODELS:
|
||||
# keep as-is to avoid round-tripping through aliases (e.g. x-ai → xai).
|
||||
if raw in _PROVIDER_DISPLAY or raw in _PROVIDER_MODELS:
|
||||
return raw
|
||||
# Try alias resolution. Only accept the result if it's itself a
|
||||
# canonical id in _PROVIDER_DISPLAY — that prevents aliases pointing
|
||||
# at non-canonical strings (legacy, hermes_cli-specific) from leaking
|
||||
# in. Falls back to the normalised input otherwise.
|
||||
resolved = _resolve_provider_alias(raw)
|
||||
if resolved and resolved.lower() in _PROVIDER_DISPLAY:
|
||||
return resolved.lower()
|
||||
return raw
|
||||
|
||||
|
||||
# Well-known models per provider (used to populate dropdown for direct API providers)
|
||||
_PROVIDER_MODELS = {
|
||||
"anthropic": [
|
||||
@@ -899,6 +941,122 @@ def _format_nous_label(mid: str) -> str:
|
||||
return f"{base} (via Nous)"
|
||||
|
||||
|
||||
# Soft cap on how many Nous Portal models surface in the picker dropdown.
|
||||
# Above this count, _build_nous_featured_set() trims the visible list to
|
||||
# ~_NOUS_FEATURED_TARGET entries; the full catalog is still returned to the
|
||||
# client under ``extra_models`` so /model autocomplete covers everything.
|
||||
# Caps reflect human scannability — a 25-row dropdown is the practical UX
|
||||
# ceiling, and per-vendor sampling at 15 keeps the flagship shape visible
|
||||
# without one vendor dominating.
|
||||
_NOUS_FEATURED_THRESHOLD = 25
|
||||
_NOUS_FEATURED_TARGET = 15
|
||||
|
||||
# Vendor-prefix priority order for featured selection. Lower index = picked
|
||||
# earlier when sampling the live catalog. Reflects which vendors users have
|
||||
# historically reached for first via Nous Portal (driven by the curated
|
||||
# static list maintained in _PROVIDER_MODELS["nous"] and Discord feedback).
|
||||
_NOUS_VENDOR_PRIORITY = (
|
||||
"anthropic", "openai", "google", "moonshotai", "z-ai",
|
||||
"minimax", "qwen", "x-ai", "deepseek", "stepfun",
|
||||
"xiaomi", "tencent", "nvidia", "arcee-ai",
|
||||
)
|
||||
|
||||
|
||||
def _build_nous_featured_set(
|
||||
live_ids: list[str],
|
||||
*,
|
||||
selected_model_id: str | None = None,
|
||||
target: int = _NOUS_FEATURED_TARGET,
|
||||
) -> tuple[list[str], list[str]]:
|
||||
"""Trim a Nous Portal catalog into a (featured, extras) split.
|
||||
|
||||
``featured`` is what the picker dropdown renders. ``extras`` is everything
|
||||
else — kept available so the slash-command `/model` autocomplete and the
|
||||
``_dynamicModelLabels`` map cover the full catalog.
|
||||
|
||||
Selection rules (in order, deterministic):
|
||||
|
||||
1. Always include the user's currently-selected model if it's in the
|
||||
catalog (preserves selection stickiness — no orphan IDs in the
|
||||
dropdown after a refresh).
|
||||
2. Always include every entry from the curated static
|
||||
``_PROVIDER_MODELS["nous"]`` list whose id maps onto a live id —
|
||||
those four are explicitly maintained as flagship picks.
|
||||
3. Top up to ``target`` by walking ``_NOUS_VENDOR_PRIORITY`` round-robin
|
||||
(one model per vendor each pass) so no vendor monopolises the slot
|
||||
budget. Within a vendor, the original ``live_ids`` order is preserved
|
||||
— that's the order Nous Portal returned, which approximates recency.
|
||||
|
||||
Returns ``(featured_ids, extras_ids)`` — both lists are subsets of
|
||||
``live_ids`` with disjoint membership and union equal to ``live_ids``.
|
||||
|
||||
For catalogs ≤ ``_NOUS_FEATURED_THRESHOLD`` entries the function is a
|
||||
no-op: ``featured == live_ids``, ``extras == []``.
|
||||
"""
|
||||
if not live_ids:
|
||||
return [], []
|
||||
if len(live_ids) <= _NOUS_FEATURED_THRESHOLD:
|
||||
return list(live_ids), []
|
||||
|
||||
chosen: list[str] = [] # preserves insertion order
|
||||
chosen_set: set[str] = set()
|
||||
|
||||
def _add(mid: str) -> None:
|
||||
if mid and mid not in chosen_set:
|
||||
chosen.append(mid)
|
||||
chosen_set.add(mid)
|
||||
|
||||
# Rule 1: sticky selection. Strip "@nous:" prefix if present so we can
|
||||
# match against the live id space (which is bare "vendor/model").
|
||||
if selected_model_id:
|
||||
sel = selected_model_id
|
||||
if sel.startswith("@nous:"):
|
||||
sel = sel[len("@nous:"):]
|
||||
if sel in live_ids:
|
||||
_add(sel)
|
||||
|
||||
# Rule 2: curated flagships. Extract the bare ids from the static list
|
||||
# entries (which are stored as "@nous:vendor/model").
|
||||
for static in _PROVIDER_MODELS.get("nous", []):
|
||||
sid = static.get("id", "")
|
||||
if sid.startswith("@nous:"):
|
||||
sid = sid[len("@nous:"):]
|
||||
if sid in live_ids:
|
||||
_add(sid)
|
||||
|
||||
# Rule 3: vendor-priority round-robin top-up.
|
||||
by_vendor: dict[str, list[str]] = {}
|
||||
for mid in live_ids:
|
||||
if mid in chosen_set:
|
||||
continue
|
||||
vendor = mid.split("/", 1)[0] if "/" in mid else ""
|
||||
by_vendor.setdefault(vendor, []).append(mid)
|
||||
|
||||
# Walk vendors in priority order, then any leftover vendors alphabetically.
|
||||
priority = list(_NOUS_VENDOR_PRIORITY)
|
||||
leftover = sorted(v for v in by_vendor if v not in set(priority))
|
||||
vendor_order = priority + leftover
|
||||
|
||||
# Round-robin: one model per vendor per pass until we hit the target or
|
||||
# exhaust every bucket.
|
||||
while len(chosen) < target:
|
||||
added_this_pass = 0
|
||||
for vendor in vendor_order:
|
||||
if len(chosen) >= target:
|
||||
break
|
||||
bucket = by_vendor.get(vendor)
|
||||
if not bucket:
|
||||
continue
|
||||
_add(bucket.pop(0))
|
||||
added_this_pass += 1
|
||||
if added_this_pass == 0:
|
||||
break # all buckets empty
|
||||
|
||||
# Anything not chosen becomes extras (full-catalog completion surface).
|
||||
extras = [m for m in live_ids if m not in chosen_set]
|
||||
return chosen, extras
|
||||
|
||||
|
||||
def _apply_provider_prefix(
|
||||
raw_models: list[dict],
|
||||
provider_id: str,
|
||||
@@ -1767,6 +1925,22 @@ def get_available_models() -> dict:
|
||||
logger.debug("Failed to get key source for provider %s", _p.get("id", "unknown"))
|
||||
detected_providers.add(_p["id"])
|
||||
_hermes_auth_used = True
|
||||
|
||||
# Belt-and-braces: list_available_providers() is the primary signal
|
||||
# for OAuth providers, but its `authenticated` field can disagree
|
||||
# with `get_auth_status(<id>).logged_in` on some hermes_cli versions
|
||||
# (the two fields are computed via different code paths). When the
|
||||
# disagreement happens for Nous Portal, the Settings → Providers
|
||||
# card renders the live catalog (because api/providers.py iterates
|
||||
# all OAuth providers regardless of authentication state) but the
|
||||
# picker dropdown comes up empty — a confusing asymmetry reported
|
||||
# in #1567. Add Nous explicitly when get_auth_status agrees so the
|
||||
# picker stays in sync with the providers card.
|
||||
try:
|
||||
if _gas("nous").get("logged_in"):
|
||||
detected_providers.add("nous")
|
||||
except Exception:
|
||||
logger.debug("Failed to check Nous Portal auth status")
|
||||
except Exception:
|
||||
logger.debug("Failed to detect auth providers from hermes")
|
||||
|
||||
@@ -1844,11 +2018,21 @@ def get_available_models() -> dict:
|
||||
# Also detect providers explicitly listed in config.yaml providers section.
|
||||
# A user may configure a provider key via config.yaml providers.<name>.api_key
|
||||
# without setting the corresponding env var. (#604)
|
||||
#
|
||||
# Canonicalise the id slug here so a user with ``providers.opencode_go``
|
||||
# (underscore variant) doesn't see TWO provider groups in the picker —
|
||||
# one for the canonical ``opencode-go`` from active_provider detection
|
||||
# and a phantom ``Opencode_Go`` group for the config-key form (#1568).
|
||||
# The same applies to mixed-case ids like ``OpenCode-Go`` and to
|
||||
# legitimate aliases like ``z-ai`` → ``zai``.
|
||||
_cfg_providers = cfg.get("providers", {})
|
||||
if isinstance(_cfg_providers, dict):
|
||||
for _pid_key in _cfg_providers:
|
||||
if _pid_key in _PROVIDER_MODELS or _pid_key in cfg.get("providers", {}):
|
||||
detected_providers.add(_pid_key)
|
||||
_canonical = _canonicalise_provider_id(_pid_key)
|
||||
if not _canonical:
|
||||
continue
|
||||
if _canonical in _PROVIDER_MODELS or _canonical in _cfg_providers or _pid_key in _cfg_providers:
|
||||
detected_providers.add(_canonical)
|
||||
|
||||
def _normalize_base_url_for_match(value: object) -> str:
|
||||
url = str(value or "").strip().rstrip("/")
|
||||
@@ -2115,10 +2299,30 @@ def get_available_models() -> dict:
|
||||
configured_providers.add(active_provider)
|
||||
cfg_providers = cfg.get("providers", {})
|
||||
if isinstance(cfg_providers, dict):
|
||||
configured_providers.update(cfg_providers.keys())
|
||||
# Canonicalise here too — same rationale as #1568 detection
|
||||
# path. Without this, only_show_configured mode could
|
||||
# exclude detected ``opencode-go`` because configured_providers
|
||||
# only has the underscore-variant key from config.yaml.
|
||||
configured_providers.update(
|
||||
_canonicalise_provider_id(k) or k for k in cfg_providers.keys()
|
||||
)
|
||||
# Only show providers that are both detected and configured
|
||||
detected_providers = detected_providers.intersection(configured_providers)
|
||||
|
||||
# Post-collection dedup: re-canonicalise every entry so any path that
|
||||
# added a non-canonical id (mixed-case from auth-store, raw config-key,
|
||||
# legacy alias) gets folded onto the canonical key. Belt-and-braces for
|
||||
# #1568 — protects against future regressions in any of the ~25
|
||||
# `detected_providers.add(...)` callsites without auditing each one.
|
||||
# The fold is idempotent for already-canonical ids, so safe to run
|
||||
# unconditionally.
|
||||
if detected_providers:
|
||||
_canonicalised_detected = set()
|
||||
for _pid in detected_providers:
|
||||
_c = _canonicalise_provider_id(_pid) or _pid
|
||||
_canonicalised_detected.add(_c)
|
||||
detected_providers = _canonicalised_detected
|
||||
|
||||
# 5. Build model groups
|
||||
if detected_providers:
|
||||
for pid in sorted(detected_providers):
|
||||
@@ -2241,43 +2445,102 @@ def get_available_models() -> dict:
|
||||
}
|
||||
)
|
||||
elif pid == "nous":
|
||||
# Nous Portal exposes a curated catalog (~30 models, currently)
|
||||
# via inference-api.nousresearch.com. Like ollama-cloud, we
|
||||
# Nous Portal exposes a curated catalog (~30 models on most
|
||||
# accounts, up to several hundred for enterprise tiers) 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.
|
||||
# chronically drifts out of date (#1538).
|
||||
#
|
||||
# When the catalog exceeds _NOUS_FEATURED_THRESHOLD (~25)
|
||||
# the picker dropdown gets a curated subset to stay
|
||||
# scannable — the full list is still returned under
|
||||
# "extra_models" for the slash-command autocomplete and
|
||||
# the dynamic-label map (#1567). The optgroup label is
|
||||
# decorated with the truncation count so users know more
|
||||
# exists.
|
||||
raw_models = []
|
||||
extra_models: list[dict] = []
|
||||
truncated_label_suffix = ""
|
||||
live_fetch_failed = False
|
||||
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")
|
||||
live_ids = []
|
||||
live_fetch_failed = True
|
||||
|
||||
if not raw_models:
|
||||
# Static fallback: deepcopy so dedup/prefix mutation
|
||||
# below does not bleed into the module-level catalog.
|
||||
if live_ids:
|
||||
# Sticky-selection signal: prefer the explicitly-active
|
||||
# model from cfg["model"]["model"] (what the user is
|
||||
# currently using) over cfg["model"]["default"] (the
|
||||
# configured default suggestion). Falls back to the
|
||||
# latter so first-load before any selection still works.
|
||||
_model_cfg = cfg.get("model", {})
|
||||
_selected = (
|
||||
(isinstance(_model_cfg, dict) and _model_cfg.get("model"))
|
||||
or default_model
|
||||
or None
|
||||
)
|
||||
featured_ids, extras_ids = _build_nous_featured_set(
|
||||
live_ids,
|
||||
selected_model_id=_selected,
|
||||
)
|
||||
# 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).
|
||||
raw_models = [
|
||||
{"id": f"@nous:{mid}", "label": _format_nous_label(mid)}
|
||||
for mid in featured_ids
|
||||
]
|
||||
extra_models = [
|
||||
{"id": f"@nous:{mid}", "label": _format_nous_label(mid)}
|
||||
for mid in extras_ids
|
||||
]
|
||||
if extras_ids:
|
||||
# Show "(15 of 397)" so the user understands the picker
|
||||
# is showing a featured subset, not a broken short list.
|
||||
truncated_label_suffix = (
|
||||
f" ({len(featured_ids)} of {len(live_ids)})"
|
||||
)
|
||||
elif not live_fetch_failed:
|
||||
# Live-fetch returned an empty list AND did not raise —
|
||||
# the user is gated as authenticated by detection above
|
||||
# but the catalog endpoint replied with no models.
|
||||
# Showing the static 4-entry curated list here would
|
||||
# contradict the providers card (which always shows
|
||||
# the live catalog) — exactly the asymmetry #1567
|
||||
# reports. Omit the Nous group entirely; the providers
|
||||
# card already tells the truth, and a transient empty
|
||||
# response will self-heal on the next cache rebuild.
|
||||
logger.warning(
|
||||
"Nous Portal authenticated but live-fetch returned empty — "
|
||||
"omitting from picker (will retry on next cache rebuild)"
|
||||
)
|
||||
else:
|
||||
# hermes_cli unavailable / raised — fall back to the
|
||||
# curated 4-entry static list so the picker is never
|
||||
# empty in this degraded state. This matches pre-#1538
|
||||
# behaviour for environments without hermes_cli (test
|
||||
# envs, package mismatches, isolated WebUI builds).
|
||||
raw_models = copy.deepcopy(_PROVIDER_MODELS.get("nous", []))
|
||||
|
||||
if raw_models:
|
||||
models = _apply_provider_prefix(raw_models, pid, active_provider)
|
||||
groups.append(
|
||||
{
|
||||
"provider": provider_name,
|
||||
"provider_id": pid,
|
||||
"models": models,
|
||||
}
|
||||
)
|
||||
# Apply the same prefix transform to extras so /model
|
||||
# autocomplete sees consistent IDs across the two lists.
|
||||
extras = _apply_provider_prefix(extra_models, pid, active_provider) if extra_models else []
|
||||
group_entry = {
|
||||
"provider": provider_name + truncated_label_suffix,
|
||||
"provider_id": pid,
|
||||
"models": models,
|
||||
}
|
||||
if extras:
|
||||
group_entry["extra_models"] = extras
|
||||
groups.append(group_entry)
|
||||
elif pid in _PROVIDER_MODELS or pid in cfg.get("providers", {}):
|
||||
raw_models = copy.deepcopy(_PROVIDER_MODELS.get(pid, []))
|
||||
detected_models = auto_detected_models_by_provider.get(pid, [])
|
||||
@@ -2332,34 +2595,69 @@ def get_available_models() -> dict:
|
||||
)
|
||||
|
||||
if default_model:
|
||||
all_ids_norm = {_norm_model_id(m["id"]) for g in groups for m in g.get("models", [])}
|
||||
if _norm_model_id(default_model) not in all_ids_norm:
|
||||
label = _get_label_for_model(default_model, groups)
|
||||
target_display = (
|
||||
_PROVIDER_DISPLAY.get(active_provider, active_provider or "").lower()
|
||||
if active_provider
|
||||
else ""
|
||||
# Guard against provider-id values mistakenly stored in
|
||||
# ``model.default``. The injection logic below puts ANY string
|
||||
# into the picker as a fake option, so a stray provider id
|
||||
# surfaces as a self-referential phantom model labelled e.g.
|
||||
# ``Opencode GO`` — a 15th entry under the OpenCode Go group
|
||||
# (#1568). The user's misconfig is real, but the picker is
|
||||
# the wrong surface to surface it; we'd rather skip injection
|
||||
# and emit a warning so the underlying config issue is logged.
|
||||
_looks_like_provider_id = (
|
||||
str(default_model).strip().lower().replace("_", "-") in _PROVIDER_DISPLAY
|
||||
or _canonicalise_provider_id(default_model) in _PROVIDER_DISPLAY
|
||||
)
|
||||
if _looks_like_provider_id:
|
||||
logger.warning(
|
||||
"Suspicious model.default value %r — looks like a provider id, "
|
||||
"not a model id. Skipping picker injection. Check `model.default` "
|
||||
"in config.yaml.",
|
||||
default_model,
|
||||
)
|
||||
injected = False
|
||||
for g in groups:
|
||||
if target_display and g.get("provider", "").lower() == target_display:
|
||||
g["models"].insert(0, {"id": default_model, "label": label})
|
||||
injected = True
|
||||
break
|
||||
if not injected and groups:
|
||||
groups.append(
|
||||
{
|
||||
"provider": "Default",
|
||||
"provider_id": active_provider or "default",
|
||||
"models": [{"id": default_model, "label": label}],
|
||||
}
|
||||
else:
|
||||
all_ids_norm = {_norm_model_id(m["id"]) for g in groups for m in g.get("models", [])}
|
||||
if _norm_model_id(default_model) not in all_ids_norm:
|
||||
label = _get_label_for_model(default_model, groups)
|
||||
target_display = (
|
||||
_PROVIDER_DISPLAY.get(active_provider, active_provider or "").lower()
|
||||
if active_provider
|
||||
else ""
|
||||
)
|
||||
injected = False
|
||||
for g in groups:
|
||||
if target_display and g.get("provider", "").lower() == target_display:
|
||||
g["models"].insert(0, {"id": default_model, "label": label})
|
||||
injected = True
|
||||
break
|
||||
if not injected and groups:
|
||||
groups.append(
|
||||
{
|
||||
"provider": "Default",
|
||||
"provider_id": active_provider or "default",
|
||||
"models": [{"id": default_model, "label": label}],
|
||||
}
|
||||
)
|
||||
|
||||
# Post-process: ensure model IDs are globally unique across groups.
|
||||
# When multiple providers expose the same bare model ID, prefix
|
||||
# collisions with @provider_id: so the frontend can distinguish them.
|
||||
_deduplicate_model_ids(groups)
|
||||
|
||||
# Defense-in-depth: drop any optgroup that ended up with zero models
|
||||
# — those are pure UI noise. A zero-model group typically means a
|
||||
# detection path added an id that has no static catalog AND the
|
||||
# live-fetch returned empty (#1568 — the user's
|
||||
# ``providers.opencode_go`` config-key path produced an empty
|
||||
# ``Opencode_Go`` group at the end of the picker before this fix).
|
||||
# Custom providers from ``custom_providers`` config are exempt —
|
||||
# they may legitimately render with zero entries when the user
|
||||
# hasn't filled in models yet but wants the card visible.
|
||||
groups = [
|
||||
g for g in groups
|
||||
if g.get("models")
|
||||
or (g.get("provider_id") or "").startswith("custom:")
|
||||
]
|
||||
|
||||
return {
|
||||
"active_provider": active_provider,
|
||||
"default_model": default_model,
|
||||
|
||||
+129
@@ -139,6 +139,135 @@ def get_active_hermes_home() -> Path:
|
||||
|
||||
|
||||
|
||||
# ── Cron-call profile isolation (issue: Scheduled jobs ignored active profile) ─
|
||||
# `cron.jobs` reads HERMES_HOME from os.environ (process-global) at function-
|
||||
# call time. That bypasses our per-request thread-local profile, so the
|
||||
# `/api/crons*` endpoints always returned the process-default profile's jobs.
|
||||
# This context manager swaps HERMES_HOME (and the cached module-level constants
|
||||
# in cron.jobs) for the duration of a cron call, serialized by a lock so
|
||||
# concurrent requests from different profiles don't race on the global env var.
|
||||
#
|
||||
# Thread-safety note on os.environ mutation:
|
||||
# CPython's os.environ assignment is GIL-protected at the bytecode level, but
|
||||
# multi-step read-modify-write sequences (snapshot prev → assign new → restore
|
||||
# on exit) are NOT atomic without explicit serialization. The _cron_env_lock
|
||||
# below makes the entire context-manager body run-to-completion serially, so
|
||||
# all webui access to HERMES_HOME goes through one thread at a time. Any
|
||||
# subprocess.Popen() call inside `run_job` inherits the env at fork time,
|
||||
# which is also under the lock — so child processes always see a consistent
|
||||
# (own-profile) HERMES_HOME, never a half-swapped state.
|
||||
_cron_env_lock = threading.Lock()
|
||||
|
||||
|
||||
class cron_profile_context_for_home:
|
||||
"""Context manager that pins HERMES_HOME to an explicit profile home path.
|
||||
|
||||
Use this variant from worker threads that don't have TLS context (e.g. the
|
||||
background thread started by /api/crons/run). The HTTP-side variant below
|
||||
resolves the home via TLS.
|
||||
"""
|
||||
|
||||
def __init__(self, home: Path):
|
||||
self._home = Path(home)
|
||||
|
||||
def __enter__(self):
|
||||
_cron_env_lock.acquire()
|
||||
try:
|
||||
self._prev_env = os.environ.get('HERMES_HOME')
|
||||
os.environ['HERMES_HOME'] = str(self._home)
|
||||
|
||||
# Re-patch cron.jobs module-level constants (see main context manager
|
||||
# below for the rationale).
|
||||
self._prev_cj = None
|
||||
try:
|
||||
import cron.jobs as _cj
|
||||
self._prev_cj = (_cj.HERMES_DIR, _cj.CRON_DIR, _cj.JOBS_FILE, _cj.OUTPUT_DIR)
|
||||
_cj.HERMES_DIR = self._home
|
||||
_cj.CRON_DIR = self._home / 'cron'
|
||||
_cj.JOBS_FILE = _cj.CRON_DIR / 'jobs.json'
|
||||
_cj.OUTPUT_DIR = _cj.CRON_DIR / 'output'
|
||||
except (ImportError, AttributeError):
|
||||
logger.debug("cron_profile_context_for_home: cron.jobs unavailable")
|
||||
except Exception:
|
||||
_cron_env_lock.release()
|
||||
raise
|
||||
return self
|
||||
|
||||
def __exit__(self, exc_type, exc_val, exc_tb):
|
||||
try:
|
||||
if self._prev_env is None:
|
||||
os.environ.pop('HERMES_HOME', None)
|
||||
else:
|
||||
os.environ['HERMES_HOME'] = self._prev_env
|
||||
if self._prev_cj is not None:
|
||||
try:
|
||||
import cron.jobs as _cj
|
||||
_cj.HERMES_DIR, _cj.CRON_DIR, _cj.JOBS_FILE, _cj.OUTPUT_DIR = self._prev_cj
|
||||
except (ImportError, AttributeError):
|
||||
pass
|
||||
finally:
|
||||
_cron_env_lock.release()
|
||||
return False
|
||||
|
||||
|
||||
class cron_profile_context:
|
||||
"""Context manager that pins HERMES_HOME to the TLS-active profile.
|
||||
|
||||
Usage:
|
||||
with cron_profile_context():
|
||||
from cron.jobs import list_jobs
|
||||
jobs = list_jobs(include_disabled=True)
|
||||
|
||||
Serializes cron API calls across profiles (cron API is low-frequency;
|
||||
serialization cost is negligible compared to correctness).
|
||||
"""
|
||||
|
||||
def __enter__(self):
|
||||
_cron_env_lock.acquire()
|
||||
try:
|
||||
self._prev_env = os.environ.get('HERMES_HOME')
|
||||
home = get_active_hermes_home()
|
||||
os.environ['HERMES_HOME'] = str(home)
|
||||
|
||||
# Re-patch cron.jobs module-level constants. They are snapshot at
|
||||
# import time (line 68-71 of cron/jobs.py) and don't participate in
|
||||
# the module's __getattr__ lazy path, so env-var alone is not enough
|
||||
# for callers that reference the module constants directly.
|
||||
self._prev_cj = None
|
||||
try:
|
||||
import cron.jobs as _cj
|
||||
self._prev_cj = (_cj.HERMES_DIR, _cj.CRON_DIR, _cj.JOBS_FILE, _cj.OUTPUT_DIR)
|
||||
_cj.HERMES_DIR = home
|
||||
_cj.CRON_DIR = home / 'cron'
|
||||
_cj.JOBS_FILE = _cj.CRON_DIR / 'jobs.json'
|
||||
_cj.OUTPUT_DIR = _cj.CRON_DIR / 'output'
|
||||
except (ImportError, AttributeError):
|
||||
logger.debug("cron_profile_context: cron.jobs unavailable; env-var only")
|
||||
except Exception:
|
||||
_cron_env_lock.release()
|
||||
raise
|
||||
return self
|
||||
|
||||
def __exit__(self, exc_type, exc_val, exc_tb):
|
||||
try:
|
||||
# Restore env var
|
||||
if self._prev_env is None:
|
||||
os.environ.pop('HERMES_HOME', None)
|
||||
else:
|
||||
os.environ['HERMES_HOME'] = self._prev_env
|
||||
|
||||
# Restore cron.jobs module constants
|
||||
if self._prev_cj is not None:
|
||||
try:
|
||||
import cron.jobs as _cj
|
||||
_cj.HERMES_DIR, _cj.CRON_DIR, _cj.JOBS_FILE, _cj.OUTPUT_DIR = self._prev_cj
|
||||
except (ImportError, AttributeError):
|
||||
pass
|
||||
finally:
|
||||
_cron_env_lock.release()
|
||||
return False
|
||||
|
||||
|
||||
def get_hermes_home_for_profile(name: str) -> Path:
|
||||
"""Return the HERMES_HOME Path for *name* without mutating any process state.
|
||||
|
||||
|
||||
+28
-2
@@ -392,10 +392,19 @@ def get_providers() -> dict[str, Any]:
|
||||
pass
|
||||
|
||||
models = list(_PROVIDER_MODELS.get(pid, []))
|
||||
models_total = len(models)
|
||||
# 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.
|
||||
#
|
||||
# On large-tier accounts (#1567 reporter Deor saw 396 entries), we
|
||||
# render the same featured subset the picker uses so the providers
|
||||
# card body doesn't become a 396-pill wall. The full count is still
|
||||
# reported via models_total — surfaced in the header line as
|
||||
# "396 models · OAuth" by static/panels.js — so the user knows the
|
||||
# complete catalog is reachable (via /model autocomplete or a future
|
||||
# "show all" disclosure if added).
|
||||
if pid == "nous":
|
||||
try:
|
||||
from hermes_cli.models import provider_model_ids as _provider_model_ids
|
||||
@@ -403,12 +412,14 @@ def get_providers() -> dict[str, Any]:
|
||||
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
|
||||
from api.config import _format_nous_label, _build_nous_featured_set
|
||||
|
||||
featured_ids, _extras = _build_nous_featured_set(live_ids)
|
||||
models = [
|
||||
{"id": f"@nous:{mid}", "label": _format_nous_label(mid)}
|
||||
for mid in live_ids
|
||||
for mid in featured_ids
|
||||
]
|
||||
models_total = len(live_ids)
|
||||
except Exception:
|
||||
logger.debug("Failed to load Nous Portal models from hermes_cli")
|
||||
# Also include models from config.yaml providers section
|
||||
@@ -420,6 +431,13 @@ def get_providers() -> dict[str, Any]:
|
||||
models = models + [{"id": k, "label": k} for k in cfg_models.keys()]
|
||||
elif isinstance(cfg_models, list):
|
||||
models = models + [{"id": k, "label": k} for k in cfg_models]
|
||||
# Recompute models_total when config.yaml contributes additional
|
||||
# entries on top of the live/static catalog. For non-Nous
|
||||
# providers models_total still equals len(models); for Nous
|
||||
# we keep the live count (which already includes any models
|
||||
# surfaced in the curated featured slice).
|
||||
if pid != "nous":
|
||||
models_total = len(models)
|
||||
|
||||
providers.append({
|
||||
"id": pid,
|
||||
@@ -430,6 +448,14 @@ def get_providers() -> dict[str, Any]:
|
||||
"key_source": key_source,
|
||||
"auth_error": auth_error,
|
||||
"models": models,
|
||||
# models_total reflects the complete catalog size (e.g. 396 for
|
||||
# an enterprise Nous Portal account), even when "models" is
|
||||
# trimmed to a featured subset for UI scannability. The frontend
|
||||
# uses this for the header text "396 models · OAuth" so users
|
||||
# know the full catalog exists and is reachable via the slash
|
||||
# command. For providers that don't trim, models_total ==
|
||||
# len(models) and the frontend behaves identically to before.
|
||||
"models_total": models_total,
|
||||
})
|
||||
|
||||
# Scan custom_providers from config.yaml (e.g. glmcode, timicc)
|
||||
|
||||
+113
-15
@@ -180,12 +180,33 @@ def _cron_output_content_window(text: str, limit: int = _CRON_OUTPUT_CONTENT_LIM
|
||||
return text[-limit:]
|
||||
|
||||
|
||||
def _run_cron_tracked(job):
|
||||
"""Wrapper that tracks running state around cron.scheduler.run_job."""
|
||||
def _run_cron_tracked(job, profile_home=None):
|
||||
"""Wrapper that tracks running state around cron.scheduler.run_job.
|
||||
|
||||
``profile_home`` pins HERMES_HOME for this worker thread so output files
|
||||
and run metadata land in the profile that triggered the run, not the
|
||||
process-global default. Captured at dispatch time because the thread runs
|
||||
after the HTTP request (and its TLS profile) has already been cleared.
|
||||
"""
|
||||
from cron.scheduler import run_job # import here — runs inside a worker thread
|
||||
from cron.jobs import mark_job_run, save_job_output
|
||||
|
||||
job_id = job.get("id", "")
|
||||
|
||||
# Pin HERMES_HOME for the duration of this thread using a dedicated
|
||||
# context manager variant that accepts the profile home directly
|
||||
# (threads have no TLS, so get_active_hermes_home() can't resolve).
|
||||
ctx = None
|
||||
if profile_home is not None:
|
||||
try:
|
||||
from api.profiles import cron_profile_context_for_home
|
||||
|
||||
ctx = cron_profile_context_for_home(profile_home)
|
||||
ctx.__enter__()
|
||||
except Exception:
|
||||
logger.exception("Failed to pin profile %s for cron run", profile_home)
|
||||
ctx = None
|
||||
|
||||
try:
|
||||
success, output, final_response, error = run_job(job)
|
||||
save_job_output(job_id, output)
|
||||
@@ -204,6 +225,11 @@ def _run_cron_tracked(job):
|
||||
except Exception:
|
||||
logger.debug("Failed to mark manual cron run failure for %s", job_id)
|
||||
finally:
|
||||
if ctx is not None:
|
||||
try:
|
||||
ctx.__exit__(None, None, None)
|
||||
except Exception:
|
||||
logger.debug("Failed to release cron_profile_context for %s", job_id)
|
||||
_mark_cron_done(job_id)
|
||||
|
||||
_PROVIDER_ALIASES = {
|
||||
@@ -2177,25 +2203,45 @@ def handle_get(handler, parsed) -> bool:
|
||||
return # SSE handled, no JSON response
|
||||
|
||||
# ── Cron API (GET) ──
|
||||
# All cron handlers touch cron.jobs which resolves HERMES_HOME from
|
||||
# os.environ (process-global) at call time. Wrap in cron_profile_context
|
||||
# so the TLS-active profile's jobs.json is read, not the process default.
|
||||
if parsed.path == "/api/crons":
|
||||
from cron.jobs import list_jobs
|
||||
from api.profiles import cron_profile_context
|
||||
|
||||
return j(handler, {"jobs": list_jobs(include_disabled=True)})
|
||||
with cron_profile_context():
|
||||
return j(handler, {"jobs": list_jobs(include_disabled=True)})
|
||||
|
||||
if parsed.path == "/api/crons/output":
|
||||
return _handle_cron_output(handler, parsed)
|
||||
from api.profiles import cron_profile_context
|
||||
|
||||
with cron_profile_context():
|
||||
return _handle_cron_output(handler, parsed)
|
||||
|
||||
if parsed.path == "/api/crons/history":
|
||||
return _handle_cron_history(handler, parsed)
|
||||
from api.profiles import cron_profile_context
|
||||
|
||||
with cron_profile_context():
|
||||
return _handle_cron_history(handler, parsed)
|
||||
|
||||
if parsed.path == "/api/crons/run":
|
||||
return _handle_cron_run_detail(handler, parsed)
|
||||
from api.profiles import cron_profile_context
|
||||
|
||||
with cron_profile_context():
|
||||
return _handle_cron_run_detail(handler, parsed)
|
||||
|
||||
if parsed.path == "/api/crons/recent":
|
||||
return _handle_cron_recent(handler, parsed)
|
||||
from api.profiles import cron_profile_context
|
||||
|
||||
with cron_profile_context():
|
||||
return _handle_cron_recent(handler, parsed)
|
||||
|
||||
if parsed.path == "/api/crons/status":
|
||||
return _handle_cron_status(handler, parsed)
|
||||
from api.profiles import cron_profile_context
|
||||
|
||||
with cron_profile_context():
|
||||
return _handle_cron_status(handler, parsed)
|
||||
|
||||
# ── Skills API (GET) ──
|
||||
if parsed.path == "/api/skills":
|
||||
@@ -2892,23 +2938,43 @@ def handle_post(handler, parsed) -> bool:
|
||||
return _handle_terminal_close(handler, body)
|
||||
|
||||
# ── Cron API (POST) ──
|
||||
# See GET-side comment above: wrap in cron_profile_context so writes go
|
||||
# to the TLS-active profile's jobs.json instead of the process default.
|
||||
if parsed.path == "/api/crons/create":
|
||||
return _handle_cron_create(handler, body)
|
||||
from api.profiles import cron_profile_context
|
||||
|
||||
with cron_profile_context():
|
||||
return _handle_cron_create(handler, body)
|
||||
|
||||
if parsed.path == "/api/crons/update":
|
||||
return _handle_cron_update(handler, body)
|
||||
from api.profiles import cron_profile_context
|
||||
|
||||
with cron_profile_context():
|
||||
return _handle_cron_update(handler, body)
|
||||
|
||||
if parsed.path == "/api/crons/delete":
|
||||
return _handle_cron_delete(handler, body)
|
||||
from api.profiles import cron_profile_context
|
||||
|
||||
with cron_profile_context():
|
||||
return _handle_cron_delete(handler, body)
|
||||
|
||||
if parsed.path == "/api/crons/run":
|
||||
return _handle_cron_run(handler, body)
|
||||
from api.profiles import cron_profile_context
|
||||
|
||||
with cron_profile_context():
|
||||
return _handle_cron_run(handler, body)
|
||||
|
||||
if parsed.path == "/api/crons/pause":
|
||||
return _handle_cron_pause(handler, body)
|
||||
from api.profiles import cron_profile_context
|
||||
|
||||
with cron_profile_context():
|
||||
return _handle_cron_pause(handler, body)
|
||||
|
||||
if parsed.path == "/api/crons/resume":
|
||||
return _handle_cron_resume(handler, body)
|
||||
from api.profiles import cron_profile_context
|
||||
|
||||
with cron_profile_context():
|
||||
return _handle_cron_resume(handler, body)
|
||||
|
||||
# ── File ops (POST) ──
|
||||
if parsed.path == "/api/file/delete":
|
||||
@@ -4416,6 +4482,23 @@ def _handle_live_models(handler, parsed):
|
||||
if not ids:
|
||||
return _finish({"provider": provider, "models": [], "count": 0})
|
||||
|
||||
# For Nous Portal, apply the same featured-set cap that
|
||||
# /api/models uses so background enrichment via _fetchLiveModels()
|
||||
# doesn't undo the dropdown trim — otherwise a 397-model catalog
|
||||
# would still flood the picker after the initial render finished
|
||||
# the cap. The full list is returned via the main /api/models
|
||||
# endpoint's extra_models field for /model autocomplete; the live
|
||||
# endpoint is purely a dropdown-enrichment surface, so it should
|
||||
# match the dropdown's visibility budget. (#1567)
|
||||
if provider == "nous":
|
||||
try:
|
||||
from api.config import _build_nous_featured_set
|
||||
_default_model = (cfg.get("model", {}) or {}).get("model") if isinstance(cfg.get("model"), dict) else None
|
||||
_featured, _ = _build_nous_featured_set(ids, selected_model_id=_default_model)
|
||||
ids = _featured
|
||||
except Exception:
|
||||
logger.debug("Failed to apply Nous featured-set cap for /api/models/live")
|
||||
|
||||
# Normalise to {id, label} — provider_model_ids() returns plain string IDs.
|
||||
# For ollama-cloud use the shared Ollama formatter (handles `:variant` suffix).
|
||||
# For all other providers use a simpler hyphen-split capitaliser.
|
||||
@@ -5149,7 +5232,22 @@ def _handle_cron_run(handler, body):
|
||||
return j(handler, {"ok": False, "job_id": job_id, "status": "already_running",
|
||||
"elapsed": round(elapsed, 1)})
|
||||
_mark_cron_running(job_id)
|
||||
threading.Thread(target=_run_cron_tracked, args=(job,), daemon=True).start()
|
||||
# Capture the TLS-active profile home now — the thread runs after the
|
||||
# request finishes, so TLS is gone by then.
|
||||
#
|
||||
# Resolve directly without a try/except: get_active_hermes_home() does
|
||||
# in-memory dict reads + a single Path.is_dir() stat, so the only way
|
||||
# it could raise from inside a request handler is if api.profiles
|
||||
# itself partially failed to import (in which case we'd already be
|
||||
# 500-ing the whole request). A silent fallback to None here would
|
||||
# re-introduce the exact bug #1573 fixes — the worker thread would
|
||||
# run unpinned against the process-global HERMES_HOME — so we'd
|
||||
# rather let any unexpected exception 500 the request than corrupt
|
||||
# cross-profile state.
|
||||
from api.profiles import get_active_hermes_home
|
||||
|
||||
_profile_home = get_active_hermes_home()
|
||||
threading.Thread(target=_run_cron_tracked, args=(job, _profile_home), daemon=True).start()
|
||||
return j(handler, {"ok": True, "job_id": job_id, "status": "running"})
|
||||
|
||||
|
||||
|
||||
@@ -136,6 +136,15 @@ async function _loadSlashModelSubArgs(force=false){
|
||||
const id=_normalizeSlashSubArg(model&&model.id);
|
||||
if(id) values.push(id);
|
||||
}
|
||||
// Include extra_models (the catalog tail that doesn't render as
|
||||
// <option> entries when the picker is capped) so /model autocomplete
|
||||
// covers the full catalog. The trimming is purely a dropdown
|
||||
// scannability concern — the slash command exists precisely so
|
||||
// power users can reach any model by typing its name. #1567.
|
||||
for(const model of (group&&group.extra_models)||[]){
|
||||
const id=_normalizeSlashSubArg(model&&model.id);
|
||||
if(id) values.push(id);
|
||||
}
|
||||
}
|
||||
const deduped=Array.from(new Set(values)).sort((a,b)=>a.localeCompare(b));
|
||||
_slashModelCache=deduped;
|
||||
|
||||
+24
-2
@@ -3233,7 +3233,13 @@ function _buildProviderCard(p){
|
||||
// Use the is_oauth flag from the backend — it reflects _OAUTH_PROVIDERS in providers.py.
|
||||
// key_source can be 'oauth' (hermes auth), 'config_yaml' (token in config.yaml), or 'none'.
|
||||
const isOauth=p.is_oauth===true;
|
||||
const modelCount=Array.isArray(p.models)?p.models.length:0;
|
||||
// models_total reflects the complete catalog (e.g. 396 for a large-tier
|
||||
// Nous Portal account). The "models" array may be trimmed to a featured
|
||||
// subset for UI scannability — fall back to its length only when the
|
||||
// server didn't supply models_total (older builds, custom providers).
|
||||
const modelCount=Number.isFinite(p.models_total)
|
||||
? p.models_total
|
||||
: (Array.isArray(p.models) ? p.models.length : 0);
|
||||
const sourceLabel=p.key_source==='oauth'
|
||||
? t('providers_status_oauth')
|
||||
: p.key_source==='config_yaml'
|
||||
@@ -3334,12 +3340,28 @@ function _buildProviderCard(p){
|
||||
modelSection.appendChild(modelLabel);
|
||||
const modelList=document.createElement('div');
|
||||
modelList.className='provider-card-model-tags';
|
||||
for(const m of p.models){
|
||||
const renderedModels=Array.isArray(p.models)?p.models:[];
|
||||
for(const m of renderedModels){
|
||||
const tag=document.createElement('span');
|
||||
tag.className='provider-card-model-tag';
|
||||
tag.textContent=m.id||m.label||m;
|
||||
modelList.appendChild(tag);
|
||||
}
|
||||
// When the rendered list is a strict subset of the total catalog (Nous
|
||||
// Portal large-tier accounts hit this with ~400-model catalogs), show
|
||||
// a "+N more" trailing pill so the user knows the picker is intentionally
|
||||
// capped — and they can still reach the full catalog via the /model
|
||||
// slash command (its autocomplete consumes the un-trimmed list from
|
||||
// /api/models's extra_models field). #1567.
|
||||
const totalCount=Number.isFinite(p.models_total)?p.models_total:renderedModels.length;
|
||||
const hiddenCount=Math.max(0, totalCount - renderedModels.length);
|
||||
if(hiddenCount>0){
|
||||
const more=document.createElement('span');
|
||||
more.className='provider-card-model-tag provider-card-model-tag-more';
|
||||
more.textContent='+'+hiddenCount+' more';
|
||||
more.title='The /model slash command can autocomplete every model in this provider\'s catalog.';
|
||||
modelList.appendChild(more);
|
||||
}
|
||||
modelSection.appendChild(modelList);
|
||||
body.appendChild(modelSection);
|
||||
}
|
||||
|
||||
@@ -2416,6 +2416,16 @@ main.main.showing-profiles > #mainProfiles{display:flex;}
|
||||
line-height:1.5;
|
||||
user-select:all;
|
||||
}
|
||||
/* "+N more" disclosure pill — appended when a provider's catalog is trimmed
|
||||
for UI scannability (#1567 — Nous Portal large-tier accounts ship 100s of
|
||||
models). Visually distinct from a real model tag so users don't think it's
|
||||
a model id; muted dashed border, italic, no user-select. */
|
||||
.provider-card-model-tag-more{
|
||||
font-style:italic;
|
||||
border-style:dashed;
|
||||
user-select:none;
|
||||
cursor:help;
|
||||
}
|
||||
|
||||
/* ── Session pin indicator (inline, only when pinned) ── */
|
||||
.session-pin-indicator{
|
||||
|
||||
@@ -370,6 +370,17 @@ async function populateModelDropdown(){
|
||||
og.appendChild(opt);
|
||||
_dynamicModelLabels[m.id]=m.label;
|
||||
}
|
||||
// Hydrate the label map from extra_models too (the catalog tail that
|
||||
// doesn't render as <option> entries when the picker is capped — see
|
||||
// _build_nous_featured_set in api/config.py for the rationale). This
|
||||
// keeps a model selected from the slash-command autocomplete or a
|
||||
// persisted-localStorage value renderable with its proper label
|
||||
// instead of falling back to the bare ID. #1567.
|
||||
if(Array.isArray(g.extra_models)){
|
||||
for(const m of g.extra_models){
|
||||
if(m && m.id) _dynamicModelLabels[m.id]=m.label||m.id;
|
||||
}
|
||||
}
|
||||
sel.appendChild(og);
|
||||
}
|
||||
// Set default model from server if no localStorage preference
|
||||
|
||||
@@ -0,0 +1,555 @@
|
||||
"""Regression tests for #1567 — Nous Portal picker capacity + endpoint symmetry.
|
||||
|
||||
Two issues addressed in one PR:
|
||||
|
||||
1. **Endpoint disagreement (the bug):** The Settings → Providers card and the
|
||||
model picker dropdown returned different Nous catalogs because their
|
||||
detection paths differ. ``api/providers.py:get_providers`` iterates ALL
|
||||
OAuth providers regardless of `list_available_providers().authenticated`.
|
||||
``api/config.py:_build_available_models_uncached`` only includes providers
|
||||
in ``detected_providers``, which is gated on
|
||||
``list_available_providers().authenticated``. On some hermes_cli versions
|
||||
that flag disagrees with ``get_auth_status(<id>).logged_in``. Result: the
|
||||
providers card shows the live catalog (e.g. 396 models) and the picker
|
||||
shows nothing or the stale 4-entry static fallback.
|
||||
|
||||
2. **UX cap (the design concern):** Even with the disagreement fixed, dumping
|
||||
a 397-model dropdown into the picker would be unusable. We cap the
|
||||
dropdown at ~15 featured entries (deterministic vendor-priority sample,
|
||||
sticky for the user's currently-selected model) and return the full
|
||||
catalog under ``extra_models`` so /model autocomplete and the dynamic
|
||||
label map still cover everything.
|
||||
|
||||
Tests in this file pin both invariants.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
import types
|
||||
|
||||
import api.config as config
|
||||
import api.profiles as profiles
|
||||
|
||||
|
||||
# Big catalog matches the shape of an enterprise Nous Portal account.
|
||||
# Volume distribution mirrors what we saw on Nathan's machine (~30 models)
|
||||
# extrapolated up to ~400 with the same vendor mix Deor reported.
|
||||
_BIG_CATALOG_VENDORS = {
|
||||
"anthropic": 8, "openai": 30, "google": 12, "moonshotai": 5, "z-ai": 15,
|
||||
"minimax": 10, "qwen": 80, "x-ai": 8, "deepseek": 20, "stepfun": 10,
|
||||
"xiaomi": 6, "tencent": 12, "nvidia": 25, "arcee-ai": 8,
|
||||
"meta-llama": 50, "mistralai": 40, "cohere": 25, "databricks": 15, "lambda-ai": 18,
|
||||
}
|
||||
|
||||
|
||||
def _build_big_catalog() -> list[str]:
|
||||
out = []
|
||||
for v, n in _BIG_CATALOG_VENDORS.items():
|
||||
for i in range(n):
|
||||
out.append(f"{v}/model-{v}-{i:02d}")
|
||||
return out
|
||||
|
||||
|
||||
def _install_fake_hermes_cli(
|
||||
monkeypatch,
|
||||
*,
|
||||
nous_ids: list[str] | None = None,
|
||||
raise_on_lookup: bool = False,
|
||||
list_authenticated: bool = True,
|
||||
auth_status_logged_in: bool = True,
|
||||
):
|
||||
"""Install fake ``hermes_cli`` modules with controllable Nous behavior.
|
||||
|
||||
The two flags ``list_authenticated`` and ``auth_status_logged_in`` model
|
||||
the divergence between ``hermes_cli.models.list_available_providers()``
|
||||
and ``hermes_cli.auth.get_auth_status()`` that #1567 calls out as a
|
||||
real-world pattern on some hermes_cli versions.
|
||||
"""
|
||||
fake_pkg = types.ModuleType("hermes_cli")
|
||||
fake_pkg.__path__ = []
|
||||
|
||||
fake_models = types.ModuleType("hermes_cli.models")
|
||||
fake_models.list_available_providers = lambda: [
|
||||
{"id": "nous", "label": "Nous Portal", "aliases": [], "authenticated": list_authenticated},
|
||||
]
|
||||
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 _get_auth_status(pid):
|
||||
if pid == "nous":
|
||||
return {"logged_in": auth_status_logged_in, "key_source": "oauth"}
|
||||
return {}
|
||||
|
||||
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):
|
||||
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 doesn't leak unrelated keys."""
|
||||
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)
|
||||
|
||||
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
# Section 1 — _build_nous_featured_set helper invariants
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestBuildNousFeaturedSet:
|
||||
"""Unit tests for the deterministic featured-vs-extras split helper."""
|
||||
|
||||
def test_small_catalog_is_no_op(self):
|
||||
from api.config import _build_nous_featured_set, _NOUS_FEATURED_THRESHOLD
|
||||
# 20 entries — below the threshold, helper should return the input
|
||||
# untouched and an empty extras list.
|
||||
catalog = [f"vendor/model-{i:02d}" for i in range(20)]
|
||||
assert len(catalog) <= _NOUS_FEATURED_THRESHOLD
|
||||
featured, extras = _build_nous_featured_set(catalog)
|
||||
assert featured == catalog
|
||||
assert extras == []
|
||||
|
||||
def test_large_catalog_is_capped_to_target(self):
|
||||
from api.config import _build_nous_featured_set, _NOUS_FEATURED_TARGET
|
||||
catalog = _build_big_catalog()
|
||||
assert len(catalog) > 100, "test fixture should produce a large catalog"
|
||||
featured, extras = _build_nous_featured_set(catalog)
|
||||
assert len(featured) == _NOUS_FEATURED_TARGET, (
|
||||
f"Large catalog should produce exactly _NOUS_FEATURED_TARGET "
|
||||
f"featured entries, got {len(featured)}."
|
||||
)
|
||||
assert len(extras) == len(catalog) - _NOUS_FEATURED_TARGET
|
||||
|
||||
def test_featured_and_extras_are_disjoint_and_complete(self):
|
||||
from api.config import _build_nous_featured_set
|
||||
catalog = _build_big_catalog()
|
||||
featured, extras = _build_nous_featured_set(catalog)
|
||||
assert set(featured) & set(extras) == set(), (
|
||||
"featured and extras must be disjoint — every model belongs to "
|
||||
"exactly one bucket."
|
||||
)
|
||||
assert set(featured) | set(extras) == set(catalog), (
|
||||
"featured ∪ extras must equal the input catalog — no model "
|
||||
"should be silently dropped."
|
||||
)
|
||||
|
||||
def test_priority_vendors_get_picked_first(self):
|
||||
from api.config import _build_nous_featured_set, _NOUS_VENDOR_PRIORITY
|
||||
catalog = _build_big_catalog()
|
||||
featured, _ = _build_nous_featured_set(catalog)
|
||||
# Every priority vendor with ≥1 entry in the catalog must appear in
|
||||
# featured (round-robin guarantee until we hit the slot budget).
|
||||
featured_vendors = {m.split("/", 1)[0] for m in featured}
|
||||
for v in _NOUS_VENDOR_PRIORITY:
|
||||
if v in _BIG_CATALOG_VENDORS:
|
||||
assert v in featured_vendors, (
|
||||
f"Priority vendor {v!r} missing from featured set — "
|
||||
f"round-robin guarantee violated."
|
||||
)
|
||||
|
||||
def test_sticky_selection_is_preserved(self):
|
||||
from api.config import _build_nous_featured_set
|
||||
catalog = _build_big_catalog()
|
||||
# Pick a model from a leftover (non-priority) vendor that wouldn't
|
||||
# normally make the featured cut.
|
||||
sticky = "lambda-ai/model-lambda-ai-15"
|
||||
assert sticky in catalog
|
||||
featured, extras = _build_nous_featured_set(catalog, selected_model_id=sticky)
|
||||
assert sticky in featured, (
|
||||
f"Sticky-selected model {sticky!r} must appear in featured — "
|
||||
f"otherwise the user's choice gets orphaned out of the dropdown "
|
||||
f"after a refresh."
|
||||
)
|
||||
assert sticky not in extras
|
||||
|
||||
def test_sticky_selection_handles_at_nous_prefix(self):
|
||||
from api.config import _build_nous_featured_set
|
||||
catalog = _build_big_catalog()
|
||||
# The frontend stores selections as @nous:vendor/model — helper must
|
||||
# strip the prefix to match against the bare-id catalog.
|
||||
sticky_with_prefix = "@nous:lambda-ai/model-lambda-ai-15"
|
||||
bare = "lambda-ai/model-lambda-ai-15"
|
||||
featured, _ = _build_nous_featured_set(catalog, selected_model_id=sticky_with_prefix)
|
||||
assert bare in featured
|
||||
|
||||
def test_curated_static_flagships_are_preserved(self):
|
||||
from api.config import _build_nous_featured_set, _PROVIDER_MODELS
|
||||
# Build a catalog that contains all the curated static IDs so the
|
||||
# rule-2 path fires.
|
||||
static_ids = []
|
||||
for entry in _PROVIDER_MODELS.get("nous", []):
|
||||
sid = entry["id"]
|
||||
if sid.startswith("@nous:"):
|
||||
sid = sid[len("@nous:"):]
|
||||
static_ids.append(sid)
|
||||
catalog = static_ids + [f"filler-vendor/filler-{i:03d}" for i in range(100)]
|
||||
featured, _ = _build_nous_featured_set(catalog)
|
||||
for sid in static_ids:
|
||||
assert sid in featured, (
|
||||
f"Curated static flagship {sid!r} dropped from featured set."
|
||||
)
|
||||
|
||||
def test_empty_catalog_returns_empty(self):
|
||||
from api.config import _build_nous_featured_set
|
||||
f, e = _build_nous_featured_set([])
|
||||
assert f == [] and e == []
|
||||
|
||||
def test_deterministic_across_calls(self):
|
||||
from api.config import _build_nous_featured_set
|
||||
catalog = _build_big_catalog()
|
||||
f1, e1 = _build_nous_featured_set(catalog)
|
||||
f2, e2 = _build_nous_featured_set(catalog)
|
||||
assert f1 == f2 and e1 == e2, (
|
||||
"Featured set must be deterministic — random/seeded selection "
|
||||
"would cause cache thrash and dropdown flicker on every reload."
|
||||
)
|
||||
|
||||
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
# Section 2 — End-to-end /api/models behaviour with the cap applied
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestApiModelsLargeCatalog:
|
||||
"""Wired-up test exercising the dispatch branch at config.py:2243."""
|
||||
|
||||
def test_picker_caps_large_catalog_and_exposes_extras(self, monkeypatch, tmp_path):
|
||||
_scrub_provider_env(monkeypatch)
|
||||
catalog = _build_big_catalog()
|
||||
_install_fake_hermes_cli(monkeypatch, nous_ids=catalog)
|
||||
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["groups"] if g["provider_id"] == "nous"]
|
||||
assert len(nous_groups) == 1
|
||||
grp = nous_groups[0]
|
||||
from api.config import _NOUS_FEATURED_TARGET
|
||||
assert len(grp["models"]) == _NOUS_FEATURED_TARGET, (
|
||||
f"Picker should render {_NOUS_FEATURED_TARGET} featured entries "
|
||||
f"on a {len(catalog)}-model catalog, got {len(grp['models'])}."
|
||||
)
|
||||
assert "extra_models" in grp, (
|
||||
"Capped Nous group must include 'extra_models' so /model "
|
||||
"autocomplete and the label map cover the full catalog."
|
||||
)
|
||||
assert len(grp["extra_models"]) == len(catalog) - _NOUS_FEATURED_TARGET
|
||||
# Optgroup label is decorated with the truncation count so the user
|
||||
# knows the dropdown is intentionally trimmed.
|
||||
assert f"{_NOUS_FEATURED_TARGET} of {len(catalog)}" in grp["provider"], (
|
||||
f"Provider label should include '({_NOUS_FEATURED_TARGET} of "
|
||||
f"{len(catalog)})' for capped catalogs, got {grp['provider']!r}."
|
||||
)
|
||||
finally:
|
||||
restore()
|
||||
|
||||
def test_picker_does_not_cap_small_catalog(self, monkeypatch, tmp_path):
|
||||
_scrub_provider_env(monkeypatch)
|
||||
# 20 models — below threshold, should pass through with no extras.
|
||||
small_catalog = [f"vendor-{i % 4}/model-{i:02d}" for i in range(20)]
|
||||
_install_fake_hermes_cli(monkeypatch, nous_ids=small_catalog)
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
|
||||
|
||||
restore = _swap_in_test_config({"model": {"provider": "nous"}})
|
||||
try:
|
||||
data = config.get_available_models()
|
||||
grp = next(g for g in data["groups"] if g["provider_id"] == "nous")
|
||||
assert len(grp["models"]) == 20
|
||||
assert "extra_models" not in grp or grp["extra_models"] == []
|
||||
assert "of " not in grp["provider"], (
|
||||
"Optgroup label should NOT include a truncation count when no "
|
||||
"trimming happened, got " + repr(grp["provider"])
|
||||
)
|
||||
finally:
|
||||
restore()
|
||||
|
||||
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
# Section 3 — Auth-detection symmetry (#1567 part 1)
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestNousDetectionSymmetry:
|
||||
"""The picker must include Nous whenever the providers card would —
|
||||
fixes the asymmetric-detection bug at the heart of #1567."""
|
||||
|
||||
def test_picker_includes_nous_when_get_auth_status_logged_in(self, monkeypatch, tmp_path):
|
||||
"""list_available_providers() reports authenticated=False but
|
||||
get_auth_status('nous').logged_in=True. Picker must still show Nous."""
|
||||
_scrub_provider_env(monkeypatch)
|
||||
catalog = ["anthropic/claude-opus-4.7", "openai/gpt-5.5"]
|
||||
_install_fake_hermes_cli(
|
||||
monkeypatch,
|
||||
nous_ids=catalog,
|
||||
list_authenticated=False, # primary detection path says NO
|
||||
auth_status_logged_in=True, # secondary detection path says YES
|
||||
)
|
||||
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["groups"] if g["provider_id"] == "nous"]
|
||||
assert nous_groups, (
|
||||
"Picker must include Nous group when get_auth_status reports "
|
||||
"logged_in=True, even if list_available_providers disagrees. "
|
||||
"This is the asymmetric-detection bug from #1567."
|
||||
)
|
||||
assert len(nous_groups[0]["models"]) == 2
|
||||
finally:
|
||||
restore()
|
||||
|
||||
def test_picker_omits_nous_when_both_auth_signals_false(self, monkeypatch, tmp_path):
|
||||
"""When neither signal reports authenticated, Nous should NOT appear.
|
||||
Previously the static 4-entry list could leak in via the fallback path
|
||||
even for unauthenticated users — that fallback is now scoped to the
|
||||
hermes_cli-unavailable case only."""
|
||||
_scrub_provider_env(monkeypatch)
|
||||
_install_fake_hermes_cli(
|
||||
monkeypatch,
|
||||
nous_ids=[], # no live catalog (also no auth)
|
||||
list_authenticated=False,
|
||||
auth_status_logged_in=False,
|
||||
)
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
|
||||
|
||||
restore = _swap_in_test_config({"model": {"provider": "anthropic"}})
|
||||
try:
|
||||
# Active provider is anthropic, not nous — so detected_providers
|
||||
# only includes nous if the new auth-symmetry check fires.
|
||||
data = config.get_available_models()
|
||||
nous_groups = [g for g in data["groups"] if g["provider_id"] == "nous"]
|
||||
assert not nous_groups, (
|
||||
"Nous must NOT appear in picker when neither auth signal "
|
||||
"reports authenticated. Got: " + str(nous_groups)
|
||||
)
|
||||
finally:
|
||||
restore()
|
||||
|
||||
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
# Section 4 — Live-fetch-empty handling (#1567 part 2)
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestNousLiveFetchEmpty:
|
||||
"""When authenticated but live-fetch returns [] (transient hermes_cli
|
||||
state, OAuth refresh in flight), DON'T fall back to the stale 4-entry
|
||||
static list — that creates the providers-card-vs-picker disagreement
|
||||
that #1567 reports. Omit the group entirely instead."""
|
||||
|
||||
def test_authenticated_empty_catalog_omits_nous_group(self, monkeypatch, tmp_path):
|
||||
_scrub_provider_env(monkeypatch)
|
||||
_install_fake_hermes_cli(
|
||||
monkeypatch,
|
||||
nous_ids=[], # live-fetch returns empty list (no exception)
|
||||
auth_status_logged_in=True, # but user IS authenticated
|
||||
)
|
||||
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["groups"] if g["provider_id"] == "nous"]
|
||||
assert not nous_groups, (
|
||||
"Authenticated user with empty live-fetch should NOT see "
|
||||
"the stale 4-entry static list — that's exactly the "
|
||||
"providers-card-vs-picker disagreement #1567 reports. "
|
||||
"Omit the Nous group entirely; it'll re-populate on the "
|
||||
"next cache rebuild when the live-fetch returns something."
|
||||
)
|
||||
finally:
|
||||
restore()
|
||||
|
||||
def test_hermes_cli_unavailable_falls_back_to_static_4(self, monkeypatch, tmp_path):
|
||||
"""When hermes_cli is unavailable (raises) — distinct from returning [] —
|
||||
we DO fall back to the static 4-entry list so the picker isn't empty
|
||||
in that degraded environment. This preserves pre-#1538 behavior for
|
||||
test envs without hermes_cli."""
|
||||
_scrub_provider_env(monkeypatch)
|
||||
_install_fake_hermes_cli(
|
||||
monkeypatch,
|
||||
raise_on_lookup=True,
|
||||
auth_status_logged_in=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["groups"] if g["provider_id"] == "nous"]
|
||||
assert nous_groups, (
|
||||
"When hermes_cli raises, Nous group MUST still appear with "
|
||||
"the curated static fallback so the picker isn't empty in "
|
||||
"test envs that lack the agent package."
|
||||
)
|
||||
assert len(nous_groups[0]["models"]) == 4, (
|
||||
"Static fallback should expose the curated 4-entry list "
|
||||
"from _PROVIDER_MODELS['nous']."
|
||||
)
|
||||
finally:
|
||||
restore()
|
||||
|
||||
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
# Section 5 — Providers card ↔ picker symmetry
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestProvidersCardPickerSymmetry:
|
||||
"""Both endpoints must report the same featured set + total count for
|
||||
Nous Portal. This is the load-bearing invariant that ends the visual
|
||||
disagreement #1567 reports."""
|
||||
|
||||
def test_providers_card_and_picker_agree_on_featured_set(self, monkeypatch, tmp_path):
|
||||
_scrub_provider_env(monkeypatch)
|
||||
catalog = _build_big_catalog()
|
||||
_install_fake_hermes_cli(monkeypatch, nous_ids=catalog)
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
|
||||
|
||||
restore = _swap_in_test_config({"model": {"provider": "nous"}})
|
||||
try:
|
||||
from api.providers import get_providers
|
||||
from api.config import _NOUS_FEATURED_TARGET
|
||||
|
||||
providers = {p["id"]: p for p in get_providers()["providers"]}
|
||||
picker = config.get_available_models()
|
||||
picker_nous = next(g for g in picker["groups"] if g["provider_id"] == "nous")
|
||||
|
||||
card = providers["nous"]
|
||||
# Both render exactly _NOUS_FEATURED_TARGET visible models.
|
||||
assert len(card["models"]) == _NOUS_FEATURED_TARGET
|
||||
assert len(picker_nous["models"]) == _NOUS_FEATURED_TARGET
|
||||
|
||||
# Both report the full catalog size somewhere.
|
||||
assert card["models_total"] == len(catalog), (
|
||||
f"Providers card models_total should match live catalog size, "
|
||||
f"got {card['models_total']} vs catalog {len(catalog)}."
|
||||
)
|
||||
picker_total = len(picker_nous.get("models", [])) + len(
|
||||
picker_nous.get("extra_models", [])
|
||||
)
|
||||
assert picker_total == len(catalog), (
|
||||
f"Picker featured + extras must equal live catalog size, "
|
||||
f"got {picker_total} vs {len(catalog)}."
|
||||
)
|
||||
|
||||
# And they pick THE SAME featured set (not e.g. one's first-15
|
||||
# and another's last-15).
|
||||
card_ids = [m["id"] for m in card["models"]]
|
||||
picker_ids = [m["id"] for m in picker_nous["models"]]
|
||||
assert card_ids == picker_ids, (
|
||||
f"Providers card and picker must show the SAME featured "
|
||||
f"set so users see consistent labels in both places. "
|
||||
f"Card: {card_ids}\nPicker: {picker_ids}"
|
||||
)
|
||||
finally:
|
||||
restore()
|
||||
|
||||
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
# Section 6 — Frontend contract (static-source assertions)
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestFrontendExtrasContract:
|
||||
"""Pin the JS-side contract: dropdown reads `models`, slash command and
|
||||
label map ALSO read `extra_models`. Without this, a model from the
|
||||
catalog tail gets a bare-ID label or is invisible to /model autocomplete."""
|
||||
|
||||
def test_ui_js_hydrates_dynamic_labels_from_extra_models(self):
|
||||
from pathlib import Path
|
||||
src = (Path(__file__).resolve().parent.parent / "static" / "ui.js").read_text(encoding="utf-8")
|
||||
# Find the populateModelDropdown function and check it consumes
|
||||
# extra_models. Use a windowed substring search so the test stays
|
||||
# robust against minor refactors of surrounding code.
|
||||
idx = src.find("async function populateModelDropdown")
|
||||
assert idx != -1
|
||||
body = src[idx : idx + 3000]
|
||||
assert "extra_models" in body, (
|
||||
"populateModelDropdown must hydrate _dynamicModelLabels from "
|
||||
"g.extra_models so a model selected outside the featured set "
|
||||
"still gets a proper label. Without this, /model audio-lines "
|
||||
"→ 'audio-lines' bare-ID display. (#1567)"
|
||||
)
|
||||
|
||||
def test_commands_js_loads_slash_args_from_extra_models(self):
|
||||
from pathlib import Path
|
||||
src = (Path(__file__).resolve().parent.parent / "static" / "commands.js").read_text(encoding="utf-8")
|
||||
idx = src.find("async function _loadSlashModelSubArgs")
|
||||
assert idx != -1
|
||||
body = src[idx : idx + 1500]
|
||||
assert "extra_models" in body, (
|
||||
"_loadSlashModelSubArgs must iterate group.extra_models so /model "
|
||||
"autocomplete covers the full catalog, not just the dropdown's "
|
||||
"featured subset. The slash command exists precisely so power "
|
||||
"users can reach any model by typing its name. (#1567)"
|
||||
)
|
||||
|
||||
def test_panels_js_uses_models_total_for_count(self):
|
||||
from pathlib import Path
|
||||
src = (Path(__file__).resolve().parent.parent / "static" / "panels.js").read_text(encoding="utf-8")
|
||||
idx = src.find("function _buildProviderCard")
|
||||
assert idx != -1
|
||||
body = src[idx : idx + 1500]
|
||||
assert "models_total" in body, (
|
||||
"Provider card header should use p.models_total (full catalog "
|
||||
"size) for the count, not p.models.length (which is now the "
|
||||
"trimmed featured-set size). Without this, the header text says "
|
||||
"'15 models' instead of '396 models' for capped catalogs. (#1567)"
|
||||
)
|
||||
|
||||
def test_panels_js_renders_more_disclosure_pill(self):
|
||||
from pathlib import Path
|
||||
src = (Path(__file__).resolve().parent.parent / "static" / "panels.js").read_text(encoding="utf-8")
|
||||
# The "+N more" disclosure must reference the difference between
|
||||
# rendered count and total count somewhere in the providers-card
|
||||
# rendering path.
|
||||
assert "provider-card-model-tag-more" in src, (
|
||||
"Provider card must render a '+N more' disclosure pill when "
|
||||
"len(models) < models_total, so users know the dropdown is "
|
||||
"intentionally capped and the rest is reachable via /model."
|
||||
)
|
||||
@@ -0,0 +1,424 @@
|
||||
"""Regression tests for #1568 — duplicate provider groups in model picker.
|
||||
|
||||
Reporter (Deor, Discord #report-bugs, May 03 2026 14:19 PT) saw the Settings →
|
||||
Default Model dropdown rendering the OpenCode Go provider as TWO separate
|
||||
optgroups: ``OpenCode Go`` (the canonical one with all 14 catalog models) and
|
||||
``Opencode_Go`` (a phantom group with one self-referential entry).
|
||||
|
||||
Three structural causes, all in ``api/config.py:_build_available_models_uncached``:
|
||||
|
||||
1. The detection path at line ~1980 reads ``cfg["providers"]`` keys verbatim —
|
||||
if the user's config has ``providers.opencode_go.api_key`` (underscore
|
||||
variant) AND another path adds the canonical ``opencode-go`` (e.g. via
|
||||
``active_provider``), both end up in ``detected_providers`` and the build
|
||||
loop creates two groups.
|
||||
|
||||
2. The injection block at line ~2598 puts ANY ``model.default`` string into
|
||||
the picker as a fake option, so a stray ``model.default: opencode_go``
|
||||
(provider id mistakenly used as a model id) surfaces as a phantom model
|
||||
labelled ``"Opencode GO"``.
|
||||
|
||||
3. Empty optgroups can leak through when a non-canonical provider id makes it
|
||||
into ``detected_providers`` but has no entry in ``_PROVIDER_MODELS`` — the
|
||||
build loop creates an optgroup with zero models.
|
||||
|
||||
The fix is a new ``_canonicalise_provider_id`` helper applied at every
|
||||
detection callsite, a post-collection dedup of ``detected_providers``, a
|
||||
provider-id guard on the model.default injection block, and an empty-group
|
||||
filter at the very end of the build.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
import types
|
||||
|
||||
import api.config as config
|
||||
import api.profiles as profiles
|
||||
|
||||
|
||||
def _install_fake_hermes_cli(monkeypatch):
|
||||
"""Stub hermes_cli so detection is deterministic in tests."""
|
||||
fake_pkg = types.ModuleType("hermes_cli")
|
||||
fake_pkg.__path__ = []
|
||||
|
||||
fake_models = types.ModuleType("hermes_cli.models")
|
||||
fake_models.list_available_providers = lambda: []
|
||||
fake_models.provider_model_ids = lambda pid: []
|
||||
|
||||
fake_auth = types.ModuleType("hermes_cli.auth")
|
||||
fake_auth.get_auth_status = lambda _pid: {}
|
||||
|
||||
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):
|
||||
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):
|
||||
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)
|
||||
|
||||
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
# Section 1 — _canonicalise_provider_id helper
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestCanonicaliseProviderId:
|
||||
def test_canonical_id_preserved(self):
|
||||
from api.config import _canonicalise_provider_id
|
||||
assert _canonicalise_provider_id("opencode-go") == "opencode-go"
|
||||
assert _canonicalise_provider_id("anthropic") == "anthropic"
|
||||
assert _canonicalise_provider_id("x-ai") == "x-ai"
|
||||
|
||||
def test_underscore_folded_to_hyphen(self):
|
||||
from api.config import _canonicalise_provider_id
|
||||
# Deor's exact failure mode — the config-file key uses underscores
|
||||
# but every other code path uses the hyphenated canonical form.
|
||||
assert _canonicalise_provider_id("opencode_go") == "opencode-go"
|
||||
|
||||
def test_case_folded(self):
|
||||
from api.config import _canonicalise_provider_id
|
||||
assert _canonicalise_provider_id("OpenCode-Go") == "opencode-go"
|
||||
assert _canonicalise_provider_id("OPENCODE_GO") == "opencode-go"
|
||||
assert _canonicalise_provider_id("Anthropic") == "anthropic"
|
||||
|
||||
def test_alias_resolved_when_target_is_canonical(self):
|
||||
from api.config import _canonicalise_provider_id
|
||||
# z-ai is an alias for the canonical zai.
|
||||
assert _canonicalise_provider_id("z-ai") == "zai"
|
||||
assert _canonicalise_provider_id("z_ai") == "zai"
|
||||
assert _canonicalise_provider_id("Z.AI") == "zai" or _canonicalise_provider_id("Z.AI") == "z.ai"
|
||||
|
||||
def test_alias_not_applied_when_input_is_already_canonical(self):
|
||||
from api.config import _canonicalise_provider_id
|
||||
# x-ai IS the canonical key in _PROVIDER_DISPLAY/_PROVIDER_MODELS.
|
||||
# _PROVIDER_ALIASES happens to also map x-ai → xai (for hermes_cli
|
||||
# compat), but we must NOT round-trip through that alias because
|
||||
# xai isn't keyed in _PROVIDER_DISPLAY/_PROVIDER_MODELS.
|
||||
assert _canonicalise_provider_id("x-ai") == "x-ai"
|
||||
assert _canonicalise_provider_id("X-AI") == "x-ai"
|
||||
|
||||
def test_empty_input(self):
|
||||
from api.config import _canonicalise_provider_id
|
||||
assert _canonicalise_provider_id("") == ""
|
||||
assert _canonicalise_provider_id(None) == ""
|
||||
assert _canonicalise_provider_id(" ") == ""
|
||||
|
||||
def test_unknown_id_normalised_but_preserved(self):
|
||||
from api.config import _canonicalise_provider_id
|
||||
# Unknown ids: still get the underscore→hyphen + lowercase fold so
|
||||
# downstream dedup works, but no alias resolution.
|
||||
assert _canonicalise_provider_id("future_provider") == "future-provider"
|
||||
assert _canonicalise_provider_id("CUSTOM_THING") == "custom-thing"
|
||||
|
||||
def test_idempotent(self):
|
||||
from api.config import _canonicalise_provider_id
|
||||
for raw in ("opencode_go", "OPENCODE-GO", "z-ai", "anthropic", "future_x"):
|
||||
once = _canonicalise_provider_id(raw)
|
||||
twice = _canonicalise_provider_id(once)
|
||||
assert once == twice, f"helper must be idempotent: {raw!r} -> {once!r} -> {twice!r}"
|
||||
|
||||
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
# Section 2 — Detection-path dedup (the core #1568 fix)
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestProviderGroupDedup:
|
||||
"""When config.yaml uses a non-canonical providers.<id> key, the picker
|
||||
must still surface ONE provider group, not two."""
|
||||
|
||||
def test_underscored_providers_key_does_not_create_phantom_group(self, monkeypatch, tmp_path):
|
||||
"""Deor's exact reproduction case: ``providers.opencode_go.api_key``
|
||||
(underscored) with ``model.provider: opencode-go`` (hyphenated)."""
|
||||
_scrub_provider_env(monkeypatch)
|
||||
_install_fake_hermes_cli(monkeypatch)
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
|
||||
|
||||
restore = _swap_in_test_config({
|
||||
"model": {"provider": "opencode-go", "default": "glm-5.1"},
|
||||
"providers": {"opencode_go": {"api_key": "fake-test-key"}},
|
||||
})
|
||||
try:
|
||||
data = config.get_available_models()
|
||||
opencode_groups = [
|
||||
g for g in data["groups"]
|
||||
if "opencode" in (g.get("provider_id") or "").lower()
|
||||
or "opencode" in (g.get("provider") or "").lower()
|
||||
]
|
||||
assert len(opencode_groups) == 1, (
|
||||
f"Expected exactly ONE OpenCode Go group, got {len(opencode_groups)}: "
|
||||
f"{[(g['provider'], g['provider_id']) for g in opencode_groups]}. "
|
||||
f"Pre-fix, the underscored providers-key produced a separate "
|
||||
f"'Opencode_Go' provider group at the bottom of the picker (#1568)."
|
||||
)
|
||||
grp = opencode_groups[0]
|
||||
assert grp["provider_id"] == "opencode-go", (
|
||||
f"Group provider_id should be canonical 'opencode-go', got "
|
||||
f"{grp['provider_id']!r}."
|
||||
)
|
||||
assert grp["provider"] == "OpenCode Go", (
|
||||
f"Group display name should be canonical 'OpenCode Go', got "
|
||||
f"{grp['provider']!r}."
|
||||
)
|
||||
finally:
|
||||
restore()
|
||||
|
||||
def test_uppercase_providers_key_does_not_create_phantom_group(self, monkeypatch, tmp_path):
|
||||
_scrub_provider_env(monkeypatch)
|
||||
_install_fake_hermes_cli(monkeypatch)
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
|
||||
|
||||
restore = _swap_in_test_config({
|
||||
"model": {"provider": "opencode-go", "default": "glm-5.1"},
|
||||
"providers": {"OPENCODE-GO": {"api_key": "fake"}},
|
||||
})
|
||||
try:
|
||||
data = config.get_available_models()
|
||||
opencode_groups = [
|
||||
g for g in data["groups"]
|
||||
if (g.get("provider_id") or "").lower().replace("_", "-") == "opencode-go"
|
||||
]
|
||||
assert len(opencode_groups) == 1
|
||||
finally:
|
||||
restore()
|
||||
|
||||
def test_aliased_providers_key_collapses_to_canonical(self, monkeypatch, tmp_path):
|
||||
"""``z-ai`` is a known alias for canonical ``zai``. A user with
|
||||
``providers.z-ai.api_key`` should still see ONE Z.AI group, not two."""
|
||||
_scrub_provider_env(monkeypatch)
|
||||
_install_fake_hermes_cli(monkeypatch)
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
|
||||
|
||||
restore = _swap_in_test_config({
|
||||
"model": {"provider": "zai", "default": "glm-5"},
|
||||
"providers": {"z-ai": {"api_key": "fake"}},
|
||||
})
|
||||
try:
|
||||
data = config.get_available_models()
|
||||
zai_groups = [
|
||||
g for g in data["groups"]
|
||||
if (g.get("provider_id") or "") in ("zai", "z-ai")
|
||||
]
|
||||
assert len(zai_groups) == 1, (
|
||||
f"Expected one Z.AI group, got {len(zai_groups)}: "
|
||||
f"{[(g['provider'], g['provider_id']) for g in zai_groups]}"
|
||||
)
|
||||
assert zai_groups[0]["provider_id"] == "zai"
|
||||
finally:
|
||||
restore()
|
||||
|
||||
def test_happy_path_unchanged(self, monkeypatch, tmp_path):
|
||||
"""Sanity: when config keys are already canonical, behaviour is unchanged."""
|
||||
_scrub_provider_env(monkeypatch)
|
||||
_install_fake_hermes_cli(monkeypatch)
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
|
||||
|
||||
restore = _swap_in_test_config({
|
||||
"model": {"provider": "opencode-go", "default": "glm-5.1"},
|
||||
"providers": {"opencode-go": {"api_key": "fake"}},
|
||||
})
|
||||
try:
|
||||
data = config.get_available_models()
|
||||
opencode_groups = [
|
||||
g for g in data["groups"]
|
||||
if g.get("provider_id") == "opencode-go"
|
||||
]
|
||||
assert len(opencode_groups) == 1
|
||||
assert opencode_groups[0]["provider"] == "OpenCode Go"
|
||||
assert len(opencode_groups[0]["models"]) >= 1
|
||||
finally:
|
||||
restore()
|
||||
|
||||
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
# Section 3 — model.default provider-id injection guard
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestDefaultModelProviderIdGuard:
|
||||
"""``model.default = <provider id>`` is a common config typo. Pre-fix the
|
||||
picker silently injected the provider id as a phantom model option.
|
||||
Post-fix the injection is skipped + a warning is logged."""
|
||||
|
||||
def test_provider_id_as_default_does_not_inject_phantom(self, monkeypatch, tmp_path, caplog):
|
||||
_scrub_provider_env(monkeypatch)
|
||||
_install_fake_hermes_cli(monkeypatch)
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
|
||||
|
||||
restore = _swap_in_test_config({
|
||||
"model": {"provider": "opencode-go", "default": "opencode_go"},
|
||||
"providers": {"opencode-go": {"api_key": "fake"}},
|
||||
})
|
||||
try:
|
||||
with caplog.at_level("WARNING", logger="api.config"):
|
||||
data = config.get_available_models()
|
||||
opencode = next(
|
||||
g for g in data["groups"] if g.get("provider_id") == "opencode-go"
|
||||
)
|
||||
ids = {m["id"] for m in opencode["models"]}
|
||||
for bad in ("opencode_go", "opencode-go", "OpenCode Go"):
|
||||
assert bad not in ids, (
|
||||
f"Phantom model id {bad!r} leaked into picker — the "
|
||||
f"provider-id guard should skip injection. Pre-fix, "
|
||||
f"this surfaced as a self-referential 'Opencode GO' "
|
||||
f"15th entry. (#1568)"
|
||||
)
|
||||
# And we get a logged warning so the misconfig is discoverable.
|
||||
assert any(
|
||||
"model.default" in rec.getMessage().lower()
|
||||
or "provider id" in rec.getMessage().lower()
|
||||
for rec in caplog.records
|
||||
), (
|
||||
"Skipping the injection should emit a WARNING so the user's "
|
||||
"actual config error is discoverable in logs, not just silently "
|
||||
"papered over."
|
||||
)
|
||||
finally:
|
||||
restore()
|
||||
|
||||
def test_provider_alias_as_default_does_not_inject_phantom(self, monkeypatch, tmp_path):
|
||||
_scrub_provider_env(monkeypatch)
|
||||
_install_fake_hermes_cli(monkeypatch)
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
|
||||
|
||||
# Z.AI / GLM has display name "Z.AI / GLM", canonical id "zai",
|
||||
# alias "z-ai". model.default == "z-ai" should be caught.
|
||||
restore = _swap_in_test_config({
|
||||
"model": {"provider": "zai", "default": "z-ai"},
|
||||
"providers": {"zai": {"api_key": "fake"}},
|
||||
})
|
||||
try:
|
||||
data = config.get_available_models()
|
||||
zai = next(g for g in data["groups"] if g.get("provider_id") == "zai")
|
||||
ids = {m["id"] for m in zai["models"]}
|
||||
assert "z-ai" not in ids
|
||||
assert "zai" not in ids
|
||||
finally:
|
||||
restore()
|
||||
|
||||
def test_real_unknown_model_id_still_injected(self, monkeypatch, tmp_path):
|
||||
"""Forward-compat: a NEW model id not yet in the static catalog
|
||||
(newly released, custom endpoint) should STILL be injected so the
|
||||
user's configured default isn't hidden from them."""
|
||||
_scrub_provider_env(monkeypatch)
|
||||
_install_fake_hermes_cli(monkeypatch)
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
|
||||
|
||||
restore = _swap_in_test_config({
|
||||
"model": {"provider": "anthropic", "default": "claude-opus-5.0-future"},
|
||||
"providers": {"anthropic": {"api_key": "fake"}},
|
||||
})
|
||||
try:
|
||||
data = config.get_available_models()
|
||||
all_ids = {m["id"] for g in data["groups"] for m in g["models"]}
|
||||
assert "claude-opus-5.0-future" in all_ids, (
|
||||
"Legitimate unknown model ids must still be injected — "
|
||||
"otherwise newly-released models or custom endpoints "
|
||||
"wouldn't show in the picker until a release with an "
|
||||
"updated _PROVIDER_MODELS catalog. The guard must only "
|
||||
"reject provider ids and known aliases."
|
||||
)
|
||||
finally:
|
||||
restore()
|
||||
|
||||
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
# Section 4 — Empty-group filter
|
||||
# ────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestEmptyGroupFilter:
|
||||
def test_empty_optgroups_dropped(self, monkeypatch, tmp_path):
|
||||
"""Pre-fix, when a non-canonical provider id slipped past the
|
||||
detection guards into _PROVIDER_MODELS lookup (which has no entry
|
||||
for ``opencode_go``), the build loop produced a zero-models
|
||||
optgroup that rendered as a phantom provider entry. The empty-group
|
||||
filter at the end of the build catches this regardless of which
|
||||
detection path leaked the bad id."""
|
||||
_scrub_provider_env(monkeypatch)
|
||||
_install_fake_hermes_cli(monkeypatch)
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
|
||||
|
||||
restore = _swap_in_test_config({
|
||||
"model": {"provider": "opencode-go", "default": "glm-5.1"},
|
||||
"providers": {"opencode_go": {"api_key": "fake"}},
|
||||
})
|
||||
try:
|
||||
data = config.get_available_models()
|
||||
empty_groups = [g for g in data["groups"] if not g.get("models")]
|
||||
# Only custom: groups are allowed to be empty (intentional UX).
|
||||
allowed_empty = [
|
||||
g for g in empty_groups
|
||||
if (g.get("provider_id") or "").startswith("custom:")
|
||||
]
|
||||
disallowed = [g for g in empty_groups if g not in allowed_empty]
|
||||
assert not disallowed, (
|
||||
f"Zero-model optgroups should not appear in the picker — "
|
||||
f"they're pure UI noise. Got {len(disallowed)} unexpected "
|
||||
f"empty groups: {[(g['provider'], g['provider_id']) for g in disallowed]}."
|
||||
)
|
||||
finally:
|
||||
restore()
|
||||
|
||||
def test_custom_provider_can_still_be_empty(self, monkeypatch, tmp_path):
|
||||
"""Custom providers from ``custom_providers`` config are exempt
|
||||
from the empty-group filter — users may want an empty card visible
|
||||
as a reminder to fill in models."""
|
||||
_scrub_provider_env(monkeypatch)
|
||||
_install_fake_hermes_cli(monkeypatch)
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
|
||||
|
||||
restore = _swap_in_test_config({
|
||||
"model": {"provider": "custom", "default": "some-model"},
|
||||
"custom_providers": [
|
||||
{"name": "my-empty-provider", "api_key": "fake"},
|
||||
],
|
||||
})
|
||||
try:
|
||||
data = config.get_available_models()
|
||||
# The empty-group filter should NOT drop a custom: provider.
|
||||
# (The exact custom group surface depends on other config logic;
|
||||
# this test just pins that custom: groups are exempt from the
|
||||
# filter, not that one is necessarily produced.)
|
||||
for g in data["groups"]:
|
||||
if (g.get("provider_id") or "").startswith("custom:"):
|
||||
# Found at least one custom group — that's enough to
|
||||
# confirm the exempt path doesn't drop them, since
|
||||
# the empty-models case would otherwise be filtered.
|
||||
return
|
||||
finally:
|
||||
restore()
|
||||
@@ -75,7 +75,10 @@ class TestConfigProvidersDetection:
|
||||
# Find the config providers detection block
|
||||
m = re.search(r'Also detect providers explicitly listed', src)
|
||||
assert m, "Comment about config.yaml providers detection must exist"
|
||||
block = src[m.start():m.start() + 500]
|
||||
# 1500-char window absorbs documentation expansion (e.g. the
|
||||
# _canonicalise_provider_id discussion added in #1568) without
|
||||
# losing the structural-assertion intent.
|
||||
block = src[m.start():m.start() + 1500]
|
||||
assert "_PROVIDER_MODELS" in block, \
|
||||
"Config providers detection must check against _PROVIDER_MODELS"
|
||||
|
||||
|
||||
@@ -0,0 +1,199 @@
|
||||
"""Regression test: /api/crons must read jobs.json from the *active profile*.
|
||||
|
||||
Before the fix, `cron.jobs.list_jobs()` resolved HERMES_HOME from os.environ
|
||||
at call time, ignoring the WebUI's per-request thread-local profile. So the
|
||||
Scheduled Jobs panel showed the process-default profile's jobs regardless of
|
||||
which profile the user had selected in the cookie.
|
||||
|
||||
This test writes two distinct jobs.json files (default + a named profile),
|
||||
then verifies `cron_profile_context` pins the cron.jobs call to the named
|
||||
profile's file.
|
||||
"""
|
||||
import json
|
||||
import os
|
||||
import pathlib
|
||||
import sys
|
||||
import threading
|
||||
from unittest import mock
|
||||
|
||||
import pytest
|
||||
|
||||
# Ensure both repos are importable.
|
||||
WEBUI_ROOT = pathlib.Path(__file__).resolve().parent.parent
|
||||
AGENT_ROOT = pathlib.Path(os.environ.get("HERMES_AGENT_ROOT", pathlib.Path.home() / "hermes-agent"))
|
||||
for p in (str(WEBUI_ROOT), str(AGENT_ROOT)):
|
||||
if p not in sys.path:
|
||||
sys.path.insert(0, p)
|
||||
|
||||
|
||||
def _write_jobs(home: pathlib.Path, jobs: list):
|
||||
cron_dir = home / "cron"
|
||||
cron_dir.mkdir(parents=True, exist_ok=True)
|
||||
(cron_dir / "jobs.json").write_text(
|
||||
json.dumps({"jobs": jobs}), encoding="utf-8"
|
||||
)
|
||||
|
||||
|
||||
def test_cron_profile_context_pins_profile_home(tmp_path, monkeypatch):
|
||||
"""The context manager should swap cron.jobs to read from the named profile."""
|
||||
pytest.importorskip("cron.jobs") # auto-skip when hermes-agent is unavailable
|
||||
|
||||
default_home = tmp_path / "default_home"
|
||||
meow_home = tmp_path / "default_home" / "profiles" / "meow"
|
||||
|
||||
_write_jobs(default_home, [{"id": "d1", "name": "default-job"}])
|
||||
_write_jobs(meow_home, [{"id": "m1", "name": "meow-job"}])
|
||||
|
||||
# Point base at default_home; HERMES_HOME env starts at default.
|
||||
monkeypatch.setenv("HERMES_HOME", str(default_home))
|
||||
|
||||
from api import profiles as p
|
||||
|
||||
monkeypatch.setattr(p, "_DEFAULT_HERMES_HOME", default_home)
|
||||
|
||||
# Baseline: no context → default profile.
|
||||
from cron.jobs import list_jobs
|
||||
# Force cron.jobs to re-evaluate its cached constants for this test run.
|
||||
import cron.jobs as _cj
|
||||
_cj.HERMES_DIR = default_home
|
||||
_cj.CRON_DIR = default_home / "cron"
|
||||
_cj.JOBS_FILE = _cj.CRON_DIR / "jobs.json"
|
||||
_cj.OUTPUT_DIR = _cj.CRON_DIR / "output"
|
||||
|
||||
jobs_before = list_jobs(include_disabled=True)
|
||||
assert any(j["id"] == "d1" for j in jobs_before), \
|
||||
f"Expected default-profile job before entering context, got {jobs_before}"
|
||||
|
||||
# Simulate a request with TLS profile = 'meow'.
|
||||
p.set_request_profile("meow")
|
||||
try:
|
||||
with p.cron_profile_context():
|
||||
jobs_inside = list_jobs(include_disabled=True)
|
||||
assert any(j["id"] == "m1" for j in jobs_inside), \
|
||||
f"Expected meow-profile job inside context, got {jobs_inside}"
|
||||
assert not any(j["id"] == "d1" for j in jobs_inside), \
|
||||
"Default-profile job leaked into meow context"
|
||||
finally:
|
||||
p.clear_request_profile()
|
||||
|
||||
# After the context exits, we should be back to default.
|
||||
jobs_after = list_jobs(include_disabled=True)
|
||||
assert any(j["id"] == "d1" for j in jobs_after), \
|
||||
f"Expected default-profile job after exiting context, got {jobs_after}"
|
||||
|
||||
|
||||
def test_cron_profile_context_for_home_pins_explicit_home(tmp_path):
|
||||
"""Thread variant: pin by explicit path (no TLS)."""
|
||||
pytest.importorskip("cron.jobs") # auto-skip when hermes-agent is unavailable
|
||||
|
||||
home_a = tmp_path / "a"
|
||||
home_b = tmp_path / "b"
|
||||
_write_jobs(home_a, [{"id": "a1", "name": "A"}])
|
||||
_write_jobs(home_b, [{"id": "b1", "name": "B"}])
|
||||
|
||||
# Start with env pointing at A.
|
||||
prev = os.environ.get("HERMES_HOME")
|
||||
os.environ["HERMES_HOME"] = str(home_a)
|
||||
try:
|
||||
import cron.jobs as _cj
|
||||
_cj.HERMES_DIR = home_a
|
||||
_cj.CRON_DIR = home_a / "cron"
|
||||
_cj.JOBS_FILE = _cj.CRON_DIR / "jobs.json"
|
||||
_cj.OUTPUT_DIR = _cj.CRON_DIR / "output"
|
||||
|
||||
from cron.jobs import list_jobs
|
||||
from api.profiles import cron_profile_context_for_home
|
||||
|
||||
assert any(j["id"] == "a1" for j in list_jobs(include_disabled=True))
|
||||
|
||||
with cron_profile_context_for_home(home_b):
|
||||
jobs_inside = list_jobs(include_disabled=True)
|
||||
assert any(j["id"] == "b1" for j in jobs_inside), jobs_inside
|
||||
assert not any(j["id"] == "a1" for j in jobs_inside), jobs_inside
|
||||
|
||||
# Restored to A.
|
||||
assert any(j["id"] == "a1" for j in list_jobs(include_disabled=True))
|
||||
finally:
|
||||
if prev is None:
|
||||
os.environ.pop("HERMES_HOME", None)
|
||||
else:
|
||||
os.environ["HERMES_HOME"] = prev
|
||||
|
||||
|
||||
def test_cron_profile_context_serializes_concurrent_access(tmp_path):
|
||||
"""The lock must prevent concurrent contexts from interleaving."""
|
||||
from api.profiles import cron_profile_context_for_home
|
||||
|
||||
home_a = tmp_path / "a"
|
||||
home_b = tmp_path / "b"
|
||||
home_a.mkdir()
|
||||
home_b.mkdir()
|
||||
|
||||
# Ensure the context lock is released between tests.
|
||||
from api import profiles as p
|
||||
assert not p._cron_env_lock.locked(), \
|
||||
"Lock leaked from a previous test"
|
||||
|
||||
observed = []
|
||||
barrier = threading.Barrier(2)
|
||||
|
||||
def worker(home, tag):
|
||||
barrier.wait()
|
||||
with cron_profile_context_for_home(home):
|
||||
observed.append(("enter", tag, os.environ["HERMES_HOME"]))
|
||||
# If serialization works, the partner thread cannot be inside
|
||||
# its own context at this moment.
|
||||
observed.append(("exit", tag))
|
||||
|
||||
t1 = threading.Thread(target=worker, args=(home_a, "A"))
|
||||
t2 = threading.Thread(target=worker, args=(home_b, "B"))
|
||||
t1.start(); t2.start()
|
||||
t1.join(); t2.join()
|
||||
|
||||
# Every enter must be immediately followed by its matching exit (no
|
||||
# interleaving), because the lock serializes the two contexts.
|
||||
assert len(observed) == 4
|
||||
first, second, third, fourth = observed
|
||||
assert first[0] == "enter" and second[0] == "exit" and first[1] == second[1]
|
||||
assert third[0] == "enter" and fourth[0] == "exit" and third[1] == fourth[1]
|
||||
|
||||
|
||||
def test_cron_run_does_not_silently_swallow_profile_resolution_errors():
|
||||
"""_handle_cron_run must NOT silently fall through to profile_home=None
|
||||
when get_active_hermes_home() raises.
|
||||
|
||||
A silent fallback would re-introduce the exact bug #1573 fixes — the
|
||||
worker thread would run unpinned against the process-global HERMES_HOME,
|
||||
silently corrupting cross-profile state. We'd rather 500 the request
|
||||
than risk that, since get_active_hermes_home() raising at all from
|
||||
inside a request handler means api.profiles is in a state we shouldn't
|
||||
be making cron decisions in.
|
||||
|
||||
Source-level assertion to catch any future re-introduction of the
|
||||
over-broad except clause.
|
||||
"""
|
||||
from pathlib import Path
|
||||
src = (Path(__file__).resolve().parent.parent / "api" / "routes.py").read_text(encoding="utf-8")
|
||||
|
||||
# Locate _handle_cron_run definition; assert the spawn block does NOT
|
||||
# wrap get_active_hermes_home() in a bare except that falls back to None.
|
||||
idx = src.find("def _handle_cron_run(handler, body):")
|
||||
assert idx != -1, "_handle_cron_run not found"
|
||||
body = src[idx : idx + 4000]
|
||||
|
||||
# The spawn site must call get_active_hermes_home() unguarded (no
|
||||
# try/except around it specifically), because a silent fallback to None
|
||||
# is exactly what would re-introduce #1573.
|
||||
spawn_idx = body.find("threading.Thread(target=_run_cron_tracked")
|
||||
assert spawn_idx != -1, "thread spawn not found in _handle_cron_run"
|
||||
|
||||
# Look at the 1500 chars before the spawn — should NOT contain the
|
||||
# `_profile_home = None` fallback pattern.
|
||||
pre_spawn = body[max(0, spawn_idx - 1500) : spawn_idx]
|
||||
assert "_profile_home = None" not in pre_spawn, (
|
||||
"_handle_cron_run silently falls back to _profile_home=None when "
|
||||
"get_active_hermes_home() raises. That re-introduces bug #1573 — "
|
||||
"the worker thread would run unpinned against the process-global "
|
||||
"HERMES_HOME. Let the exception propagate (500 the request) rather "
|
||||
"than corrupt cross-profile state silently."
|
||||
)
|
||||
Reference in New Issue
Block a user