diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index 9c158a7f5a673..92b3e4bdff3ec 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -4902,6 +4902,43 @@ def _slack_message_matches_mention_patterns(self, text: str) -> bool: # ────────────────────────────────────────────────────────────────────────── +async def _standalone_upload_file( + client, + chat_id: str, + media_path: str, + *, + initial_comment: str = "", + thread_id: Optional[str] = None, +) -> Dict[str, Any]: + """Upload one local file via ``files_upload_v2`` (same API as the live adapter).""" + kwargs: Dict[str, Any] = { + "channel": chat_id, + "file": media_path, + "filename": os.path.basename(media_path), + "initial_comment": initial_comment or "", + } + if thread_id: + kwargs["thread_ts"] = thread_id + result = await client.files_upload_v2(**kwargs) + if isinstance(result, dict) and result.get("ok") is False: + return {"error": f"Slack API error: {result.get('error', 'unknown')}"} + # files_upload_v2 responses vary by sdk version; prefer file timestamp when present. + message_id = None + if isinstance(result, dict): + file_obj = result.get("file") or {} + shares = file_obj.get("shares") or {} + for share_bucket in shares.values(): + if isinstance(share_bucket, dict): + for entries in share_bucket.values(): + if isinstance(entries, list) and entries: + message_id = entries[0].get("ts") or message_id + break + if message_id: + break + message_id = message_id or file_obj.get("timestamp") or result.get("ts") + return {"success": True, "message_id": message_id, "raw": result} + + async def _standalone_send( pconfig, chat_id, @@ -4910,33 +4947,153 @@ async def _standalone_send( thread_id=None, media_files=None, force_document=False, + caption=None, ): - """Out-of-process Slack delivery via the Web API ``chat.postMessage``. + """Out-of-process Slack delivery via the Web API. Implements the ``standalone_sender_fn`` contract so ``deliver=slack`` cron - jobs succeed when the cron process is not co-located with the gateway (the - in-process adapter weakref is ``None`` in that case). Replaces the legacy - ``_send_slack`` helper that used to live in ``tools/send_message_tool.py``. + jobs and ``send_message`` MEDIA attachments succeed when the cron/tool + process is not co-located with the gateway (the in-process adapter weakref + is ``None`` in that case). Replaces the legacy ``_send_slack`` helper that + used to live in ``tools/send_message_tool.py``. - mrkdwn formatting is applied exactly as the legacy core path did — via a - throwaway ``SlackAdapter`` instance's ``format_message`` — so cron-delivered - Slack messages render identically to gateway-delivered ones. + Text uses ``chat.postMessage`` (aiohttp). Media uses ``files_upload_v2`` via + ``AsyncWebClient`` — the same upload path as the live Slack adapter — so + PDFs/images/documents arrive as native Slack file shares. + + ``force_document`` is accepted for signature parity but unused — Slack + treats every upload as a generic file share. + + When ``caption`` is set (single captionable MEDIA: + short text), the + text rides as ``initial_comment`` on the upload instead of a separate + ``chat.postMessage``. """ + del force_document # signature parity with other standalone senders token = getattr(pconfig, "token", None) or os.getenv("SLACK_BOT_TOKEN", "") if not token: return {"error": "Slack send failed: SLACK_BOT_TOKEN not configured"} - formatted = message - if message: + media_files = media_files or [] + warnings: List[str] = [] + + def _format_mrkdwn(text: str) -> str: + if not text: + return text try: _fmt_adapter = SlackAdapter.__new__(SlackAdapter) - formatted = _fmt_adapter.format_message(message) + return _fmt_adapter.format_message(text) except Exception: logger.debug( "Failed to apply Slack mrkdwn formatting in _standalone_send", exc_info=True, ) + return text + + formatted = _format_mrkdwn(message) if message else message + formatted_caption = _format_mrkdwn(caption) if caption else caption + + # --- Media path: AsyncWebClient.files_upload_v2 (+ optional text) --- + if media_files: + try: + from slack_sdk.web.async_client import AsyncWebClient as _AsyncWebClient + except ImportError: + return { + "error": ( + "slack_sdk not installed. Run: pip install 'slack-sdk' " + "(required for Slack MEDIA delivery via send_message)" + ) + } + + client = _AsyncWebClient(token=token) + last_message_id = None + + # Caption mode: skip a separate text post; comment rides the upload. + text_to_send = "" if formatted_caption else (formatted or "") + if text_to_send.strip(): + post_kwargs: Dict[str, Any] = { + "channel": chat_id, + "text": text_to_send, + "mrkdwn": True, + } + if thread_id: + post_kwargs["thread_ts"] = thread_id + try: + post_resp = await client.chat_postMessage(**post_kwargs) + if isinstance(post_resp, dict) and not post_resp.get("ok", True): + return { + "error": f"Slack API error: {post_resp.get('error', 'unknown')}" + } + last_message_id = ( + post_resp.get("ts") if isinstance(post_resp, dict) else None + ) + except Exception as e: + return {"error": f"Slack send failed: {e}"} + + caption_pending = bool(formatted_caption) + uploaded_any = False + for media_path, _is_voice in media_files: + if not os.path.exists(media_path): + warning = f"Media file not found, skipping: {media_path}" + logger.warning("[Slack] %s", warning) + warnings.append(warning) + if caption_pending: + # Keep caption deliverable even when the file is missing. + try: + fallback_kwargs: Dict[str, Any] = { + "channel": chat_id, + "text": formatted_caption, + "mrkdwn": True, + } + if thread_id: + fallback_kwargs["thread_ts"] = thread_id + fb = await client.chat_postMessage(**fallback_kwargs) + if isinstance(fb, dict) and fb.get("ok", True): + last_message_id = fb.get("ts") or last_message_id + caption_pending = False + except Exception: + logger.warning( + "[Slack] Caption-fallback send failed for missing media", + exc_info=True, + ) + continue + try: + upload_result = await _standalone_upload_file( + client, + chat_id, + media_path, + initial_comment=formatted_caption if caption_pending else "", + thread_id=thread_id, + ) + if upload_result.get("error"): + warnings.append( + f"Failed to send media {media_path}: {upload_result['error']}" + ) + continue + uploaded_any = True + caption_pending = False + last_message_id = upload_result.get("message_id") or last_message_id + except Exception as e: + warning = f"Failed to send media {media_path}: {e}" + logger.error("[Slack] %s", warning, exc_info=True) + warnings.append(warning) + + if last_message_id is None and not uploaded_any and not text_to_send.strip(): + error = "No deliverable text or media remained after processing" + if warnings: + return {"error": error, "warnings": warnings} + return {"error": error} + + result: Dict[str, Any] = { + "success": True, + "platform": "slack", + "chat_id": chat_id, + "message_id": last_message_id, + } + if warnings: + result["warnings"] = warnings + return result + # --- Text-only path (existing aiohttp chat.postMessage) --- try: import aiohttp except ImportError: diff --git a/tests/tools/test_slack_send_message_media.py b/tests/tools/test_slack_send_message_media.py new file mode 100644 index 0000000000000..9b6711835272b --- /dev/null +++ b/tests/tools/test_slack_send_message_media.py @@ -0,0 +1,251 @@ +"""Slack media delivery for send_message. + +Covers ``plugins/platforms/slack/adapter.py::_standalone_send`` media path: +text+file, media-only, caption-on-upload, missing-file warnings. + +``slack_sdk`` is optional in CI, so tests inject a fake module into +``sys.modules`` (same pattern as ``tests/gateway/test_slack.py``). +""" + +from __future__ import annotations + +import asyncio +import contextlib +import os +import sys +import tempfile +from types import ModuleType, SimpleNamespace +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from plugins.platforms.slack.adapter import _standalone_send + + +def _pconfig(token: str = "xoxb-test"): + return SimpleNamespace(token=token, extra={}) + + +def _tmpfile(suffix: str) -> str: + f = tempfile.NamedTemporaryFile(suffix=suffix, delete=False) + f.write(b"%PDF-1.4 test") + f.close() + return f.name + + +def _mock_client(*, post_ok=True, upload_ok=True): + client = MagicMock() + client.chat_postMessage = AsyncMock( + return_value={ + "ok": post_ok, + "ts": "111.222", + "error": None if post_ok else "channel_not_found", + } + ) + if upload_ok: + client.files_upload_v2 = AsyncMock( + return_value={ + "ok": True, + "file": { + "id": "F123", + "timestamp": 1234567890, + "shares": {"public": {"C012AB3CD": [{"ts": "333.444"}]}}, + }, + } + ) + else: + client.files_upload_v2 = AsyncMock( + return_value={"ok": False, "error": "not_in_channel"} + ) + return client + + +@contextlib.contextmanager +def _fake_slack_sdk(client): + """Make ``from slack_sdk.web.async_client import AsyncWebClient`` resolve to a factory.""" + sdk = ModuleType("slack_sdk") + web = ModuleType("slack_sdk.web") + async_client = ModuleType("slack_sdk.web.async_client") + async_client.AsyncWebClient = MagicMock(return_value=client) + sdk.web = web + web.async_client = async_client + + modules = { + "slack_sdk": sdk, + "slack_sdk.web": web, + "slack_sdk.web.async_client": async_client, + } + old = {name: sys.modules.get(name) for name in modules} + sys.modules.update(modules) + try: + yield + finally: + for name, prev in old.items(): + if prev is None: + sys.modules.pop(name, None) + else: + sys.modules[name] = prev + + +def test_text_plus_pdf_uploads_via_files_upload_v2(): + pdf = _tmpfile(".pdf") + client = _mock_client() + try: + with _fake_slack_sdk(client): + result = asyncio.run( + _standalone_send( + _pconfig(), + "C012AB3CD", + "Here is the report", + media_files=[(pdf, False)], + ) + ) + assert result["success"] is True + assert result["platform"] == "slack" + client.chat_postMessage.assert_awaited_once() + client.files_upload_v2.assert_awaited_once() + upload_kwargs = client.files_upload_v2.await_args.kwargs + assert upload_kwargs["channel"] == "C012AB3CD" + assert upload_kwargs["file"] == pdf + assert upload_kwargs["filename"] == os.path.basename(pdf) + assert upload_kwargs["initial_comment"] == "" + finally: + os.unlink(pdf) + + +def test_media_only_skips_text_post(): + pdf = _tmpfile(".pdf") + client = _mock_client() + try: + with _fake_slack_sdk(client): + result = asyncio.run( + _standalone_send( + _pconfig(), + "C012AB3CD", + "", + media_files=[(pdf, False)], + ) + ) + assert result["success"] is True + client.chat_postMessage.assert_not_awaited() + client.files_upload_v2.assert_awaited_once() + finally: + os.unlink(pdf) + + +def test_caption_rides_initial_comment_no_separate_text(): + pdf = _tmpfile(".pdf") + client = _mock_client() + try: + with _fake_slack_sdk(client): + result = asyncio.run( + _standalone_send( + _pconfig(), + "C012AB3CD", + "", + media_files=[(pdf, False)], + caption="Q3 summary PDF", + ) + ) + assert result["success"] is True + client.chat_postMessage.assert_not_awaited() + upload_kwargs = client.files_upload_v2.await_args.kwargs + assert upload_kwargs["initial_comment"] == "Q3 summary PDF" + finally: + os.unlink(pdf) + + +def test_missing_media_file_warns_and_falls_back_caption(): + client = _mock_client() + with _fake_slack_sdk(client): + result = asyncio.run( + _standalone_send( + _pconfig(), + "C012AB3CD", + "", + media_files=[("/no/such/file.pdf", False)], + caption="still deliver this", + ) + ) + assert result["success"] is True + assert result.get("warnings") + assert any("not found" in w.lower() for w in result["warnings"]) + client.chat_postMessage.assert_awaited_once() + assert client.chat_postMessage.await_args.kwargs["text"] == "still deliver this" + client.files_upload_v2.assert_not_awaited() + + +def test_missing_token_errors(monkeypatch): + monkeypatch.delenv("SLACK_BOT_TOKEN", raising=False) + result = asyncio.run( + _standalone_send( + _pconfig(token=""), + "C012AB3CD", + "hi", + media_files=[("/tmp/x.pdf", False)], + ) + ) + assert "error" in result + assert "SLACK_BOT_TOKEN" in result["error"] + + +def test_thread_id_passed_to_upload(): + pdf = _tmpfile(".pdf") + client = _mock_client() + try: + with _fake_slack_sdk(client): + asyncio.run( + _standalone_send( + _pconfig(), + "C012AB3CD", + "", + thread_id="999.000", + media_files=[(pdf, False)], + ) + ) + assert client.files_upload_v2.await_args.kwargs["thread_ts"] == "999.000" + finally: + os.unlink(pdf) + + +def test_send_to_platform_routes_slack_media(): + """_send_to_platform must call Slack standalone_sender with media_files.""" + import httpx + + if not hasattr(httpx, "Proxy") or not hasattr(httpx, "URL"): + pytest.skip("httpx type annotations incompatible with telegram library") + + from gateway.config import Platform + from hermes_cli.plugins import discover_plugins + from gateway.platform_registry import platform_registry + from tools.send_message_tool import _send_to_platform + + pdf = _tmpfile(".pdf") + discover_plugins() + entry = platform_registry.get("slack") + assert entry is not None and entry.standalone_sender_fn is not None + original = entry.standalone_sender_fn + mock_sender = AsyncMock( + return_value={"success": True, "platform": "slack", "message_id": "1.2"} + ) + entry.standalone_sender_fn = mock_sender + try: + result = asyncio.run( + _send_to_platform( + Platform.SLACK, + _pconfig(), + "C012AB3CD", + "Here is the report", + media_files=[(pdf, False)], + ) + ) + assert result["success"] is True + mock_sender.assert_awaited() + call_kwargs = mock_sender.await_args.kwargs + assert call_kwargs.get("media_files") == [(pdf, False)] + # Single captionable file + short text → caption rides the upload. + assert call_kwargs.get("caption") == "Here is the report" + assert not result.get("warnings") + finally: + entry.standalone_sender_fn = original + os.unlink(pdf) diff --git a/tools/send_message_tool.py b/tools/send_message_tool.py index 6cccee7eddfb9..41e99dfb2ebb0 100644 --- a/tools/send_message_tool.py +++ b/tools/send_message_tool.py @@ -982,6 +982,48 @@ async def _send_to_platform(platform, pconfig, chat_id, message, thread_id=None, last_result = result return last_result + # --- Slack: native media via files_upload_v2 in the plugin's + # standalone_sender_fn (plugins/platforms/slack/adapter.py::_standalone_send). + # Gateway in-channel MEDIA: delivery already worked; send_message previously + # omitted Slack attachments and told the model media was unsupported. + if platform == Platform.SLACK and media_files: + from gateway.platform_registry import platform_registry as _pr_slack + from hermes_cli.plugins import discover_plugins as _dp_slack + _dp_slack() + _slack_entry = _pr_slack.get("slack") + if _slack_entry is None or _slack_entry.standalone_sender_fn is None: + return {"error": "Slack plugin not registered or missing standalone_sender_fn"} + _sl_caption, _ = _media_caption_split( + message, media_files, + max_caption_len=(max_len or _DEFAULT_CAPTION_LIMIT), + ) + if _sl_caption is not None: + result = await _slack_entry.standalone_sender_fn( + pconfig, + chat_id, + "", + thread_id=thread_id, + media_files=media_files, + caption=_sl_caption, + ) + if isinstance(result, dict) and result.get("error"): + return result + return result + last_result = None + for i, chunk in enumerate(chunks): + is_last = (i == len(chunks) - 1) + result = await _slack_entry.standalone_sender_fn( + pconfig, + chat_id, + chunk, + thread_id=thread_id, + media_files=media_files if is_last else [], + ) + if isinstance(result, dict) and result.get("error"): + return result + last_result = result + return last_result + # --- WhatsApp: native media attachment support via the registry's # standalone_sender_fn (plugins/platforms/whatsapp/adapter.py::_standalone_send). # The plugin uploads each file through the local Baileys bridge /send-media @@ -1036,7 +1078,7 @@ async def _send_to_platform(platform, pconfig, chat_id, message, thread_id=None, if media_files and not message.strip(): return { "error": ( - f"send_message MEDIA delivery is currently only supported for telegram, discord, matrix, weixin, signal, yuanbao, feishu and whatsapp; " + f"send_message MEDIA delivery is currently only supported for telegram, discord, matrix, weixin, signal, yuanbao, feishu, whatsapp and slack; " f"target {platform.value} had only media attachments" ) } @@ -1044,7 +1086,7 @@ async def _send_to_platform(platform, pconfig, chat_id, message, thread_id=None, if media_files: warning = ( f"MEDIA attachments were omitted for {platform.value}; " - "native send_message media delivery is currently only supported for telegram, discord, matrix, weixin, signal, yuanbao, feishu and whatsapp" + "native send_message media delivery is currently only supported for telegram, discord, matrix, weixin, signal, yuanbao, feishu, whatsapp and slack" ) last_result = None