Skip to content

fix(gateway): mark final voice reply as notify-worthy so Telegram delivers it audibly#28107

Closed
briandevans wants to merge 1 commit into
NousResearch:mainfrom
briandevans:fix/voice-reply-notify-27970
Closed

fix(gateway): mark final voice reply as notify-worthy so Telegram delivers it audibly#28107
briandevans wants to merge 1 commit into
NousResearch:mainfrom
briandevans:fix/voice-reply-notify-27970

Conversation

@briandevans

@briandevans briandevans commented May 18, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Marks Hermes's auto voice reply (the TTS-generated final response for users with voice.auto_tts enabled) as notify-worthy so Telegram delivers the final voice note as a normal push instead of a silent message. Mirrors the final-text precedent in gateway/platforms/base.py:3214, which already sets metadata["notify"] = True for the final text response. In Telegram's default notifications=important mode, TelegramPlatformAdapter._notification_kwargs() returns disable_notification=True for every send unless metadata["notify"] is truthy; the auto-TTS path through GatewayRunner._send_voice_reply was forwarding thread metadata unmodified, so the final voice note arrived silently. The fix clones the thread metadata and sets notify=True before passing it to adapter.send_voice (clone-then-mark so we don't leak notify=True back 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

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • gateway/run.py — in _send_voice_reply, clone thread_meta and set notify=True before passing it to adapter.send_voice so Telegram's _notification_kwargs does not strip the push notification. Clone-then-mark to avoid leaking notify=True into 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 gets notify=True added without mutating the source dict).
  • tests/gateway/test_voice_command.py — updated TestSendVoiceReply::test_auto_voice_reply_uses_thread_metadata_helper to assert notify: True in the dispatched metadata.

How to Test

  1. 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.
  2. Adjacent suites also pass: tests/gateway/test_tts_media_routing.py, tests/gateway/test_voice_mode_platform_isolation.py, tests/gateway/test_matrix_voice.py.
  3. Regression guard: the two new tests fail against origin/main with AssertionError: notify is None, and pass with the fix applied.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run focused tests for the touched code and all pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15.x

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Sibling Audit

Sibling code paths that may need the same fix: GatewayRunner._deliver_media_from_response (gateway/run.py:10630) also delivers final-turn voice/audio media via adapter.send_voice without notify=True. Intentionally left out of this PR's scope to keep the diff small — happy to widen if preferred.

Other platforms (Discord, Slack, Matrix) ignore metadata["notify"], so they are unaffected.

…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>
Copilot AI review requested due to automatic review settings May 18, 2026 17:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 populate metadata for voice replies, cloning existing thread metadata before adding notify=True.
  • New regression tests in tests/gateway/test_send_voice_reply_notify.py covering DM and DM-topic paths.
  • Updated existing test expectation in tests/gateway/test_voice_command.py to include notify: 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.

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter tool/tts Text-to-speech and transcription P2 Medium — degraded but workaround exists labels May 18, 2026
@BoardJames-Bot

Copy link
Copy Markdown

BoardJames CI triage for the current head (9c4a719): all non-full-suite checks are green (lint, supply-chain, both Docker builds, both Nix jobs, e2e, attribution/history). Tests / test is still in progress.

Focused local validation of this PR's changed gateway coverage passes:

python -m pytest tests/gateway/test_send_voice_reply_notify.py tests/gateway/test_voice_command.py -q -o 'addopts='
# 183 passed in 4.56s

No branch-local failure found so far. Remaining risk is only the shared long-running full-suite Tests / test lane; if it later fails with the already-seen repo-wide full-suite cancellation/timeout or unrelated main-branch failures, this PR should not churn on it.

@outsourc-e outsourc-e left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #28504. Authorship preserved. #28504

@teknium1 teknium1 closed this May 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/telegram Telegram bot adapter 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.

[Bug]: Telegram auto voice replies can fall back to MP3 attachments and silent final notifications

6 participants