mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-27 17:58:07 +00:00
fix(discord): notify user on attachmentless MEDIA drop
Surface a user-visible delivery notice when non-streaming media dispatch gets success=False after MEDIA tags were stripped, and validate forum starter-message attachments the same way as direct channel sends (#66797).
This commit is contained in:
parent
8eb29a1bb9
commit
384b0a0b5b
4 changed files with 253 additions and 5 deletions
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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."]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue