diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index 06983f910f05..afe1756cd2d3 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -19,11 +19,12 @@ import time from dataclasses import dataclass, field from typing import Callable, Dict, Optional, Any, Tuple, List +import aiohttp + try: from slack_bolt.async_app import AsyncApp from slack_bolt.adapter.socket_mode.async_handler import AsyncSocketModeHandler from slack_sdk.web.async_client import AsyncWebClient - import aiohttp SLACK_AVAILABLE = True except ImportError: @@ -746,6 +747,7 @@ class SlackAdapter(BasePlatformAdapter): self._CHANNEL_TEAM_MAX = 10000 # user target (team_id:user_id) → opened DM conversation ID (D...) self._dm_conversation_cache: Dict[str, str] = {} + self._DM_CONVERSATION_CACHE_MAX = 5000 # Dedup cache: prevents duplicate bot responses when Socket Mode # reconnects redeliver events (#4777). The TTL must outlast Slack's # worst-case reconnect-redelivery gap, not just a few seconds — the @@ -2025,9 +2027,12 @@ class SlackAdapter(BasePlatformAdapter): dm_id = ((response or {}).get("channel") or {}).get("id") if dm_id: self._dm_conversation_cache[cache_key] = dm_id + self._trim_oldest_dict_entries( + self._dm_conversation_cache, self._DM_CONVERSATION_CACHE_MAX + ) # DM belongs to the same workspace as the user target. if team_id: - self._channel_team[dm_id] = team_id + self._remember_channel_team(dm_id, team_id) return dm_id except Exception as e: logger.warning( @@ -3228,7 +3233,9 @@ class SlackAdapter(BasePlatformAdapter): or (isinstance(profile, dict) and profile.get("bot_id")) ) self._user_is_bot_cache[cache_key] = is_bot - + self._trim_oldest_dict_entries( + self._user_is_bot_cache, self._USER_NAME_CACHE_MAX + ) # Populate the name cache from the same users.info response so the # later source construction does not need a second API lookup. name = ( @@ -7089,6 +7096,13 @@ class SlackAdapter(BasePlatformAdapter): # Cache for Slack user ID -> DM conversation ID resolution in the standalone # send path. Keyed by "{token}:{user_id}" to support multi-workspace setups. _slack_dm_cache: Dict[str, str] = {} +_SLACK_DM_CACHE_MAX = 5000 + + +def _trim_slack_dm_cache() -> None: + """Bound the module-level DM cache, oldest-insertion-first (C16 policy).""" + while len(_slack_dm_cache) > _SLACK_DM_CACHE_MAX: + _slack_dm_cache.pop(next(iter(_slack_dm_cache))) async def _resolve_slack_user_dm(token: str, user_id: str) -> Optional[str]: @@ -7128,6 +7142,7 @@ async def _resolve_slack_user_dm(token: str, user_id: str) -> Optional[str]: if data.get("ok") and data.get("channel", {}).get("id"): channel_id = data["channel"]["id"] _slack_dm_cache[cache_key] = channel_id + _trim_slack_dm_cache() return channel_id logger.warning( "[Slack] conversations.open failed for %s: %s", @@ -7249,6 +7264,9 @@ async def _standalone_send( # --- Media path: AsyncWebClient.files_upload_v2 (+ optional text) --- if media_files: + # Function-local import: tests inject a fake slack_sdk via + # sys.modules, and installs without slack_sdk get a clean error + # instead of an ImportError at module load. try: from slack_sdk.web.async_client import AsyncWebClient as _AsyncWebClient except ImportError: @@ -7260,6 +7278,7 @@ async def _standalone_send( } client = _AsyncWebClient(token=token) + _apply_slack_proxy(client, resolve_proxy_url()) last_message_id = None # Caption mode: skip a separate text post; comment rides the upload. diff --git a/tests/gateway/test_slack.py b/tests/gateway/test_slack.py index f9f6b73c2f98..fb7dd1e149eb 100644 --- a/tests/gateway/test_slack.py +++ b/tests/gateway/test_slack.py @@ -1221,25 +1221,63 @@ class TestSlackProxyBehavior: # --------------------------------------------------------------------------- + +from contextlib import contextmanager +from types import ModuleType + + +@contextmanager +def _fake_slack_sdk_modules(client): + """Route ``from slack_sdk.web.async_client import AsyncWebClient`` to a mock.""" + import sys as _sys + + 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 + + class TestStandaloneSendMedia: @pytest.mark.asyncio async def test_uploads_local_media_with_message_as_caption(self, tmp_path): - """Standalone cron sends should use files_upload_v2, not omit the image.""" + """Standalone cron sends must upload via files_upload_v2 — the text + posts as its own message and the file follows (caption-mode, where + text rides the upload as initial_comment, is chosen by the tool + layer via the ``caption=`` kwarg — covered in + tests/tools/test_slack_send_message_media.py).""" image = tmp_path / "daily-report.png" image.write_bytes(b"\x89PNG\r\n\x1a\n") client = MagicMock() + client.chat_postMessage = AsyncMock(return_value={"ok": True, "ts": "1.0"}) client.files_upload_v2 = AsyncMock( return_value={"ok": True, "files": [{"id": "F123"}]} ) config = PlatformConfig(enabled=True, token="xoxb-fake-token") with ( - patch.object(_slack_mod, "AsyncWebClient", return_value=client), + _fake_slack_sdk_modules(client), patch.object(_slack_mod, "resolve_proxy_url", return_value=None), patch.object( _slack_mod.aiohttp, "ClientSession", - side_effect=AssertionError("media delivery used text-only chat.postMessage"), + side_effect=AssertionError("media delivery used text-only aiohttp path"), ), ): result = await _slack_mod._standalone_send( @@ -1251,12 +1289,45 @@ class TestStandaloneSendMedia: ) assert result["success"] is True - client.files_upload_v2.assert_awaited_once_with( - channel="C123", - file=str(image), - filename="daily-report.png", - initial_comment="daily report", - thread_ts=None, + client.chat_postMessage.assert_awaited_once() + assert client.chat_postMessage.await_args.kwargs["text"] == "daily report" + client.files_upload_v2.assert_awaited_once() + up_kwargs = client.files_upload_v2.await_args.kwargs + assert up_kwargs["channel"] == "C123" + assert up_kwargs["file"] == str(image) + assert up_kwargs["filename"] == "daily-report.png" + assert up_kwargs["initial_comment"] == "" + + @pytest.mark.asyncio + async def test_caption_kwarg_rides_upload_as_initial_comment(self, tmp_path): + """When the tool layer passes caption=, it rides the upload and no + separate text message is posted (C8 caption-mode contract).""" + image = tmp_path / "chart.png" + image.write_bytes(b"\x89PNG\r\n\x1a\n") + client = MagicMock() + client.chat_postMessage = AsyncMock(return_value={"ok": True, "ts": "1.0"}) + client.files_upload_v2 = AsyncMock( + return_value={"ok": True, "files": [{"id": "F123"}]} + ) + config = PlatformConfig(enabled=True, token="xoxb-fake-token") + + with ( + _fake_slack_sdk_modules(client), + patch.object(_slack_mod, "resolve_proxy_url", return_value=None), + ): + result = await _slack_mod._standalone_send( + config, + "C123", + "", + thread_id=None, + media_files=[(str(image), False)], + caption="Q3 chart", + ) + + assert result["success"] is True + client.chat_postMessage.assert_not_awaited() + assert ( + client.files_upload_v2.await_args.kwargs["initial_comment"] == "Q3 chart" ) @@ -1371,12 +1442,13 @@ class TestStandaloneSendUserDmResolution: open_resp = self._mock_resp({"ok": True, "channel": {"id": "D777666555"}}) session = self._mock_session(open_resp) client = MagicMock() + client.chat_postMessage = AsyncMock(return_value={"ok": True, "ts": "1.0"}) client.files_upload_v2 = AsyncMock(return_value={"ok": True, "ts": "9.9"}) config = PlatformConfig(enabled=True, token="xoxb-fake-token") with ( patch.object(_slack_mod.aiohttp, "ClientSession", return_value=session), - patch.object(_slack_mod, "AsyncWebClient", return_value=client), + _fake_slack_sdk_modules(client), patch.object(_slack_mod, "resolve_proxy_url", return_value=None), ): result = await _slack_mod._standalone_send( diff --git a/tests/tools/test_send_message_tool.py b/tests/tools/test_send_message_tool.py index 38d28bcc4599..d7201f9aff6d 100644 --- a/tests/tools/test_send_message_tool.py +++ b/tests/tools/test_send_message_tool.py @@ -785,13 +785,15 @@ class TestSendToPlatformChunking: entry.standalone_sender_fn = original assert result["success"] is True + # C8 caption-mode: short text rides the upload as `caption=` and the + # message slot is emptied — one Slack bubble, not text + bare file. send.assert_awaited_once_with( pconfig, "C123", - "daily report", + "", thread_id=None, media_files=media_files, - force_document=False, + caption="daily report", ) def test_slack_bold_italic_formatted_before_send(self, monkeypatch):