diff --git a/nanobot/channels/slack/runtime.py b/nanobot/channels/slack/runtime.py index 56512165c..67ef6ee45 100644 --- a/nanobot/channels/slack/runtime.py +++ b/nanobot/channels/slack/runtime.py @@ -493,12 +493,11 @@ class SlackChannel(BaseChannel): except Exception as e: self.logger.debug("reactions_add failed: {}", e) - # Thread-scoped session key whenever the user is in a real thread - # (raw_thread_ts is set). DM threads get their own session, separate - # from the DM root, so context doesn't bleed across thread boundaries. - session_key = ( - f"slack:{chat_id}:{thread_ts}" if thread_ts and raw_thread_ts else None - ) + # Thread-scoped session key whenever the turn lives in a thread: either the + # message arrived inside one (raw_thread_ts) or reply_in_thread opens a new + # thread for this channel message. DM roots have no thread_ts and keep the + # default per-chat session, so context doesn't bleed across thread boundaries. + session_key = f"slack:{chat_id}:{thread_ts}" if thread_ts else None media_paths: list[str] = [] file_markers: list[str] = [] for file_info in _as_json_list(event.get("files")) or []: diff --git a/nanobot/channels/slack/tests/test_slack_channel.py b/nanobot/channels/slack/tests/test_slack_channel.py index a79c564a5..2299f2793 100644 --- a/nanobot/channels/slack/tests/test_slack_channel.py +++ b/nanobot/channels/slack/tests/test_slack_channel.py @@ -555,6 +555,113 @@ async def test_dm_thread_message_keeps_thread_ts_and_threaded_session() -> None: assert kwargs["metadata"]["slack"]["thread_ts"] == "1700000000.000100" +def _channel_mention_request(envelope_id: str, ts: str) -> SimpleNamespace: + return SimpleNamespace( + type="events_api", + envelope_id=envelope_id, + payload={ + "event": { + "type": "app_mention", + "user": "U1", + "channel": "C123", + "text": "<@UBOT> hello", + "ts": ts, + } + }, + ) + + +@pytest.mark.asyncio +async def test_channel_root_message_uses_thread_scoped_session() -> None: + """A channel mention that opens a thread belongs to that thread's session.""" + channel = SlackChannel(SlackConfig(enabled=True), MessageBus()) + channel._bot_user_id = "UBOT" + channel._web_client = _FakeAsyncWebClient() + channel._handle_message = AsyncMock() # type: ignore[method-assign] + client = SimpleNamespace(send_socket_mode_response=AsyncMock()) + + req = _channel_mention_request("env-c1", "1700000000.000100") + + await channel._on_socket_request(client, req) + + channel._handle_message.assert_awaited_once() + kwargs = channel._handle_message.await_args.kwargs + assert kwargs["session_key"] == "slack:C123:1700000000.000100" + assert kwargs["metadata"]["slack"]["thread_ts"] == "1700000000.000100" + + +@pytest.mark.asyncio +async def test_channel_root_messages_do_not_share_one_session() -> None: + """Two threads opened in the same channel must not collapse into one session.""" + channel = SlackChannel(SlackConfig(enabled=True), MessageBus()) + channel._bot_user_id = "UBOT" + channel._web_client = _FakeAsyncWebClient() + channel._handle_message = AsyncMock() # type: ignore[method-assign] + client = SimpleNamespace(send_socket_mode_response=AsyncMock()) + + first = _channel_mention_request("env-c1", "1700000000.000100") + second = _channel_mention_request("env-c2", "1700000000.000200") + + await channel._on_socket_request(client, first) + await channel._on_socket_request(client, second) + + session_keys = [call.kwargs["session_key"] for call in channel._handle_message.await_args_list] + assert session_keys == [ + "slack:C123:1700000000.000100", + "slack:C123:1700000000.000200", + ] + + +@pytest.mark.asyncio +async def test_channel_root_message_without_reply_in_thread_uses_channel_session() -> None: + """With reply_in_thread disabled no thread is opened, so the channel session is used.""" + channel = SlackChannel(SlackConfig(enabled=True, reply_in_thread=False), MessageBus()) + channel._bot_user_id = "UBOT" + channel._web_client = _FakeAsyncWebClient() + channel._handle_message = AsyncMock() # type: ignore[method-assign] + client = SimpleNamespace(send_socket_mode_response=AsyncMock()) + + req = _channel_mention_request("env-c3", "1700000000.000300") + + await channel._on_socket_request(client, req) + + channel._handle_message.assert_awaited_once() + kwargs = channel._handle_message.await_args.kwargs + assert kwargs["session_key"] is None + assert kwargs["metadata"]["slack"]["thread_ts"] is None + + +@pytest.mark.asyncio +async def test_channel_thread_reply_keeps_thread_session() -> None: + """A reply inside a channel thread stays in the session opened by the root message.""" + channel = SlackChannel(SlackConfig(enabled=True), MessageBus()) + channel._bot_user_id = "UBOT" + channel._web_client = _FakeAsyncWebClient() + channel._handle_message = AsyncMock() # type: ignore[method-assign] + channel._with_thread_context = AsyncMock(return_value="hello") # type: ignore[method-assign] + client = SimpleNamespace(send_socket_mode_response=AsyncMock()) + req = SimpleNamespace( + type="events_api", + envelope_id="env-c4", + payload={ + "event": { + "type": "app_mention", + "user": "U1", + "channel": "C123", + "text": "<@UBOT> follow up", + "ts": "1700000000.000400", + "thread_ts": "1700000000.000100", + } + }, + ) + + await channel._on_socket_request(client, req) + + channel._handle_message.assert_awaited_once() + kwargs = channel._handle_message.await_args.kwargs + assert kwargs["session_key"] == "slack:C123:1700000000.000100" + + @pytest.mark.asyncio async def test_slack_slash_command_skips_thread_context() -> None: channel = SlackChannel(SlackConfig(enabled=True, allow_from=[]), MessageBus())