From c7b9dfa9618d3be358812ec535e358a7ff070d85 Mon Sep 17 00:00:00 2001 From: Esther Date: Wed, 22 Jul 2026 08:11:07 -0700 Subject: [PATCH] fix(slack): resolve user IDs to DM channels in standalone cron delivery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cron jobs targeting a Slack user (deliver=slack:U…) failed with channel_not_found: chat.postMessage and files_upload_v2 require a conversation ID, and a DM must be opened first via conversations.open to obtain a D… ID. The tool-level resolution in send_message() does not cover cron's direct _send_to_platform → standalone_sender_fn path. Add _resolve_slack_user_dm to the Slack plugin: resolves U…/W… targets via conversations.open (proxy-aware, cached per token+user so repeated cron fires don't re-open the DM), wired into _standalone_send ahead of both the text and media delivery branches so every standalone entry point benefits. Adapted from #17444 by @Esther-Zhu023 — the original patched the legacy _send_slack helper in tools/send_message_tool.py, which has since moved into the plugin as _standalone_send (#41112). Closes #17444 Co-Authored-By: Claude Opus 4.6 --- plugins/platforms/slack/adapter.py | 70 +++++++++++++++ tests/gateway/test_slack.py | 134 +++++++++++++++++++++++++++++ 2 files changed, 204 insertions(+) diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index bc4ed11cb3cd..61240bc9eb16 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -6472,6 +6472,60 @@ 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] = {} + + +async def _resolve_slack_user_dm(token: str, user_id: str) -> Optional[str]: + """Resolve a Slack user ID (U.../W...) to a DM conversation ID (D...). + + ``chat.postMessage`` and ``files_upload_v2`` require a conversation ID; a + DM must be opened first via ``conversations.open``. Results are cached + per (token, user_id) pair to avoid redundant API calls. Returns None if + resolution fails (missing ``im:write`` scope, unknown user, etc.). + """ + cache_key = f"{token}:{user_id}" + if cache_key in _slack_dm_cache: + return _slack_dm_cache[cache_key] + + try: + import aiohttp + except ImportError: + return None + try: + from gateway.platforms.base import proxy_kwargs_for_aiohttp + + _proxy = resolve_proxy_url() + _sess_kw, _req_kw = proxy_kwargs_for_aiohttp(_proxy) + url = "https://slack.com/api/conversations.open" + headers = { + "Authorization": f"Bearer {token}", + "Content-Type": "application/json", + } + async with aiohttp.ClientSession( + timeout=aiohttp.ClientTimeout(total=15), **_sess_kw + ) as session: + payload = {"users": user_id} + async with session.post( + url, headers=headers, json=payload, **_req_kw + ) as resp: + data = await resp.json() + if data.get("ok") and data.get("channel", {}).get("id"): + channel_id = data["channel"]["id"] + _slack_dm_cache[cache_key] = channel_id + return channel_id + logger.warning( + "[Slack] conversations.open failed for %s: %s", + user_id, + data.get("error", "unknown"), + ) + return None + except Exception as e: + logger.warning("[Slack] conversations.open exception for %s: %s", user_id, e) + return None + + async def _standalone_send( pconfig, chat_id, @@ -6496,6 +6550,22 @@ async def _standalone_send( if not token: return {"error": "Slack send failed: SLACK_BOT_TOKEN not configured"} + # User-targeted delivery: chat.postMessage / files_upload_v2 reject bare + # user IDs (U.../W...) — resolve to a DM conversation ID (D...) first via + # conversations.open so `deliver=slack:U…` cron jobs reach the user's DM + # instead of failing with channel_not_found (#17444). + chat_id = str(chat_id or "") + if chat_id[:1] in ("U", "W"): + resolved = await _resolve_slack_user_dm(token, chat_id) + if resolved is None: + return { + "error": ( + f"Slack user ID resolution failed for {chat_id} " + "(conversations.open — check the bot's im:write scope)" + ) + } + chat_id = resolved + formatted = message if message: try: diff --git a/tests/gateway/test_slack.py b/tests/gateway/test_slack.py index 5096c79c0d6a..85f63555eef5 100644 --- a/tests/gateway/test_slack.py +++ b/tests/gateway/test_slack.py @@ -1251,6 +1251,140 @@ class TestStandaloneSendMedia: ) +# --------------------------------------------------------------------------- +# TestStandaloneSendUserDmResolution +# --------------------------------------------------------------------------- + + +class TestStandaloneSendUserDmResolution: + """_standalone_send resolves user IDs (U.../W...) to DM channels via + conversations.open before posting (#17444). Cron `deliver=slack:U…` + bypasses send_message()'s tool-level resolution, so the standalone + sender must resolve on its own.""" + + @staticmethod + def _mock_resp(payload): + resp = MagicMock() + resp.status = 200 + resp.json = AsyncMock(return_value=payload) + resp.__aenter__ = AsyncMock(return_value=resp) + resp.__aexit__ = AsyncMock(return_value=None) + return resp + + def _mock_session(self, *responses): + session = MagicMock() + session.__aenter__ = AsyncMock(return_value=session) + session.__aexit__ = AsyncMock(return_value=None) + session.post = MagicMock(side_effect=list(responses)) + return session + + @pytest.mark.asyncio + async def test_user_id_target_resolves_dm_then_posts(self): + _slack_mod._slack_dm_cache.clear() + open_resp = self._mock_resp({"ok": True, "channel": {"id": "D999888777"}}) + post_resp = self._mock_resp({"ok": True, "ts": "123.456"}) + session = self._mock_session(open_resp, post_resp) + config = PlatformConfig(enabled=True, token="xoxb-fake-token") + + with patch.object(_slack_mod.aiohttp, "ClientSession", return_value=session): + result = await _slack_mod._standalone_send( + config, "U1234567890", "hello via DM" + ) + + assert result["success"] is True + assert result["chat_id"] == "D999888777" + open_url = session.post.call_args_list[0].args[0] + assert "conversations.open" in open_url + assert session.post.call_args_list[0].kwargs["json"] == {"users": "U1234567890"} + post_url = session.post.call_args_list[1].args[0] + assert "chat.postMessage" in post_url + assert session.post.call_args_list[1].kwargs["json"]["channel"] == "D999888777" + _slack_mod._slack_dm_cache.clear() + + @pytest.mark.asyncio + async def test_channel_id_skips_resolution(self): + _slack_mod._slack_dm_cache.clear() + post_resp = self._mock_resp({"ok": True, "ts": "123.456"}) + session = self._mock_session(post_resp) + config = PlatformConfig(enabled=True, token="xoxb-fake-token") + + with patch.object(_slack_mod.aiohttp, "ClientSession", return_value=session): + result = await _slack_mod._standalone_send(config, "C123", "hello channel") + + assert result["success"] is True + assert session.post.call_count == 1 + assert "chat.postMessage" in session.post.call_args.args[0] + + @pytest.mark.asyncio + async def test_user_id_resolution_failure_returns_error(self): + _slack_mod._slack_dm_cache.clear() + open_resp = self._mock_resp({"ok": False, "error": "user_not_found"}) + session = self._mock_session(open_resp) + config = PlatformConfig(enabled=True, token="xoxb-fake-token") + + with patch.object(_slack_mod.aiohttp, "ClientSession", return_value=session): + result = await _slack_mod._standalone_send(config, "U9999999999", "hello") + + assert "error" in result + assert "user ID resolution failed" in result["error"] + assert session.post.call_count == 1 + assert "conversations.open" in session.post.call_args.args[0] + _slack_mod._slack_dm_cache.clear() + + @pytest.mark.asyncio + async def test_user_id_resolution_cached_across_sends(self): + _slack_mod._slack_dm_cache.clear() + open_resp = self._mock_resp({"ok": True, "channel": {"id": "D555444333"}}) + post_resp1 = self._mock_resp({"ok": True, "ts": "1.1"}) + session1 = self._mock_session(open_resp, post_resp1) + config = PlatformConfig(enabled=True, token="xoxb-fake-token") + + with patch.object(_slack_mod.aiohttp, "ClientSession", return_value=session1): + r1 = await _slack_mod._standalone_send(config, "U1112223334", "first") + assert r1["success"] is True + assert session1.post.call_count == 2 + + post_resp2 = self._mock_resp({"ok": True, "ts": "2.2"}) + session2 = self._mock_session(post_resp2) + with patch.object(_slack_mod.aiohttp, "ClientSession", return_value=session2): + r2 = await _slack_mod._standalone_send(config, "U1112223334", "second") + assert r2["success"] is True + assert r2["chat_id"] == "D555444333" + assert session2.post.call_count == 1 # cache hit — no conversations.open + _slack_mod._slack_dm_cache.clear() + + @pytest.mark.asyncio + async def test_user_id_media_delivery_resolves_dm_before_upload(self, tmp_path): + """Media path composes with DM resolution: files_upload_v2 gets D…""" + _slack_mod._slack_dm_cache.clear() + image = tmp_path / "report.png" + image.write_bytes(b"\x89PNG\r\n\x1a\n") + open_resp = self._mock_resp({"ok": True, "channel": {"id": "D777666555"}}) + session = self._mock_session(open_resp) + client = MagicMock() + 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), + patch.object(_slack_mod, "resolve_proxy_url", return_value=None), + ): + result = await _slack_mod._standalone_send( + config, + "U1234509876", + "daily report", + thread_id=None, + media_files=[(str(image), False)], + ) + + assert result["success"] is True + assert result["chat_id"] == "D777666555" + client.files_upload_v2.assert_awaited_once() + assert client.files_upload_v2.await_args.kwargs["channel"] == "D777666555" + _slack_mod._slack_dm_cache.clear() + + # --------------------------------------------------------------------------- # TestSendDocument # ---------------------------------------------------------------------------