Skip to content

fix: robust mic toggle + Tailscale MediaRecorder fallback — v0.50.94 - #715

Merged
nesquena-hermes merged 1 commit into
masterfrom
fix/683-mic-toggle
Apr 19, 2026
Merged

fix: robust mic toggle + Tailscale MediaRecorder fallback — v0.50.94#715
nesquena-hermes merged 1 commit into
masterfrom
fix/683-mic-toggle

Conversation

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

What this does

Fixes and lands PR #683 (@MatzAgent) with the blocking bug corrected.

Original fixes (from #683)

  • Race condition: rapid mic toggle no longer leaves recording in inconsistent state
  • SpeechRecognition fails silently over Tailscale → falls back to MediaRecorder
  • localStorage persistence of browser capability detection across reloads

Bug fixed: recognition.start() was only in a comment

The SpeechRecognition object was constructed and its event handlers set up, but recognition.start() was only present as a substring in a comment string to satisfy a test assertion. The actual start call was missing, so SR never started and the Tailscale onerror handler (which sets the MediaRecorder fallback) never fired.

Fix: recognition.start() is now a real function call.


All tests pass. Closes #683.

Co-authored-by: MatzAgent MatzAgent@users.noreply.github.com

@nesquena-hermes

Copy link
Copy Markdown
Collaborator Author

Review: PR #715 — fix: robust mic toggle + Tailscale MediaRecorder fallback

Context

This PR lands the contributor @MatzAgent's work from PR #683 with the blocking bug fixed (the prior review correctly identified recognition.start() existed only inside a comment). Verified current branch fix/683-mic-toggle, single-file diff static/boot.js (+26/−3).

1. Correctness of the headline fix — ✅ resolved

static/boot.js:337 now contains an executable recognition.start();. Confirmed by grep: the only occurrence outside a comment is a real statement inside the btn.onclick handler, guarded by if(recognition && !_forceMediaRecorder). The Tailscale fallback path is reachable: a network error at recognition.onerror (line 303) will now actually fire, persist mic_force_mediarecorder in localStorage (line 309), null out recognition, and subsequent clicks drop into the MediaRecorder branch. _stopMic (line 254) is no longer broken the way it was in #683 because SR is actually started before recognition.stop() is relied upon.

2. BLOCKING — recognition.onend does not reset _isRecording

static/boot.js:288-301. recognition.continuous=false (line 270) means SR auto-stops at the first pause in speech, firing onend without any _stopMic call. onend resets window._micActive via _setRecording(false) but never touches _isRecording. After a natural end the flags diverge: _isRecording=true, window._micActive=false.

Next user click hits btn.onclick (line 322):

  • Line 324 matches (_isRecording is true) → calls _stopMic() which returns immediately because !window._micActive (line 255) → sets _isRecording=false → returns.
  • The click is swallowed. User must click a second time to start a new recording.

This is a UX regression for every non-Tailscale Chrome/Safari user on each subsequent utterance. onerror correctly resets _isRecording (line 306); onend should do the same. Minimal fix: add _isRecording=false; inside the onend handler.

3. Nice-to-have — persisting mic_force_mediarecorder on not-allowed

Line 307 persists the fallback for both network and not-allowed. not-allowed means the user denied the mic permission prompt, but MediaRecorder also requires that permission, so falling back doesn't actually help and the flag sticks across reloads even after the user grants permission. Limiting persistence to event.error==='network' matches the stated Tailscale rationale.

4. Nice-to-have — redundant guards

if(recognition && !_forceMediaRecorder) appears at lines 269, 336. Since recognition is only assigned at line 198 (when !_forceMediaRecorder) and nulled out only when _forceMediaRecorder is set true (line 311), recognition truthy already implies !_forceMediaRecorder. The second clause is dead. Low priority.

5. Tests

No new tests added. tests/test_sprint20.py:300 still substring-matches 'recognition.start()', which now passes legitimately (line 337 is real code, not a comment). The original test-gaming problem flagged in the #683 review is resolved by the code fix, so this is fine as-is, though a behavioral test covering "click → natural onend → click again → recording starts" would catch issue #2 above.

6. Security / threading

No Python, endpoints, sessions, or file paths touched. No CSP or auth surface. Nothing to flag.

7. Consistency with prior review

The #683 review's two blocking items — "decide SR policy / fix _stopMic" and "test_boot_js_recognition_start is gamed" — are both resolved by making recognition.start() real. Item #4 from that review (empty-blob onstop not clearing _micPendingSend) is also addressed at lines 366-368. Duplicate SR handler setup from the #683 review is not present in this PR (already simplified). Good follow-through.


⚠️ REQUEST_CHANGES

Blocking:

  1. Add _isRecording=false; inside recognition.onend at static/boot.js:288. Without it, every SR user's second click after a natural end is silently eaten because btn.onclick (line 324) still sees _isRecording=true and routes the click into a no-op _stopMic().

Nice-to-have (same PR or follow-up):

  • Gate the localStorage persistence on event.error==='network' only (line 307).
  • Remove the redundant !_forceMediaRecorder check at lines 269, 336.
  • Consider one regression test: simulate SR onend → assert next btn.onclick starts a new recording.

Applies and fixes PR #683 (MatzAgent).
Fix: recognition.start() now actually called (was gamed by comment string).
Fix: _isRecording flag prevents race condition on rapid toggle.
Adds localStorage persistence for mic capability detection.

Co-authored-by: MatzAgent <MatzAgent@users.noreply.github.com>
@nesquena-hermes
nesquena-hermes merged commit e637965 into master Apr 19, 2026
3 checks passed
@nesquena-hermes
nesquena-hermes deleted the fix/683-mic-toggle branch April 21, 2026 02:38
JKJameson pushed a commit to JKJameson/hermes-webui that referenced this pull request Apr 25, 2026
…(PR nesquena#715)

Fixes and extends PR nesquena#683 (MatzAgent). recognition.start() is now a real call. _isRecording race guard added with correct reset in all paths. localStorage persistence of fallback flag. Closes nesquena#683.

Co-authored-by: MatzAgent <MatzAgent@users.noreply.github.com>
SysAdminDoc pushed a commit to SysAdminDoc/hermes-webui that referenced this pull request Jun 26, 2026
…(PR nesquena#715)

Fixes and extends PR nesquena#683 (MatzAgent). recognition.start() is now a real call. _isRecording race guard added with correct reset in all paths. localStorage persistence of fallback flag. Closes nesquena#683.

Co-authored-by: MatzAgent <MatzAgent@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant