Skip to content

fix(gateway): skip the auto-TTS voice reply for the desktop platform - #91249

Open
liuhao1024 wants to merge 2 commits into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-90297
Open

liuhao1024 wants to merge 2 commits into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-90297

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Stops the double auto-TTS playback on the desktop surface. When voice.auto_tts is enabled, two independent paths synthesize and play the same reply on the desktop: the gateway's _send_voice_reply (calling text_to_speech_tool and delivering via adapter.send_voice) and the desktop app's useAutoSpeakReplies hook (calling /api/audio/speak → playSpeechText). Neither knows about the other, so every desktop reply plays twice — logs show one "Generating speech" but two "TTS audio saved" entries per reply from two different PIDs (#90297).

_should_send_voice_reply now returns False for the desktop platform, deferring to the desktop's own hook (the authoritative speaker for that surface — it fires on every reply regardless of mode, and the gateway has no way to know whether the client spoke). This is the issue's first suggested fix; the alternative (teaching the hook to detect an already-spoken gateway message) adds cross-process coordination the simple skip avoids. Every other platform keeps its gateway voice reply unchanged.

Related Issue

Fixes #90297

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • gateway/run.py — _should_send_voice_reply skips event.source.platform.value == "desktop" right after the mode check, with the double-synthesis mechanism documented inline (auto_tts plays audio twice on desktop — gateway _send_voice_reply and useAutoSpeakReplies both fire #90297)
  • tests/gateway/test_desktop_no_gateway_reply.py — three regressions: desktop is skipped even when global auto_tts would qualify; desktop is skipped even with an explicit all mode for the chat (the hook fires regardless); and Telegram with auto_tts on still gets its gateway voice reply (the skip is desktop-only)

How to Test

  1. .venv/bin/python -m pytest tests/gateway/test_desktop_no_gateway_voice_reply.py -q — should pass (3 passed)
  2. Observed result: on unmodified main (verified via git stash control), both desktop tests fail — _should_send_voice_reply returns True for the desktop platform and the gateway sends a voice reply the desktop hook will speak again; with this fix both shapes return False
  3. Regression: tests/gateway/test_auto_voice_reply_format.py + tests/gateway/test_voice_mode_platform_isolation.py — 10 passed (voice-reply format selection and per-platform mode isolation unchanged)

Checklist

  • Code follows the project's style guidelines
  • Self-review completed
  • Comments added for complex logic (why the desktop hook is authoritative and the gateway cannot know whether the client spoke)
  • Tests added that prove the fix (both desktop shapes + other-platform guard)
  • All new and existing tests pass locally (3 + 10 passed; main-control fail confirmed)
  • Platform: macOS (pure predicate logic, platform-independent)

With voice.auto_tts on, the gateway and the desktop's useAutoSpeakReplies
hook each synthesize and play the same reply — two TTS generations per
message. The desktop hook is the authoritative speaker for that surface;
the gateway now steps out for platform 'desktop'.

Fixes NousResearch#90297
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery comp/desktop Electron desktop app (apps/desktop/*) tool/tts Text-to-speech and transcription sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Aug 21, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

Correct fix for the double-speak (#90297): the guard sits after the mode/auto-TTS gate and before any return True, so it covers every qualifying path, and the tests pin all three mode combinations. Suggestions:

  1. gateway/run.py:21976 — the skip is completely silent, unlike every other exit in _should_send_voice_reply which logs "Auto voice reply skipped: ...". Add the same log line (with reason="desktop surface speaks locally") so a user who enabled /voice all on desktop and hears nothing isn't left debugging a mystery.
  2. gateway/run.py:21976 — explicit per-chat /voice all is now silently ignored on desktop: there is no remaining way to get a gateway-synthesized reply on that surface even when the client hook is disabled. If that's intended, document it next to the voice-mode command help; otherwise consider keying the skip off the desktop hook being active instead of the platform alone.
  3. gateway/run.py:21976 — magic string "desktop" is compared here and re-spelled in three test places plus wherever the session routing layer produces it. Promote a shared constant (or a real Platform member) so a future case drift ("Desktop", "desktop-app") can't silently disable the guard.
  4. tests/gateway/test_desktop_no_gateway_voice_reply.py:18 — from types import SimpleNamespace # noqa: F401 (docstring reference only) is dead code; imports don't serve docstrings. Drop it.
  5. tests/gateway/test_desktop_no_gateway_voice_reply.py:24 — _DesktopPlatform(str) with .value returning self is a clever stand-in; since the stated root cause is that the value comes from the session routing layer, importing/borrowing that layer's own factory would keep the stub honest if the runtime shape ever changes. Acceptable as-is with the docstring rationale. (nit)

No blocking issues found.

@liuhao1024

Copy link
Copy Markdown
Contributor Author

Thanks — addressed in d1e9047:

  • Silent skip (point 1): fixed. The desktop exit now logs Auto voice reply skipped: desktop surface speaks locally (mode=%s chat=%s) at debug, matching the other exits in _should_send_voice_reply.
  • Dead import (point 4): dropped.
  • /voice all having no desktop effect even with the client hook off (point 2) and the shared "desktop" constant (point 3): both change behavior/scope beyond this fix — whether the skip should key off the desktop hook being active rather than the platform, and promoting a shared constant across the session routing layer, are maintainer calls. Noted here so they aren't lost; happy to follow up on either if a direction is preferred.
  • Test stub (point 5): keeping as-is with the docstring rationale, per your note.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages tool/tts Text-to-speech and transcription type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

auto_tts plays audio twice on desktop — gateway _send_voice_reply and useAutoSpeakReplies both fire

3 participants