Skip to content

fix(gateway): preserve Telegram DM topic routing metadata in synthetic notifications - #34144

Closed
Dusk1e wants to merge 1 commit into
NousResearch:mainfrom
Dusk1e:fix/telegram-synthetic-notification-topic-routing
Closed

fix(gateway): preserve Telegram DM topic routing metadata in synthetic notifications#34144
Dusk1e wants to merge 1 commit into
NousResearch:mainfrom
Dusk1e:fix/telegram-synthetic-notification-topic-routing

Conversation

@Dusk1e

@Dusk1e Dusk1e commented May 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Preserve Telegram DM topic routing metadata in synthetic gateway notifications.

This change fixes a routing gap where synthetic notifications such as /update, /restart, and startup home-channel messages could bypass the shared Telegram DM topic metadata path and send with only thread_id. For Telegram DM topic lanes, that could route notifications outside the intended topic.

What Changed

  • Centralized synthetic thread metadata generation through the same helper logic used by normal thread-aware replies.
  • Preserved chat_type in update and restart marker payloads so post-restart notification paths can reconstruct Telegram DM topic routing correctly.
  • Added Telegram DM topic fallback metadata for synthetic sends when the target is a DM topic:
    • telegram_dm_topic_reply_fallback
    • direct_messages_topic_id
  • Kept home-channel startup notifications on the same metadata path so configured Telegram DM topic homes stay correctly routed.
  • Tightened regression coverage to assert the full Telegram DM topic metadata, not just thread_id.

Testing

Focused regression verification:

  • tests/gateway/test_update_command.py::TestHandleUpdateCommand::test_writes_pending_marker — passed
  • tests/gateway/test_update_command.py::TestHandleUpdateCommand::test_writes_pending_marker_with_thread_id — passed
  • tests/gateway/test_update_command.py::TestSendUpdateNotification::test_sends_notification_with_thread_metadata — passed
  • tests/gateway/test_update_command.py::TestSendUpdateNotification::test_defers_notification_while_update_still_running — passed
  • tests/gateway/test_update_command.py::TestSendUpdateNotification::test_sends_generic_message_when_no_output — passed
  • tests/gateway/test_restart_notification.py::test_restart_command_writes_notify_file — passed
  • tests/gateway/test_restart_notification.py::test_restart_command_preserves_thread_id — passed
  • tests/gateway/test_restart_notification.py::test_send_home_channel_startup_notification_preserves_thread_metadata — passed
  • tests/gateway/test_restart_notification.py::test_send_restart_notification_with_thread — passed
  • tests/gateway/test_restart_notification.py::test_shutdown_notifications_use_cached_live_thread_source_when_origin_missing — passed

Focused regression result:

  • 10 passed

Broader targeted suite command:

pytest tests/gateway/test_update_command.py tests/gateway/test_restart_notification.py -v --timeout-method=thread

Broader targeted suite result:

  • 53 collected
  • 47 passed
  • 6 failed

Remaining failures from the broader targeted suite:

  • tests/gateway/test_update_command.py::TestHandleUpdateCommand::test_spawns_setsid
  • tests/gateway/test_update_command.py::TestHandleUpdateCommand::test_fallback_when_no_setsid
  • tests/gateway/test_update_command.py::TestSendUpdateNotification::test_sends_notification_with_output
  • tests/gateway/test_update_command.py::TestSendUpdateNotification::test_strips_ansi_codes
  • tests/gateway/test_update_command.py::TestSendUpdateNotification::test_cleans_up_files_after_notification
  • tests/gateway/test_update_command.py::TestSendUpdateNotification::test_cleans_up_on_error

Notes

The scoped regression coverage for this fix is green. The remaining failures are outside the routing change itself and should be handled separately.

@Dusk1e
Dusk1e marked this pull request as draft May 28, 2026 23:11
@Dusk1e
Dusk1e marked this pull request as ready for review May 28, 2026 23:11
@alt-glitch alt-glitch added P2 Medium — degraded but workaround exists type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter labels May 29, 2026
@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Merged via #36028 — your commit was cherry-picked onto current main with authorship preserved (it was ~320 commits behind, so a salvage was cleaner than rebasing the branch).

This also fixes the /restart + home-channel "Telegram DM topic delivery requires a reply anchor; refusing to send outside the requested topic" failure: the direct_messages_topic_id metadata you added lets _is_private_dm_topic_send route the notification through the anchorless DM-topic path, so no reply anchor is needed.

I added one small follow-up on top: _is_telegram_dm_topic_target now resolves _get_dm_topic_info on the adapter class (type(adapter)) and treats only dict-shaped returns as a declared topic — instance-level getattr was a false positive for MagicMock test doubles. Mirrors the existing guard in _rename_telegram_topic_for_session_title.

Thanks for the fix! 🙏

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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants