diff --git a/CHANGELOG.md b/CHANGELOG.md index c07c5542..37bd4330 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 `..//` 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 diff --git a/api/providers.py b/api/providers.py index a36a935b..424a7927 100644 --- a/api/providers.py +++ b/api/providers.py @@ -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", } diff --git a/static/panels.js b/static/panels.js index f4c9ef4f..2d325edc 100644 --- a/static/panels.js +++ b/static/panels.js @@ -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(){ diff --git a/static/style.css b/static/style.css index f12693f2..b6f37d43 100644 --- a/static/style.css +++ b/static/style.css @@ -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}} diff --git a/tests/test_499_tts_playback.py b/tests/test_499_tts_playback.py index 21630dc9..efbc1933 100644 --- a/tests/test_499_tts_playback.py +++ b/tests/test_499_tts_playback.py @@ -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[^}]*)\}', + 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)." + ) diff --git a/tests/test_provider_management.py b/tests/test_provider_management.py index 05adb75c..002f6ade 100644 --- a/tests/test_provider_management.py +++ b/tests/test_provider_management.py @@ -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