diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index 1bfb3dfa89ab..708a8b62367e 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -3706,6 +3706,45 @@ class BasePlatformAdapter(ABC): text = f"{caption}\n{text}" return await self.send(chat_id=chat_id, content=text, reply_to=reply_to, metadata=metadata) + async def _notify_media_delivery_failure( + self, + chat_id: str, + media_path: str, + *, + is_voice: bool = False, + metadata: Optional[Dict[str, Any]] = None, + ) -> None: + """Send a user-visible notice when a MEDIA attachment could not be delivered. + + The non-streaming dispatch loop strips ``MEDIA:`` tags before sending + attachments. When the subsequent upload returns ``success=False`` (for + example Discord accepted the message but attached nothing), the user + must see a failure notice instead of a silent drop (#66797). + """ + ext = Path(media_path).suffix.lower() + _VIDEO_EXTS = {".mp4", ".mov", ".avi", ".mkv", ".webm", ".3gp"} + if is_voice or should_send_media_as_audio(self.platform, ext, is_voice=is_voice): + text = "⚠️ Couldn't deliver the audio attachment." + elif ext in _VIDEO_EXTS: + text = "⚠️ Couldn't deliver the video attachment." + else: + file_name = os.path.basename(media_path) + text = f"⚠️ Couldn't deliver the file attachment ({file_name})." + try: + notice = await self.send(chat_id=chat_id, content=text, metadata=metadata) + if not notice.success: + logger.debug( + "[%s] Could not send media-delivery-failure notice: %s", + self.name, + notice.error, + ) + except Exception as notify_err: + logger.debug( + "[%s] Could not send media-delivery-failure notice: %s", + self.name, + notify_err, + ) + async def send_image_file( self, chat_id: str, @@ -5498,6 +5537,12 @@ class BasePlatformAdapter(ABC): if not media_result.success: logger.warning("[%s] Failed to send media (%s): %s", self.name, ext, media_result.error) + await self._notify_media_delivery_failure( + event.source.chat_id, + media_path, + is_voice=is_voice, + metadata=_final_thread_metadata, + ) except Exception as media_err: logger.warning("[%s] Error sending media: %s", self.name, media_err) @@ -5508,17 +5553,29 @@ class BasePlatformAdapter(ABC): try: ext = Path(file_path).suffix.lower() if ext in _VIDEO_EXTS: - await self.send_video( + file_result = await self.send_video( chat_id=event.source.chat_id, video_path=file_path, metadata=_final_thread_metadata, ) else: - await self.send_document( + file_result = await self.send_document( chat_id=event.source.chat_id, file_path=file_path, metadata=_final_thread_metadata, ) + if not file_result.success: + logger.warning( + "[%s] Failed to send local file (%s): %s", + self.name, + ext, + file_result.error, + ) + await self._notify_media_delivery_failure( + event.source.chat_id, + file_path, + metadata=_final_thread_metadata, + ) except Exception as file_err: logger.error("[%s] Error sending local file %s: %s", self.name, file_path, file_err) diff --git a/plugins/platforms/discord/adapter.py b/plugins/platforms/discord/adapter.py index d19ba5831393..4787bc6b5225 100644 --- a/plugins/platforms/discord/adapter.py +++ b/plugins/platforms/discord/adapter.py @@ -3106,6 +3106,30 @@ class DiscordAdapter(BasePlatformAdapter): starter_msg = getattr(thread, "message", None) message_id = str(getattr(starter_msg, "id", thread_id)) if starter_msg else thread_id + if file is not None or files: + attachments = getattr(starter_msg, "attachments", None) or [] + if not attachments: + filename = "" + if file is not None: + filename = getattr(file, "filename", "") or "" + elif files: + filename = getattr(files[0], "filename", "") or "" + logger.warning( + "[%s] Forum thread %s starter has no attachments for %s", + self.name, + thread_id, + filename or "file", + ) + return SendResult( + success=False, + error=( + "Discord created the forum thread but attached no files" + + (f" ({filename})" if filename else "") + ), + message_id=message_id or None, + raw_response={"thread_id": thread_id}, + ) + return SendResult( success=True, message_id=message_id, diff --git a/tests/gateway/test_discord_send.py b/tests/gateway/test_discord_send.py index 7aaf800e76fc..dd178ac52b3a 100644 --- a/tests/gateway/test_discord_send.py +++ b/tests/gateway/test_discord_send.py @@ -326,7 +326,14 @@ async def test_forum_post_file_creates_thread_with_attachment(): adapter = DiscordAdapter(PlatformConfig(enabled=True, token="***")) thread_ch = SimpleNamespace(id=777, send=AsyncMock()) - thread = SimpleNamespace(id=777, message=SimpleNamespace(id=800), thread=thread_ch) + thread = SimpleNamespace( + id=777, + message=SimpleNamespace( + id=800, + attachments=[SimpleNamespace(filename="photo.png")], + ), + thread=thread_ch, + ) forum_channel = _discord_mod.ForumChannel() forum_channel.id = 999 forum_channel.name = "ideas" @@ -356,7 +363,14 @@ async def test_forum_post_file_uses_filename_when_no_content(): """Thread name falls back to file.filename when no content is provided.""" adapter = DiscordAdapter(PlatformConfig(enabled=True, token="***")) - thread = SimpleNamespace(id=1, message=SimpleNamespace(id=2), thread=SimpleNamespace(id=1, send=AsyncMock())) + thread = SimpleNamespace( + id=1, + message=SimpleNamespace( + id=2, + attachments=[SimpleNamespace(filename="voice-message.ogg")], + ), + thread=SimpleNamespace(id=1, send=AsyncMock()), + ) forum_channel = _discord_mod.ForumChannel() forum_channel.id = 10 forum_channel.name = "forum" @@ -390,6 +404,32 @@ async def test_forum_post_file_creation_failure(): assert "missing perms" in (result.error or "") +@pytest.mark.asyncio +async def test_forum_post_file_fails_when_starter_has_no_attachments(): + """Forum create_thread can succeed yet return an attachmentless starter (#66797).""" + adapter = DiscordAdapter(PlatformConfig(enabled=True, token="***")) + + thread = SimpleNamespace( + id=7, + message=SimpleNamespace(id=8, attachments=[]), + thread=SimpleNamespace(id=7, send=AsyncMock()), + ) + forum_channel = _discord_mod.ForumChannel() + forum_channel.id = 999 + forum_channel.create_thread = AsyncMock(return_value=thread) + + fake_file = SimpleNamespace(filename="clip.mp4") + result = await adapter._forum_post_file( + forum_channel, + content="video clip", + files=[fake_file], + ) + + assert result.success is False + assert "no files" in (result.error or "").lower() + forum_channel.create_thread.assert_awaited_once() + + # --------------------------------------------------------------------------- # Typing indicator task lifecycle # --------------------------------------------------------------------------- @@ -622,7 +662,10 @@ async def test_send_file_attachment_forum_uses_files_kwarg(tmp_path, monkeypatch adapter = DiscordAdapter(PlatformConfig(enabled=True, token="***")) created_thread = SimpleNamespace( id=7, - message=SimpleNamespace(id=8), + message=SimpleNamespace( + id=8, + attachments=[SimpleNamespace(filename="clip.mp4")], + ), ) forum_channel = SimpleNamespace( id=7, @@ -641,3 +684,39 @@ async def test_send_file_attachment_forum_uses_files_kwarg(tmp_path, monkeypatch thread_kwargs = forum_channel.create_thread.await_args.kwargs assert thread_kwargs.get("file") is None assert isinstance(thread_kwargs.get("files"), list) and len(thread_kwargs["files"]) == 1 + + +@pytest.mark.asyncio +async def test_forum_send_video_fails_loud_when_starter_has_no_attachments(tmp_path, monkeypatch): + """Forum-parent send_video must fail loud when the starter message drops attachments.""" + import plugins.platforms.discord.adapter as discord_platform + + video = tmp_path / "clip.mp4" + video.write_bytes(b"fake-mp4") + + monkeypatch.setattr( + discord_platform.discord, + "File", + lambda fp, filename=None, **kwargs: SimpleNamespace(fp=fp, filename=filename), + ) + + adapter = DiscordAdapter(PlatformConfig(enabled=True, token="***")) + created_thread = SimpleNamespace( + id=7, + message=SimpleNamespace(id=8, attachments=[]), + ) + forum_channel = SimpleNamespace( + id=7, + create_thread=AsyncMock(return_value=created_thread), + ) + adapter._client = SimpleNamespace( + get_channel=lambda _chat_id: forum_channel, + fetch_channel=AsyncMock(), + ) + monkeypatch.setattr(adapter, "_is_forum_parent", lambda _ch: True) + + result = await adapter.send_video("555", str(video)) + + assert result.success is False + assert "no files" in (result.error or "").lower() + forum_channel.create_thread.assert_awaited_once() diff --git a/tests/gateway/test_tts_media_routing.py b/tests/gateway/test_tts_media_routing.py index e152b99c2743..50381fb6ba7c 100644 --- a/tests/gateway/test_tts_media_routing.py +++ b/tests/gateway/test_tts_media_routing.py @@ -261,3 +261,91 @@ async def test_streaming_delivery_blocks_media_path_outside_allowed_roots(tmp_pa adapter.send_document.assert_not_awaited() adapter.send_voice.assert_not_awaited() + + +class _DiscordMediaFailureAdapter(BasePlatformAdapter): + """Minimal adapter to exercise non-streaming MEDIA failure notification.""" + + def __init__(self): + super().__init__(PlatformConfig(enabled=True, token="test"), Platform.DISCORD) + self.notices: list[str] = [] + + async def connect(self, *, is_reconnect: bool = False): + return True + + async def disconnect(self): + pass + + async def send(self, chat_id, content=None, **kwargs): + self.notices.append(content or "") + return SendResult(success=True, message_id="notice") + + async def get_chat_info(self, chat_id): + return {"id": chat_id, "type": "dm"} + + +@pytest.mark.asyncio +async def test_non_streaming_media_failure_notifies_user(tmp_path, monkeypatch): + """Attachmentless send_video results must surface a user-visible notice (#66797).""" + adapter = _DiscordMediaFailureAdapter() + event = _event() + media_file = _allowed_media_path(tmp_path, monkeypatch, "clip.mp4") + adapter._message_handler = AsyncMock(return_value=f"MEDIA:{media_file}") + adapter.send_video = AsyncMock( + return_value=SendResult( + success=False, + error="Discord accepted the message but attached no files (clip.mp4)", + ) + ) + adapter.send_document = AsyncMock(return_value=SendResult(success=True, message_id="doc")) + adapter.send_voice = AsyncMock(return_value=SendResult(success=True, message_id="voice")) + adapter.send_multiple_images = AsyncMock() + + await adapter._process_message_background(event, build_session_key(event.source)) + + adapter.send_video.assert_awaited_once() + assert adapter.notices == ["⚠️ Couldn't deliver the video attachment."] + + +class _DiscordMediaFailureAdapter(BasePlatformAdapter): + """Minimal adapter to exercise non-streaming MEDIA failure notification.""" + + def __init__(self): + super().__init__(PlatformConfig(enabled=True, token="test"), Platform.DISCORD) + self.notices: list[str] = [] + + async def connect(self, *, is_reconnect: bool = False): + return True + + async def disconnect(self): + pass + + async def send(self, chat_id, content=None, **kwargs): + self.notices.append(content or "") + return SendResult(success=True, message_id="notice") + + async def get_chat_info(self, chat_id): + return {"id": chat_id, "type": "dm"} + + +@pytest.mark.asyncio +async def test_non_streaming_media_failure_notifies_user(tmp_path, monkeypatch): + """Attachmentless send_video results must surface a user-visible notice (#66797).""" + adapter = _DiscordMediaFailureAdapter() + event = _event() + media_file = _allowed_media_path(tmp_path, monkeypatch, "clip.mp4") + adapter._message_handler = AsyncMock(return_value=f"MEDIA:{media_file}") + adapter.send_video = AsyncMock( + return_value=SendResult( + success=False, + error="Discord accepted the message but attached no files (clip.mp4)", + ) + ) + adapter.send_document = AsyncMock(return_value=SendResult(success=True, message_id="doc")) + adapter.send_voice = AsyncMock(return_value=SendResult(success=True, message_id="voice")) + adapter.send_multiple_images = AsyncMock() + + await adapter._process_message_background(event, build_session_key(event.source)) + + adapter.send_video.assert_awaited_once() + assert adapter.notices == ["⚠️ Couldn't deliver the video attachment."]