Skip to content

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

Merged
kshitijk4poor merged 2 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/telegram-dm-topic-notify-34144
May 31, 2026
Merged

fix(gateway): preserve Telegram DM topic routing metadata in synthetic notifications#36028
kshitijk4poor merged 2 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/telegram-dm-topic-notify-34144

Conversation

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Summary

Salvage of #34144 (@Dusk1e) onto current main, plus a hardening follow-up.

Synthetic gateway notifications — /update, /restart, shutdown warnings, and home-channel startup pings — built their Telegram send metadata inline as {"thread_id": thread_id}, bypassing the richer DM-topic routing metadata that normal replies carry. For Telegram private DM topic lanes, a thread-id-only payload with no reply anchor is rejected by the Bot API send path:

Telegram DM topic delivery requires a reply anchor; refusing to send outside the requested topic

so the notification was silently dropped (observed on /restart and home-channel startup).

What changed

  • Centralize synthetic-send metadata through a shared _thread_metadata_for_target() helper (the existing _thread_metadata_for_source() now delegates to it, with byte-identical behavior for its 18 callers).
  • Preserve chat_type in the /update and /restart marker payloads so post-restart paths can reconstruct DM-topic routing.
  • For DM-topic targets, attach the anchorless routing keys (telegram_dm_topic_reply_fallback, direct_messages_topic_id). The Telegram adapter's _is_private_dm_topic_send() short-circuits to False when direct_messages_topic_id is present, so the send routes via the topic id and no longer requires a reply anchor.
  • Keep home-channel startup notifications on the same metadata path.
  • Tighten regression coverage to assert the full DM-topic metadata, not just thread_id.

Follow-up commit (hardening)

The contributor's _is_telegram_dm_topic_target probed adapter._get_dm_topic_info via instance-level getattr. A MagicMock auto-creates that attribute as a truthy callable, so any test double with a non-dm chat_type + a thread_id would be misclassified as a DM topic lane and get the fallback keys injected. The follow-up resolves the method on type(adapter) and treats only dict-shaped returns as an operator-declared topic — mirroring the existing guard in _rename_telegram_topic_for_session_title. The home-channel test is updated to declare _get_dm_topic_info on a real adapter subclass instead of patching a MagicMock onto the instance.

Why no message_id anchor was needed

The reported root cause suggested storing event.message_id as a reply anchor. That works, but the adapter already supports an anchorless routing path for synthetic sends (introduced in #27937): when direct_messages_topic_id is present, _is_private_dm_topic_send() returns False and _thread_kwargs_for_send() returns {"message_thread_id": None, "direct_messages_topic_id": <tid>}. This fix uses that path, which is the correct mechanism for sends that have no originating message to reply to.

Testing

  • Cherry-pick applies cleanly onto current main (+121/-25, matching the original PR).
  • tests/gateway/test_update_command.py + tests/gateway/test_restart_notification.py: 53 passed.
  • Full sweep across the 13 test files exercising the refactored helper: 489 passed.
  • ruff clean, py_compile clean.
  • E2E: reproduced the exact failure — wrote a .restart_notify.json marker for a DM topic (thread 777, chat_type dm), ran _send_restart_notification(), fed the resulting metadata into the real TelegramAdapter decision logic. On main _is_private_dm_topic_sendTrue (→ the error); with this PR → False, routing via direct_messages_topic_id. Confirmed the notification now delivers.
  • E2E (hardening): a bare-MagicMock adapter with a non-dm chat_type is no longer misclassified; real adapters returning a dict / None behave correctly.

Closes #34144

Co-authored-by: Dusk1e yusufalweshdemir@gmail.com

Dusk1e and others added 2 commits June 1, 2026 00:15
Follow-up to the synthetic-notification DM-topic routing fix. The new
_is_telegram_dm_topic_target probed the adapter's _get_dm_topic_info via
instance-level getattr, which a MagicMock auto-creates as a truthy callable —
so any test double with a non-dm chat_type and a thread_id would be
misclassified as a DM topic lane and have the fallback routing keys injected.

Resolve the method on type(adapter) and treat only dict-shaped returns as an
operator-declared topic, mirroring the existing guard in
_rename_telegram_topic_for_session_title. Update the home-channel startup test
to declare _get_dm_topic_info on a real adapter subclass instead of patching a
MagicMock onto the instance.
@kshitijk4poor
kshitijk4poor enabled auto-merge (rebase) May 31, 2026 18:49
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter labels May 31, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Supersedes #34144 — salvage onto current main with additional hardening (MagicMock DM-topic misclassification fix). Related to merged #28488 and #28487.

@kshitijk4poor
kshitijk4poor disabled auto-merge May 31, 2026 19:13
@kshitijk4poor
kshitijk4poor merged commit eb3cf97 into NousResearch:main May 31, 2026
21 of 22 checks passed
@kshitijk4poor
kshitijk4poor deleted the salvage/telegram-dm-topic-notify-34144 branch May 31, 2026 19:14
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