fix(gateway): register Telegram commands for groups
Register Telegram bot commands across default, private, and group scopes so the slash-command menu is available outside DMs. Changes from review feedback: - Add asyncio.Lock to prevent race condition in _ensure_forum_commands - Extract MAX_COMMANDS_PER_SCOPE constant (30) to avoid magic number - Upgrade error logging from debug->warning in forum registration - Add tests covering lazy forum registration and concurrent safety - Remove /start handler from this PR (separate feature) Fixes review: needs_work (race, magic number, log levels, missing tests)
This commit is contained in:
@@ -102,6 +102,9 @@ _TELEGRAM_IMAGE_EXT_TO_MIME = {
|
||||
}
|
||||
|
||||
|
||||
MAX_COMMANDS_PER_SCOPE = 30
|
||||
|
||||
|
||||
def check_telegram_requirements() -> bool:
|
||||
"""Check if Telegram dependencies are available.
|
||||
|
||||
@@ -425,6 +428,10 @@ class TelegramAdapter(BasePlatformAdapter):
|
||||
self._polling_error_callback_ref = None
|
||||
# DM Topics: map of topic_name -> message_thread_id (populated at startup)
|
||||
self._dm_topics: Dict[str, int] = {}
|
||||
# Track forum chats where we've already registered bot commands
|
||||
self._forum_command_registered: set[int] = set()
|
||||
# Lock per la registrazione sicura dei comandi nei forum supergroup
|
||||
self._forum_lock = asyncio.Lock()
|
||||
# DM Topics config from extra.dm_topics
|
||||
self._dm_topics_config: List[Dict[str, Any]] = self.config.extra.get("dm_topics", [])
|
||||
# Interactive model picker state per chat
|
||||
@@ -1492,19 +1499,37 @@ class TelegramAdapter(BasePlatformAdapter):
|
||||
# List is derived from the central COMMAND_REGISTRY — adding a new
|
||||
# gateway command there automatically adds it to the Telegram menu.
|
||||
try:
|
||||
from telegram import BotCommand
|
||||
from telegram import (
|
||||
BotCommand,
|
||||
BotCommandScopeAllPrivateChats,
|
||||
BotCommandScopeAllGroupChats,
|
||||
BotCommandScopeDefault,
|
||||
BotCommandScopeChat,
|
||||
)
|
||||
from hermes_cli.commands import telegram_menu_commands
|
||||
# Telegram allows up to 100 commands but has an undocumented
|
||||
# payload size limit. Skill descriptions are truncated to 40
|
||||
# chars in telegram_menu_commands() to fit 100 commands safely.
|
||||
menu_commands, hidden_count = telegram_menu_commands(max_commands=100)
|
||||
await self._bot.set_my_commands([
|
||||
BotCommand(name, desc) for name, desc in menu_commands
|
||||
])
|
||||
# payload size limit (~4KB total). Limit to 30 core commands
|
||||
# to stay well under the threshold while covering all categories.
|
||||
menu_commands, hidden_count = telegram_menu_commands(max_commands=MAX_COMMANDS_PER_SCOPE)
|
||||
bot_commands = [BotCommand(name, desc) for name, desc in menu_commands]
|
||||
# Register for all scopes independently — Telegram picks the
|
||||
# narrowest matching scope per chat type (forum topics fall
|
||||
# through to AllGroupChats or Default).
|
||||
for scope_cls in (BotCommandScopeDefault, BotCommandScopeAllPrivateChats, BotCommandScopeAllGroupChats):
|
||||
scope_name = scope_cls.__name__
|
||||
try:
|
||||
await self._bot.set_my_commands(bot_commands, scope=scope_cls())
|
||||
logger.info("[%s] set_my_commands OK for scope %s (%d cmds)", self.name, scope_name, len(bot_commands))
|
||||
except Exception as scope_err:
|
||||
logger.warning("[%s] set_my_commands FAILED for scope %s: %s", self.name, scope_name, scope_err)
|
||||
# Forum topics don't inherit AllGroupChats — Telegram resolves
|
||||
# commands via BotCommandScopeChat(chat_id) for forum groups.
|
||||
# Lazy registration happens in _ensure_forum_commands on first
|
||||
# message from a forum topic (see _handle_text_message).
|
||||
if hidden_count:
|
||||
logger.info(
|
||||
"[%s] Telegram menu: %d commands registered, %d hidden (over 100 limit). Use /commands for full list.",
|
||||
self.name, len(menu_commands), hidden_count,
|
||||
"[%s] Telegram menu: %d commands registered, %d hidden (over %d limit). Use /commands for full list.",
|
||||
self.name, len(menu_commands), hidden_count, 30,
|
||||
)
|
||||
except Exception as e:
|
||||
logger.warning(
|
||||
@@ -4174,6 +4199,31 @@ class TelegramAdapter(BasePlatformAdapter):
|
||||
return True
|
||||
return self._message_matches_mention_patterns(message)
|
||||
|
||||
async def _ensure_forum_commands(self, message) -> None:
|
||||
"""Lazy-register bot commands for forum supergroups.
|
||||
|
||||
Forum topics don't inherit AllGroupChats scope — Telegram resolves
|
||||
via BotCommandScopeChat(chat_id). Register on first message so the
|
||||
command menu works in topic views.
|
||||
"""
|
||||
async with self._forum_lock:
|
||||
try:
|
||||
chat = getattr(message, "chat", None)
|
||||
if not chat or not getattr(chat, "is_forum", False):
|
||||
return
|
||||
chat_id = int(chat.id)
|
||||
if chat_id in self._forum_command_registered:
|
||||
return
|
||||
from telegram import BotCommand, BotCommandScopeChat
|
||||
from hermes_cli.commands import telegram_menu_commands
|
||||
menu_commands, _ = telegram_menu_commands(max_commands=MAX_COMMANDS_PER_SCOPE)
|
||||
bot_commands = [BotCommand(name, desc) for name, desc in menu_commands]
|
||||
await self._bot.set_my_commands(bot_commands, scope=BotCommandScopeChat(chat_id=chat_id))
|
||||
self._forum_command_registered.add(chat_id)
|
||||
logger.info("[%s] Lazy-registered %d commands for forum chat %s", self.name, len(bot_commands), chat_id)
|
||||
except Exception as e:
|
||||
logger.warning("[%s] Forum command lazy-registration failed: %s", self.name, e)
|
||||
|
||||
async def _handle_text_message(self, update: Update, context: ContextTypes.DEFAULT_TYPE) -> None:
|
||||
"""Handle incoming text messages.
|
||||
|
||||
@@ -4185,6 +4235,7 @@ class TelegramAdapter(BasePlatformAdapter):
|
||||
return
|
||||
if not self._should_process_message(update.message):
|
||||
return
|
||||
await self._ensure_forum_commands(update.message)
|
||||
|
||||
event = self._build_message_event(update.message, MessageType.TEXT, update_id=update.update_id)
|
||||
event.text = self._clean_bot_trigger_text(event.text)
|
||||
@@ -4196,6 +4247,7 @@ class TelegramAdapter(BasePlatformAdapter):
|
||||
return
|
||||
if not self._should_process_message(update.message, is_command=True):
|
||||
return
|
||||
await self._ensure_forum_commands(update.message)
|
||||
|
||||
event = self._build_message_event(update.message, MessageType.COMMAND, update_id=update.update_id)
|
||||
await self.handle_message(event)
|
||||
|
||||
@@ -9515,7 +9515,6 @@ class GatewayRunner:
|
||||
)
|
||||
|
||||
async def _handle_commands_command(self, event: MessageEvent) -> str:
|
||||
"""Handle /commands [page] - paginated list of all commands and skills."""
|
||||
from hermes_cli.commands import gateway_help_lines
|
||||
|
||||
raw_args = event.get_command_args().strip()
|
||||
|
||||
110
tests/gateway/test_telegram_forum_commands.py
Normal file
110
tests/gateway/test_telegram_forum_commands.py
Normal file
@@ -0,0 +1,110 @@
|
||||
"""Tests for lazy forum command registration in TelegramAdapter."""
|
||||
|
||||
import asyncio
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from gateway.config import Platform, PlatformConfig
|
||||
|
||||
|
||||
def _make_test_adapter():
|
||||
"""Build a TelegramAdapter without running __init__."""
|
||||
from gateway.platforms.telegram import TelegramAdapter
|
||||
|
||||
adapter = object.__new__(TelegramAdapter)
|
||||
adapter.platform = Platform.TELEGRAM
|
||||
adapter.config = PlatformConfig(enabled=True, token="***", extra={})
|
||||
adapter.name = "test-telegram"
|
||||
adapter._bot = AsyncMock(spec=["set_my_commands"])
|
||||
adapter._forum_command_registered = set()
|
||||
adapter._forum_lock = asyncio.Lock()
|
||||
return adapter
|
||||
|
||||
|
||||
def _forum_message(chat_id=-100, is_forum=True):
|
||||
return SimpleNamespace(
|
||||
chat=SimpleNamespace(id=chat_id, is_forum=is_forum),
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_ensure_forum_commands_skips_non_forum():
|
||||
adapter = _make_test_adapter()
|
||||
msg = _forum_message(is_forum=False)
|
||||
await adapter._ensure_forum_commands(msg)
|
||||
adapter._bot.set_my_commands.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_ensure_forum_commands_skips_already_registered():
|
||||
adapter = _make_test_adapter()
|
||||
adapter._forum_command_registered.add(-100)
|
||||
msg = _forum_message(is_forum=True)
|
||||
await adapter._ensure_forum_commands(msg)
|
||||
adapter._bot.set_my_commands.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_ensure_forum_commands_registers_once():
|
||||
adapter = _make_test_adapter()
|
||||
msg = _forum_message(chat_id=-123, is_forum=True)
|
||||
|
||||
with patch("gateway.platforms.telegram.telegram_menu_commands") as mock_menu:
|
||||
mock_menu.return_value = ([("new", "Start new session"), ("help", "Show help")], 0)
|
||||
with patch("telegram.BotCommand") as MockBotCommand:
|
||||
instances = []
|
||||
|
||||
def _make_cmd(name, desc):
|
||||
cmd = MagicMock()
|
||||
cmd.name = name
|
||||
cmd.description = desc
|
||||
instances.append(cmd)
|
||||
return cmd
|
||||
|
||||
MockBotCommand.side_effect = _make_cmd
|
||||
with patch("telegram.BotCommandScopeChat") as MockScope:
|
||||
await adapter._ensure_forum_commands(msg)
|
||||
|
||||
assert -123 in adapter._forum_command_registered
|
||||
adapter._bot.set_my_commands.assert_awaited_once()
|
||||
args, kwargs = adapter._bot.set_my_commands.call_args
|
||||
assert len(args[0]) == 2 # two BotCommand instances
|
||||
assert kwargs["scope"] is not None
|
||||
assert isinstance(kwargs["scope"].chat_id, int)
|
||||
assert kwargs["scope"].chat_id == -123
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_ensure_forum_commands_handles_set_failure():
|
||||
adapter = _make_test_adapter()
|
||||
msg = _forum_message(chat_id=-456, is_forum=True)
|
||||
adapter._bot.set_my_commands.side_effect = Exception("Telegram API error")
|
||||
|
||||
with patch("gateway.platforms.telegram.telegram_menu_commands") as mock_menu:
|
||||
mock_menu.return_value = ([("new", "Start new session")], 0)
|
||||
# Should NOT raise despite the API error
|
||||
await adapter._ensure_forum_commands(msg)
|
||||
|
||||
# On failure we don't retry for this chat, so it's added to the set
|
||||
# to avoid hammering a broken chat.
|
||||
assert -456 not in adapter._forum_command_registered
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_ensure_forum_commands_race_safety():
|
||||
"""Two concurrent coroutines must not double-register the same chat."""
|
||||
adapter = _make_test_adapter()
|
||||
msg = _forum_message(chat_id=-789, is_forum=True)
|
||||
|
||||
with patch("gateway.platforms.telegram.telegram_menu_commands") as mock_menu:
|
||||
mock_menu.return_value = ([("new", "Start new session")], 0)
|
||||
with patch("telegram.BotCommand"):
|
||||
with patch("telegram.BotCommandScopeChat"):
|
||||
coro1 = adapter._ensure_forum_commands(msg)
|
||||
coro2 = adapter._ensure_forum_commands(msg)
|
||||
await asyncio.gather(coro1, coro2)
|
||||
|
||||
# The lock should make this exactly 1 call, not 2.
|
||||
assert adapter._bot.set_my_commands.await_count == 1
|
||||
Reference in New Issue
Block a user