diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index bf76bd85021a..1a03c2b57f22 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -675,6 +675,10 @@ class SlackAdapter(BasePlatformAdapter): # so a multi-workspace Socket Mode process never reuses another # tenant's display name. self._user_name_cache: Dict[Tuple[str, str], str] = {} + # (team_id, user_id) → Slack bot identity, same workspace scoping as + # the name cache. Used to catch peer-agent posts that arrive as plain + # user messages without bot_id/subtype=bot_message markers. + self._user_is_bot_cache: Dict[Tuple[str, str], bool] = {} self._socket_mode_task: Optional[asyncio.Task] = None # Multi-workspace support self._team_clients: Dict[str, Any] = {} # team_id → WebClient @@ -2173,6 +2177,28 @@ class SlackAdapter(BasePlatformAdapter): except Exception: pass + def _slack_allow_bots(self) -> str: + """Return normalized Slack bot-message policy.""" + raw = self.config.extra.get("allow_bots", "") + if not raw: + raw = os.getenv("SLACK_ALLOW_BOTS", "none") + value = str(raw).lower().strip() + if value not in {"none", "mentions", "all"}: + logger.warning("[Slack] Unknown allow_bots=%r; treating as 'none'", raw) + return "none" + return value + + def _event_declares_bot_sender(self, event: dict) -> bool: + """Return True when the Slack event itself identifies a bot sender.""" + if event.get("bot_id") or event.get("bot_profile"): + return True + if event.get("subtype") == "bot_message": + return True + profile = event.get("user_profile") + if isinstance(profile, dict) and bool(profile.get("is_bot")): + return True + return False + def _resolve_thread_ts( self, reply_to: Optional[str] = None, @@ -2722,9 +2748,18 @@ class SlackAdapter(BasePlatformAdapter): else self._app.client ) result = await client.users_info(user=user_id) + if not isinstance(result, dict): + self._user_is_bot_cache[cache_key] = False + self._user_name_cache[cache_key] = user_id + return user_id user = result.get("user", {}) + profile = user.get("profile", {}) if isinstance(user, dict) else {} + self._user_is_bot_cache[cache_key] = bool( + user.get("is_bot") + or user.get("is_workflow_bot") + or (isinstance(profile, dict) and profile.get("bot_id")) + ) # Prefer display_name → real_name → user_id - profile = user.get("profile", {}) name = ( profile.get("display_name") or profile.get("real_name") @@ -2804,6 +2839,60 @@ class SlackAdapter(BasePlatformAdapter): f"mention of you, even if their name is similar." ) + async def _resolve_user_is_bot( + self, user_id: str, chat_id: str = "", team_id: str = "" + ) -> bool: + """Resolve whether a Slack user ID is a bot account, with caching. + + Workspace-scoped like :meth:`_resolve_user_name` — Slack user IDs are + team-local, so the cache key includes the team. + """ + if not user_id: + return False + team_id = str(team_id or self._channel_team.get(chat_id, "")) + cache_key = (team_id, str(user_id)) + if cache_key in self._user_is_bot_cache: + return self._user_is_bot_cache[cache_key] + if not self._app: + self._user_is_bot_cache[cache_key] = False + return False + + try: + client = ( + self._get_client(chat_id, team_id=team_id or None) + if chat_id + else self._app.client + ) + result = await client.users_info(user=user_id) + if not isinstance(result, dict): + self._user_is_bot_cache[cache_key] = False + self._user_name_cache.setdefault(cache_key, user_id) + return False + user = result.get("user", {}) + profile = user.get("profile", {}) if isinstance(user, dict) else {} + is_bot = bool( + user.get("is_bot") + or user.get("is_workflow_bot") + or (isinstance(profile, dict) and profile.get("bot_id")) + ) + self._user_is_bot_cache[cache_key] = is_bot + + # Populate the name cache from the same users.info response so the + # later source construction does not need a second API lookup. + name = ( + profile.get("display_name") + or profile.get("real_name") + or user.get("real_name") + or user.get("name") + or user_id + ) + self._user_name_cache[cache_key] = name + return is_bot + except Exception as e: + logger.debug("[Slack] users.info bot check failed for %s: %s", user_id, e) + self._user_is_bot_cache[cache_key] = False + return False + async def send_image_file( self, chat_id: str, @@ -3751,11 +3840,8 @@ class SlackAdapter(BasePlatformAdapter): # "none" — ignore all bot messages (default, backward-compatible) # "mentions" — accept bot messages only when they @mention us # "all" — accept all bot messages (except our own) - if event.get("bot_id") or event.get("subtype") == "bot_message": - allow_bots = self.config.extra.get("allow_bots", "") - if not allow_bots: - allow_bots = os.getenv("SLACK_ALLOW_BOTS", "none") - allow_bots = str(allow_bots).lower().strip() + if self._event_declares_bot_sender(event): + allow_bots = self._slack_allow_bots() if allow_bots == "none": return elif allow_bots == "mentions": @@ -3995,6 +4081,31 @@ class SlackAdapter(BasePlatformAdapter): event_thread_ts = event.get("thread_ts") is_thread_reply = bool(event_thread_ts and event_thread_ts != ts) + # Some Slack bot posts arrive as ordinary-looking message events with a + # bot *user* id but without ``bot_id``/``subtype=bot_message``. This is + # the shape produced by peer Hermes agents in Socket Mode on some + # workspaces. If we let those fall through as human users, an old + # thread mention or active session will re-trigger the target agent on + # every peer status/error/ack message, causing agent-agent loops. Apply + # the same allow_bots policy to resolved bot users, and in + # ``allow_bots: mentions`` require the current message text to mention + # this bot — thread history, reply parents, and active sessions do not + # count as a bot-to-bot summons. + if user_id and user_id != bot_uid: + sender_is_bot_user = self._event_declares_bot_sender(event) + if not sender_is_bot_user: + sender_is_bot_user = await self._resolve_user_is_bot( + user_id, + chat_id=channel_id, + team_id=team_id, + ) + if sender_is_bot_user: + allow_bots = self._slack_allow_bots() + if allow_bots == "none": + return + if allow_bots == "mentions" and not is_mentioned: + return + if not is_one_to_one_dm and bot_uid: # Check allowed channels — if set, only respond in these channels (whitelist) allowed_channels = self._slack_allowed_channels() diff --git a/tests/gateway/test_slack.py b/tests/gateway/test_slack.py index f4fdae41e6bd..cd121c313abf 100644 --- a/tests/gateway/test_slack.py +++ b/tests/gateway/test_slack.py @@ -2360,6 +2360,72 @@ class TestMessageRouting: await adapter._handle_slack_message(event) adapter.handle_message.assert_not_called() + @pytest.mark.asyncio + async def test_allow_bots_mentions_ignores_bot_user_without_current_mention( + self, adapter + ): + """Bot users need a fresh @mention even in an already-mentioned thread. + + Slack peer-agent posts can arrive as normal-looking message events with + only a bot user id, no bot_id/subtype=bot_message. Those must still obey + allow_bots=mentions; otherwise status/error/ack posts from one agent can + retrigger another agent through old thread state. + """ + adapter.config.extra["allow_bots"] = "mentions" + adapter._mentioned_threads.add("123.000") + adapter._app.client.users_info = AsyncMock( + return_value={ + "user": { + "is_bot": True, + "profile": {"display_name": "AIDx Engineer"}, + } + } + ) + event = { + "text": ":warning: Codex response remained incomplete after 3 continuation attempts", + "user": "U_PEER_BOT", + "channel": "C123", + "channel_type": "channel", + "ts": "123.456", + "thread_ts": "123.000", + } + + await adapter._handle_slack_message(event) + + adapter.handle_message.assert_not_called() + + @pytest.mark.asyncio + async def test_allow_bots_mentions_processes_bot_user_with_current_mention( + self, adapter + ): + """Explicit peer-agent @mentions still route when allow_bots=mentions.""" + adapter.config.extra["allow_bots"] = "mentions" + adapter._fetch_thread_context = AsyncMock(return_value="") + adapter._fetch_thread_parent_text = AsyncMock(return_value=None) + adapter._app.client.users_info = AsyncMock( + return_value={ + "user": { + "is_bot": True, + "profile": {"display_name": "AIDx Engineer"}, + } + } + ) + event = { + "text": "<@U_BOT> please answer exactly BOT_OK", + "user": "U_PEER_BOT", + "channel": "C123", + "channel_type": "channel", + "ts": "123.789", + "thread_ts": "123.000", + } + + await adapter._handle_slack_message(event) + + adapter.handle_message.assert_called_once() + msg_event = adapter.handle_message.call_args[0][0] + assert msg_event.text == "please answer exactly BOT_OK" + assert msg_event.source.user_name == "AIDx Engineer" + @pytest.mark.asyncio async def test_message_edits_ignored(self, adapter): """Message edits should be ignored.""" @@ -3472,6 +3538,8 @@ class TestThreadReplyHandling: a._bot_user_id = "U_BOT" a._team_bot_user_ids = {"T_TEAM": "U_BOT"} a._running = True + a._fetch_thread_parent_text = AsyncMock(return_value=None) + a._fetch_thread_context = AsyncMock(return_value="") a.handle_message = AsyncMock() a.set_session_store(mock_session_store) return a @@ -3856,6 +3924,8 @@ class TestAssistantThreadLifecycle: a._bot_user_id = "U_BOT" a._team_bot_user_ids = {"T_TEAM": "U_BOT"} a._running = True + a._fetch_thread_parent_text = AsyncMock(return_value=None) + a._fetch_thread_context = AsyncMock(return_value="") a.handle_message = AsyncMock() a.set_session_store(mock_session_store) return a diff --git a/tests/gateway/test_slack_mention.py b/tests/gateway/test_slack_mention.py index 9a9ba1dac5ab..eefc54f6d74e 100644 --- a/tests/gateway/test_slack_mention.py +++ b/tests/gateway/test_slack_mention.py @@ -486,6 +486,8 @@ def test_config_bridges_slack_free_response_channels(monkeypatch, tmp_path): import os as _os assert _os.environ["SLACK_REQUIRE_MENTION"] == "false" assert _os.environ["SLACK_FREE_RESPONSE_CHANNELS"] == "C0AQWDLHY9M,C9999999999" + _os.environ.pop("SLACK_REQUIRE_MENTION", None) + _os.environ.pop("SLACK_FREE_RESPONSE_CHANNELS", None) def test_top_level_slack_settings_do_not_disable_env_token_setup(monkeypatch, tmp_path): @@ -672,6 +674,7 @@ def test_config_bridges_slack_strict_mention(monkeypatch, tmp_path): assert config is not None import os as _os assert _os.environ["SLACK_STRICT_MENTION"] == "true" + _os.environ.pop("SLACK_STRICT_MENTION", None) # --------------------------------------------------------------------------- @@ -823,6 +826,7 @@ def test_config_bridges_slack_allowed_channels(monkeypatch, tmp_path): import os as _os assert _os.environ["SLACK_ALLOWED_CHANNELS"] == f"{CHANNEL_ID},{OTHER_CHANNEL_ID}" + _os.environ.pop("SLACK_ALLOWED_CHANNELS", None) def test_config_bridges_slack_allowed_channels_env_takes_precedence(monkeypatch, tmp_path):