diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index 5828e02ddded..b7cc4c939302 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -287,7 +287,8 @@ class _ThreadContextCache: content: str fetched_at: float = field(default_factory=time.monotonic) message_count: int = 0 - parent_text: str = "" # Raw text of the thread parent (for reply_to_text injection) + # Cached root text used by mention wake checks. + parent_text: str = "" # The Slack user_id of the thread parent message author. Used by # _bot_authored_thread_root (#63530) to detect threads whose root was # posted by the bot via direct chat.postMessage (outside the gateway's @@ -4897,10 +4898,8 @@ async def _handle_slack_reaction(self, event: dict, removed: bool = False) -> No used by the Feishu and Photon adapters β€” ``reaction:added:`` / ``reaction:removed:`` β€” with common Slack reaction names translated to unicode emoji (πŸ‘, πŸ‘Ž, βœ…, …) so agents and skills see - the same shape on every platform. Because the synthesized event is - threaded under the reacted-to message, the existing reply-context - plumbing injects the target message's text as ``reply_to_text`` and - the agent sees WHAT was reacted to. + the same shape on every platform. The event is routed to the + reacted-to message's thread, whose history supplies the context. Message-pipeline routing is OPT-IN via ``slack.reaction_triggers`` (default off) so busy channels don't wake the agent on every emoji. @@ -6367,29 +6366,6 @@ async def _handle_slack_message( None, ) - # Extract reply context if this message is a thread reply. - # Mirrors the Telegram/Discord implementations so that gateway.run - # can inject a `[Replying to: "..."]` prefix when the parent is not - # already in the session history. Uses the thread-context cache when - # available to avoid redundant conversations.replies calls. - reply_to_text = None - if thread_ts and thread_ts != ts: - try: - reply_to_text = ( - await self._fetch_thread_parent_text( - channel_id=channel_id, - thread_ts=thread_ts, - team_id=team_id, - ) - or None - ) - if reply_to_text: - reply_to_text = await self._humanize_user_mentions( - reply_to_text, chat_id=channel_id, team_id=team_id - ) - except Exception: # pragma: no cover - defensive - reply_to_text = None - # Humanize remaining user mentions: the bot's own mention was already # stripped above, so any ``<@UID>`` left in the trigger text refers to # OTHER participants. Render them as ``@DisplayName`` so the agent can @@ -6412,7 +6388,9 @@ async def _handle_slack_message( reply_to_message_id=thread_ts if thread_ts != ts else None, channel_prompt=_channel_prompt, channel_context=channel_context, - reply_to_text=reply_to_text, + # thread_ts identifies the thread root, not an explicit reply; + # channel_context hydrates the root separately. + reply_to_text=None, auto_skill=_auto_skill, metadata={ "slack_team_id": team_id, @@ -7463,8 +7441,7 @@ async def _format_thread_context( When ``after_ts`` is set, only messages with ts strictly greater than the watermark are included (delta refresh, #23918); the thread parent - text is still captured regardless so reply_to_text callers keep - working from the shared cache. + text is still captured in the shared cache. Returns ``(content, parent_text)``. """ @@ -7613,16 +7590,15 @@ async def _fetch_thread_parent_text( ) -> str: """Return the text of the thread parent message. - Used for reply_to_text injection (mention stripped) and for the - parent-mentioned-bot wake check (#24848 β€” pass - ``strip_bot_mention=False`` so the ``<@bot>`` token is preserved). + Used to check whether the root mentions the bot (#24848). Set + ``strip_bot_mention=False`` to preserve the mention. Uses the same per-thread cache as :meth:`_fetch_thread_context` to avoid hitting ``conversations.replies`` twice. Falls back to a cheap single- message fetch (``limit=1, inclusive=True``) when the cache is cold. Returns empty string on any failure β€” callers should treat an empty - return as "no parent context to inject". + return as an unavailable parent message. """ cache_key = f"{channel_id}:{thread_ts}:{team_id}" now = time.monotonic() diff --git a/tests/gateway/test_slack.py b/tests/gateway/test_slack.py index fcdcb701cfa1..3e0b162f9668 100644 --- a/tests/gateway/test_slack.py +++ b/tests/gateway/test_slack.py @@ -22,7 +22,7 @@ import pytest import agent.secret_scope as secret_scope -from gateway.config import Platform, PlatformConfig +from gateway.config import GatewayConfig, Platform, PlatformConfig from gateway.run import GatewayRunner from gateway.platforms.base import ( MessageEvent, @@ -3711,15 +3711,11 @@ async def test_dm_toplevel_progress_uses_message_ts_as_thread(self, adapter): ) -class TestSlackReplyToText: - """Ensure MessageEvent.reply_to_text is populated on thread replies so - gateway.run can inject a ``[Replying to: "..."]`` prefix (parity with - Telegram/Discord/Feishu/WeCom).""" +class TestSlackThreadParentContext: + """Ensure Slack thread roots are hydrated once, not injected every turn.""" @pytest.mark.asyncio - async def test_slack_reply_to_text_set_on_thread_reply(self, adapter): - """When a thread reply arrives and the parent was posted by a bot - (e.g. cron summary), reply_to_text must carry the parent's text.""" + async def test_thread_root_uses_channel_context_not_reply_to_text(self, adapter): adapter._channel_team = {} # primary workspace only adapter._team_bot_user_ids = {} @@ -3757,10 +3753,49 @@ async def test_slack_reply_to_text_set_on_thread_reply(self, adapter): ), "handle_message must be invoked for thread-reply DM" msg_event = adapter.handle_message.call_args[0][0] assert msg_event.reply_to_message_id == "1000.0" - # The critical assertion: parent text is exposed as reply_to_text so the - # gateway can inject it when not already in the session history. - assert msg_event.reply_to_text is not None - assert "パール要約" in msg_event.reply_to_text + assert "パール要約" in msg_event.channel_context + assert msg_event.reply_to_text is None + + @pytest.mark.asyncio + async def test_active_thread_does_not_refetch_root_as_reply_text(self, adapter): + adapter._has_active_session_for_thread = MagicMock(return_value=True) + adapter._fetch_thread_parent_text = AsyncMock(return_value="original task") + + event = { + "text": "one more detail", + "user": "U_USER", + "channel": "D123", + "channel_type": "im", + "ts": "1001.0", + "thread_ts": "1000.0", + } + + with patch.object( + adapter, "_resolve_user_name", new=AsyncMock(return_value="Alice") + ): + await adapter._handle_slack_message(event) + + msg_event = adapter.handle_message.call_args[0][0] + assert msg_event.channel_context is None + assert msg_event.reply_to_message_id == "1000.0" + assert msg_event.reply_to_text is None + adapter._fetch_thread_parent_text.assert_not_awaited() + + runner = object.__new__(GatewayRunner) + runner.config = GatewayConfig( + platforms={ + Platform.SLACK: PlatformConfig(enabled=True, token="fake") + } + ) + runner.adapters = {} + prepared = await runner._prepare_inbound_message_text( + event=msg_event, + source=msg_event.source, + history=[{"role": "user", "content": "original task"}], + ) + + assert prepared == "one more detail" + assert "[Replying to:" not in prepared # ---------------------------------------------------------------------------