diff --git a/tests/tools/test_send_message_tool.py b/tests/tools/test_send_message_tool.py index fa810eb5c54d..8a907dd5199e 100644 --- a/tests/tools/test_send_message_tool.py +++ b/tests/tools/test_send_message_tool.py @@ -23,6 +23,7 @@ def _reset_signal_scheduler(): from gateway.config import Platform from tools.send_message_tool import ( _derive_forum_thread_name, + _is_telegram_thread_not_found, _parse_target_ref, _send_discord, _send_matrix_via_adapter, @@ -799,6 +800,59 @@ def test_general_topic_thread_id_int_input_also_dropped(self, monkeypatch): kwargs = bot.send_message.await_args.kwargs assert "message_thread_id" not in kwargs + def test_thread_not_found_retries_without_message_thread_id(self, monkeypatch): + """When send_message raises "thread not found", retry without thread_id (#27012).""" + bot = self._make_bot() + _install_telegram_mock(monkeypatch, bot) + + # First call raises thread-not-found, second succeeds + bot.send_message = AsyncMock(side_effect=[ + Exception("Bad Request: message thread not found"), + SimpleNamespace(message_id=2), + ]) + + asyncio.run( + _send_telegram("tok", "-1001234567890", "hello", thread_id="17585") + ) + + assert bot.send_message.await_count == 2 + # First call: should include message_thread_id=17585 + call1_kwargs = bot.send_message.await_args_list[0].kwargs + assert call1_kwargs["message_thread_id"] == 17585 + # Second call (retry): should NOT include message_thread_id + call2_kwargs = bot.send_message.await_args_list[1].kwargs + assert "message_thread_id" not in call2_kwargs + + def test_thread_not_found_for_media_retries_without_message_thread_id(self, monkeypatch, tmp_path): + """Media send with stale thread_id retries without it (#27012).""" + bot = self._make_bot() + # Mock send_document to fail with thread-not-found, then succeed + bot.send_document = AsyncMock(side_effect=[ + Exception("Bad Request: message thread not found"), + SimpleNamespace(message_id=3), + ]) + _install_telegram_mock(monkeypatch, bot) + + # Create a test file + test_file = tmp_path / "doc.txt" + test_file.write_text("test content") + + asyncio.run( + _send_telegram( + "tok", "-1001234567890", "", + media_files=[(str(test_file), False)], + thread_id="17585", + ) + ) + + assert bot.send_document.await_count == 2 + # First call: should include message_thread_id=17585 + call1_kwargs = bot.send_document.await_args_list[0].kwargs + assert call1_kwargs["message_thread_id"] == 17585 + # Second call (retry): should NOT include message_thread_id + call2_kwargs = bot.send_document.await_args_list[1].kwargs + assert "message_thread_id" not in call2_kwargs + # --------------------------------------------------------------------------- # Tests for Discord thread_id support @@ -2332,3 +2386,94 @@ def test_gateway_status_import_error_is_swallowed(self, monkeypatch): patch("gateway.status.is_gateway_running", side_effect=ImportError("simulated")): assert _check_send_message() is False + + +class TestSendTelegramThreadNotFoundRetry: + """Tests for thread-not-found retry behaviour in _send_telegram (#27012).""" + + def test_is_thread_not_found_matches_expected_errors(self): + """_is_telegram_thread_not_found should detect thread-not-found errors.""" + class FakeError(Exception): + pass + + assert _is_telegram_thread_not_found(FakeError("message thread not found")) is True + assert _is_telegram_thread_not_found(FakeError("THREAD NOT FOUND")) is True + assert _is_telegram_thread_not_found(FakeError("Bad Request: thread not found")) is True + assert _is_telegram_thread_not_found(FakeError("chat not found")) is False + assert _is_telegram_thread_not_found(FakeError("parse error")) is False + assert _is_telegram_thread_not_found(FakeError("")) is False + + def test_text_send_retries_without_thread_id_on_thread_not_found(self): + """When thread is not found, the text send should retry without + message_thread_id.""" + call_args = [] + + async def fake_retry(bot, *, chat_id, text, parse_mode, **kwargs): + call_args.append(dict(kwargs, chat_id=chat_id, text=text)) + if len(call_args) == 1: + raise Exception("Bad Request: message thread not found") + return SimpleNamespace(message_id=42) + + async def run_test(): + with patch( + "tools.send_message_tool._send_telegram_message_with_retry", + fake_retry, + ): + # _send_telegram imports Bot locally; we only need to mock + # the send path, not Bot itself (Bot import falls through + # normally since python-telegram-bot is installed). + return await _send_telegram( + "fake-token", "-100123", "hello from topic 17585", + thread_id="17585", + ) + + result = asyncio.run(run_test()) + assert result["success"] is True + assert result["message_id"] == "42" + assert len(call_args) == 2, f"expected 2 calls, got {len(call_args)}" + # First call should have message_thread_id + assert call_args[0].get("message_thread_id") is not None + # Second call (retry) should NOT have message_thread_id + assert "message_thread_id" not in call_args[1], \ + "retry should drop message_thread_id after thread-not-found" + + def test_disable_web_page_preview_not_leaked_to_media_sends(self): + """disable_web_page_preview should only appear in text send, not media sends.""" + text_kwargs_seen = [] + media_kwargs_seen = [] + + class FakeBot: + async def send_message(self, **kwargs): + text_kwargs_seen.append(kwargs) + return SimpleNamespace(message_id=1) + + async def send_document(self, **kwargs): + media_kwargs_seen.append(kwargs) + return SimpleNamespace(message_id=2) + + import tempfile + media_path = None + try: + with tempfile.NamedTemporaryFile(suffix=".pdf", delete=False) as tf: + tf.write(b"%PDF-1.4 test content") + media_path = tf.name + + async def run_test(): + with patch("telegram.Bot", return_value=FakeBot()): + return await _send_telegram( + "fake-token", "-100123", "check preview", + media_files=[(media_path, False)], + disable_link_previews=True, + ) + + result = asyncio.run(run_test()) + assert result["success"] is True + # Text send should have disable_web_page_preview + assert text_kwargs_seen[0].get("disable_web_page_preview") is True + # Media send should NOT have disable_web_page_preview + assert "disable_web_page_preview" not in media_kwargs_seen[0], \ + "disable_web_page_preview leaked into send_document kwargs" + finally: + if media_path and os.path.exists(media_path): + os.unlink(media_path) + diff --git a/tools/send_message_tool.py b/tools/send_message_tool.py index d5b2c0c782cd..ab471c61ecaa 100644 --- a/tools/send_message_tool.py +++ b/tools/send_message_tool.py @@ -754,6 +754,15 @@ async def _send_to_platform(platform, pconfig, chat_id, message, thread_id=None, return last_result +def _is_telegram_thread_not_found(error: Exception) -> bool: + """Check if a Telegram error is a thread-not-found failure. + + Matches the gateway adapter's ``_is_thread_not_found_error`` for + the standalone ``_send_telegram`` path (issue #27012). + """ + return "thread not found" in str(error).lower() + + async def _send_telegram(token, chat_id, message, media_files=None, thread_id=None, disable_link_previews=False, force_document=False): """Send via Telegram Bot API (one-shot, no polling needed). @@ -810,8 +819,12 @@ async def _send_telegram(token, chat_id, message, media_files=None, thread_id=No ) if effective_thread_id is not None: thread_kwargs["message_thread_id"] = effective_thread_id + # disable_web_page_preview is only valid for send_message, not + # send_photo/send_video/etc. Keep it separate so media sends + # don't inherit an invalid parameter (issue #27012). + text_kwargs = dict(thread_kwargs) if disable_link_previews: - thread_kwargs["disable_web_page_preview"] = True + text_kwargs["disable_web_page_preview"] = True last_msg = None warnings = [] @@ -821,11 +834,24 @@ async def _send_telegram(token, chat_id, message, media_files=None, thread_id=No last_msg = await _send_telegram_message_with_retry( bot, chat_id=int_chat_id, text=formatted, - parse_mode=send_parse_mode, **thread_kwargs + parse_mode=send_parse_mode, **text_kwargs ) except Exception as md_error: - # Parse failed, fall back to plain text - if "parse" in str(md_error).lower() or "markdown" in str(md_error).lower() or "html" in str(md_error).lower(): + # Thread not found — retry without message_thread_id so the + # message still delivers (matching the gateway adapter's + # fallback behaviour, issue #27012). + if _is_telegram_thread_not_found(md_error) and thread_kwargs: + logger.warning( + "Thread %s not found in _send_telegram, retrying without message_thread_id", + thread_kwargs.get("message_thread_id"), + ) + text_kwargs.pop("message_thread_id", None) + last_msg = await _send_telegram_message_with_retry( + bot, + chat_id=int_chat_id, text=formatted, + parse_mode=send_parse_mode, **text_kwargs + ) + elif "parse" in str(md_error).lower() or "markdown" in str(md_error).lower() or "html" in str(md_error).lower(): logger.warning( "Parse mode %s failed in _send_telegram, falling back to plain text: %s", send_parse_mode, @@ -842,7 +868,7 @@ async def _send_telegram(token, chat_id, message, media_files=None, thread_id=No last_msg = await _send_telegram_message_with_retry( bot, chat_id=int_chat_id, text=plain, - parse_mode=None, **thread_kwargs + parse_mode=None, **text_kwargs ) else: raise @@ -857,26 +883,61 @@ async def _send_telegram(token, chat_id, message, media_files=None, thread_id=No ext = os.path.splitext(media_path)[1].lower() try: with open(media_path, "rb") as f: - if ext in _IMAGE_EXTS and not force_document: - last_msg = await bot.send_photo( - chat_id=int_chat_id, photo=f, **thread_kwargs - ) - elif ext in _VIDEO_EXTS: - last_msg = await bot.send_video( - chat_id=int_chat_id, video=f, **thread_kwargs - ) - elif ext in _VOICE_EXTS and is_voice: - last_msg = await bot.send_voice( - chat_id=int_chat_id, voice=f, **thread_kwargs - ) - elif ext in _TELEGRAM_SEND_AUDIO_EXTS: - last_msg = await bot.send_audio( - chat_id=int_chat_id, audio=f, **thread_kwargs - ) - else: - last_msg = await bot.send_document( - chat_id=int_chat_id, document=f, **thread_kwargs - ) + media_kwargs = dict(thread_kwargs) + try: + if ext in _IMAGE_EXTS and not force_document: + last_msg = await bot.send_photo( + chat_id=int_chat_id, photo=f, **media_kwargs + ) + elif ext in _VIDEO_EXTS: + last_msg = await bot.send_video( + chat_id=int_chat_id, video=f, **media_kwargs + ) + elif ext in _VOICE_EXTS and is_voice: + last_msg = await bot.send_voice( + chat_id=int_chat_id, voice=f, **media_kwargs + ) + elif ext in _TELEGRAM_SEND_AUDIO_EXTS: + last_msg = await bot.send_audio( + chat_id=int_chat_id, audio=f, **media_kwargs + ) + else: + last_msg = await bot.send_document( + chat_id=int_chat_id, document=f, **media_kwargs + ) + except Exception as media_err: + if _is_telegram_thread_not_found(media_err) and media_kwargs.get("message_thread_id"): + # Thread not found for media — retry without + # message_thread_id (issue #27012). + logger.warning( + "Thread %s not found for media send, retrying without message_thread_id", + media_kwargs["message_thread_id"], + ) + # Re-seek the file since the first attempt consumed it + f.seek(0) + media_kwargs.pop("message_thread_id", None) + if ext in _IMAGE_EXTS and not force_document: + last_msg = await bot.send_photo( + chat_id=int_chat_id, photo=f, **media_kwargs + ) + elif ext in _VIDEO_EXTS: + last_msg = await bot.send_video( + chat_id=int_chat_id, video=f, **media_kwargs + ) + elif ext in _VOICE_EXTS and is_voice: + last_msg = await bot.send_voice( + chat_id=int_chat_id, voice=f, **media_kwargs + ) + elif ext in _TELEGRAM_SEND_AUDIO_EXTS: + last_msg = await bot.send_audio( + chat_id=int_chat_id, audio=f, **media_kwargs + ) + else: + last_msg = await bot.send_document( + chat_id=int_chat_id, document=f, **media_kwargs + ) + else: + raise except Exception as e: warning = _sanitize_error_text(f"Failed to send media {media_path}: {e}") logger.error(warning)