fix(gateway): mark final voice reply as notify-worthy so Telegram delivers it audibly#28107
fix(gateway): mark final voice reply as notify-worthy so Telegram delivers it audibly#28107briandevans wants to merge 1 commit into
Conversation
…ivers it audibly In Telegram "important" notifications mode (default), TelegramPlatformAdapter sets ``disable_notification=True`` on every send unless metadata carries ``notify=True``. GatewayRunner._send_voice_reply already passes thread metadata through to ``adapter.send_voice``, but never marks the final auto-TTS voice reply as notify-worthy — so users with the default mode get the final voice note delivered silently with no push notification. Mirror the final-text path in gateway/platforms/base.py (the existing text-response final send already adds ``metadata["notify"] = True``). Issue NousResearch#27970 Bug 2. Bug 1 (MP3 vs. native OGG voice-note) is being addressed by existing PRs NousResearch#20182 / NousResearch#20878 — this PR is intentionally scoped to the silent-delivery bug only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds notify=True to the metadata of auto voice replies in GatewayRunner._send_voice_reply so push-gating adapters (e.g., Telegram "important" mode) deliver them as normal notifications, matching the final-text path. Includes regression tests.
Changes:
- In
gateway/run.py, always populatemetadatafor voice replies, cloning existing thread metadata before addingnotify=True. - New regression tests in
tests/gateway/test_send_voice_reply_notify.pycovering DM and DM-topic paths. - Updated existing test expectation in
tests/gateway/test_voice_command.pyto includenotify: True.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| gateway/run.py | Clone thread metadata and set notify=True for auto voice replies. |
| tests/gateway/test_send_voice_reply_notify.py | New tests verifying notify flag and non-mutation of source metadata. |
| tests/gateway/test_voice_command.py | Updated expected metadata to include notify: True. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
BoardJames CI triage for the current head ( Focused local validation of this PR's changed gateway coverage passes: No branch-local failure found so far. Remaining risk is only the shared long-running full-suite |
outsourc-e
left a comment
There was a problem hiding this comment.
Reviewed locally on macOS. I ran python -m pytest tests/gateway/test_voice_command.py tests/gateway/test_send_voice_reply_notify.py -q and got 183 passing tests. Diff is narrow, behavior matches the existing final-text notify pattern, and I don’t see security concerns in the metadata copy + notify flag approach.
What does this PR do?
Marks Hermes's auto voice reply (the TTS-generated final response for users with
voice.auto_ttsenabled) as notify-worthy so Telegram delivers the final voice note as a normal push instead of a silent message. Mirrors the final-text precedent ingateway/platforms/base.py:3214, which already setsmetadata["notify"] = Truefor the final text response. In Telegram's defaultnotifications=importantmode,TelegramPlatformAdapter._notification_kwargs()returnsdisable_notification=Truefor every send unlessmetadata["notify"]is truthy; the auto-TTS path throughGatewayRunner._send_voice_replywas forwarding thread metadata unmodified, so the final voice note arrived silently. The fix clones the thread metadata and setsnotify=Truebefore passing it toadapter.send_voice(clone-then-mark so we don't leaknotify=Trueback into the typing-indicator state that shares this metadata).Related Issue
Fixes #27970 (Bug 2 of 2 — the silent-final-voice-reply piece; Bug 1, MP3 vs. native OGG fallback, is already covered by open PRs #20182 and #20878 per alt-glitch's pointer on the issue thread, and the reporter agreed to split).
Type of Change
Changes Made
gateway/run.py— in_send_voice_reply, clonethread_metaand setnotify=Truebefore passing it toadapter.send_voiceso Telegram's_notification_kwargsdoes not strip the push notification. Clone-then-mark to avoid leakingnotify=Trueinto the shared typing-indicator metadata.tests/gateway/test_send_voice_reply_notify.py— new regression file with two cases (DM with no thread metadata gets a fresh{"notify": True}; DM topic with existing thread metadata getsnotify=Trueadded without mutating the source dict).tests/gateway/test_voice_command.py— updatedTestSendVoiceReply::test_auto_voice_reply_uses_thread_metadata_helperto assertnotify: Truein the dispatched metadata.How to Test
uv run --with pytest --with pytest-xdist --with pytest-asyncio python3 -m pytest tests/gateway/test_voice_command.py tests/gateway/test_send_voice_reply_notify.py -v— 8 passed.tests/gateway/test_tts_media_routing.py,tests/gateway/test_voice_mode_platform_isolation.py,tests/gateway/test_matrix_voice.py.origin/mainwithAssertionError: notify is None, and pass with the fix applied.Checklist
Code
fix(scope):,feat(scope):, etc.)Documentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/ASibling Audit
Other platforms (Discord, Slack, Matrix) ignore
metadata["notify"], so they are unaffected.