diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index 0ba00d89017..4335a51f112 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -569,6 +569,16 @@ class BasePlatformAdapter(ABC): """ self._message_handler = handler + def set_session_store(self, session_store: Any) -> None: + """ + Set the session store for checking active sessions. + + Used by adapters that need to check if a thread/conversation + has an active session before processing messages (e.g., Slack + thread replies without explicit mentions). + """ + self._session_store = session_store + @abstractmethod async def connect(self) -> bool: """ diff --git a/gateway/platforms/slack.py b/gateway/platforms/slack.py index b4bf5ffd73a..384f379d768 100644 --- a/gateway/platforms/slack.py +++ b/gateway/platforms/slack.py @@ -766,11 +766,28 @@ class SlackAdapter(BasePlatformAdapter): else: thread_ts = event.get("thread_ts") or ts # ts fallback for channels - # In channels, only respond if bot is mentioned + # In channels, only respond if bot is mentioned OR if this is a + # reply in a thread where the bot has an active session. bot_uid = self._team_bot_user_ids.get(team_id, self._bot_user_id) - if not is_dm and bot_uid: - if f"<@{bot_uid}>" not in text: + is_mentioned = bot_uid and f"<@{bot_uid}>" in text + + if not is_dm and bot_uid and not is_mentioned: + # Check if this is a thread reply (thread_ts exists and differs from ts) + event_thread_ts = event.get("thread_ts") + is_thread_reply = event_thread_ts and event_thread_ts != ts + + if is_thread_reply and self._has_active_session_for_thread( + channel_id=channel_id, + thread_ts=event_thread_ts, + user_id=user_id, + ): + # Allow thread replies without mention if there's an active session + pass + else: + # Not a thread reply or no active session - ignore return + + if is_mentioned: # Strip the bot mention from the text text = text.replace(f"<@{bot_uid}>", "").strip() @@ -936,6 +953,68 @@ class SlackAdapter(BasePlatformAdapter): await self.handle_message(event) + def _has_active_session_for_thread( + self, + channel_id: str, + thread_ts: str, + user_id: str, + ) -> bool: + """Check if there's an active session for a thread. + + Used to determine if thread replies without @mentions should be + processed (they should if there's an active session). + + Args: + channel_id: The Slack channel ID + thread_ts: The thread timestamp (parent message ts) + user_id: The user ID of the sender + + Returns: + True if there's an active session for this thread + """ + session_store = getattr(self, "_session_store", None) + if not session_store: + return False + + try: + # Build a SessionSource for this thread + from gateway.session import SessionSource + from gateway.config import Platform + + source = SessionSource( + platform=Platform.SLACK, + chat_id=channel_id, + chat_type="group", + user_id=user_id, + thread_id=thread_ts, + ) + + # Generate the session key using the same logic as SessionStore + # This mirrors the logic in build_session_key for group sessions + key_parts = ["agent:main", "slack", "group", channel_id, thread_ts] + + # Include user_id if group_sessions_per_user is enabled + # We check the session store config if available + group_sessions_per_user = getattr( + session_store, "config", {} + ) + if hasattr(group_sessions_per_user, "group_sessions_per_user"): + group_sessions_per_user = group_sessions_per_user.group_sessions_per_user + else: + group_sessions_per_user = True # Default + + if group_sessions_per_user and user_id: + key_parts.append(str(user_id)) + + session_key = ":".join(key_parts) + + # Check if the session exists in the store + session_store._ensure_loaded() + return session_key in session_store._entries + except Exception: + # If anything goes wrong, default to False (require mention) + return False + async def _download_slack_file(self, url: str, ext: str, audio: bool = False, team_id: str = "") -> str: """Download a Slack file using the bot token for auth, with retry.""" import asyncio diff --git a/gateway/run.py b/gateway/run.py index 82cb10b4e34..9d5ac5aa2c6 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -1127,6 +1127,7 @@ class GatewayRunner: # Set up message + fatal error handlers adapter.set_message_handler(self._handle_message) adapter.set_fatal_error_handler(self._handle_adapter_fatal_error) + adapter.set_session_store(self.session_store) # Try to connect logger.info("Connecting to %s...", platform.value) @@ -1424,6 +1425,7 @@ class GatewayRunner: adapter.set_message_handler(self._handle_message) adapter.set_fatal_error_handler(self._handle_adapter_fatal_error) + adapter.set_session_store(self.session_store) success = await adapter.connect() if success: diff --git a/tests/gateway/test_slack.py b/tests/gateway/test_slack.py index 81f8077ad6b..89b44718344 100644 --- a/tests/gateway/test_slack.py +++ b/tests/gateway/test_slack.py @@ -699,6 +699,147 @@ class TestReactions: assert remove_calls[0].kwargs["name"] == "eyes" +# --------------------------------------------------------------------------- +# TestThreadReplyHandling +# --------------------------------------------------------------------------- + + +class TestThreadReplyHandling: + """Test thread reply processing without explicit bot mentions.""" + + @pytest.fixture() + def mock_session_store(self): + """Create a mock session store with entries dict.""" + store = MagicMock() + store._entries = {} + store._ensure_loaded = MagicMock() + store.config = MagicMock() + store.config.group_sessions_per_user = True + return store + + @pytest.fixture() + def adapter_with_session_store(self, mock_session_store): + """Create an adapter with a mock session store attached.""" + config = PlatformConfig(enabled=True, token="***") + a = SlackAdapter(config) + a._app = MagicMock() + a._app.client = AsyncMock() + a._bot_user_id = "U_BOT" + a._team_bot_user_ids = {"T_TEAM": "U_BOT"} + a._running = True + a.handle_message = AsyncMock() + a.set_session_store(mock_session_store) + return a + + @pytest.mark.asyncio + async def test_thread_reply_without_mention_no_session_ignored( + self, adapter_with_session_store, mock_session_store + ): + """Thread replies without mention should be ignored if no active session.""" + mock_session_store._entries = {} # No active sessions + + event = { + "text": "Just replying in the thread", + "user": "U_USER", + "channel": "C123", + "ts": "123.456", + "thread_ts": "123.000", # Different from ts - this is a reply + "channel_type": "channel", + "team": "T_TEAM", + } + await adapter_with_session_store._handle_slack_message(event) + adapter_with_session_store.handle_message.assert_not_called() + + @pytest.mark.asyncio + async def test_thread_reply_without_mention_with_session_processed( + self, adapter_with_session_store, mock_session_store + ): + """Thread replies without mention should be processed if there's an active session.""" + # Simulate an active session for this thread + session_key = "agent:main:slack:group:C123:123.000:U_USER" + mock_session_store._entries = {session_key: MagicMock()} + + event = { + "text": "Follow-up question", + "user": "U_USER", + "channel": "C123", + "ts": "123.456", + "thread_ts": "123.000", # Reply in thread 123.000 + "channel_type": "channel", + "team": "T_TEAM", + } + await adapter_with_session_store._handle_slack_message(event) + adapter_with_session_store.handle_message.assert_called_once() + + # Verify the text is passed through unchanged (no mention stripping needed) + msg_event = adapter_with_session_store.handle_message.call_args[0][0] + assert msg_event.text == "Follow-up question" + + @pytest.mark.asyncio + async def test_thread_reply_with_mention_strips_bot_id( + self, adapter_with_session_store, mock_session_store + ): + """Thread replies with @mention should still strip the bot ID.""" + # Even with a session, mentions should be stripped + session_key = "agent:main:slack:group:C123:123.000:U_USER" + mock_session_store._entries = {session_key: MagicMock()} + + event = { + "text": "<@U_BOT> thanks for the help", + "user": "U_USER", + "channel": "C123", + "ts": "123.456", + "thread_ts": "123.000", + "channel_type": "channel", + "team": "T_TEAM", + } + await adapter_with_session_store._handle_slack_message(event) + adapter_with_session_store.handle_message.assert_called_once() + + msg_event = adapter_with_session_store.handle_message.call_args[0][0] + assert "<@U_BOT>" not in msg_event.text + assert msg_event.text == "thanks for the help" + + @pytest.mark.asyncio + async def test_top_level_message_requires_mention_even_with_session( + self, adapter_with_session_store, mock_session_store + ): + """Top-level channel messages should require mention even if session exists.""" + # Session exists but this is a top-level message (no thread_ts) + session_key = "agent:main:slack:group:C123:123.000:U_USER" + mock_session_store._entries = {session_key: MagicMock()} + + event = { + "text": "New question without mention", + "user": "U_USER", + "channel": "C123", + "ts": "456.789", + # No thread_ts - this is a top-level message + "channel_type": "channel", + "team": "T_TEAM", + } + await adapter_with_session_store._handle_slack_message(event) + adapter_with_session_store.handle_message.assert_not_called() + + @pytest.mark.asyncio + async def test_no_session_store_ignores_thread_replies( + self, adapter + ): + """If no session store is attached, thread replies without mention should be ignored.""" + # adapter fixture has no session store attached + event = { + "text": "Thread reply without mention", + "user": "U_USER", + "channel": "C123", + "ts": "123.456", + "thread_ts": "123.000", + "channel_type": "channel", + "team": "T_TEAM", + } + await adapter._handle_slack_message(event) + adapter.handle_message.assert_not_called() + + # --------------------------------------------------------------------------- # TestUserNameResolution # ---------------------------------------------------------------------------