diff --git a/nanobot/channels/slack.py b/nanobot/channels/slack.py index 2503f6a2d..af03d4973 100644 --- a/nanobot/channels/slack.py +++ b/nanobot/channels/slack.py @@ -5,6 +5,7 @@ import re from typing import Any from loguru import logger +from pydantic import Field from slack_sdk.socket_mode.request import SocketModeRequest from slack_sdk.socket_mode.response import SocketModeResponse from slack_sdk.socket_mode.websockets import SocketModeClient @@ -13,8 +14,6 @@ from slackify_markdown import slackify_markdown from nanobot.bus.events import OutboundMessage from nanobot.bus.queue import MessageBus -from pydantic import Field - from nanobot.channels.base import BaseChannel from nanobot.config.schema import Base @@ -50,6 +49,9 @@ class SlackChannel(BaseChannel): name = "slack" display_name = "Slack" + _SLACK_ID_RE = re.compile(r"^[CDGUW][A-Z0-9]{2,}$") + _SLACK_CHANNEL_REF_RE = re.compile(r"^<#([A-Z0-9]+)(?:\|[^>]+)?>$") + _SLACK_USER_REF_RE = re.compile(r"^<@([A-Z0-9]+)(?:\|[^>]+)?>$") @classmethod def default_config(cls) -> dict[str, Any]: @@ -63,6 +65,7 @@ class SlackChannel(BaseChannel): self._web_client: AsyncWebClient | None = None self._socket_client: SocketModeClient | None = None self._bot_user_id: str | None = None + self._target_cache: dict[str, str] = {} async def start(self) -> None: """Start the Slack Socket Mode client.""" @@ -113,6 +116,7 @@ class SlackChannel(BaseChannel): logger.warning("Slack client not running") return try: + target_chat_id = await self._resolve_target_chat_id(msg.chat_id) slack_meta = msg.metadata.get("slack", {}) if msg.metadata else {} thread_ts = slack_meta.get("thread_ts") channel_type = slack_meta.get("channel_type") @@ -123,7 +127,7 @@ class SlackChannel(BaseChannel): # but send a single blank message when the bot has no text or files to send. if msg.content or not (msg.media or []): await self._web_client.chat_postMessage( - channel=msg.chat_id, + channel=target_chat_id, text=self._to_mrkdwn(msg.content) if msg.content else " ", thread_ts=thread_ts_param, ) @@ -131,7 +135,7 @@ class SlackChannel(BaseChannel): for media_path in msg.media or []: try: await self._web_client.files_upload_v2( - channel=msg.chat_id, + channel=target_chat_id, file=media_path, thread_ts=thread_ts_param, ) @@ -141,12 +145,123 @@ class SlackChannel(BaseChannel): # Update reaction emoji when the final (non-progress) response is sent if not (msg.metadata or {}).get("_progress"): event = slack_meta.get("event", {}) - await self._update_react_emoji(msg.chat_id, event.get("ts")) + await self._update_react_emoji(event.get("channel") or msg.chat_id, event.get("ts")) except Exception as e: logger.error("Error sending Slack message: {}", e) raise + async def _resolve_target_chat_id(self, target: str) -> str: + """Resolve human-friendly Slack targets to concrete IDs when needed.""" + if not self._web_client: + return target + + target = target.strip() + if not target: + return target + + if match := self._SLACK_CHANNEL_REF_RE.fullmatch(target): + return match.group(1) + if match := self._SLACK_USER_REF_RE.fullmatch(target): + return await self._open_dm_for_user(match.group(1)) + if self._SLACK_ID_RE.fullmatch(target): + if target.startswith(("U", "W")): + return await self._open_dm_for_user(target) + return target + + if target.startswith("#"): + return await self._resolve_channel_name(target[1:]) + if target.startswith("@"): + return await self._resolve_user_handle(target[1:]) + + try: + return await self._resolve_channel_name(target) + except ValueError: + return await self._resolve_user_handle(target) + + async def _resolve_channel_name(self, name: str) -> str: + normalized = self._normalize_target_name(name) + if not normalized: + raise ValueError("Slack target channel name is empty") + + cache_key = f"channel:{normalized}" + if cache_key in self._target_cache: + return self._target_cache[cache_key] + + cursor: str | None = None + while True: + response = await self._web_client.conversations_list( + types="public_channel,private_channel", + exclude_archived=True, + limit=200, + cursor=cursor, + ) + for channel in response.get("channels", []): + if self._normalize_target_name(str(channel.get("name") or "")) == normalized: + channel_id = str(channel.get("id") or "") + if channel_id: + self._target_cache[cache_key] = channel_id + return channel_id + cursor = ((response.get("response_metadata") or {}).get("next_cursor") or "").strip() + if not cursor: + break + + raise ValueError( + f"Slack channel '{name}' was not found. Use a joined channel name like " + f"'#general' or a concrete channel ID." + ) + + async def _resolve_user_handle(self, handle: str) -> str: + normalized = self._normalize_target_name(handle) + if not normalized: + raise ValueError("Slack target user handle is empty") + + cache_key = f"user:{normalized}" + if cache_key in self._target_cache: + return self._target_cache[cache_key] + + cursor: str | None = None + while True: + response = await self._web_client.users_list(limit=200, cursor=cursor) + for member in response.get("members", []): + if self._member_matches_handle(member, normalized): + user_id = str(member.get("id") or "") + if not user_id: + continue + dm_id = await self._open_dm_for_user(user_id) + self._target_cache[cache_key] = dm_id + return dm_id + cursor = ((response.get("response_metadata") or {}).get("next_cursor") or "").strip() + if not cursor: + break + + raise ValueError( + f"Slack user '{handle}' was not found. Use '@name' or a concrete DM/channel ID." + ) + + async def _open_dm_for_user(self, user_id: str) -> str: + response = await self._web_client.conversations_open(users=user_id) + channel_id = str(((response.get("channel") or {}).get("id")) or "") + if not channel_id: + raise ValueError(f"Slack DM target for user '{user_id}' could not be opened.") + return channel_id + + @staticmethod + def _normalize_target_name(value: str) -> str: + return value.strip().lstrip("#@").lower() + + @classmethod + def _member_matches_handle(cls, member: dict[str, Any], normalized: str) -> bool: + profile = member.get("profile") or {} + candidates = { + str(member.get("name") or ""), + str(profile.get("display_name") or ""), + str(profile.get("display_name_normalized") or ""), + str(profile.get("real_name") or ""), + str(profile.get("real_name_normalized") or ""), + } + return normalized in {cls._normalize_target_name(candidate) for candidate in candidates if candidate} + async def _on_socket_request( self, client: SocketModeClient, diff --git a/tests/channels/test_slack_channel.py b/tests/channels/test_slack_channel.py index f7eec95c0..6fb05a912 100644 --- a/tests/channels/test_slack_channel.py +++ b/tests/channels/test_slack_channel.py @@ -10,8 +10,7 @@ except ImportError: from nanobot.bus.events import OutboundMessage from nanobot.bus.queue import MessageBus -from nanobot.channels.slack import SlackChannel -from nanobot.channels.slack import SlackConfig +from nanobot.channels.slack import SlackChannel, SlackConfig class _FakeAsyncWebClient: @@ -20,6 +19,12 @@ class _FakeAsyncWebClient: self.file_upload_calls: list[dict[str, object | None]] = [] self.reactions_add_calls: list[dict[str, object | None]] = [] self.reactions_remove_calls: list[dict[str, object | None]] = [] + self.conversations_list_calls: list[dict[str, object | None]] = [] + self.users_list_calls: list[dict[str, object | None]] = [] + self.conversations_open_calls: list[dict[str, object | None]] = [] + self._conversations_pages: list[dict[str, object]] = [] + self._users_pages: list[dict[str, object]] = [] + self._open_dm_response: dict[str, object] = {"channel": {"id": "D_OPENED"}} async def chat_postMessage( self, @@ -81,6 +86,22 @@ class _FakeAsyncWebClient: } ) + async def conversations_list(self, **kwargs): + self.conversations_list_calls.append(kwargs) + if self._conversations_pages: + return self._conversations_pages.pop(0) + return {"channels": [], "response_metadata": {"next_cursor": ""}} + + async def users_list(self, **kwargs): + self.users_list_calls.append(kwargs) + if self._users_pages: + return self._users_pages.pop(0) + return {"members": [], "response_metadata": {"next_cursor": ""}} + + async def conversations_open(self, **kwargs): + self.conversations_open_calls.append(kwargs) + return self._open_dm_response + @pytest.mark.asyncio async def test_send_uses_thread_for_channel_messages() -> None: @@ -151,3 +172,115 @@ async def test_send_updates_reaction_when_final_response_sent() -> None: assert fake_web.reactions_add_calls == [ {"channel": "C123", "name": "white_check_mark", "timestamp": "1700000000.000100"} ] + + +@pytest.mark.asyncio +async def test_send_resolves_channel_name_to_channel_id() -> None: + channel = SlackChannel(SlackConfig(enabled=True), MessageBus()) + fake_web = _FakeAsyncWebClient() + fake_web._conversations_pages = [ + { + "channels": [{"id": "C999", "name": "channel_x"}], + "response_metadata": {"next_cursor": ""}, + } + ] + channel._web_client = fake_web + + await channel.send( + OutboundMessage( + channel="slack", + chat_id="#channel_x", + content="hello", + ) + ) + + assert fake_web.chat_post_calls == [ + {"channel": "C999", "text": "hello\n", "thread_ts": None} + ] + assert len(fake_web.conversations_list_calls) == 1 + + +@pytest.mark.asyncio +async def test_send_resolves_user_handle_to_dm_channel() -> None: + channel = SlackChannel(SlackConfig(enabled=True), MessageBus()) + fake_web = _FakeAsyncWebClient() + fake_web._users_pages = [ + { + "members": [ + { + "id": "U234", + "name": "alice", + "profile": {"display_name": "Alice"}, + } + ], + "response_metadata": {"next_cursor": ""}, + } + ] + fake_web._open_dm_response = {"channel": {"id": "D234"}} + channel._web_client = fake_web + + await channel.send( + OutboundMessage( + channel="slack", + chat_id="@alice", + content="hello", + ) + ) + + assert fake_web.conversations_open_calls == [{"users": "U234"}] + assert fake_web.chat_post_calls == [ + {"channel": "D234", "text": "hello\n", "thread_ts": None} + ] + + +@pytest.mark.asyncio +async def test_send_updates_reaction_on_origin_channel_for_cross_channel_send() -> None: + channel = SlackChannel(SlackConfig(enabled=True, react_emoji="eyes"), MessageBus()) + fake_web = _FakeAsyncWebClient() + fake_web._conversations_pages = [ + { + "channels": [{"id": "C999", "name": "channel_x"}], + "response_metadata": {"next_cursor": ""}, + } + ] + channel._web_client = fake_web + + await channel.send( + OutboundMessage( + channel="slack", + chat_id="channel_x", + content="done", + metadata={ + "slack": { + "event": {"ts": "1700000000.000100", "channel": "D_ORIGIN"}, + "channel_type": "im", + }, + }, + ) + ) + + assert fake_web.chat_post_calls == [ + {"channel": "C999", "text": "done\n", "thread_ts": None} + ] + assert fake_web.reactions_remove_calls == [ + {"channel": "D_ORIGIN", "name": "eyes", "timestamp": "1700000000.000100"} + ] + assert fake_web.reactions_add_calls == [ + {"channel": "D_ORIGIN", "name": "white_check_mark", "timestamp": "1700000000.000100"} + ] + + +@pytest.mark.asyncio +async def test_send_raises_when_named_target_cannot_be_resolved() -> None: + channel = SlackChannel(SlackConfig(enabled=True), MessageBus()) + fake_web = _FakeAsyncWebClient() + channel._web_client = fake_web + + with pytest.raises(ValueError, match="was not found"): + await channel.send( + OutboundMessage( + channel="slack", + chat_id="#missing-channel", + content="hello", + ) + )