mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-05-25 11:10:18 +00:00
Merge PR #1411 from nesquena-hermes: TTS toggle CSS specificity collision (#1409) + Ollama env var bleed (#1410)
# Conflicts: # CHANGELOG.md
This commit is contained in:
+4
-1
@@ -18,6 +18,10 @@
|
||||
### Fixed
|
||||
- **Session provider context preserved across model picker → runtime resolution** (#1240) — the WebUI model picker can show multiple providers exposing the same bare model id (e.g. `gpt-5.5` from OpenAI Codex, OpenRouter, Copilot). Previously sessions persisted only the bare model, so a session selected as "gpt-5.5 from OpenAI Codex" silently rerouted through whatever provider became default after a config change. New `model_provider: str | None` field on `Session` is persisted in metadata, threaded through every chat path (`/api/session/new`, `/api/session/update`, `/api/chat/start`, `/api/chat/sync`, `/btw`, `/background`, `_run_agent_streaming`), and is gated in `compact()` to emit only when truthy (matches v0.50.251 lineage end_reason gating). New `model_with_provider_context(model_id, model_provider)` in `api/config.py` builds the `@provider:model` form when provider differs from configured default, then passes through `resolve_model_provider()`. New `_should_attach_codex_provider_context()` narrow exception detects bare GPT-* models under active OpenAI Codex (because Codex/OpenRouter/Copilot expose overlapping GPT names). New `_resolve_compatible_session_model_state()` returns `(effective_model, effective_provider, model_was_normalized)`. Frontend adds `MODEL_STATE_KEY='hermes-webui-model-state'` localStorage with structured persistence and migrates from the legacy `hermes-webui-model` key. 13 new tests in `test_provider_mismatch.py`, 2 in `test_model_picker_badges.py`. (`api/config.py`, `api/models.py`, `api/routes.py`, `api/streaming.py`, `static/boot.js`, `static/messages.js`, `static/panels.js`, `static/sessions.js`, `static/ui.js`) @starship-s — PR #1390, refs #1240
|
||||
|
||||
|
||||
- **TTS toggle: speaker icon never appeared when "Text-to-Speech for responses" was ticked** (#1409, closes #1409) — `_applyTtsEnabled()` set `btn.style.display=enabled?'':'none'` on every `.msg-tts-btn`. The `''` branch removes the inline override, after which the `.msg-tts-btn{display:none;}` rule from `style.css` re-hides the button. Both the "enabled" and "disabled" branches left the icon hidden, so the toggle had no visible effect since the feature shipped in #499. Fixed by switching to a body-class toggle (`body.tts-enabled`) plus a compound CSS selector (`body.tts-enabled .msg-tts-btn{display:inline-flex;}`). The new shape bypasses the `.msg-action-btn` / `.msg-tts-btn` cascade collision and survives subsequent `renderMd()` re-renders without re-querying every button. (`static/panels.js`, `static/style.css`, `tests/test_499_tts_playback.py`) — fixes #1409
|
||||
|
||||
- **Ollama (local) no longer falsely reports "API key configured" when only Ollama Cloud key is set** (#1410, closes #1410) — both providers were mapped to the same `OLLAMA_API_KEY` env var in `_PROVIDER_ENV_VAR`, so configuring Ollama Cloud lit up the local Ollama card too. The runtime in `hermes_cli/runtime_provider.py` only consumes `OLLAMA_API_KEY` when the base URL hostname is `ollama.com` — local Ollama is keyless by design — so the WebUI was reporting "configured" for a key local Ollama doesn't even read. Dropped the bare `"ollama": "OLLAMA_API_KEY"` mapping; local Ollama users who genuinely need a key can still set `providers.ollama.api_key` in `config.yaml`, and `_provider_has_key()` continues to honor that path. (`api/providers.py`, `tests/test_provider_management.py`) — fixes #1410, reported by @AvidFuturist
|
||||
### Changed
|
||||
|
||||
- **`api/rollback.py` — checkpoint id regex validation (defense-in-depth)** — Opus pre-release follow-up. The `checkpoint` parameter on `/api/rollback/diff` and `/api/rollback/restore` was joined into the path via `_checkpoint_root() / ws_hash / checkpoint`. `Path("/a/b") / "../escape"` does NOT normalize, so an authenticated caller could pass `../<other-ws-hash>/<sha>` and read or restore from another allowlisted workspace's checkpoint store. New `_validate_checkpoint_id()` regex-guards with `^[A-Za-z0-9_-][A-Za-z0-9_.-]{0,63}$` and rejects literal `.` / `..`. (`api/rollback.py`)
|
||||
@@ -28,7 +32,6 @@
|
||||
|
||||
- **`api/rollback.py::_inspect_checkpoint` drops bare `Exception` from except tuple** — Opus pre-release follow-up. The previous `except (subprocess.TimeoutExpired, OSError, Exception)` made the specific catches redundant and swallowed everything. Now `(subprocess.TimeoutExpired, OSError)` only. (`api/rollback.py`, `tests/test_v050255_opus_followups.py`)
|
||||
|
||||
|
||||
## [v0.50.254] — 2026-05-01
|
||||
|
||||
### Fixed
|
||||
|
||||
+8
-1
@@ -44,7 +44,14 @@ _PROVIDER_ENV_VAR: dict[str, str] = {
|
||||
"x-ai": "XAI_API_KEY",
|
||||
"opencode-zen": "OPENCODE_ZEN_API_KEY",
|
||||
"opencode-go": "OPENCODE_GO_API_KEY",
|
||||
"ollama": "OLLAMA_API_KEY",
|
||||
# NOTE: bare "ollama" (local) deliberately omitted — local Ollama is keyless
|
||||
# by default and the runtime in hermes_cli/runtime_provider.py only consumes
|
||||
# OLLAMA_API_KEY when the base URL hostname is ollama.com (Ollama Cloud).
|
||||
# If we mapped both providers to the same env var, configuring Ollama Cloud
|
||||
# would falsely flip the local Ollama card to "API key configured" (#1410).
|
||||
# Users who genuinely run an authenticated local Ollama can still set a key
|
||||
# via providers.ollama.api_key in config.yaml — that path remains supported
|
||||
# by _provider_has_key().
|
||||
"ollama-cloud": "OLLAMA_API_KEY",
|
||||
"nvidia": "NVIDIA_API_KEY",
|
||||
}
|
||||
|
||||
+6
-4
@@ -2747,11 +2747,13 @@ function _markSettingsDirty(){
|
||||
_settingsDirty = true;
|
||||
}
|
||||
|
||||
// Apply TTS enabled state: show/hide TTS buttons on all assistant messages
|
||||
// Apply TTS enabled state: toggles a body class so the CSS rule
|
||||
// `body.tts-enabled .msg-tts-btn` shows/hides the speaker icon. We toggle the
|
||||
// body class instead of writing inline `style.display` because the parent
|
||||
// `.msg-action-btn` has no display rule, so clearing the inline style let the
|
||||
// `.msg-tts-btn{display:none;}` cascade re-hide the button (#1409).
|
||||
function _applyTtsEnabled(enabled){
|
||||
document.querySelectorAll('.msg-tts-btn').forEach(btn=>{
|
||||
btn.style.display=enabled?'':'none';
|
||||
});
|
||||
document.body.classList.toggle('tts-enabled', !!enabled);
|
||||
}
|
||||
|
||||
function _appearancePayloadFromUi(){
|
||||
|
||||
+5
-1
@@ -1423,8 +1423,12 @@
|
||||
.msg-row:hover .msg-actions{opacity:1;}
|
||||
.msg-action-btn{background:none;border:none;color:var(--muted);cursor:pointer;font-size:13px;padding:2px 5px;border-radius:5px;transition:color .12s,background .12s;line-height:1;}
|
||||
.msg-action-btn:hover{color:var(--accent-text);background:var(--accent-bg);}
|
||||
/* TTS speaker button: hidden by default, shown when TTS enabled */
|
||||
/* TTS speaker button: hidden by default, shown when TTS is enabled.
|
||||
* Use body-class selector instead of JS inline-style so the rule survives
|
||||
* subsequent renderMd() passes and is not subject to inline-style cascade
|
||||
* collisions with the .msg-action-btn parent (#1409). */
|
||||
.msg-tts-btn{display:none;}
|
||||
body.tts-enabled .msg-tts-btn{display:inline-flex;align-items:center;}
|
||||
.msg-tts-btn[data-speaking="1"]{color:var(--accent);animation:tts-pulse 1s ease-in-out infinite;}
|
||||
@keyframes tts-pulse{0%,100%{opacity:1}50%{opacity:.5}}
|
||||
|
||||
|
||||
@@ -193,3 +193,57 @@ class TestTtsStyles:
|
||||
src = _read('style.css')
|
||||
assert 'tts-pulse' in src, \
|
||||
"tts-pulse animation not found in style.css"
|
||||
|
||||
|
||||
class TestIssue1409TtsToggleBodyClass:
|
||||
"""Regression: #1409 — TTS toggle had no effect because of CSS specificity collision.
|
||||
|
||||
Original bug: ``_applyTtsEnabled`` set ``btn.style.display=enabled?'':'none'``.
|
||||
The empty-string branch removes the inline override, after which the
|
||||
``.msg-tts-btn { display:none; }`` rule from style.css applies — so both
|
||||
"enabled" and "disabled" states left the button hidden.
|
||||
|
||||
Fix: toggle a body-level class (``body.tts-enabled``) and gate the speaker
|
||||
icon on a compound selector ``body.tts-enabled .msg-tts-btn``. This bypasses
|
||||
the inline-style cascade collision and survives ``renderMd()`` re-renders.
|
||||
"""
|
||||
|
||||
def test_apply_tts_enabled_uses_body_class(self):
|
||||
"""_applyTtsEnabled must toggle the document body's `tts-enabled` class."""
|
||||
src = _read('panels.js')
|
||||
# The new shape: toggle body class instead of writing inline display
|
||||
assert "document.body.classList.toggle('tts-enabled'" in src, (
|
||||
"_applyTtsEnabled must toggle the body.tts-enabled class — see #1409. "
|
||||
"Reverting to inline `style.display` will silently break the toggle "
|
||||
"again because of the .msg-action-btn / .msg-tts-btn cascade."
|
||||
)
|
||||
|
||||
def test_apply_tts_enabled_does_not_use_inline_display(self):
|
||||
"""_applyTtsEnabled must NOT set inline `style.display` on .msg-tts-btn."""
|
||||
src = _read('panels.js')
|
||||
# Find the function body and check it doesn't set inline display
|
||||
# on individual buttons (the broken pattern).
|
||||
m = re.search(
|
||||
r'function _applyTtsEnabled\([^)]*\)\s*\{(?P<body>[^}]*)\}',
|
||||
src,
|
||||
)
|
||||
assert m, "_applyTtsEnabled function body not found in panels.js"
|
||||
body = m.group('body')
|
||||
assert '.style.display' not in body, (
|
||||
"_applyTtsEnabled body must not set inline style.display — that's "
|
||||
"the #1409 bug. Use body.classList.toggle('tts-enabled') instead."
|
||||
)
|
||||
|
||||
def test_body_class_selector_in_css(self):
|
||||
"""style.css must show .msg-tts-btn only when body.tts-enabled is set."""
|
||||
src = _read('style.css')
|
||||
assert 'body.tts-enabled .msg-tts-btn' in src, (
|
||||
"Missing `body.tts-enabled .msg-tts-btn` selector in style.css — "
|
||||
"without this rule the body class has no visual effect (#1409)."
|
||||
)
|
||||
# The default-hidden rule must still be present (so no body class = no icon).
|
||||
assert '.msg-tts-btn{display:none;}' in src or \
|
||||
re.search(r'\.msg-tts-btn\s*\{[^}]*display\s*:\s*none', src), (
|
||||
"Default `.msg-tts-btn{display:none;}` rule must remain so the "
|
||||
"icon is hidden by default (#1409)."
|
||||
)
|
||||
|
||||
@@ -372,3 +372,92 @@ class TestProvidersEndpoints:
|
||||
"""POST /api/providers/delete without provider should return 400."""
|
||||
body, status = _post("/api/providers/delete", {})
|
||||
assert status == 400
|
||||
|
||||
|
||||
class TestIssue1410OllamaEnvVarBleed:
|
||||
"""Regression: Ollama Cloud key must not flip local Ollama to has_key=True.
|
||||
|
||||
Both providers used to share OLLAMA_API_KEY in _PROVIDER_ENV_VAR. After
|
||||
a user added a key for Ollama Cloud, the local Ollama card also lit up
|
||||
"API key configured" — incorrect because the runtime in
|
||||
hermes_cli/runtime_provider.py only consumes OLLAMA_API_KEY when the
|
||||
base URL hostname is ollama.com. Local Ollama is keyless by default.
|
||||
|
||||
Fix: drop bare "ollama" from _PROVIDER_ENV_VAR so the env-var check is
|
||||
only applied to ollama-cloud. Local Ollama users who genuinely need a
|
||||
key can still set providers.ollama.api_key in config.yaml.
|
||||
"""
|
||||
|
||||
def test_ollama_local_not_configured_when_only_cloud_env_var_set(
|
||||
self, monkeypatch, tmp_path,
|
||||
):
|
||||
"""OLLAMA_API_KEY in env should mark ollama-cloud configured but not bare ollama."""
|
||||
_install_fake_hermes_cli(monkeypatch)
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
|
||||
monkeypatch.setenv("OLLAMA_API_KEY", "sk-cloud-key-xyz")
|
||||
|
||||
old_cfg = dict(config.cfg)
|
||||
old_mtime = config._cfg_mtime
|
||||
config.cfg.clear()
|
||||
config.cfg["model"] = {}
|
||||
try:
|
||||
config._cfg_mtime = config.Path(config._get_config_path()).stat().st_mtime
|
||||
except Exception:
|
||||
config._cfg_mtime = 0.0
|
||||
|
||||
from api.providers import get_providers
|
||||
try:
|
||||
result = get_providers()
|
||||
by_id = {p["id"]: p for p in result["providers"]}
|
||||
assert "ollama-cloud" in by_id, "ollama-cloud should appear in provider list"
|
||||
assert "ollama" in by_id, "ollama (local) should appear in provider list"
|
||||
assert by_id["ollama-cloud"]["has_key"] is True, \
|
||||
"ollama-cloud should be has_key=True when OLLAMA_API_KEY is set"
|
||||
assert by_id["ollama"]["has_key"] is False, (
|
||||
"ollama (local) must NOT be has_key=True when only the cloud env "
|
||||
"var is set — local Ollama is keyless and shares no env var with "
|
||||
"Ollama Cloud (#1410)."
|
||||
)
|
||||
# ollama-cloud should be configurable, but local ollama should not
|
||||
# (it has no env var mapping — keys go through providers.ollama.api_key
|
||||
# in config.yaml if the user explicitly opts in).
|
||||
assert by_id["ollama-cloud"]["configurable"] is True
|
||||
assert by_id["ollama"]["configurable"] is False
|
||||
finally:
|
||||
config.cfg.clear()
|
||||
config.cfg.update(old_cfg)
|
||||
config._cfg_mtime = old_mtime
|
||||
|
||||
def test_ollama_local_still_configured_via_config_yaml(
|
||||
self, monkeypatch, tmp_path,
|
||||
):
|
||||
"""providers.ollama.api_key in config.yaml should still mark local ollama configured."""
|
||||
_install_fake_hermes_cli(monkeypatch)
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path)
|
||||
# Important: clear the env var so the only signal is config.yaml.
|
||||
monkeypatch.delenv("OLLAMA_API_KEY", raising=False)
|
||||
|
||||
old_cfg = dict(config.cfg)
|
||||
old_mtime = config._cfg_mtime
|
||||
config.cfg.clear()
|
||||
config.cfg["model"] = {}
|
||||
config.cfg["providers"] = {"ollama": {"api_key": "local-token-abc"}}
|
||||
try:
|
||||
config._cfg_mtime = config.Path(config._get_config_path()).stat().st_mtime
|
||||
except Exception:
|
||||
config._cfg_mtime = 0.0
|
||||
|
||||
from api.providers import get_providers
|
||||
try:
|
||||
result = get_providers()
|
||||
by_id = {p["id"]: p for p in result["providers"]}
|
||||
assert by_id["ollama"]["has_key"] is True, (
|
||||
"Local Ollama users with providers.ollama.api_key in config.yaml "
|
||||
"should still report configured (#1410 fix must not regress this)."
|
||||
)
|
||||
# And ollama-cloud should NOT be configured by ollama's config entry.
|
||||
assert by_id["ollama-cloud"]["has_key"] is False
|
||||
finally:
|
||||
config.cfg.clear()
|
||||
config.cfg.update(old_cfg)
|
||||
config._cfg_mtime = old_mtime
|
||||
|
||||
Reference in New Issue
Block a user