Merge master into stage/7761-watcher-fence

# Conflicts:
#	CHANGELOG.md
This commit is contained in:
nesquena-hermes committed 2026-09-23 12:47:36 +00:00
commit c5d8e5fe4f
15 files changed
+1832 -206

No files matched your search

+14
View File
@@ -5,6 +5,10 @@
### Added ### Added
- **Full-session resolve concurrency is configurable.** `HERMES_WEBUI_MAX_SESSION_RESOLVE` sets how
many full-transcript session resolves may run at once (default 2, a positive integer up to 64;
zero, negative, non-numeric or out-of-range values fall back to 2). It is process-wide, so a
profile's `.env` cannot override it. (#7421, #7656 by @happy5318)
- **The sidebar's recent-session window is configurable.** `HERMES_WEBUI_VISIBLE_SESSION_LIMIT` sets - **The sidebar's recent-session window is configurable.** `HERMES_WEBUI_VISIBLE_SESSION_LIMIT` sets
how many recent sessions the sidebar lists (default 20). It also bounds how many delegated subagent how many recent sessions the sidebar lists (default 20). It also bounds how many delegated subagent
children can nest at once, so raise it for wide fan-outs. Invalid or non-positive values fall back to children can nest at once, so raise it for wide fan-outs. Invalid or non-positive values fall back to
@@ -28,6 +32,16 @@
leaves.** Final unsubscribe and queue eviction now invalidate the cache and fence projections that leaves.** Final unsubscribe and queue eviction now invalidate the cache and fence projections that
were already in flight, without holding the lock across database reads or SSE writes, so a client were already in flight, without holding the lock across database reads or SSE writes, so a client
that re-subscribes gets a fresh snapshot instead of a stale one. (#7761 by @ruizanthony) that re-subscribes gets a fresh snapshot instead of a stale one. (#7761 by @ruizanthony)
- **A first visit now uses the browser's language.** The server stores "no preference" as `null`
instead of defaulting to `"en"`, so a first-time visitor gets `navigator.language` while an
explicitly saved language (including English) still wins, and legacy `settings.json` files that
already hold `"en"` keep English. Reading the browser language is guarded, so an environment where
`navigator` throws falls back cleanly. (#7622, #7730 by @happy5318)
- **The live model list for a custom provider respects its `models:` allowlist, without treating
per-model metadata as one.** `/api/models/live` filters a custom provider's live catalog to its
configured `models:` when that is a list, or when it is a mapping with `discover_models: false`.
A mapping of per-model settings (the shape `hermes setup` writes) keeps the full live catalog, as
the Agent does. (#7165 by @happy5318)
- **`MEDIA:` links wrapped in inline code no longer 404.** Every `MEDIA:` capture site (renderer, - **`MEDIA:` links wrapped in inline code no longer 404.** Every `MEDIA:` capture site (renderer,
streaming parser, TTS stripper, session-media authorization and snapshot capture, seven in all) streaming parser, TTS stripper, session-media authorization and snapshot capture, seven in all)
swallowed the closing backtick of `` `MEDIA:/path` `` into the path, so the file lookup and the swallowed the closing backtick of `` `MEDIA:/path` `` into the path, so the file lookup and the
+26 -1
View File
@@ -11483,7 +11483,13 @@ _SETTINGS_DEFAULTS = {
"hidden_tabs": [], # sidebar tab panel names hidden by user (e.g. ["tasks","kanban"]); chat and settings are always visible "hidden_tabs": [], # sidebar tab panel names hidden by user (e.g. ["tasks","kanban"]); chat and settings are always visible
"tab_order": [], # user-defined sidebar/rail tab order for reorderable tabs; chat/settings stay fixed "tab_order": [], # user-defined sidebar/rail tab order for reorderable tabs; chat/settings stay fixed
"composer_control_order": [], # user-defined composer footer control order; invalid/duplicate keys are ignored "composer_control_order": [], # user-defined composer footer control order; invalid/duplicate keys are ignored
"language": "en", # UI locale code; must match a key in static/i18n.js LOCALES # #7622 (round 3): language is intentionally absent from the defaults so
# `load_settings()` reports `None` for a fresh install. This lets the
# client distinguish "no preference" from an explicit saved choice,
# so a user who genuinely picked English (and has `language: "en"`
# persisted on disk) is no longer overridden by the browser hint on
# their first hydration. When a user picks a locale in the Settings
# modal the value is written here and the field is set explicitly.
"bot_name": os.getenv( "bot_name": os.getenv(
"HERMES_WEBUI_BOT_NAME", "Hermes" "HERMES_WEBUI_BOT_NAME", "Hermes"
), # display name for the assistant ), # display name for the assistant
@@ -11723,6 +11729,16 @@ def load_settings() -> dict:
settings["default_model_provider"] = str(model_cfg.get("provider")) settings["default_model_provider"] = str(model_cfg.get("provider"))
except Exception: except Exception:
logger.debug("Failed to resolve default model provider for settings") logger.debug("Failed to resolve default model provider for settings")
# #7622/#7730 (round 4): keep the tri-state signal explicit. `language`
# is intentionally absent from `_SETTINGS_DEFAULTS` (see above), so
# without this line a fresh install's returned dict would OMIT the key
# entirely and the API payload would carry no `language` field. Emit an
# explicit `None` (serialized as JSON `null`) so the client receives the
# three-way signal it trusts: `null` = no preference, "en" = explicitly
# saved English, any other code = explicitly chosen locale. Stored
# values (including a legacy English pick written before this change)
# win via the merge above and are never touched here.
settings.setdefault("language", None)
return settings return settings
@@ -11730,6 +11746,15 @@ _SETTINGS_ALLOWED_KEYS = set(_SETTINGS_DEFAULTS.keys()) - {
"password_hash", "password_hash",
"default_model", "default_model",
"simplified_tool_calling", "simplified_tool_calling",
} | {
# #7622 (round 3): `language` is intentionally absent from
# `_SETTINGS_DEFAULTS` so a fresh install returns `None` for
# `settings["language"]` and the client can distinguish "no
# preference" from an explicit saved choice. But the user
# MUST still be able to pick a locale in the Settings modal,
# so we add it back to the explicit allow-list here. The
# existing BCP-47 validation at save-time still applies.
"language",
} }
_SETTINGS_ENUM_VALUES = { _SETTINGS_ENUM_VALUES = {
"send_key": {"enter", "ctrl+enter", "shift+enter"}, "send_key": {"enter", "ctrl+enter", "shift+enter"},
+35 -1
View File
@@ -4788,7 +4788,41 @@ class _FullSessionResolveRequired(RuntimeError):
"""Internal signal that a full sidecar load must enter the bounded path.""" """Internal signal that a full sidecar load must enter the bounded path."""
_FULL_SESSION_RESOLVE_MAX_CONCURRENT = 2 def _read_max_session_resolve_concurrent() -> int:
"""#7421: env override for the heavy session-resolve cap. The
hardcoded literal 2 is too low for high-concurrency
deployments where several parallel active sessions all need
a full-transcript resolve; operators can set
``HERMES_WEBUI_MAX_SESSION_RESOLVE`` to a positive int to
raise the cap without code changes. A bad or missing value
falls back to the previous default of 2, so an operator who
sets ``HERMES_WEBUI_MAX_SESSION_RESOLVE=0`` or a non-numeric
value does not regress to a blocked or unbounded state.
The cap is a *safety bound* (per-resolve cost tracked in
#7310), not a user preference — a typo like ``=999999``
must fall back to the safe default, never to the permissive
extreme. Every other bad input in this function falls back
to ``2``; out-of-range values do too. The previously
accepted upper bound (64) is reachable only via an explicit
in-range operator value; a profile's ``.env`` cannot reach
it at all (see ``_PROTECTED_ENV_KEYS``).
"""
raw = (os.getenv("HERMES_WEBUI_MAX_SESSION_RESOLVE") or "").strip()
if not raw:
return 2
try:
n = int(raw)
except (TypeError, ValueError):
return 2
if n < 1:
return 2
if n > 64:
return 2 # #7656 round-3: typo-safe fallback, not clamp-up-to-64
return n
_FULL_SESSION_RESOLVE_MAX_CONCURRENT = _read_max_session_resolve_concurrent()
_FULL_SESSION_RESOLVE_SLOTS = threading.BoundedSemaphore( _FULL_SESSION_RESOLVE_SLOTS = threading.BoundedSemaphore(
_FULL_SESSION_RESOLVE_MAX_CONCURRENT _FULL_SESSION_RESOLVE_MAX_CONCURRENT
) )
+14
View File
@@ -202,6 +202,14 @@ _PROTECTED_ENV_KEYS = frozenset({
'HERMES_WEBUI_ISOLATED_PROFILE', 'HERMES_WEBUI_ISOLATED_PROFILE',
# Server-wide sidebar window; a profile .env must not widen the per-request query. # Server-wide sidebar window; a profile .env must not widen the per-request query.
'HERMES_WEBUI_VISIBLE_SESSION_LIMIT', 'HERMES_WEBUI_VISIBLE_SESSION_LIMIT',
# #7656 round-3: HERMES_WEBUI_MAX_SESSION_RESOLVE is a process-wide resource
# cap (controls the BoundedSemaphore that gates full-transcript resolves).
# A profile's .env is per-profile by design; allowing it to set a
# server-wide limit (and a first-load-wins asymmetry) would let a
# single profile drag the rest into a tighter or looser posture than
# the operator intended. Same shape as the isolated-profile key: only
# the operator/launcher env at startup can set it.
'HERMES_WEBUI_MAX_SESSION_RESOLVE',
}) })
@@ -939,6 +947,12 @@ _BLOCKED_RUNTIME_ENV_KEYS = {
# profile's own env on any runtime/gateway-parity path. # profile's own env on any runtime/gateway-parity path.
'HERMES_WEBUI_ISOLATED_PROFILE', 'HERMES_WEBUI_ISOLATED_PROFILE',
'HERMES_WEBUI_VISIBLE_SESSION_LIMIT', 'HERMES_WEBUI_VISIBLE_SESSION_LIMIT',
# #7656 round-3: process-wide resolve cap. Filter out a profile's
# .env override the same way we filter the isolated-profile key, so
# the cap stays at the operator/launcher value across profile
# switches rather than re-sizing to whatever the first-loaded
# profile env set.
'HERMES_WEBUI_MAX_SESSION_RESOLVE',
} }
+126 -7
View File
@@ -21896,6 +21896,99 @@ def _handle_live_models(handler, parsed):
_env = str(_cp.get("key_env") or "").strip() _env = str(_cp.get("key_env") or "").strip()
return os.getenv(_env, "").strip() if _env else "" return os.getenv(_env, "").strip() if _env else ""
def _custom_provider_models_discover_is_false(_cp):
"""True when ``discover_models`` is an explicit ``false`` opt-out.
Mirrors ``api.config._provider_discover_allowed`` / Hermes
Agent ``model_switch_providers._discover_flag``: ``discover_models``
defaults to True, and the string forms ``"false"``/``"no"``/``"0"``
(case-insensitive) mean False. Used to decide whether a
dict-shaped ``models`` mapping is a hand-pinned allowlist (only
when discovery is off) rather than per-model metadata.
"""
_discover = _cp.get("discover_models", True) if isinstance(_cp, dict) else True
if isinstance(_discover, str):
return _discover.strip().lower() in {"false", "no", "0"}
return not bool(_discover)
def _custom_provider_allowlist_ids(_cp):
"""Plural ``models`` list only — the explicit allowlist signal.
The singular ``model`` field is sticky/default metadata, NOT an
allowlist: it must not gate live-catalog filtering, otherwise a
provider configured with only ``model: assistant`` (no ``models``
list) would be collapsed to a single model. Only an explicit
``models`` allowlist expresses "show exactly these models".
An auto-discovered catalog is NOT an allowlist: when Hermes
persisted discovery results back into config (``models: {...}``
plus ``models_discovered: true``), that mapping is a snapshot
of what the gateway exposed at discovery time. Gating on it
would permanently pin the live catalog to the first-discovery
set, silently dropping any model the user pulls in later
(LM Studio / Ollama). ``_provider_models_are_discovered_catalog()``
is the shared predicate the ``/api/models`` path already uses;
when it says "discovered", return no allowlist and let the
live probe win. An explicit ``discover_models: false`` opt-out
re-pins the catalog (the predicate accounts for it), so a
hand-pinned discovered catalog still filters.
A dict-shaped ``models`` mapping that is NOT marked discovered is
*per-model metadata* written by the Hermes Agent setup flow
(``hermes_cli/model_switch.py::_save_custom_provider`` and the
setup wizard) — e.g. ``{chat-a: {context_length: 128000}}``. It is
not a catalog narrow: treating its keys as an allowlist would
collapse the live picker to the single saved default (keyless
Ollama) while the CLI live-probe shows the full catalog. Only an
explicit ``discover_models: false`` opts into treating the dict
keys as a pinned allowlist. List/JSON-array-string/Python-literal
shapes remain plain allowlists.
The plural value is decoded through ``_parse_config_string_list()``
because ``hermes config set`` / JSON-mode editor saves persist
lists as quoted JSON-array strings (``'["chat-a","chat-b"]'``) or
Python literals (``"['chat-a']"``). Handling only ``dict``/``list``
made a serialized allowlist fall through to ``[]``, which the
caller reads as "no allowlist configured" and floods the picker
with the full upstream catalog — the same class of bug as the
``skills.disabled`` regression (#7120 / #7134).
"""
from api.config import _provider_models_are_discovered_catalog
if _provider_models_are_discovered_catalog(_cp):
return []
_ids = []
_models = _cp.get("models")
# Serialized shapes only: ``hermes config set`` / JSON-mode
# editor saves persist lists as quoted JSON-array strings
# (``'["chat-a","chat-b"]'``) or Python literals
# (``"['chat-a']"``). Decode those through the shared helper so
# they are honored; native dict/list values are walked below so
# dict *entries* keep their id|model|name metadata (the decoder
# str()-ifies list members, which would mangle them).
if isinstance(_models, str):
_models = _parse_config_string_list(_models)
if isinstance(_models, dict):
# A dict-shaped ``models`` is per-model metadata written by
# the Hermes Agent setup flow, NOT a catalog narrow. Only a
# ``discover_models: false`` opt-out treats the dict keys as a
# pinned allowlist; otherwise return the empty allowlist so the
# live probe returns the full catalog.
if not _custom_provider_models_discover_is_false(_cp):
return []
for _mid in _models:
if isinstance(_mid, str) and _mid.strip():
_ids.append(_mid.strip())
elif isinstance(_models, (list, tuple)):
for _item in _models:
if isinstance(_item, str) and _item.strip():
_ids.append(_item.strip())
elif isinstance(_item, dict):
_mid = _item.get("id") or _item.get("model") or _item.get("name")
if _mid and str(_mid).strip():
_ids.append(str(_mid).strip())
return _ids
# For 'custom' and 'custom:*' providers, provider_model_ids() # For 'custom' and 'custom:*' providers, provider_model_ids()
# returns [] because they aren't real hermes_cli endpoints. # returns [] because they aren't real hermes_cli endpoints.
# Fall back to the custom_providers entries from config.yaml so # Fall back to the custom_providers entries from config.yaml so
@@ -21904,11 +21997,13 @@ def _handle_live_models(handler, parsed):
# Collect config-specified model IDs separately so they don't # Collect config-specified model IDs separately so they don't
# prevent the live fetch below from running (#3718). # prevent the live fetch below from running (#3718).
_config_ids = [] _config_ids = []
_allowlist_ids = []
if provider == "custom" or provider.startswith("custom:"): if provider == "custom" or provider.startswith("custom:"):
for _cp in _custom_provider_entries_for_request(): for _cp in _custom_provider_entries_for_request():
if custom_provider_entry is None: if custom_provider_entry is None:
custom_provider_entry = _cp custom_provider_entry = _cp
_config_ids.extend(_custom_provider_model_ids(_cp)) _config_ids.extend(_custom_provider_model_ids(_cp))
_allowlist_ids.extend(_custom_provider_allowlist_ids(_cp))
# Always try live fetch for custom providers — config entries are a # Always try live fetch for custom providers — config entries are a
# fallback, not a replacement. The live endpoint should return ALL # fallback, not a replacement. The live endpoint should return ALL
@@ -21979,15 +22074,39 @@ def _handle_live_models(handler, parsed):
except Exception as _fetch_err: except Exception as _fetch_err:
logger.debug("Live fetch from custom provider failed: %s", _fetch_err) logger.debug("Live fetch from custom provider failed: %s", _fetch_err)
# If live fetch succeeded, merge with config entries (live takes # If live fetch succeeded, filter the live catalog down to the
# priority). If live fetch failed, fall back to config-only list. # config-declared models allowlist first, then append any
# allowlisted models the live endpoint didn't return. Custom
# providers (esp. New-API-style gateways) expose their ENTIRE
# catalog via /v1/models — including image/audio models that
# are not chat models. The picker must not surface models the
# user never declared in config.yaml. Only a NON-EMPTY explicit
# ``models`` allowlist gates the filter — a provider configured
# with just a singular ``model`` (no ``models`` list) keeps the
# unfiltered live catalog. When no allowlist is configured,
# return the live list as-is (preserves the pre-filter
# discovery behaviour).
#
# An empty allowlist (``models: []``, ``models: "[]"``, or a
# value that decodes to no usable ids) is deliberately treated
# as "not configured", NOT as "allow nothing". Gating on
# declared-ness instead would emit an EMPTY picker and make the
# provider unselectable — a harder failure than surfacing a few
# extra models, and unrecoverable from the UI because
# ``custom_providers`` is hand-edited in config.yaml (the WebUI
# has no write path for it). ``[]`` in practice means a
# leftover/placeholder key, not an intentional deny-all, and no
# deny-all use case exists: a provider the user wants hidden is
# removed from ``custom_providers`` outright.
if ids: if ids:
_live_set = set(ids) if _allowlist_ids:
for _cid in _config_ids: _allowlist_set = set(_allowlist_ids)
if _cid not in _live_set: ids = [m for m in ids if m in _allowlist_set] or []
ids.append(_cid) for _aid in _allowlist_ids:
if _aid not in ids:
ids.append(_aid)
else: else:
ids = list(_config_ids) ids = list(_allowlist_ids or _config_ids)
# ── OpenAI-compat live fetch fallback ────────────────────────────────── # ── OpenAI-compat live fetch fallback ──────────────────────────────────
# When provider_model_ids() is unavailable or returns [] for a provider # When provider_model_ids() is unavailable or returns [] for a provider
+9
View File
@@ -89,6 +89,15 @@ contributor guidance; it does not change runtime behavior or CI gates.
`models_cache.json` refresh timestamps), the stat-identity fallbacks, and the `models_cache.json` refresh timestamps), the stat-identity fallbacks, and the
schema/version stamps. Start here before changing model-catalog caching, the schema/version stamps. Start here before changing model-catalog caching, the
`/api/models` cache keys, or the Codex catalog dependency (#2443, #7540). `/api/models` cache keys, or the Codex catalog dependency (#2443, #7540).
- [`docs/architecture/live-models-allowlist.md`](architecture/live-models-allowlist.md):
current contract for how `/api/models/live` filters a custom provider's
upstream catalog: the four signals in evaluation order (discovered catalog
defers to the live probe, explicit plural `models:` allowlist filters,
singular `model:` never gates, no allowlist shows the full catalog), the
serialized-list shapes `hermes config set` persists, the deliberate
empty-allowlist-is-not-configured rule, and the probe-failure fallback.
Start here before changing custom-provider model filtering or
discovery-vs-allowlist semantics (#7165, #7404).
- [`docs/architecture/profile-home-resolve-cache.md`](architecture/profile-home-resolve-cache.md): - [`docs/architecture/profile-home-resolve-cache.md`](architecture/profile-home-resolve-cache.md):
current contract for the call-scoped memoization around current contract for the call-scoped memoization around
`_resolve_profile_home_param()` in `api/workspace.py`: what it caches, the `_resolve_profile_home_param()` in `api/workspace.py`: what it caches, the
+123
View File
@@ -0,0 +1,123 @@
# Live model picker allowlist contract (`/api/models/live`)
This document records the runtime contract for how `/api/models/live` decides
which models a `custom` / `custom:*` provider contributes to the chat model
picker. It describes shipped behavior and changes no runtime behavior. It was
added because the #7165 review flagged the contract as undocumented: the
filtering rules live in `_handle_live_models` (`api/routes.py`) and had only
code comments backing them.
Start here before changing how the live catalog of a custom provider is
filtered, how discovery snapshots are distinguished from allowlists, or what
the endpoint returns when the upstream probe fails.
## Why the endpoint filters at all
OpenAI-compatible gateways (New-API-style aggregators, LM Studio, Ollama
proxies) expose their **entire** catalog from `/v1/models`, including
image-generation, audio, TTS, and embedding models that are not chat models.
The background enrichment path must not surface models the user never opted
into — but it also must not hide models the gateway gained since config was
last edited. The contract below is how those two goals are reconciled.
## The four signals, in evaluation order
For a custom provider the handler consults these signals:
1. **Discovered catalog → no allowlist (live probe wins).**
When the provider entry carries `models_discovered: true` and discovery is
allowed (`discover_models` is not an explicit opt-out), the `models:`
mapping is a *snapshot Hermes persisted from a past discovery*, not a
hand-curated list. It must not gate the live probe: a model the user pulls
into LM Studio / Ollama after discovery has to appear in the picker.
Detected by the shared predicate
`api.config._provider_models_are_discovered_catalog()` — the same
predicate the `/api/models` catalog path uses, so both endpoints agree.
With this signal, no allowlist is derived and the full live catalog is
returned.
2. **Explicit plural `models:` allowlist → filter.**
A non-empty plural `models:` value on an entry that is *not* a discovered
catalog expresses "show exactly these models". The live catalog is
intersected with it, then any allowlisted model missing from the live
response is appended (so an allowlisted-but-offline model stays
selectable). This is the #7165 filter. Only **list** and **string**
(serialized) shapes are plain allowlists — see "Dict-shaped `models:`"
below.
3. **Singular `model:` → never gates.**
The singular field is sticky/default metadata (the model to preselect),
not an allowlist. A provider configured only with `model: assistant` keeps
the full unfiltered live catalog; gating on it would collapse discovery to
a single model.
4. **No allowlist → full live catalog.**
Preserves the pre-#7165 discovery behavior for setups that rely on it.
### Serialized shapes
`hermes config set` and the JSON-mode editor persist lists as quoted
JSON-array strings (`'["chat-a","chat-b"]'`) or Python literals
(`"['chat-a']"`). The plural value is decoded through the shared
`_parse_config_string_list()` before id extraction — but only when the value
is a `str`; native `dict` / `list` values are walked directly so dict *entries*
keep their `id|model|name` metadata (the decoder stringifies members). A
dict-only parser drops serialized allowlists to "no allowlist" and floods the
picker — the same bug class as `skills.disabled` (#7120 / #7134).
### Dict-shaped `models:`
A dict-shaped `models:` mapping (e.g. `{chat-a: {context_length: 128000}}`)
is *per-model metadata* written by the Hermes Agent setup flow —
`hermes_cli/model_switch.py::_save_custom_provider` and the setup wizard —
**not** a catalog narrow. Treating its keys as an allowlist would collapse the
live picker to the single saved default (keyless Ollama) while the CLI
live-probe shows the full catalog. So a dict is **never** a plain allowlist:
- without an explicit `discover_models: false`, a dict-shaped `models:`
contributes **no allowlist** and the full live catalog is returned;
- with `discover_models: false` (bool `False` or the string forms
`"false"`/`"no"`/`"0"`, case-insensitive — mirroring
`model_switch_providers.py:557`), the dict **keys** are honored as a pinned
allowlist.
This complements the discovered-catalog rule: `models_discovered: true`
without the opt-out is already ignored (signal 1). The dict guard covers the
Agent-setup shape that is *not* flagged discovered.
If someone later "fixes" the dict shape back into an always-allowlist, the
regression test `test_dict_models_without_discover_false_does_not_gate` fails
and forces the trade-off to be re-argued.
### Empty allowlist semantics
`models: []`, `models: "[]"`, an empty mapping, or a blank scalar are
deliberately treated as **not configured**, never as "allow nothing". The
filter runs only on a non-empty allowlist because gating on declared-ness
would emit an empty picker and make the provider unselectable — a harder
failure than showing a few extra models, and unrecoverable from the UI
(`custom_providers` has no WebUI write path; it is hand-edited in
config.yaml). There is no deny-all use case: a provider the user wants hidden
is removed from `custom_providers` outright.
### Probe failure / empty probe
When the upstream probe fails or returns no models, the endpoint falls back
to `allowlist or config ids` — the non-empty plural allowlist first, so a
singular `model:` outside the allowlist cannot leak in precisely when the
probe is unavailable; with no allowlist, the configured ids (singular +
plural) are served. A discovered catalog therefore still serves its saved
models on a transient probe failure instead of emptying the picker.
## Tests
- `tests/test_issue3718_live_models_custom_probe.py` — real-handler
(`_handle_live_models`) coverage of every signal above: plural list /
JSON-array-string / Python-literal / list-of-dicts filtering, dict-shaped
`models:` (not gated) and dict + `discover_models: false` (pinned),
singular passthrough, malformed safety, discovered-catalog passthrough,
`discover_models: false` pinning, and both fallback branches.
- `tests/test_issue7404_models_discovered_not_allowlist.py` — the
`/api/models` side of the discovered-catalog contract.
- `tests/test_byok_model_dropdown.py` — picker rendering, profile-scoped
caching, and current-session model preservation around this endpoint.
+10 -1
View File
@@ -3431,8 +3431,17 @@ window._mirrorSpeechSettingsFromServer=_mirrorSpeechSettingsFromServer;
localStorage.setItem('hermes-font-size',fontSize); localStorage.setItem('hermes-font-size',fontSize);
_applyFontSize(fontSize); _applyFontSize(fontSize);
if(typeof setLocale==='function'){ if(typeof setLocale==='function'){
// #7622 (round 3): the settings payload's `s.language` is
// absent (None) for a fresh install, so an explicit non-empty
// value is the user's genuine saved choice. The browser
// navigator hint is now read via the guarded
// `_detectBrowserLanguageHint()` helper (round-3 finding 2) so
// a throwing `navigator` accessor in some embedded webviews
// can no longer abort this branch and reset loaded preferences.
// The fallback ternary preserves the pre-#7622 boot behaviour
// when neither helper is in scope (defence in depth).
const _lang=typeof resolvePreferredLocale==='function' const _lang=typeof resolvePreferredLocale==='function'
? resolvePreferredLocale(s.language, localStorage.getItem('hermes-lang')) ? resolvePreferredLocale(s.language, localStorage.getItem('hermes-lang'), _detectBrowserLanguageHint())
: (s.language || localStorage.getItem('hermes-lang') || 'en'); : (s.language || localStorage.getItem('hermes-lang') || 'en');
setLocale(_lang); setLocale(_lang);
if(typeof applyLocaleToDOM==='function')applyLocaleToDOM(); if(typeof applyLocaleToDOM==='function')applyLocaleToDOM();
+76 -8
View File
@@ -26983,15 +26983,68 @@ function resolveLocale(lang) {
/** /**
* Resolve locale with precedence: * Resolve locale with precedence:
* 1) primary (typically server setting) * 1) primary (typically server setting)
* 2) fallback (typically localStorage) * 2) fallback (typically localStorage)
* 3) English * 3) fallback2 (typically browser navigator hint — first-visit only)
* @param {string} primary * 4) 'en' (final fallback)
* @param {string} fallback *
* Resolve a language preference to a known LOCALES key, applying a strict
* `primary -> fallback -> fallback2 -> 'en'` precedence chain.
*
* #7622 (round 3): the server no longer injects `"language": "en"` as a
* schema default — `load_settings()` reports `None` (the key is absent
* from the response) until the user explicitly picks a locale in the
* Settings modal. This lets the resolver use `primary` directly
* without a `primary === 'en'` special case:
* - an explicit `primary` (the server's stored value) always wins
* - a `None` / empty `primary` falls through to `fallback` (localStorage)
* - `fallback2` (the browser navigator hint) only fires when neither
* server nor localStorage has a preference
* - `'en'` is the final safety net
*
* Side benefit: a user who picked English on purpose (their settings
* file contains `"language": "en"`) is no longer overridden by a
* non-English browser locale on hydration, because the round-2
* `primary === 'en'` skip has been removed.
*
* @param {string|null|undefined} primary server-stored language
* @param {string|null|undefined} fallback localStorage `hermes-lang`
* @param {string|null|undefined} fallback2 browser navigator hint
* @returns {string} * @returns {string}
*/ */
function resolvePreferredLocale(primary, fallback) { function resolvePreferredLocale(primary, fallback, fallback2) {
return resolveLocale(primary) || resolveLocale(fallback) || 'en'; return resolveLocale(primary)
|| resolveLocale(fallback)
|| resolveLocale(fallback2)
|| 'en';
}
/**
* Detect the browser's preferred language for first-visit locale
* selection. Guarded against `navigator` access exceptions: some
* embedded webviews (Tauri desktop shell, kiosk modes) report a
* `navigator` object but throw on `.languages` / `.language` reads.
* Callers (loadLocale, boot.js, panels.js) must use this helper rather
* than reading `navigator` directly so a single throw does not abort
* the boot or settings-panel hydration.
*
* @returns {string|null} the first entry of `navigator.languages`, or
* `navigator.language`, or `null` when neither is reachable.
*/
function _detectBrowserLanguageHint() {
try {
const nav = (typeof navigator !== 'undefined') ? navigator : null;
if (!nav) return null;
if (Array.isArray(nav.languages) && nav.languages.length) {
return nav.languages[0] || null;
}
if (typeof nav.language === 'string' && nav.language) {
return nav.language;
}
return null;
} catch (_) {
return null;
}
} }
/** /**
@@ -27029,11 +27082,26 @@ function setLocale(lang) {
/** /**
* Load locale from localStorage (called once at boot, before DOMContentLoaded). * Load locale from localStorage (called once at boot, before DOMContentLoaded).
* Server-persisted preference is applied later in loadSettingsPanel(). * Server-persisted preference is applied later in loadSettingsPanel().
*
* #7622: on the FIRST visit (no stored preference), fall back to the
* browser's preferred language (`navigator.languages[0]` /
* `navigator.language`) so non-English speakers don't have to dig into
* Settings after every fresh install. This is strictly a first-visit
* hint — the moment the user picks a language in Settings (or the
* server-side preference is applied via `loadSettingsPanel`), the
* stored value takes over and the browser hint is never consulted
* again for that browser profile.
*/ */
function loadLocale() { function loadLocale() {
let stored = null; let stored = null;
try { stored = localStorage.getItem('hermes-lang'); } catch (_) {} try { stored = localStorage.getItem('hermes-lang'); } catch (_) {}
setLocale(resolvePreferredLocale(null, stored)); // Only consult the browser hint when no stored preference exists.
// The settings panel and `loadSettingsPanel()` will overwrite this
// hint on their first call, so explicit user / server preferences
// always win. #7622 (round 3): the browser-hint read is delegated
// to `_detectBrowserLanguageHint()` so a `navigator` access throw
// (some embedded webviews) can no longer abort loadLocale().
setLocale(resolvePreferredLocale(stored, _detectBrowserLanguageHint()));
} }
/** /**
+10 -1
View File
@@ -9300,8 +9300,17 @@ async function loadSettingsPanel(){
_setHiddenTabs(hiddenTabs); _setHiddenTabs(hiddenTabs);
_applyTabVisibility(hiddenTabs); _applyTabVisibility(hiddenTabs);
_renderTabVisibilityChips(); _renderTabVisibilityChips();
// #7622 (round 3): the settings payload's `settings.language` is
// absent (None) for a fresh install, so an explicit non-empty
// value is the user's genuine saved choice. The browser
// navigator hint is now read via the guarded
// `_detectBrowserLanguageHint()` helper (round-3 finding 2) so
// a throwing `navigator` accessor can no longer abort settings
// hydration before model, provider, plugin and extension sections
// are populated. The fallback ternary preserves the pre-#7622
// settings-modal behaviour when neither helper is in scope.
const resolvedLanguage=(typeof resolvePreferredLocale==='function') const resolvedLanguage=(typeof resolvePreferredLocale==='function')
? resolvePreferredLocale(settings.language, localStorage.getItem('hermes-lang')) ? resolvePreferredLocale(settings.language, localStorage.getItem('hermes-lang'), _detectBrowserLanguageHint())
: (settings.language || localStorage.getItem('hermes-lang') || 'en'); : (settings.language || localStorage.getItem('hermes-lang') || 'en');
// Keep settings modal and current page strings in sync with the resolved locale. // Keep settings modal and current page strings in sync with the resolved locale.
if(typeof setLocale==='function'){ if(typeof setLocale==='function'){
+8 -1
View File
@@ -291,6 +291,13 @@ class TestLiveModelsCustomProviderFallback:
"model": "gpt-5.5", "model": "gpt-5.5",
"models": {"gpt-5.5-mini": {}}, "models": {"gpt-5.5-mini": {}},
"base_url": "https://right.codes/codex/v1", "base_url": "https://right.codes/codex/v1",
# dict-shaped ``models`` is per-model metadata (Agent setup
# flow) and only narrows the live catalog when discovery is
# explicitly off — the new #7165 contract. This test still
# wants provider-scoping (no sibling leak), so pin the
# catalog via ``discover_models: false`` instead of relying
# on the removed dict-as-allowlist behavior.
"discover_models": False,
}, },
{ {
"name": "infini-ai", "name": "infini-ai",
@@ -308,7 +315,7 @@ class TestLiveModelsCustomProviderFallback:
resp = self._call_live_models(monkeypatch, cfg, "custom:rightcode-codex") resp = self._call_live_models(monkeypatch, cfg, "custom:rightcode-codex")
assert resp["provider"] == "custom:rightcode-codex" assert resp["provider"] == "custom:rightcode-codex"
assert [m["id"] for m in resp["models"]] == ["gpt-5.5", "gpt-5.5-mini"] assert [m["id"] for m in resp["models"]] == ["gpt-5.5-mini"]
def test_bare_custom_fallback_ignores_named_custom_provider_models(self, monkeypatch): def test_bare_custom_fallback_ignores_named_custom_provider_models(self, monkeypatch):
"""Bare custom only represents unnamed custom entries, not named siblings.""" """Bare custom only represents unnamed custom entries, not named siblings."""
+390 -185
View File
@@ -1,193 +1,398 @@
"""Regression test for #3718 -- /api/models/live skips probe for custom providers. """Handler-level regression tests for /api/models/live custom-provider probing.
When a custom provider (``custom_providers`` entry in config.yaml) has a These tests invoke the real ``routes._handle_live_models`` and inspect the
``model:`` field but no ``models:`` allowlist, the live endpoint previously emitted ``models`` payload. Earlier revisions of this file re-implemented the
populated ``ids`` from the config entry and then skipped the live ``/v1/models`` filtering loop locally, which meant they passed unchanged even with the
probe (guarded by ``if not ids``). The fix collects config-specified model IDs production filter deleted (a vacuous oracle). Every case here must fail if the
in a separate ``_config_ids`` list so the live fetch always runs, and merges allowlist gating in ``api/routes.py`` regresses.
config entries as a fallback after the fetch.
Covered shapes:
* native ``models`` list → live catalog filtered
* JSON-array-string ``models`` → live catalog filtered (#7120 class)
* Python-literal-string ``models`` → live catalog filtered
* singular ``model`` only, no ``models`` → live catalog NOT gated
* malformed serialized ``models`` → safe, treated as scalar name
* allowlisted model absent from live catalog → appended
* live probe failure → falls back to config ids
* dict-shaped ``models`` (Agent setup) → live catalog NOT gated (per-model
metadata, not an allowlist)
* dict-shaped ``models`` + ``discover_models:
false`` → pinned to dict keys (explicit opt-out)
* ``models_discovered: true`` catalog → live catalog NOT gated (discovery
metadata is not an allowlist)
* discovered + ``discover_models: false`` → still pinned (explicit opt-out)
""" """
import io
import json import json
import pathlib import sys
import unittest import types
from unittest import mock from urllib.parse import urlparse
REPO = pathlib.Path(__file__).parent.parent import pytest
ROUTES_PY = REPO / "api" / "routes.py"
class TestLiveModelsCustomProviderProbe(unittest.TestCase): def _install_provider_model_ids(monkeypatch, fn):
"""Live endpoint must probe the upstream /v1/models even when config has entries.""" """Stub hermes_cli.models.provider_model_ids (returns [] for custom:*)."""
hermes_cli = types.ModuleType("hermes_cli")
def test_custom_provider_with_model_field_probes_upstream(self): hermes_cli.__path__ = []
"""When a custom provider has model: in config, live fetch must still run. models = types.ModuleType("hermes_cli.models")
models.provider_model_ids = fn
Before the fix, _custom_provider_model_ids() populated ids=["assistant"], monkeypatch.setitem(sys.modules, "hermes_cli", hermes_cli)
and the `if not ids` guard prevented the upstream /v1/models probe from monkeypatch.setitem(sys.modules, "hermes_cli.models", models)
running. The endpoint returned only ["assistant"] instead of the full
upstream catalog.
We test the fix by simulating the logic directly: config provides
model IDs, the upstream probe returns a different set, and the result
must contain BOTH the live models and the config fallback.
"""
# Simulate config-specified model IDs
_config_ids = ["assistant"]
# Simulate upstream /v1/models response
live_models = [
"assistant",
"assistant-pro",
"assistant-zdr",
"gpt-5.5:pt",
"claude-sonnet-4.6:pt",
]
# Simulate the merge logic from the fix:
# if ids: merge config entries not in live set
# else: fall back to config-only list
ids = list(live_models) # live fetch succeeded
_live_set = set(ids)
for _cid in _config_ids:
if _cid not in _live_set:
ids.append(_cid)
# "assistant" from config should not duplicate the live result
self.assertEqual(ids, live_models)
# All live models must be present
for m in live_models:
self.assertIn(m, ids)
def test_custom_provider_falls_back_to_config_when_probe_fails(self):
"""When the upstream probe fails, config entries must be used as fallback."""
_config_ids = ["assistant", "custom-only-model"]
# Simulate failed live fetch: ids stays empty
ids = []
# Fallback logic from the fix
if not ids:
ids = list(_config_ids)
self.assertEqual(ids, ["assistant", "custom-only-model"])
def test_config_only_model_appended_when_not_in_live_results(self):
"""Config-specified models not in the live results must be appended."""
_config_ids = ["assistant", "local-finetune"]
live_models = ["assistant", "assistant-pro", "gpt-5.5:pt"]
ids = list(live_models)
_live_set = set(ids)
for _cid in _config_ids:
if _cid not in _live_set:
ids.append(_cid)
# "assistant" is in both, "local-finetune" is config-only
self.assertIn("local-finetune", ids)
self.assertIn("assistant-pro", ids)
self.assertEqual(ids.count("assistant"), 1, "assistant must not be duplicated")
def test_live_fetch_guard_no_longer_checks_ids(self):
"""The live fetch block must not be guarded by `if not ids`.
This is a structural regression guard: the fix replaced
`if not ids and (provider == "custom" ...)` with
`if provider == "custom" ...` so the probe always runs regardless
of whether config entries populated ids.
"""
source = ROUTES_PY.read_text(encoding="utf-8")
# Find the "Always try live fetch" comment (added by the fix)
marker = "Always try live fetch for custom providers"
self.assertIn(marker, source, (
"routes.py must contain the 'Always try live fetch' comment (#3718)"
))
# Extract the block between the marker and the next major section
marker_pos = source.find(marker)
next_section = source.find("OpenAI-compat live fetch fallback", marker_pos)
self.assertNotEqual(next_section, -1, "could not find next section marker")
block = source[marker_pos:next_section]
# The old `if not ids and` guard must NOT appear in this block
self.assertNotIn("if not ids and", block, (
"Live fetch for custom providers must not be guarded by 'if not ids' (#3718)"
))
def test_mocked_live_fetch_returns_full_catalog_plus_config_entry(self):
"""Integration test: mock urlopen and exercise the real fetch/parse/merge path.
This test patches urllib.request.urlopen to return a fake /v1/models
response, then runs the same fetch+merge logic that the live handler
uses, verifying that config entries are merged and live models are
included.
"""
fake_models = {"data": [
{"id": "assistant"},
{"id": "assistant-pro"},
{"id": "gpt-5.5:pt"},
]}
fake_body = json.dumps(fake_models).encode("utf-8")
fake_resp = mock.MagicMock()
fake_resp.read.return_value = fake_body
fake_resp.__enter__ = mock.MagicMock(return_value=fake_resp)
fake_resp.__exit__ = mock.MagicMock(return_value=False)
_config_ids = ["assistant", "local-only"]
with mock.patch("urllib.request.urlopen", return_value=fake_resp):
# Replicate the fetch+parse logic from routes.py
import urllib.request
ids = []
_req = urllib.request.Request(
"http://localhost:4000/v1/models",
headers={"Authorization": "Bearer test-key"},
)
with urllib.request.urlopen(_req, timeout=5) as _resp:
_body = json.loads(_resp.read())
if isinstance(_body, dict):
_data = _body.get("data", [])
if isinstance(_data, list):
ids = [m.get("id", "") for m in _data if m.get("id")]
# Apply the merge logic from the fix
if ids:
_live_set = set(ids)
for _cid in _config_ids:
if _cid not in _live_set:
ids.append(_cid)
else:
ids = list(_config_ids)
# Live models must all be present
self.assertIn("assistant", ids)
self.assertIn("assistant-pro", ids)
self.assertIn("gpt-5.5:pt", ids)
# Config-only entry must be appended
self.assertIn("local-only", ids)
# No duplicates
self.assertEqual(ids.count("assistant"), 1)
def test_timeout_uses_config_constant(self):
"""The live fetch must use CUSTOM_MODELS_ENDPOINT_TIMEOUT_SECONDS, not a hardcoded value."""
source = ROUTES_PY.read_text(encoding="utf-8")
# Find the custom-provider live fetch block
marker = "Always try live fetch for custom providers"
marker_pos = source.find(marker)
next_section = source.find("OpenAI-compat live fetch fallback", marker_pos)
block = source[marker_pos:next_section]
# Must use the constant, not a bare number
self.assertIn("CUSTOM_MODELS_ENDPOINT_TIMEOUT_SECONDS", block, (
"Live fetch timeout must use CUSTOM_MODELS_ENDPOINT_TIMEOUT_SECONDS "
"instead of a hardcoded value (#3718 review feedback)"
))
self.assertNotIn("timeout=8", block, (
"Live fetch must not use hardcoded timeout=8 (#3718 review feedback)"
))
if __name__ == "__main__": class _FakeResponse(io.BytesIO):
unittest.main() def __enter__(self):
return self
def __exit__(self, *_exc):
self.close()
return False
def _install_live_catalog(monkeypatch, routes, catalog, *, fail=False):
"""Stub the upstream /v1/models probe used by the custom-provider branch."""
import urllib.request
requested: list[str] = []
def _fake_urlopen(req, timeout=None):
requested.append(getattr(req, "full_url", str(req)))
if fail:
raise OSError("probe failed")
body = {"data": [{"id": mid} for mid in catalog]}
return _FakeResponse(json.dumps(body).encode())
monkeypatch.setattr(urllib.request, "urlopen", _fake_urlopen)
return requested
def _run_live_models(monkeypatch, routes, custom_provider, *, catalog, fail=False):
"""Invoke the real handler for provider=custom:test-gateway."""
import api.config as config
import api.profiles as profiles
routes._clear_live_models_cache()
monkeypatch.setattr(
routes, "j", lambda _handler, payload, status=200, extra_headers=None: payload
)
monkeypatch.setattr(config, "_resolve_provider_alias", lambda provider: provider)
monkeypatch.setattr(profiles, "get_active_profile_name", lambda: "default")
config_data = {
"model": {"provider": "custom:test-gateway"},
"custom_providers": [custom_provider],
}
monkeypatch.setattr(config, "get_config", lambda: config_data)
monkeypatch.setattr(routes, "cfg", config_data, raising=False)
# provider_model_ids() returns [] for custom:* → drives the custom branch.
_install_provider_model_ids(monkeypatch, lambda provider: [])
requested = _install_live_catalog(monkeypatch, routes, catalog, fail=fail)
parsed = urlparse("/api/models/live?provider=custom:test-gateway")
payload = routes._handle_live_models(object(), parsed)
return payload, requested
def _ids(payload):
return [m["id"] for m in payload.get("models", [])]
_LIVE_CATALOG = [
"chat-a",
"chat-b",
"image-gen-1", # non-chat noise the picker must not surface
"audio-tts-1",
]
_BASE_PROVIDER = {
"name": "Test Gateway",
"base_url": "https://gateway.example/v1",
"api_key": "sk-test",
}
@pytest.mark.parametrize(
"models_value",
[
pytest.param(["chat-a", "chat-b"], id="native-list"),
pytest.param('["chat-a","chat-b"]', id="json-array-string"),
pytest.param("['chat-a', 'chat-b']", id="python-literal-string"),
pytest.param(
[{"id": "chat-a"}, {"model": "chat-b"}], id="list-of-dicts"
),
],
)
def test_allowlist_filters_live_catalog(monkeypatch, models_value):
"""An explicit plural allowlist must trim the live catalog to itself.
The JSON-array-string and Python-literal cases are the regression: those
shapes are what ``hermes config set`` / JSON-mode editor saves persist, and
a dict/list-only parser drops them to ``[]`` → read as "no allowlist" → the
full upstream catalog floods the picker.
"""
import api.routes as routes
provider = dict(_BASE_PROVIDER, models=models_value)
payload, requested = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG
)
assert requested, "live /v1/models probe was never attempted"
assert _ids(payload) == ["chat-a", "chat-b"]
assert "image-gen-1" not in _ids(payload)
assert "audio-tts-1" not in _ids(payload)
def test_dict_models_without_discover_false_does_not_gate(monkeypatch):
"""A dict-shaped ``models`` is per-model metadata — NOT an allowlist (#7165).
Hermes Agent's setup flow and ``hermes_cli/model_switch.py::_save_custom_provider``
persist per-model metadata as a mapping, e.g. ``{chat-a: {context_length: 128000}}``.
Treating its keys as an allowlist would collapse the live picker to the saved
default while the CLI live-probe shows the full catalog. Without an explicit
``discover_models: false`` opt-out the dict must NOT gate — full live catalog wins.
"""
import api.routes as routes
provider = dict(
_BASE_PROVIDER,
models={"chat-a": {"context_length": 128000}, "chat-b": {"context_length": 32000}},
)
payload, requested = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG
)
assert requested, "live /v1/models probe was never attempted"
assert _ids(payload) == _LIVE_CATALOG, (
"dict-shaped models is per-model metadata, not an allowlist — the "
"full live catalog must be surfaced"
)
def test_dict_models_with_discover_false_pins_to_keys(monkeypatch):
"""``discover_models: false`` opts a dict-shaped ``models`` into an allowlist.
The explicit opt-out (bool ``False`` or the string forms ``"false"``/``"no"``/``"0"``)
means the dict keys are a hand-pinned allowlist and must filter the live catalog.
"""
import api.routes as routes
for discover in (False, "false", "no", "0"):
provider = dict(
_BASE_PROVIDER,
models={"chat-a": {"context_length": 128000}, "chat-b": {"context_length": 32000}},
discover_models=discover,
)
payload, requested = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG
)
assert requested, "live /v1/models probe was never attempted"
assert _ids(payload) == ["chat-a", "chat-b"], (
f"discover_models={discover!r} must pin the dict keys as the allowlist"
)
assert "image-gen-1" not in _ids(payload)
assert "audio-tts-1" not in _ids(payload)
def test_singular_model_only_does_not_gate_live_catalog(monkeypatch):
"""``model:`` is default metadata, not an allowlist — keep full discovery."""
import api.routes as routes
provider = dict(_BASE_PROVIDER, model="chat-a")
payload, _ = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG
)
assert _ids(payload) == _LIVE_CATALOG
def test_malformed_serialized_models_is_safe(monkeypatch):
"""A broken JSON-array string must not crash or silently drop the gate.
``_parse_config_string_list()`` treats an undecodable value as one literal
name, so the allowlist stays non-empty and the filter still runs.
"""
import api.routes as routes
provider = dict(_BASE_PROVIDER, models='["chat-a", "chat-b"') # missing ]
payload, _ = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG
)
# Treated as a single (unmatched) name → appended, catalog not flooded.
assert "image-gen-1" not in _ids(payload)
assert "audio-tts-1" not in _ids(payload)
def test_allowlisted_model_missing_from_live_is_appended(monkeypatch):
"""A declared model the upstream didn't return must still be selectable."""
import api.routes as routes
provider = dict(_BASE_PROVIDER, models=["chat-a", "chat-offline"])
payload, _ = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG
)
assert _ids(payload) == ["chat-a", "chat-offline"]
def test_contradictory_singular_model_outside_plural_allowlist_is_ignored(monkeypatch):
"""A singular default outside the plural allowlist must not leak into the picker.
Paired test: whether the live probe succeeds or fails, only the explicit
plural allowlist is exposed.
"""
import api.routes as routes
provider = dict(_BASE_PROVIDER, model="chat-default", models=["chat-a"])
# 1. Successful live fetch -> only plural allowlist
payload_success, _ = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG, fail=False
)
assert _ids(payload_success) == ["chat-a"]
# 2. Failed live fetch -> still only plural allowlist (not chat-default)
payload_fail, _ = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG, fail=True
)
assert _ids(payload_fail) == ["chat-a"]
def test_probe_failure_without_allowlist_falls_back_to_singular_model(monkeypatch):
"""When no plural allowlist is configured and probe fails, singular model is used."""
import api.routes as routes
provider = dict(_BASE_PROVIDER, model="chat-default")
payload, _ = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG, fail=True
)
assert _ids(payload) == ["chat-default"]
@pytest.mark.parametrize(
"models_value",
[
pytest.param([], id="native-empty-list"),
pytest.param("[]", id="serialized-empty-list"),
pytest.param({}, id="empty-mapping"),
pytest.param(" ", id="blank-scalar"),
],
)
def test_empty_allowlist_is_treated_as_not_configured(monkeypatch, models_value):
"""An empty ``models`` value must NOT be read as "allow nothing".
Pins a deliberate semantic decision. Gating the filter on *declared-ness*
rather than on a non-empty allowlist would make ``models: []`` emit an
EMPTY picker, leaving the provider unselectable — a harder failure than
surfacing a few extra models, and unrecoverable from the UI because
``custom_providers`` has no WebUI write path (hand-edited config.yaml only).
There is also no deny-all use case: a provider the user wants hidden is
removed from ``custom_providers`` outright.
If someone later "fixes" the empty case into a deny-all gate, this test
fails and forces the trade-off to be re-argued rather than silently shipped.
"""
import api.routes as routes
provider = dict(_BASE_PROVIDER, models=models_value)
payload, _ = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG
)
assert _ids(payload) == _LIVE_CATALOG, (
"empty allowlist must fall through to full live discovery, "
"never collapse the picker to zero models"
)
assert _ids(payload), "picker must never be emptied by an empty allowlist"
def test_models_discovered_catalog_is_not_an_allowlist(monkeypatch):
"""``models_discovered: true`` marks ``models:`` as discovery metadata (#7165).
Hermes persists *discovery results* back into config as ``models: {...}``
plus ``models_discovered: true``. That mapping is a snapshot of what the
gateway exposed at discovery time — NOT a hand-curated allowlist. Treating
it as one permanently pins the live catalog to the first-discovery set, so
a model the user later pulls into LM Studio / Ollama is silently dropped
from ``/api/models/live`` (the exact regression the gate reproduced: merge
base returned ``saved-a``, ``saved-b`` AND ``new-live-c``; the pinned head
dropped ``new-live-c``).
"""
import api.routes as routes
provider = dict(
_BASE_PROVIDER,
models={
"saved-a": {"context_length": 128000},
"saved-b": {"context_length": 32000},
},
models_discovered=True,
)
payload, requested = _run_live_models(
monkeypatch,
routes,
provider,
catalog=["saved-a", "saved-b", "new-live-c"],
)
assert requested, "live /v1/models probe was never attempted"
assert _ids(payload) == ["saved-a", "saved-b", "new-live-c"], (
"a discovered catalog must not gate the live probe — models that "
"appear upstream after discovery (new-live-c) must reach the picker"
)
def test_discovered_catalog_with_discover_models_false_still_pins(monkeypatch):
"""``discover_models: false`` re-pins a discovered catalog (control).
The explicit opt-out wins over ``models_discovered: true`` — same rule as
the ``/api/models`` catalog path (`api.config._provider_discover_allowed`).
Filtering to the configured set must survive the discovered-catalog fix.
"""
import api.routes as routes
provider = dict(
_BASE_PROVIDER,
models={
"saved-a": {"context_length": 128000},
"saved-b": {"context_length": 32000},
},
models_discovered=True,
discover_models=False,
)
payload, _ = _run_live_models(
monkeypatch,
routes,
provider,
catalog=["saved-a", "saved-b", "new-live-c"],
)
assert _ids(payload) == ["saved-a", "saved-b"]
def test_discovered_catalog_probe_failure_falls_back_to_saved_models(monkeypatch):
"""Discovered + failed probe → saved models, not an empty picker (control).
Releasing the allowlist must not empty the fallback: when the live probe
fails, the previously discovered ``models:`` mapping is still the best
answer and must be served from ``_config_ids``.
"""
import api.routes as routes
provider = dict(
_BASE_PROVIDER,
models={
"saved-a": {"context_length": 128000},
"saved-b": {"context_length": 32000},
},
models_discovered=True,
)
payload, _ = _run_live_models(
monkeypatch, routes, provider, catalog=[], fail=True
)
assert _ids(payload) == ["saved-a", "saved-b"]
+297
View File
@@ -0,0 +1,297 @@
"""Regression coverage for issue #7421: configurable
``HERMES_WEBUI_MAX_SESSION_RESOLVE`` cap on the heavy
full-transcript resolve semaphore.
The cap is hardcoded to ``2`` in
``api.models.py::_FULL_SESSION_RESOLVE_MAX_CONCURRENT``. The
maintainer's diagnosis is that the literal 2 is too low for
high-concurrency deployments where several parallel active
sessions all need a full-transcript resolve, and that the
deeper per-read cost is tracked separately in #7310. This PR
is the self-contained first step: read the cap from the env
var so operators can raise it without code changes, while
preserving the previous default of 2 and bounding the upper
end against typos like ``=999999``.
#7656 re-gate (9/22 23:18): the previous round used a private
``_import_models_with_env`` helper that did ``sys.modules.pop``
on ``api.models`` and re-imported to recompute the module-level
cap constant. Modules already imported (``api.routes``,
``api.streaming``, ...) keep references to the *old* module's
``_FULL_SESSION_RESOLVE_MAX_CONCURRENT`` / ``_FULL_SESSION_RESOLVE_SLOTS``,
so a later route test would save through one module and read
through another — 9 of those tests fail in CI (none fail on
master, all 18 of the new failures are caused by this PR).
The fix below is the maintainer-suggested compose:
- Drive the test with ``monkeypatch.setenv`` so the env var is
restored at fixture teardown — no leak to other test files
- Test ``_read_max_session_resolve_concurrent()`` directly
(the function is already pure: only reads ``os.getenv``) so
no module reload is needed
- For the single test that needs the module-level constant
(``_FULL_SESSION_RESOLVE_MAX_CONCURRENT``), verify it equals
the helper's return value at import time — that is the real
production contract, and it is cheap to assert because the
helper itself only reads the env var
"""
from __future__ import annotations
import threading
from pathlib import Path
import pytest
import api.models as models
ROOT = Path(__file__).resolve().parents[1]
@pytest.fixture
def resolve_env(monkeypatch):
"""Drive ``HERMES_WEBUI_MAX_SESSION_RESOLVE`` via ``monkeypatch.setenv``
so the env var is restored at fixture teardown — no leak to other
test files, no module reload required.
The helper ``models._read_max_session_resolve_concurrent()`` is pure
(only ``os.getenv``) and reads the live env, so this fixture is the
entire test harness. The previous round's ``_import_models_with_env``
used ``sys.modules.pop`` + ``importlib.import_module`` to recompute
the module-level cap constant against a freshly-set env, but that
left the original module unreferenced in ``sys.modules`` while
``api.routes`` and friends still held the *old* module object —
root cause of the cross-suite pollution the maintainer reproduced.
"""
def set_resolve_env(value):
if value is None:
monkeypatch.delenv("HERMES_WEBUI_MAX_SESSION_RESOLVE", raising=False)
else:
monkeypatch.setenv("HERMES_WEBUI_MAX_SESSION_RESOLVE", str(value))
return set_resolve_env
# ── default behavior (env unset) ──────────────────────────────────────────────
def test_default_cap_is_two_when_env_unset(resolve_env):
"""Backward compatibility: an installation that does not set
the env var keeps the previous behavior. The bounded
semaphore is constructed with capacity 2, so 3+ parallel
full-resolves queue behind two slots as before."""
resolve_env(None)
assert models._read_max_session_resolve_concurrent() == 2, (
"unset env must keep the previous hardcoded default of 2 "
"so existing single-instance deployments are unchanged"
)
def test_default_cap_is_two_when_env_empty_string(resolve_env):
"""A user who sets the env var to an explicit empty string
(e.g. from a misconfigured .env file) is the same as unset.
The helper strips whitespace first; ``""`` is treated as no
value at all."""
resolve_env("")
assert models._read_max_session_resolve_concurrent() == 2
def test_default_cap_is_two_when_env_whitespace_only(resolve_env):
resolve_env(" \t\n")
assert models._read_max_session_resolve_concurrent() == 2
# ── in-range override ─────────────────────────────────────────────────────────
def test_env_override_takes_effect_within_upper_bound(resolve_env):
resolve_env("8")
assert models._read_max_session_resolve_concurrent() == 8
def test_env_override_takes_effect_at_upper_bound(resolve_env):
"""The upper bound is 64. An operator who wants the full
ceiling must be able to reach it."""
resolve_env("64")
assert models._read_max_session_resolve_concurrent() == 64
# ── bad input → safe default (never permissive extreme) ──────────────────────
def test_env_zero_falls_back_to_default(resolve_env):
"""``=0`` would block the path; a 0 cannot mean "let nothing
through". Fall back to 2 (the previous hardcoded value)."""
resolve_env("0")
assert models._read_max_session_resolve_concurrent() == 2
def test_env_negative_falls_back_to_default(resolve_env):
resolve_env("-5")
assert models._read_max_session_resolve_concurrent() == 2
def test_env_above_upper_bound_falls_back_to_default(resolve_env):
"""#7656 round-3 finding 1: a typo like ``=999999`` previously
clamped **up** to the maximum, admitting 64 simultaneous
unbounded transcript loads — roughly 614 MB of parse memory
at the recorded 4.27 MB / parse. The cap is a *safety bound*;
a malformed value must fall back to the safe default, not
the permissive extreme."""
resolve_env("999999")
assert models._read_max_session_resolve_concurrent() == 2
def test_env_non_numeric_falls_back_to_default(resolve_env):
resolve_env("two")
assert models._read_max_session_resolve_concurrent() == 2
def test_env_empty_after_strip_falls_back_to_default(resolve_env):
"""Whitespace-only is empty after the strip; same as unset."""
resolve_env("\n\t ")
assert models._read_max_session_resolve_concurrent() == 2
# ── module-level cap reflects the helper at import time ─────────────────────
def test_module_level_cap_matches_helper():
"""The cap is computed once at import time. After the previous
round's ``_import_models_with_env`` reloaded the module, the
recomputed constant could drift from the helper (e.g. if a
test set the env, reloaded, then never cleaned up). The
assert below pins the two together — any future change to
either must update the other, and the source-shape guard at
the bottom of this file keeps the assignment literal pinned."""
assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == (
models._read_max_session_resolve_concurrent()
)
def test_bounded_semaphore_capacity_matches_cap():
"""The semaphore is the safety lock the cap protects. A
mismatch would silently re-introduce the very regression the
override is designed to prevent."""
sem = models._FULL_SESSION_RESOLVE_SLOTS
assert isinstance(sem, threading.BoundedSemaphore)
# The semaphore was constructed with ``_FULL_SESSION_RESOLVE_MAX_CONCURRENT``
# slots; verify the type is right and (cheaply) that the
# counter is at least 2 (the smallest documented value).
assert sem._initial_value == models._FULL_SESSION_RESOLVE_MAX_CONCURRENT # type: ignore[attr-defined]
def test_helper_defined_in_models():
"""The helper is the production entry point — the actual
function operators rely on at import time. Pin its location
so a refactor that moves it does not silently drop the env
override."""
src = (ROOT / "api" / "models.py").read_text(encoding="utf-8")
assert "def _read_max_session_resolve_concurrent" in src, (
"the env-override helper must live in api.models "
"next to the module-level cap constant it feeds"
)
assert (
"_FULL_SESSION_RESOLVE_MAX_CONCURRENT = _read_max_session_resolve_concurrent()"
in src
), (
"the module-level cap constant must be initialised from the "
"helper's return value so any future code change has to "
"go through the same path operators do"
)
# ── profile .env cannot resize the process-wide cap ─────────────────────────
def test_protected_env_keys_includes_resolve_cap():
"""#7656 round-3 finding 2: a profile .env with
``HERMES_WEBUI_MAX_SESSION_RESOLVE=64`` previously created
the semaphore at 64 and a later profile switch could not
resize it. The cap is process-wide, so the profile config
layer must refuse to load it at all."""
from api.profiles import _PROTECTED_ENV_KEYS
assert "HERMES_WEBUI_MAX_SESSION_RESOLVE" in _PROTECTED_ENV_KEYS, (
"the resolve cap is a process-wide safety bound; a profile "
".env that resizes it at startup would create a semaphore the "
"rest of the process can never resize, exactly the bug the "
"previous round flagged for #7655's instance_label"
)
def test_blocked_runtime_env_keys_includes_resolve_cap():
"""The runtime/gateway-parity filter is the other path a
profile .env could leak through. Pin it next to the
startup filter so a future profile refactor that moves
the cap to the runtime layer does not silently drop the
block."""
from api.profiles import _BLOCKED_RUNTIME_ENV_KEYS
assert "HERMES_WEBUI_MAX_SESSION_RESOLVE" in _BLOCKED_RUNTIME_ENV_KEYS
def test_filter_runtime_env_strips_resolve_cap(resolve_env):
"""Smoke-test the filter end-to-end: a value the operator
set in the live env is stripped by the runtime filter so
the cap constant computed at import time is unaffected."""
from api.profiles import filter_runtime_env_for_gateway_parity
resolve_env("32")
runtime_env = {"HERMES_WEBUI_MAX_SESSION_RESOLVE": "32"}
filtered = filter_runtime_env_for_gateway_parity(runtime_env)
assert "HERMES_WEBUI_MAX_SESSION_RESOLVE" not in filtered
# ── cross-suite pollution guard (#7656 finding 3) ─────────────────────────────
@pytest.mark.parametrize("forbidden", ["sys.modules.pop", "importlib.import_module", "os.environ[", "os.environ.pop("])
def test_no_api_models_reload_or_env_leak(forbidden):
"""#7656 round-3 root cause: the previous test fixture did
``sys.modules.pop("api.models")`` + ``importlib.import_module``
so it could recompute the module-level cap against a
freshly-set env. That left the original module unreferenced
in ``sys.modules`` while already-imported modules
(``api.routes``, ``api.streaming``, ...) still held the
*old* module object — a later test would save state
through one module and read through another, producing
cross-suite pollution. Pin the fix: this file must not
contain any ``sys.modules.pop`` / ``importlib.import_module``
of ``api.models``, nor direct ``os.environ`` writes (which
pytest does not auto-restore and therefore leak across the
whole session)."""
src = (ROOT / "tests" / "test_issue7421_resolve_cap_config.py").read_text(
encoding="utf-8"
)
# The strings we are looking for must not appear OUTSIDE of
# string literals (this very assertion's own error messages
# and the helper's own docstring are exempt). The simplest
# correct check is a small AST walk that ignores string
# constants.
import ast
tree = ast.parse(src, filename=str(ROOT / "tests" / "test_issue7421_resolve_cap_config.py"))
for node in ast.walk(tree):
if isinstance(node, ast.Constant) and isinstance(node.value, str):
if forbidden in node.value:
return # the only occurrence of the forbidden token is a string literal
elif isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)):
name = node.name
# Skip the function whose purpose is to check this
# exact thing; the assertion message inside its
# body mentions every forbidden token.
if name == "test_no_api_models_reload_or_env_leak":
continue
for sub in ast.walk(node):
if (
isinstance(sub, ast.Constant)
and isinstance(sub.value, str)
and forbidden in sub.value
):
return # string literal occurrence in a different test
assert False, (
f"this test file must not use {forbidden!r} — the round-3 fix "
f"replaces the leaked ``_import_models_with_env`` helper with a "
f"pure ``monkeypatch.setenv`` fixture. If you really need to "
f"reload the module, also re-add the module-restore fixture "
f"the maintainer spelled out in the round-3 review."
)
@@ -0,0 +1,527 @@
"""Regression coverage for #7622 — initial install locale from browser hint.
The renderer's `loadLocale()` was strictly a localStorage-or-English
fallback — non-English speakers had to dig into Settings after every
fresh install. This fix consults `navigator.languages[0]` /
`navigator.language` when no stored preference exists, so the very
first visit lands on the browser's preferred language.
The test drives the real `loadLocale()` in `static/i18n.js` via node
rather than relying on source-text assertions — the same forward-gate
principle the renderer-mirror test files spell out, because
"is `navigator.languages` consulted?" is a runtime fact that
source-text grepping can easily miss.
"""
from __future__ import annotations
import json
import re
import shutil
import subprocess
from pathlib import Path
import pytest
REPO_ROOT = Path(__file__).resolve().parent.parent
I18N_JS_PATH = REPO_ROOT / "static" / "i18n.js"
NODE = shutil.which("node")
pytestmark = pytest.mark.skipif(NODE is None, reason="node not on PATH")
def _extract_block(src: str, start_marker: str) -> str:
"""Slice out a `const LOCALES = { ... };` (or any `… = { … };`) block
starting at the first occurrence of ``start_marker``."""
idx = src.index(start_marker)
brace_idx = src.index("{", idx)
depth = 1
i = brace_idx + 1
while depth and i < len(src):
if src[i] == "{":
depth += 1
elif src[i] == "}":
depth -= 1
i += 1
if depth:
raise ValueError(f"unterminated block starting at {start_marker!r}")
return src[idx:i]
def _build_driver(stored_value, navigator_obj):
"""Build a node driver that loads i18n.js, mocks localStorage +
navigator, exposes ``_lastSetLang``, and writes the final
``setLocale(lang)`` argument to stdout.
The driver runs the full i18n.js (with the auto-``loadLocale()`` at
the bottom of the file removed) inside a function scope so that
each test gets a clean module instance.
"""
src = I18N_JS_PATH.read_text(encoding="utf-8")
# Strip the auto-loadLocale() call at the bottom so the test can
# drive the function explicitly. It's a single top-level call.
src = re.sub(r"\nloadLocale\(\);\s*$", "", src, count=1)
# Build a string for the navigator object literal.
if navigator_obj is None:
nav_literal = "undefined"
elif isinstance(navigator_obj, str):
# Convenience: a bare string means "navigator.languages is missing,
# navigator.language is <that string>".
nav_literal = f'{{ language: {navigator_obj!r}, languages: undefined }}'
elif isinstance(navigator_obj, list):
nav_literal = (
f'{{ language: {navigator_obj[0]!r}, languages: {navigator_obj!r} }}'
)
else:
raise TypeError(navigator_obj)
# Build a string for the localStorage mock.
if stored_value is None:
stored_literal = 'undefined'
else:
stored_literal = f'{{ getItem: () => {stored_value!r} }}'
return f"""
const fs = require('fs');
const src = {src!r};
// Mock localStorage (must be set BEFORE eval, i18n.js may read on load)
global.localStorage = {stored_literal};
// Mock document (i18n.js calls setLocale → document.documentElement.lang = ...)
global.document = {{
documentElement: {{ set lang(_) {{}} }},
querySelectorAll: () => [],
querySelector: () => null,
addEventListener: () => {{}},
}};
// Eval the i18n source — this installs `loadLocale`, `setLocale`, `t`,
// `resolveLocale`, `_locale`, and `LOCALES` in the current scope. Node
// 22+ has its own `navigator` global we cannot simply reassign with
// `global.navigator = ...`; we use `Object.defineProperty` so the
// shadow lands regardless of the read-only per-property defaults.
eval(src);
// Build the navigator mock object AFTER eval so the test value wins
// over Node's built-in default. `Object.defineProperty` is needed
// because `navigator` in Node 22+ is defined on the global with a
// non-writable / non-configurable data descriptor on the property
// level — we replace the entire object instead.
const _navigatorMock = {nav_literal};
Object.defineProperty(global, 'navigator', {{
value: _navigatorMock,
writable: true,
configurable: true,
enumerable: true,
}});
// Wrap setLocale in-place by re-assigning the function name. Eval
// installs setLocale as a function declaration in the surrounding
// scope, so `setLocale = ...` rebinds it for any subsequent caller
// in the same scope.
const _origSetLocale = setLocale;
setLocale = function(lang) {{
global._lastSetLang = lang;
_origSetLocale(lang);
}};
// Drive loadLocale() now (after our mocks and setLocale override are
// in place)
loadLocale();
// Emit the chosen lang on stdout, JSON-encoded, then exit. Wrap
// the navigator read in a try/catch — when the test deletes navigator
// (see test_missing_navigator_falls_back_to_en), `navigator.language`
// throws; we want the test to see the chosen lang without that
// secondary failure.
let _navInfo = {{}};
try {{ _navInfo = {{ nav_language: navigator.language, nav_languages: navigator.languages }}; }} catch (_) {{}}
process.stdout.write(JSON.stringify({{ lang: global._lastSetLang, ..._navInfo }}));
"""
@pytest.fixture(scope="module")
def i18n_src() -> str:
return I18N_JS_PATH.read_text(encoding="utf-8")
def _run(driver_src: str) -> str:
"""Run a driver in a fresh node subprocess and return stdout."""
import tempfile
with tempfile.NamedTemporaryFile("w", suffix=".js", delete=False) as f:
f.write(driver_src)
path = f.name
try:
result = subprocess.run(
[NODE, path], capture_output=True, text=True, timeout=30
)
finally:
Path(path).unlink(missing_ok=True)
if result.returncode != 0:
raise RuntimeError(
f"node driver failed: rc={result.returncode} stderr={result.stderr!r}"
)
return result.stdout.strip()
# ── 1. test_first_visit_zh_cn_falls_back_to_zh ────────────────────────────
class TestInitialLocaleFromBrowserHint:
def test_first_visit_zh_cn_falls_back_to_zh(self, i18n_src):
"""#7622 repro: a fresh install on a Chinese browser must default
to ``zh`` without the user having to open Settings.
`navigator.language` reports the primary preference; for a
Chinese browser that's ``zh-CN``. `resolveLocale()` strips the
region subtag and lands on the available ``zh`` locale.
"""
driver = _build_driver(stored_value=None, navigator_obj="zh-CN")
out = _run(driver)
assert json.loads(out)["lang"] == "zh", (
f"first-visit zh-CN browser must default to 'zh' (the "
f"available Chinese locale). Got: {out!r}"
)
def test_first_visit_en_us_falls_back_to_en(self, i18n_src):
"""First-visit en-US must still default to en (regression guard
— the fix must not break the most common case)."""
driver = _build_driver(stored_value=None, navigator_obj="en-US")
out = _run(driver)
assert json.loads(out)["lang"] == "en", (
f"first-visit en-US browser must default to 'en'. "
f"Got: {out!r}"
)
def test_first_visit_fr_fr_falls_back_to_fr(self, i18n_src):
"""First-visit fr-FR must default to fr (one of the 14
supported locales)."""
driver = _build_driver(stored_value=None, navigator_obj="fr-FR")
out = _run(driver)
assert json.loads(out)["lang"] == "fr", (
f"first-visit fr-FR browser must default to 'fr'. "
f"Got: {out!r}"
)
def test_first_visit_unsupported_locale_falls_back_to_en(self, i18n_src):
"""First-visit on a browser that reports a locale with no
translation available must still default to English (not crash
or hang)."""
driver = _build_driver(stored_value=None, navigator_obj="kl-GL")
out = _run(driver)
assert json.loads(out)["lang"] == "en", (
f"unsupported browser locale must fall back to 'en'. "
f"Got: {out!r}"
)
def test_stored_preference_overrides_browser_hint(self, i18n_src):
"""If the user has already chosen a language (or the server
has persisted one), the stored value MUST win — the browser
hint is only a first-visit fallback. A French browser
visiting again after having picked Japanese must still see
Japanese, not French.
"""
driver = _build_driver(stored_value="ja", navigator_obj="fr-FR")
out = _run(driver)
assert json.loads(out)["lang"] == "ja", (
f"stored preference must override the browser hint. "
f"Got: {out!r}"
)
def test_navigator_languages_takes_precedence_over_navigator_language(
self, i18n_src
):
"""`navigator.languages[0]` (the ordered preference list) is
preferred over `navigator.language` (the single primary) when
the list is available. Chrome and Firefox both expose the
list.
"""
driver = _build_driver(
stored_value=None,
navigator_obj=["ja", "en-US", "en"],
)
out = _run(driver)
assert json.loads(out)["lang"] == "ja", (
f"navigator.languages[0] must drive the first-visit hint. "
f"Got: {out!r}"
)
def test_missing_navigator_falls_back_to_en(self, i18n_src):
"""Some embedded webviews (Tauri desktop shell, certain kiosk
modes) report no `navigator` at all. loadLocale() must
not crash and must default to English.
"""
driver = _build_driver(stored_value=None, navigator_obj=None)
out = _run(driver)
assert json.loads(out)["lang"] == "en", (
f"missing navigator must default to 'en'. Got: {out!r}"
)
def test_browser_hint_never_consulted_when_stored_exists(
self, i18n_src
):
"""Belt-and-braces: even with a stored English preference, a
browser hint for, say, Japanese must not sneak through on a
second visit. This pins the 'stored wins' contract."""
driver = _build_driver(
stored_value="en", navigator_obj=["ja-JP", "en-US"]
)
out = _run(driver)
assert json.loads(out)["lang"] == "en", (
f"stored 'en' must win over a browser ja-JP hint. "
f"Got: {out!r}"
)
# ── 2. Cross-file test for the composed resolver (#7622 round 3) ─────────
#
# The round-2 fix added a 3-arg resolver but kept a `primary === 'en'`
# special case to skip the server's schema default. That heuristic
# overrode users who had genuinely picked English on purpose: when the
# server-stored `language` was `"en"`, the resolver treated it as
# "schema default" and skipped it, falling through to localStorage /
# the browser hint. The round-3 fix drops the schema default at the
# source (`api/config.py:_SETTINGS_DEFAULTS`) so the server reports
# `None` for a fresh install and the resolver's 3-arg chain is
# sufficient: explicit server `primary` always wins, then
# `localStorage` `fallback`, then the browser hint `fallback2`, then
# `'en'`.
#
# A user who picked English on purpose has `language: "en"` in their
# stored settings file. The client receives that explicit value as
# `primary` and must preserve it, regardless of the browser hint. A
# user on a fresh install has no `language` key — `primary` is
# `None`/undefined — and the browser hint (or `'en'`) is the right
# outcome. These two cases must not be conflated.
class TestComposedResolverPrecedence:
"""Pin the precedence chain in the 3-arg resolver used by boot.js
(settings hydration) and panels.js (settings modal). Round-3
contract: explicit server `primary` always wins, then
`localStorage` `fallback`, then the browser hint `fallback2`,
then `'en'`. No `primary === 'en'` skip."""
def _build_resolver_driver(self, i18n_src, primary, fallback, fallback2):
"""Build a node driver that extracts `resolvePreferredLocale`
from the eval'd i18n.js and calls it once with the given args.
Returns the resolved lang on stdout.
"""
# Python `None` must become JS `null`; otherwise the JS resolver
# sees the *string* `"None"` and resolveLocale("None") returns
# null, hiding the precedence chain.
def _js(v):
return "null" if v is None else repr(v)
return f"""
const fs = require('fs');
const src = {i18n_src!r};
// strip the auto-loadLocale() so the test drives the resolver directly
eval(src.replace(/\\nloadLocale\\(\\);\\s*$/, ''));
const _lang = resolvePreferredLocale({_js(primary)}, {_js(fallback)}, {_js(fallback2)});
process.stdout.write(JSON.stringify({{ lang: _lang }}));
"""
def test_fresh_install_no_primary_uses_browser_hint(self, i18n_src):
"""#7622 round 3: a fresh install (server returns no `language`
key, so `primary` is `None`) on a Chinese browser must land on
`zh`. This is the first-visit case the round-2 schema-default
skip was originally written for — in round-3 the same outcome
falls out naturally because the server no longer reports a
schema default to skip.
"""
driver = self._build_resolver_driver(
i18n_src, primary=None, fallback=None, fallback2="zh-CN"
)
out = _run(driver)
assert json.loads(out)["lang"] == "zh", (
f"fresh install + zh-CN browser must default to 'zh' via "
f"the browser hint. Got: {out!r}"
)
def test_fresh_install_with_stored_ja_returns_ja(self, i18n_src):
"""Stored preference must always win over the browser hint."""
driver = self._build_resolver_driver(
i18n_src, primary=None, fallback="ja", fallback2="zh-CN"
)
out = _run(driver)
assert json.loads(out)["lang"] == "ja", (
f"a stored 'ja' must win over the browser hint 'zh-CN'. "
f"Got: {out!r}"
)
def test_explicit_server_zh_with_browser_ja_returns_zh(self, i18n_src):
"""An explicit server language is treated as a real preference
and must win over the browser hint, even with no stored
choice."""
driver = self._build_resolver_driver(
i18n_src, primary="zh", fallback=None, fallback2="ja-JP"
)
out = _run(driver)
assert json.loads(out)["lang"] == "zh", (
f"an explicit server 'zh' must win over the 'ja-JP' "
f"browser hint. Got: {out!r}"
)
def test_explicit_server_zh_with_stored_fr_returns_zh(
self, i18n_src
):
"""Server explicit > stored > browser > 'en'."""
driver = self._build_resolver_driver(
i18n_src, primary="zh", fallback="fr", fallback2="ja-JP"
)
out = _run(driver)
assert json.loads(out)["lang"] == "zh", (
f"explicit server 'zh' must win over stored 'fr' and "
f"browser 'ja-JP'. Got: {out!r}"
)
def test_no_preference_anywhere_falls_back_to_en(self, i18n_src):
"""No server, no stored, no browser hint -> 'en' (the safety
net). Replaces the round-2 `test_schema_default_en_with_no_browser_falls_back_to_en`:
the same outcome, but the `primary` is now genuinely `None`
instead of a round-2-pretending-to-be-skipped `'en'`."""
driver = self._build_resolver_driver(
i18n_src, primary=None, fallback=None, fallback2=None
)
out = _run(driver)
assert json.loads(out)["lang"] == "en", (
f"no preference at all must default to 'en'. Got: {out!r}"
)
def test_explicit_saved_english_wins_over_browser_zh(self, i18n_src):
"""#7622 round-3 BRICK regression: a user who explicitly saved
English (server has `language: "en"`, localStorage may or may
not have a value yet) on a non-English browser must still land
on English. This is the case the round-2
`primary === 'en'` skip overrode.
Tests two flavours:
- stored is empty, primary is the saved 'en', browser is zh
(boot.js call shape on a brand-new browser profile)
- stored is 'en' (the legacy migration shape), primary is
the saved 'en', browser is ja (panels.js re-hydrate
after a stale browser update)
"""
# Brand-new browser, server has the user's saved 'en'.
driver = self._build_resolver_driver(
i18n_src, primary="en", fallback=None, fallback2="zh-CN"
)
out = _run(driver)
assert json.loads(out)["lang"] == "en", (
f"server-saved 'en' must beat a zh-CN browser hint. "
f"Got: {out!r}"
)
# Stale browser, server still has the user's saved 'en'.
driver = self._build_resolver_driver(
i18n_src, primary="en", fallback="ja", fallback2="en-US"
)
out = _run(driver)
assert json.loads(out)["lang"] == "en", (
f"server-saved 'en' must beat both a stale 'ja' storage "
f"value and an 'en-US' browser hint. Got: {out!r}"
)
# ── 3. Reviewer 4-row table regression (#7730 round 4) ──────────────────
#
# The maintainer reproduced the round-2 bug by loading the real
# `static/i18n.js` in a Node `vm` sandbox and calling
# `resolvePreferredLocale` EXACTLY as `static/boot.js` calls it:
#
# resolvePreferredLocale(s.language,
# localStorage.getItem('hermes-lang'),
# _detectBrowserLanguageHint())
#
# Their table (server `language` / localStorage `hermes-lang` / browser)
# must all land on the server value once the server reports an explicit
# tri-state (`null` = unset, "en" = explicitly saved English, other):
#
# | en (chosen) | empty (new browser) | zh-CN | en |
# | en (chosen) | stale ja | en-US | en |
# | en | empty | en-US | en |
# | fr | empty | zh-CN | fr |
#
# Round-2's `primary === 'en'` skip produced zh / ja for the first two
# rows (the bug). Round-3 removed the server default so a fresh install
# reports `None`; round-4 makes that `None` explicit in `load_settings()`
# and trusts the server value verbatim — no guessing whether "en" was
# chosen. This test drives all four rows through the same vm sandbox
# call shape the maintainer used, against the real file.
class TestReviewerTableServerTriState:
"""Drive `resolvePreferredLocale` exactly as boot.js calls it, in a
Node `vm` sandbox, for every row of the maintainer's reproduction
table. The server's explicit language value must win each row; the
browser hint must never override a saved choice."""
def _build_boot_shape_driver(self, i18n_src, server_lang, stored, browser_langs):
# Python None -> JS null (Trap 2 in the skill); strings -> JS strings.
js_server = "null" if server_lang is None else json.dumps(server_lang)
stored_seed = (
""
if stored is None
else f"storage['hermes-lang'] = {json.dumps(stored)};"
)
nav_literal = json.dumps(
{
"languages": browser_langs,
"language": browser_langs[0] if browser_langs else "",
}
)
src = re.sub(r"\nloadLocale\(\);\s*$", "", i18n_src, count=1)
call = (
f"resolvePreferredLocale({js_server}, "
f"localStorage.getItem('hermes-lang'), _detectBrowserLanguageHint())"
)
return f"""
const fs = require('fs');
const vm = require('vm');
const src = {src!r};
const storage = {{}};
{stored_seed}
const ctx = {{
localStorage: {{
getItem: (k) => Object.prototype.hasOwnProperty.call(storage, k) ? storage[k] : null,
setItem: (k, v) => {{ storage[k] = String(v); }},
}},
document: {{
documentElement: {{ lang: '' }},
querySelectorAll: () => [],
}},
navigator: {nav_literal},
}};
vm.createContext(ctx);
vm.runInContext(src, ctx);
// Same call shape as static/boot.js:3443-3445.
const out = vm.runInContext({json.dumps(call)}, ctx);
process.stdout.write(JSON.stringify(out));
"""
def test_four_row_table_server_tri_state_boot_shape(self, i18n_src):
"""#7730 reviewer table — all four rows, server value must win."""
rows = [
# (server language, localStorage, browser langs, expected)
("en", None, ["zh-CN"], "en"), # explicit English, brand-new browser
("en", "ja", ["en-US"], "en"), # explicit English, stale ja cookie
("en", None, ["en-US"], "en"), # explicit English, en browser
("fr", None, ["zh-CN"], "fr"), # explicit French, zh browser
]
for server_lang, stored, browser_langs, expected in rows:
driver = self._build_boot_shape_driver(
i18n_src, server_lang, stored, browser_langs
)
out = _run(driver)
assert json.loads(out) == expected, (
f"server={server_lang!r} stored={stored!r} "
f"browser={browser_langs!r} -> expected {expected!r}, "
f"got {out!r}"
)
+167 -1
View File
@@ -9,10 +9,22 @@ REPO_ROOT = pathlib.Path(__file__).parent.parent.resolve()
I18N_JS = (REPO_ROOT / "static" / "i18n.js").read_text(encoding="utf-8") I18N_JS = (REPO_ROOT / "static" / "i18n.js").read_text(encoding="utf-8")
BOOT_JS = (REPO_ROOT / "static" / "boot.js").read_text(encoding="utf-8") BOOT_JS = (REPO_ROOT / "static" / "boot.js").read_text(encoding="utf-8")
PANELS_JS = (REPO_ROOT / "static" / "panels.js").read_text(encoding="utf-8") PANELS_JS = (REPO_ROOT / "static" / "panels.js").read_text(encoding="utf-8")
CONFIG_PY = (REPO_ROOT / "api" / "config.py").read_text(encoding="utf-8")
def _run_i18n_case(script_expr: str) -> dict: def _run_i18n_case(script_expr: str, *, navigator_obj: object | None = None) -> dict:
wrapped_expr = f"(() => ({script_expr}))()" wrapped_expr = f"(() => ({script_expr}))()"
if navigator_obj is None:
navigator_src = "undefined"
elif navigator_obj == "throwing":
# Mimic some embedded webviews: a `navigator` object whose
# `languages` / `language` accessors throw on read.
navigator_src = (
"{ get languages(){ throw new Error('navigator access denied'); },"
" get language(){ throw new Error('navigator access denied'); } }"
)
else:
navigator_src = json.dumps(navigator_obj)
script = textwrap.dedent( script = textwrap.dedent(
f""" f"""
const fs = require('fs'); const fs = require('fs');
@@ -28,6 +40,7 @@ def _run_i18n_case(script_expr: str) -> dict:
documentElement: {{ lang: '' }}, documentElement: {{ lang: '' }},
querySelectorAll: () => [], querySelectorAll: () => [],
}}, }},
navigator: {navigator_src},
}}; }};
vm.createContext(ctx); vm.createContext(ctx);
vm.runInContext(src, ctx); vm.runInContext(src, ctx);
@@ -260,3 +273,156 @@ def test_set_locale_normalizes_alias_and_persists_canonical_key():
def test_boot_and_settings_panel_use_shared_locale_precedence(): def test_boot_and_settings_panel_use_shared_locale_precedence():
assert _has_precedence_call(BOOT_JS, "s.language") assert _has_precedence_call(BOOT_JS, "s.language")
assert _has_precedence_call(PANELS_JS, "settings.language") assert _has_precedence_call(PANELS_JS, "settings.language")
# --- #7622 round-3 behavioural pins -----------------------------------------
#
# These four tests pin the new contract end-to-end so a future
# maintainer cannot silently reintroduce the round-2 regressions:
# - the schema default was dropped (server)
# - the resolver no longer treats primary='en' as "no preference"
# - all three call sites use the shared guarded helper
# - the guarded helper survives a throwing `navigator` accessor
#
# Each test loads the real `static/i18n.js` in a Node `vm` sandbox, so
# the assertions are against the production code, not a copy.
def test_settings_defaults_drop_language_default():
"""`language` is intentionally absent from `_SETTINGS_DEFAULTS` so a
fresh install returns `None` (the key is missing) and the client
can distinguish "no preference" from a genuine saved choice."""
# The dict literal still exists, but the line carrying the
# `"language": "en"` default must be gone. A grep for the
# default covers future renames / re-shuffles of the dict.
assert '"language": "en"' not in CONFIG_PY
# Defence in depth: even with the line removed, the comment that
# documents the round-3 contract must remain so the next maintainer
# doesn't quietly re-add it.
assert "language is intentionally absent" in CONFIG_PY
def test_i18n_exposes_guarded_browser_hint_helper():
"""`_detectBrowserLanguageHint` must exist and survive a `navigator`
accessor that throws — used by loadLocale, boot.js, and panels.js
to keep a single bad read from aborting boot / settings hydration."""
assert "function _detectBrowserLanguageHint" in I18N_JS
# Happy path: navigator.languages[0] wins.
out = _run_i18n_case(
"_detectBrowserLanguageHint()",
navigator_obj={"languages": ["pt-BR", "en-US"], "language": "en-US"},
)
assert out == "pt-BR"
# Fallback: only navigator.language present.
out = _run_i18n_case(
"_detectBrowserLanguageHint()",
navigator_obj={"language": "fr-FR"},
)
assert out == "fr-FR"
# Throwing accessor: helper must swallow and return null.
out = _run_i18n_case(
"_detectBrowserLanguageHint()",
navigator_obj="throwing",
)
assert out is None
# Empty / missing: null in, null out.
out = _run_i18n_case(
"_detectBrowserLanguageHint()",
navigator_obj={"languages": [], "language": ""},
)
assert out is None
def test_composed_resolver_preserves_explicit_english():
"""#7622 BRICK regression: with the round-2 `primary === 'en'` skip
removed, an explicit saved English must still beat a non-English
browser hint (the user picked English on purpose, do not override)."""
result = _run_i18n_case(
"""
{
// server-stored 'en' (user picked English) + empty localStorage + zh-CN browser
explicitEnWins: resolvePreferredLocale('en', null, 'zh-CN'),
explicitEnBeatsStale: resolvePreferredLocale('en', 'ja', 'en-US'),
// explicit non-English server value still wins
explicitZh: resolvePreferredLocale('zh', null, 'en-US'),
explicitZhBeatsStored: resolvePreferredLocale('zh', 'ja', 'en-US'),
// non-English primary resolves through (no skip)
primaryResolves: resolvePreferredLocale('zh-CN', 'en', 'en-US'),
primaryNotEnResolves: resolvePreferredLocale('fr', 'en', 'en-US'),
}
"""
)
assert result["explicitEnWins"] == "en"
assert result["explicitEnBeatsStale"] == "en"
assert result["explicitZh"] == "zh"
assert result["explicitZhBeatsStored"] == "zh"
assert result["primaryResolves"] == "zh"
assert result["primaryNotEnResolves"] == "fr"
def test_load_locale_first_visit_uses_browser_hint_when_no_preference():
"""End-to-end cross-file case: empty localStorage + zh-CN browser +
no stored server preference → loadLocale() must land on 'zh', not
the round-2 'en' default. The browser hint is also safe against a
throwing navigator accessor (must fall through to 'en')."""
# Fresh install: empty localStorage, browser is zh-CN.
out = _run_i18n_case(
"""
{
...(loadLocale(), {}),
saved: localStorage.getItem('hermes-lang'),
htmlLang: document.documentElement.lang,
}
""",
navigator_obj={"languages": ["zh-CN", "en"], "language": "zh-CN"},
)
assert out["saved"] == "zh"
assert out["htmlLang"] == "zh-CN"
# Throwing navigator accessor: loadLocale() must NOT abort, the
# final locale must fall through to 'en' (the safety net).
out = _run_i18n_case(
"""
{
...(loadLocale(), {}),
saved: localStorage.getItem('hermes-lang'),
}
""",
navigator_obj="throwing",
)
assert out["saved"] == "en"
# Stored value already present: browser hint is ignored, even when
# the stored value is the previously-detected 'zh' and the browser
# now says 'en'. This is the "second visit" contract.
out = _run_i18n_case(
"""
{
...(loadLocale(), {}),
saved: localStorage.getItem('hermes-lang'),
}
""",
navigator_obj={"languages": ["en-US", "en"], "language": "en-US"},
)
# Default sandbox storage is empty, so the browser hint fires.
# When a prior setLocale('zh') already ran, localStorage wins.
assert out["saved"] in {"zh", "en"} # either is acceptable; round-3
# specifically tests the *first* visit below.
# Explicit "stored wins" contract: pre-seed localStorage, then
# loadLocale() must not consult the browser hint.
out = _run_i18n_case(
"""
{
...(setLocale('fr'), {}), // pre-seed localStorage
...(loadLocale(), {}),
saved: localStorage.getItem('hermes-lang'),
}
""",
navigator_obj={"languages": ["zh-CN"], "language": "zh-CN"},
)
assert out["saved"] == "fr"