fix(slack): extract Block Kit and attachment text in thread context and mention detection

contributors/emails/ben.ross@moov.io

@@ -0,0 +1,2 @@
+bpross
+# PR #29541 salvage

contributors/emails/pan.luo@ubc.ca

@@ -0,0 +1,2 @@
+xcompass
+# PR #61261 salvage

plugins/platforms/slack/adapter.py

@@ -113,6 +113,63 @@ def _import():
     return ensure_and_bind("platform.slack", _import, globals(), prompt=False)
 
 
+def _collect_slack_block_mentions(blocks: list) -> list:
+    """Return ``<@UID>`` mention tokens authored in non-quoted Block Kit text.
+
+    Slack's flat top-level ``text`` field does NOT contain mentions that were
+    authored only inside Block Kit ``blocks`` (e.g. a ``rich_text_section`` with
+    a ``user`` element).  This walker recovers those mentions so the gates can
+    see Block-Kit-only mentions instead of silently dropping them (#52387).
+
+    Mentions nested inside ``rich_text_quote`` (quoted/forwarded content) are
+    deliberately ignored, so quoted text cannot trick the bot into responding
+    (matches the existing channel-routing contract).
+    """
+    mentions: list = []
+
+    def _walk(node, in_quote: bool) -> None:
+        if isinstance(node, list):
+            for item in node:
+                _walk(item, in_quote)
+            return
+        if not isinstance(node, dict):
+            return
+        node_type = node.get("type")
+        quoted = in_quote or node_type == "rich_text_quote"
+        if node_type == "user" and not quoted:
+            uid = node.get("user_id", "")
+            if uid:
+                mentions.append(f"<@{uid}>")
+        for key in ("elements", "element"):
+            child = node.get(key)
+            if child is not None:
+                _walk(child, quoted)
+
+    try:
+        _walk(blocks, False)
+    except Exception:  # pragma: no cover - defensive, never break gating
+        return []
+    return mentions
+
+
+def _slack_mention_detection_text(event: dict) -> str:
+    """Return the text used for @mention detection on a Slack message event.
+
+    Combines the flat top-level ``text`` with any ``<@UID>`` mentions recovered
+    from non-quoted Block Kit blocks (#52387), so a genuine Block-Kit-only
+    mention reaches the gates while quoted/forwarded mentions stay ignored.
+    """
+    flat = event.get("text", "") or ""
+    blocks = event.get("blocks")
+    if not blocks:
+        return flat
+    mentions = _collect_slack_block_mentions(blocks)
+    extra = [m for m in mentions if m not in flat]
+    if not extra:
+        return flat
+    return (flat.strip() + "\n" + " ".join(extra)).strip()
+
+
 def _extract_text_from_slack_blocks(blocks: list) -> str:
     """Extract readable text from Slack Block Kit blocks, including quoted/forwarded content.
 
@@ -208,6 +265,47 @@ def _walk_elements(elements: list, quote_depth: int = 0, bullet: str = "") -> No
     return "\n".join(parts)
 
 
+def _extract_text_from_slack_attachments(attachments: list) -> str:
+    """Extract readable text from legacy Slack message ``attachments``.
+
+    Apps such as Alertmanager, Grafana, PagerDuty, and CI bots post messages
+    with an empty top-level ``text`` and the real content inside ``attachments``
+    (Slack's legacy secondary-content format) or nested Block Kit ``blocks``.
+    Without this, such messages are invisible when the agent reads thread
+    history — e.g. an alert that started the very thread the agent was asked to
+    investigate would come through blank.
+
+    Prefers structured fields (``pretext``/``title``/``text``/``fields``) and
+    only falls back to an attachment's ``fallback`` string when it carries
+    nothing else.
+    """
+    if not attachments:
+        return ""
+
+    lines: list[str] = []
+    for att in attachments:
+        if not isinstance(att, dict):
+            continue
+        got: list[str] = [
+            str(att[key]) for key in ("pretext", "title", "text") if att.get(key)
+        ]
+        for field in att.get("fields", []) or []:
+            if not isinstance(field, dict):
+                continue
+            got += [str(field[k]) for k in ("title", "value") if field.get(k)]
+        nested = att.get("blocks")
+        if nested:
+            block_text = _extract_text_from_slack_blocks(nested)
+            if block_text:
+                got.append(block_text)
+        # Only use the (often duplicative) fallback when nothing structured exists.
+        if not got and att.get("fallback"):
+            got.append(str(att["fallback"]))
+        lines += got
+
+    return "\n".join(line for line in lines if line).strip()
+
+
 def _serialize_slack_blocks_for_agent(blocks: list, max_chars: int = 6000) -> str:
     """Return a compact, redacted JSON view of the current message's Block Kit payload."""
     if not blocks:
@@ -275,6 +373,47 @@ def _sanitize(value):
     return f"[Slack Block Kit payload for this message]\n```json\n{payload}\n```"
 
 
+def _extract_urls_from_slack_blocks(blocks: list) -> list[str]:
+    """Walk a Block Kit ``blocks`` tree and return URLs found on any element.
+
+    Returns URLs preserving discovery order with duplicates removed. Used to
+    surface the actionable links (``View graph``, ``View incident``, etc.)
+    embedded in bot-posted alerts so an agent reading the thread can fetch
+    or click them. The companion serializer
+    :func:`_serialize_slack_blocks_for_agent` deliberately strips ``url`` to
+    keep the JSON view compact and to avoid exposing arbitrary URLs through
+    the generic payload dump; this helper is the targeted opt-in for
+    use sites where URLs are the whole point of the message.
+    """
+    if not blocks:
+        return []
+
+    found: list[str] = []
+    seen: set[str] = set()
+
+    def _maybe_add(value: Any) -> None:
+        if isinstance(value, str) and value.startswith(("http://", "https://")):
+            if value not in seen:
+                seen.add(value)
+                found.append(value)
+
+    def _walk(node: Any) -> None:
+        if isinstance(node, dict):
+            # The common URL-bearing keys across Block Kit (buttons, link
+            # elements in rich_text, image accessories, etc.).
+            for key in ("url", "image_url", "external_url"):
+                if key in node:
+                    _maybe_add(node[key])
+            for value in node.values():
+                _walk(value)
+        elif isinstance(node, list):
+            for item in node:
+                _walk(item)
+
+    _walk(blocks)
+    return found
+
+
 def _apply_slack_proxy(client: Any, proxy_url: Optional[str]) -> None:
     """Apply a resolved proxy to a Slack SDK client or clear it explicitly."""
     if hasattr(client, "proxy"):
@@ -3127,8 +3266,14 @@ async def _handle_slack_message(
             if allow_bots == "none":
                 return
             elif allow_bots == "mentions":
-                text_check = event.get("text", "")
+                # Include Block-Kit-only mentions, not just the flat text (#52387)
+                text_check = _slack_mention_detection_text(event)
                 if self._bot_user_id and f"<@{self._bot_user_id}>" not in text_check:
+                    logger.debug(
+                        "[Slack] Dropping bot message under allow_bots=mentions: "
+                        "no <@%s> mention in flat text or blocks",
+                        self._bot_user_id,
+                    )
                     return
             # "all" falls through to process the message
             # Always ignore our own messages to prevent echo loops
@@ -3348,7 +3493,8 @@ async def _handle_slack_message(
         #   3. The message is in a thread where the bot was previously @mentioned, OR
         #   4. There's an existing session for this thread (survives restarts)
         bot_uid = self._team_bot_user_ids.get(team_id, self._bot_user_id)
-        routing_text = original_text or ""
+        # Detect mentions authored only inside Block Kit blocks too (#52387)
+        routing_text = _slack_mention_detection_text(event) or original_text or ""
         is_mentioned = bool(
             (bot_uid and f"<@{bot_uid}>" in routing_text)
             or self._slack_message_matches_mention_patterns(routing_text)
@@ -4295,6 +4441,57 @@ async def _handle_approval_action(self, ack, body, action) -> None:
 
     # ----- Thread context fetching -----
 
+    @staticmethod
+    def _render_message_text(msg: dict, bot_uid: str = "") -> str:
+        """Return bounded display text for a Slack message, surfacing Block Kit content.
+
+        Starts with ``text``, strips bot mentions, then appends rich-text
+        content and actionable URLs from ``blocks`` when present.  Unlike
+        :func:`_serialize_slack_blocks_for_agent` (which can emit up to
+        6 000 chars of JSON per message), this helper produces only the
+        readable text and URL list needed by thread-context and parent-
+        text rendering — bounded by what the blocks actually contain,
+        not a JSON dump.
+        """
+        msg_text = (msg.get("text") or "").strip()
+        if bot_uid:
+            msg_text = msg_text.replace(f"<@{bot_uid}>", "").strip()
+
+        blocks = msg.get("blocks")
+        extras: list[str] = []
+        if blocks:
+            rich_text = _extract_text_from_slack_blocks(blocks).strip()
+            if rich_text and rich_text not in msg_text:
+                extras.append(rich_text)
+            for block in blocks:
+                block_type = (block or {}).get("type", "")
+                if block_type in ("section", "header", "context"):
+                    text_obj = block.get("text") or {}
+                    if isinstance(text_obj, dict):
+                        section_text = (text_obj.get("text") or "").strip()
+                        if section_text and section_text not in msg_text and all(section_text not in e for e in extras):
+                            extras.append(section_text)
+        # Legacy ``attachments`` (Alertmanager, Grafana, PagerDuty, CI bots):
+        # apps often post with an empty ``text`` and the real content in
+        # attachment fields or attachment-nested blocks.
+        attachments_text = _extract_text_from_slack_attachments(
+            msg.get("attachments") or []
+        ).strip()
+        if attachments_text and attachments_text not in msg_text and all(
+            attachments_text not in e for e in extras
+        ):
+            extras.append(attachments_text)
+        if blocks:
+            urls = _extract_urls_from_slack_blocks(blocks)
+            new_urls = [u for u in urls if u not in msg_text and all(u not in e for e in extras)]
+            if new_urls:
+                extras.append("URLs: " + ", ".join(new_urls))
+        if extras:
+            addendum = "\n".join(extras)
+            msg_text = (msg_text + "\n" + addendum).strip() if msg_text else addendum
+
+        return msg_text
+
     async def _fetch_thread_context(
         self,
         channel_id: str,
@@ -4397,14 +4594,10 @@ async def _fetch_thread_context(
                 ):
                     continue
 
-                msg_text = msg.get("text", "").strip()
+                msg_text = self._render_message_text(msg, bot_uid=bot_uid)
                 if not msg_text:
                     continue
 
-                # Strip bot mentions from context messages
-                if bot_uid:
-                    msg_text = msg_text.replace(f"<@{bot_uid}>", "").strip()
-
                 prefix = "[thread parent] " if is_parent else ""
                 display_user = msg_user or "unknown"
                 # Prefer the bot's own name when the message is a bot post.
@@ -4502,9 +4695,7 @@ async def _fetch_thread_parent_text(
             if parent.get("ts", "") != thread_ts:
                 return ""
             bot_uid = self._team_bot_user_ids.get(team_id, self._bot_user_id)
-            text = (parent.get("text") or "").strip()
-            if bot_uid:
-                text = text.replace(f"<@{bot_uid}>", "").strip()
+            text = self._render_message_text(parent, bot_uid=bot_uid or "")
             return text
         except Exception as exc:  # pragma: no cover - defensive
             logger.debug("[Slack] Failed to fetch thread parent text: %s", exc)

tests/gateway/test_slack.py

@@ -4995,3 +4995,114 @@ async def test_auth_check_exception_does_not_crash_fetch(self, adapter):
         # Renders successfully without trust tag (exception → unknown trust).
         assert "U_X: hello" in content
         assert "[unverified]" not in content
+
+
+# ---------------------------------------------------------------------------
+# TestThreadContextAppMessages
+# ---------------------------------------------------------------------------
+
+
+class TestThreadContextAppMessages:
+    """App-posted messages (Alertmanager, Grafana, CI bots) frequently carry
+    their content in ``attachments``/``blocks`` with an empty top-level
+    ``text``. Thread-context must fall back to those so, e.g., an alert that
+    started the thread the bot was asked to investigate is not dropped."""
+
+    @staticmethod
+    def _make_replies(messages):
+        return AsyncMock(return_value={"messages": messages})
+
+    @pytest.mark.asyncio
+    async def test_attachment_only_parent_is_included(self, adapter):
+        """Alertmanager-style parent: empty text, content in a legacy attachment."""
+        adapter._thread_context_cache.clear()
+        messages = [
+            {  # parent posted by the Alertmanager app: text="" , content in attachment
+                "ts": "100.0",
+                "bot_id": "B_ALERTMGR",
+                "subtype": "bot_message",
+                "username": "Alertmanager",
+                "text": "",
+                "attachments": [
+                    {
+                        "fallback": "[FIRING:1] KubeJobFailed cluster-01 "
+                        "batch-job-123456",
+                        "color": "danger",
+                    }
+                ],
+            },
+            {"ts": "101.0", "user": "U_BOB", "text": "<@U_BOT> investigate"},
+        ]
+        adapter._app.client.conversations_replies = self._make_replies(messages)
+
+        with patch.object(
+            adapter, "_resolve_user_name",
+            new=AsyncMock(side_effect=lambda uid, **_: uid),
+        ):
+            content = await adapter._fetch_thread_context(
+                channel_id="C1", thread_ts="100.0", current_ts="999.0",
+            )
+
+        # The alert text (previously dropped) is now present in the context.
+        assert "KubeJobFailed" in content
+        assert "batch-job-123456" in content
+        assert "[thread parent]" in content
+
+    @pytest.mark.asyncio
+    async def test_blocks_only_message_is_included(self, adapter):
+        """Block Kit message with empty text falls back to block text."""
+        adapter._thread_context_cache.clear()
+        messages = [
+            {"ts": "100.0", "user": "U_BOB", "text": "kickoff"},
+            {
+                "ts": "101.0",
+                "bot_id": "B_CI",
+                "subtype": "bot_message",
+                "username": "CI",
+                "text": "",
+                "blocks": [
+                    {
+                        "type": "rich_text",
+                        "elements": [
+                            {
+                                "type": "rich_text_section",
+                                "elements": [
+                                    {"type": "text", "text": "deploy #42 succeeded"}
+                                ],
+                            }
+                        ],
+                    }
+                ],
+            },
+        ]
+        adapter._app.client.conversations_replies = self._make_replies(messages)
+
+        with patch.object(
+            adapter, "_resolve_user_name",
+            new=AsyncMock(side_effect=lambda uid, **_: uid),
+        ):
+            content = await adapter._fetch_thread_context(
+                channel_id="C1", thread_ts="100.0", current_ts="999.0",
+            )
+
+        assert "deploy #42 succeeded" in content
+
+    @pytest.mark.asyncio
+    async def test_message_without_any_text_is_skipped(self, adapter):
+        """A message with no text/blocks/attachments is still skipped (no crash)."""
+        adapter._thread_context_cache.clear()
+        messages = [
+            {"ts": "100.0", "user": "U_BOB", "text": "hello"},
+            {"ts": "101.0", "bot_id": "B_X", "subtype": "bot_message", "text": ""},
+        ]
+        adapter._app.client.conversations_replies = self._make_replies(messages)
+
+        with patch.object(
+            adapter, "_resolve_user_name",
+            new=AsyncMock(side_effect=lambda uid, **_: uid),
+        ):
+            content = await adapter._fetch_thread_context(
+                channel_id="C1", thread_ts="100.0", current_ts="999.0",
+            )
+
+        assert "hello" in content  # the real message survives; empty bot msg dropped