fix(telegram): normalize forum topics and bot command mentions

gateway/platforms/telegram.py

@@ -4300,6 +4300,33 @@ def _is_group_chat(self, message: Message) -> bool:
         chat_type = str(getattr(chat, "type", "")).split(".")[-1].lower()
         return chat_type in {"group", "supergroup"}
 
+    @classmethod
+    def _effective_message_thread_id(cls, message: Message) -> Optional[str]:
+        """Return the routable thread id for a Telegram message.
+
+        Forum supergroup messages posted in the General topic arrive with
+        ``message_thread_id=None``, while Telegram itself addresses that topic
+        as thread id ``1``.  Private chats are the opposite footgun: Telegram
+        may put ``message_thread_id`` on ordinary DM replies, but those ids are
+        not valid send targets unless Telegram marks the message as a real topic
+        message.  Gates, skill binding, and outbound routing must agree on the
+        same normalized value.
+        """
+        chat = getattr(message, "chat", None)
+        chat_type = str(getattr(chat, "type", "")).split(".")[-1].lower() if chat else ""
+        raw = getattr(message, "message_thread_id", None)
+        is_topic_message = bool(getattr(message, "is_topic_message", False))
+        is_forum_group = chat_type in ("group", "supergroup") and getattr(chat, "is_forum", False)
+        if raw is not None:
+            if is_forum_group or (chat_type in ("group", "supergroup") and is_topic_message):
+                return str(raw)
+            if chat_type == "private" and is_topic_message:
+                return str(raw)
+            return None
+        if is_forum_group:
+            return cls._GENERAL_TOPIC_THREAD_ID
+        return None
+
     def _is_reply_to_bot(self, message: Message) -> bool:
         if not self._bot or not getattr(message, "reply_to_message", None):
             return False
@@ -4372,45 +4399,45 @@ def _iter_sources():
             yield getattr(message, "text", None) or "", getattr(message, "entities", None) or []
             yield getattr(message, "caption", None) or "", getattr(message, "caption_entities", None) or []
 
-        # Telegram parses mentions server-side and emits MessageEntity objects
-        # (type=mention for @username, type=text_mention for @FirstName targeting
-        # a user without a public username). Those entities are authoritative:
-        # raw substring matches like "foo@hermes_bot.example" are not mentions
-        # (bug #12545). Entities also correctly handle @handles inside URLs, code
-        # blocks, and quoted text, where a regex scan would over-match.
+        # Telegram parses mentions server-side and emits MessageEntity objects;
+        # trust those rather than substring-scanning the raw text. A naive
+        # ``"@hermes_bot" in text`` would over-match: ``foo@hermes_bot.example``
+        # in a URL or code block is not a mention (bug #12545), while entities
+        # correctly delimit only the addressable spans.
+        #
+        # Three entity shapes count as addressing this bot:
+        #   - ``mention``:      inline ``@botname``
+        #   - ``text_mention``: tap-mention of a user that has no @username
+        #   - ``bot_command``:  ``/cmd@botname`` — Telegram's group command
+        #                       menu emits the whole token as a single
+        #                       bot_command entity (no separate mention).
+        #                       Accept only when the ``@suffix`` matches this
+        #                       bot; reject ``/cmd`` (no suffix) and
+        #                       ``/cmd@other_bot`` so multi-bot groups stay
+        #                       disambiguated under require_mention (#15415).
         for source_text, entities in _iter_sources():
             for entity in entities:
                 entity_type = str(getattr(entity, "type", "")).split(".")[-1].lower()
-                if entity_type == "mention" and expected:
-                    offset = int(getattr(entity, "offset", -1))
-                    length = int(getattr(entity, "length", 0))
-                    if offset < 0 or length <= 0:
-                        continue
-                    if source_text[offset:offset + length].strip().lower() == expected:
-                        return True
-                elif entity_type == "text_mention":
+                if entity_type == "text_mention":
                     user = getattr(entity, "user", None)
                     if user and getattr(user, "id", None) == bot_id:
                         return True
-                elif entity_type == "bot_command" and expected:
-                    # Telegram's official group-disambiguation form for slash
-                    # commands (``/cmd@botname``) is emitted as a single
-                    # ``bot_command`` entity covering the whole span — there
-                    # is no accompanying ``mention`` entity. Treat it as a
-                    # direct address to this bot when the ``@botname`` suffix
-                    # matches. This is the form Telegram's own command menu
-                    # autocomplete produces in groups, so dropping it at the
-                    # mention gate would break /new, /reset, /help, ... for
-                    # every group that has ``require_mention`` enabled (#15415).
-                    offset = int(getattr(entity, "offset", -1))
-                    length = int(getattr(entity, "length", 0))
-                    if offset < 0 or length <= 0:
-                        continue
-                    command_text = source_text[offset:offset + length]
-                    at_index = command_text.find("@")
+                    continue
+                if not expected:
+                    continue
+                offset = int(getattr(entity, "offset", -1))
+                length = int(getattr(entity, "length", 0))
+                if offset < 0 or length <= 0:
+                    continue
+                span = source_text[offset:offset + length]
+                if entity_type == "mention":
+                    if span.strip().lower() == expected:
+                        return True
+                elif entity_type == "bot_command":
+                    at_index = span.find("@")
                     if at_index < 0:
                         continue
-                    if command_text[at_index:].strip().lower() == expected:
+                    if span[at_index:].strip().lower() == expected:
                         return True
         if bot_username and re.fullmatch(r"[a-z0-9_]{2,29}bot", bot_username, re.IGNORECASE):
             return bot_username in self._extract_bot_mention_usernames(message)
@@ -4491,7 +4518,7 @@ def _should_process_message(self, message: Message, *, is_command: bool = False)
         if not self._is_group_chat(message):
             return True
 
-        thread_id = getattr(message, "message_thread_id", None)
+        thread_id = self._effective_message_thread_id(message)
         allowed_topics = self._telegram_allowed_topics()
         if allowed_topics:
             topic_id = str(thread_id) if thread_id is not None else self._GENERAL_TOPIC_THREAD_ID
@@ -5262,29 +5289,8 @@ def _build_message_event(
         elif telegram_chat_type == "channel":
             chat_type = "channel"
 
-        # Resolve Telegram topic name and skill binding.
-        # Only preserve message_thread_id when Telegram marks the message as
-        # a real topic/forum message. Telegram can also populate
-        # message_thread_id for ordinary reply UI anchors; treating those as
-        # durable session threads fragments workflows such as CAPTCHA/login
-        # handoffs where the user later replies "done" in the same group.
-        # Private chats have the same pitfall: only real DM topic messages
-        # (is_topic_message=True) should keep the thread id, otherwise sends
-        # can hit Telegram's 'Message thread not found' error (#3206).
-        thread_id_raw = message.message_thread_id
-        is_topic_message = bool(getattr(message, "is_topic_message", False))
-        is_forum_group = getattr(chat, "is_forum", False) is True
-        thread_id_str = None
-        if thread_id_raw is not None:
-            if chat_type == "group" and (is_topic_message or is_forum_group):
-                thread_id_str = str(thread_id_raw)
-            elif chat_type == "dm" and is_topic_message:
-                thread_id_str = str(thread_id_raw)
-        # For forum groups without an explicit topic, default to the
-        # General-topic id so the gateway routes back to the General topic
-        # rather than dropping into the bot's main channel (#22423).
-        if chat_type == "group" and thread_id_str is None and is_forum_group:
-            thread_id_str = self._GENERAL_TOPIC_THREAD_ID
+        # Resolve routable thread id for DM topics and forum group topics.
+        thread_id_str = self._effective_message_thread_id(message)
         chat_topic = None
         topic_skill = None
 

tests/gateway/conftest.py

@@ -55,10 +55,14 @@ def _ensure_telegram_mock() -> None:
     mod.constants.ParseMode.MARKDOWN = "Markdown"
     mod.constants.ParseMode.MARKDOWN_V2 = "MarkdownV2"
     mod.constants.ParseMode.HTML = "HTML"
-    mod.constants.ChatType.PRIVATE = "private"
-    mod.constants.ChatType.GROUP = "group"
-    mod.constants.ChatType.SUPERGROUP = "supergroup"
-    mod.constants.ChatType.CHANNEL = "channel"
+    for chat_type_name, chat_type_value in (
+        ("PRIVATE", "private"),
+        ("GROUP", "group"),
+        ("SUPERGROUP", "supergroup"),
+        ("CHANNEL", "channel"),
+    ):
+        setattr(mod.constants.ChatType, chat_type_name, chat_type_value)
+        setattr(mod.ChatType, chat_type_name, chat_type_value)
 
     # Real exception classes so ``except (NetworkError, ...)`` clauses
     # in production code don't blow up with TypeError.

tests/gateway/test_dm_topics.py

@@ -28,10 +28,14 @@ def _ensure_telegram_mock():
     telegram_mod = MagicMock()
     telegram_mod.ext.ContextTypes.DEFAULT_TYPE = type(None)
     telegram_mod.constants.ParseMode.MARKDOWN_V2 = "MarkdownV2"
-    telegram_mod.constants.ChatType.GROUP = "group"
-    telegram_mod.constants.ChatType.SUPERGROUP = "supergroup"
-    telegram_mod.constants.ChatType.CHANNEL = "channel"
-    telegram_mod.constants.ChatType.PRIVATE = "private"
+    for chat_type_name, chat_type_value in (
+        ("GROUP", "group"),
+        ("SUPERGROUP", "supergroup"),
+        ("CHANNEL", "channel"),
+        ("PRIVATE", "private"),
+    ):
+        setattr(telegram_mod.constants.ChatType, chat_type_name, chat_type_value)
+        setattr(telegram_mod.ChatType, chat_type_name, chat_type_value)
 
     for name in ("telegram", "telegram.ext", "telegram.constants", "telegram.request"):
         sys.modules.setdefault(name, telegram_mod)
@@ -225,7 +229,7 @@ def test_persist_dm_topic_thread_id_writes_config(tmp_path):
 
     config_file = tmp_path / ".hermes" / "config.yaml"
     config_file.parent.mkdir(parents=True)
-    with open(config_file, "w") as f:
+    with open(config_file, "w", encoding="utf-8") as f:
         yaml.dump(config_data, f)
 
     adapter = _make_adapter()
@@ -234,7 +238,7 @@ def test_persist_dm_topic_thread_id_writes_config(tmp_path):
          patch.dict(os.environ, {"HERMES_HOME": str(tmp_path / ".hermes")}):
         adapter._persist_dm_topic_thread_id(111, "General", 999)
 
-    with open(config_file) as f:
+    with open(config_file, encoding="utf-8") as f:
         result = yaml.safe_load(f)
 
     topics = result["platforms"]["telegram"]["extra"]["dm_topics"][0]["topics"]
@@ -265,15 +269,15 @@ def test_persist_dm_topic_thread_id_skips_if_already_set(tmp_path):
 
     config_file = tmp_path / ".hermes" / "config.yaml"
     config_file.parent.mkdir(parents=True)
-    with open(config_file, "w") as f:
+    with open(config_file, "w", encoding="utf-8") as f:
         yaml.dump(config_data, f)
 
     adapter = _make_adapter()
 
     with patch.object(Path, "home", return_value=tmp_path):
         adapter._persist_dm_topic_thread_id(111, "General", 999)
 
-    with open(config_file) as f:
+    with open(config_file, encoding="utf-8") as f:
         result = yaml.safe_load(f)
 
     topics = result["platforms"]["telegram"]["extra"]["dm_topics"][0]["topics"]
@@ -409,7 +413,7 @@ def test_get_dm_topic_info_hot_reloads_from_config(tmp_path):
     }
     config_file = tmp_path / ".hermes" / "config.yaml"
     config_file.parent.mkdir(parents=True)
-    with open(config_file, "w") as f:
+    with open(config_file, "w", encoding="utf-8") as f:
         yaml.dump(config_data, f)
 
     with patch.object(Path, "home", return_value=tmp_path), \
@@ -449,15 +453,14 @@ def test_cache_dm_topic_from_message_no_overwrite():
 
 def _make_mock_message(chat_id=111, chat_type="private", text="hello", thread_id=None,
                        user_id=42, user_name="Test User", forum_topic_created=None,
-                       is_topic_message=None, is_forum=None):
+                       is_topic_message=None, is_forum=False):
     """Create a mock Telegram Message for _build_message_event tests."""
     chat = SimpleNamespace(
         id=chat_id,
         type=chat_type,
         title=None,
+        is_forum=is_forum,
     )
-    if is_forum is not None:
-        chat.is_forum = is_forum
     # Add full_name attribute for DM chats
     if not hasattr(chat, "full_name"):
         chat.full_name = user_name
@@ -574,10 +577,9 @@ def test_build_message_event_preserves_true_dm_topic_thread_id():
 
 # ── _build_message_event: group_topics skill binding ──
 
-# The telegram mock sets sys.modules["telegram.constants"] = telegram_mod (root mock),
-# so `from telegram.constants import ChatType` in telegram.py resolves to
-# telegram_mod.ChatType — not telegram_mod.constants.ChatType.  We must use
-# the same ChatType object the production code sees so equality checks work.
+# The telegram mock maps both ``telegram.constants.ChatType`` and root
+# ``telegram.ChatType`` to the same string values so imports in telegram.py and
+# these tests exercise the same comparisons as python-telegram-bot constants.
 from telegram.constants import ChatType as _ChatType  # noqa: E402
 
 
@@ -664,6 +666,39 @@ def test_group_topic_no_skill_binding():
     assert event.source.chat_topic == "General"
 
 
+def test_group_topic_general_topic_normalization_sets_skill_binding():
+    """Forum General-topic messages should bind using normalized thread id 1.
+
+    Telegram forum supergroup messages in the General topic arrive with
+    ``message_thread_id=None``. ``_build_message_event`` must use the same
+    effective thread id as the gating path so configured ``group_topics``
+    bindings for thread id 1 still set source metadata and auto_skill.
+    """
+    from gateway.platforms.base import MessageType
+
+    adapter = _make_adapter(group_topics_config=[
+        {
+            "chat_id": -1001234567890,
+            "topics": [
+                {"name": "General", "thread_id": 1, "skill": "daily-review"},
+            ],
+        }
+    ])
+
+    msg = _make_mock_message(
+        chat_id=-1001234567890,
+        chat_type=_ChatType.SUPERGROUP,
+        thread_id=None,
+        text="general update",
+        is_forum=True,
+    )
+    event = adapter._build_message_event(msg, MessageType.TEXT)
+
+    assert event.source.thread_id == "1"
+    assert event.source.chat_topic == "General"
+    assert event.auto_skill == "daily-review"
+
+
 def test_group_topic_unmapped_thread_id():
     """Thread ID not in config should fall through — no skill, no topic name."""
     from gateway.platforms.base import MessageType