From 3d1076d67a1e403d700445c1b5d5beb220f2f1a8 Mon Sep 17 00:00:00 2001 From: Bartok9 Date: Thu, 25 Jun 2026 03:27:34 -0400 Subject: [PATCH 1/4] fix(slack): detect Block-Kit-only @mentions in bot filter and router MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #52387 Slack messages can carry the bot @mention only inside Block Kit `blocks` (a `rich_text` section with a `user` element), with the flat top-level `text` containing just a fallback string. Both mention gates read only the flat `text`: - the `allow_bots: mentions` bot-message filter, and - the `is_mentioned` channel router (`routing_text = original_text`) so such messages were silently dropped — `allow_bots: mentions` was effectively non-functional for Block-Kit senders, and the same blind spot hit `require_mention` / `strict_mention`. Add `_collect_slack_block_mentions()` (walks blocks, recovers `<@UID>` tokens from non-quoted rich-text `user` elements) and `_slack_mention_detection_text()` (flat text + recovered mentions). Both gates now use the merged detection text. Mentions nested in `rich_text_quote` are deliberately ignored, preserving the existing contract that quoted/forwarded content can't trick the bot. Also emit a debug line when a bot message is dropped so silent drops are diagnosable. Tests: 4 new cases in tests/gateway/test_slack_mention.py (Block-Kit mention recovered, flat-text passthrough, no-mention, quoted-mention ignored). 275 slack tests pass. --- plugins/platforms/slack/adapter.py | 68 +++++++++++++++++++++- tests/gateway/test_slack_mention.py | 89 +++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+), 2 deletions(-) diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index bde89f4ad1af..374a2c418fba 100644 --- a/plugins/platforms/slack/adapter.py +++ b/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. @@ -3127,8 +3184,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 +3411,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) diff --git a/tests/gateway/test_slack_mention.py b/tests/gateway/test_slack_mention.py index b3fb0685b3a8..9a9ba1dac5ab 100644 --- a/tests/gateway/test_slack_mention.py +++ b/tests/gateway/test_slack_mention.py @@ -903,3 +903,92 @@ def test_mention_patterns_trigger_in_channel_without_literal_mention(): assert _would_process(adapter, text="hey hermes what's the status") is True # Unrelated channel chatter is still ignored. assert _would_process(adapter, text="lunch anyone?") is False + + +# --------------------------------------------------------------------------- +# Tests: Block-Kit-only mention detection (#52387) +# --------------------------------------------------------------------------- + +from plugins.platforms.slack.adapter import _slack_mention_detection_text # noqa: E402 + + +def _blockkit_mention_event(bot_user_id=BOT_USER_ID, flat_text="Release notification"): + """A Slack event whose @mention lives ONLY inside Block Kit blocks.""" + return { + "text": flat_text, + "blocks": [ + { + "type": "rich_text", + "elements": [ + { + "type": "rich_text_section", + "elements": [ + {"type": "text", "text": "Hey "}, + {"type": "user", "user_id": bot_user_id}, + {"type": "text", "text": "! I will do a release"}, + ], + } + ], + } + ], + } + + +def test_mention_detection_text_recovers_blockkit_mention(): + event = _blockkit_mention_event() + merged = _slack_mention_detection_text(event) + # The flat text alone never contains the mention... + assert f"<@{BOT_USER_ID}>" not in event.get("text", "") + # ...but the merged detection text does. + assert f"<@{BOT_USER_ID}>" in merged + + +def test_mention_detection_text_no_blocks_returns_flat_text(): + event = {"text": f"<@{BOT_USER_ID}> hello"} + assert _slack_mention_detection_text(event) == f"<@{BOT_USER_ID}> hello" + + +def test_mention_detection_text_no_mention_anywhere(): + event = { + "text": "lunch anyone?", + "blocks": [ + { + "type": "rich_text", + "elements": [ + { + "type": "rich_text_section", + "elements": [{"type": "text", "text": "lunch anyone?"}], + } + ], + } + ], + } + assert f"<@{BOT_USER_ID}>" not in _slack_mention_detection_text(event) + + +def test_mention_detection_text_ignores_quoted_blockkit_mention(): + """A mention inside rich_text_quote (forwarded content) must NOT count.""" + event = { + "text": "please review", + "blocks": [ + { + "type": "rich_text", + "elements": [ + { + "type": "rich_text_quote", + "elements": [ + { + "type": "rich_text_section", + "elements": [ + {"type": "text", "text": "Contains "}, + {"type": "user", "user_id": BOT_USER_ID}, + {"type": "text", "text": " in quoted text"}, + ], + } + ], + } + ], + } + ], + } + assert f"<@{BOT_USER_ID}>" not in _slack_mention_detection_text(event) From 3d711321eca2d82eb6e59dd3179e200eadbea15e Mon Sep 17 00:00:00 2001 From: Pan Luo Date: Wed, 8 Jul 2026 17:49:02 -0700 Subject: [PATCH 2/4] fix(slack): read thread context from attachments and blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_fetch_thread_context` and `_fetch_thread_parent_text` only read each message's plain `text` field, so messages posted by apps (Alertmanager, Grafana, PagerDuty, CI bots) — which carry their content in legacy `attachments` or Block Kit `blocks` with an empty `text` — were dropped entirely. When such a message *starts* a thread (e.g. an alert), a bot mentioned mid-thread to investigate sees an empty thread and can only ask "what should I investigate?". Fall back to the existing `_extract_text_from_slack_blocks` and a new `_extract_text_from_slack_attachments` helper when `text` is empty, so app-posted alerts and notifications are visible in fetched thread history. Adds TestThreadContextAppMessages (attachment-only, blocks-only, and empty-message cases). --- plugins/platforms/slack/adapter.py | 62 +++++++++++++++- tests/gateway/test_slack.py | 111 +++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+), 1 deletion(-) diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index 374a2c418fba..578b256714e2 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -265,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: @@ -4461,7 +4502,18 @@ async def _fetch_thread_context( ): continue - msg_text = msg.get("text", "").strip() + msg_text = (msg.get("text") or "").strip() + # Apps (Alertmanager, Grafana, CI bots) often post with an empty + # ``text`` and the content in blocks/attachments — fall back so + # messages that started or populate the thread aren't dropped. + if not msg_text: + msg_text = _extract_text_from_slack_blocks( + msg.get("blocks") + ).strip() + if not msg_text: + msg_text = _extract_text_from_slack_attachments( + msg.get("attachments") + ).strip() if not msg_text: continue @@ -4567,6 +4619,14 @@ async def _fetch_thread_parent_text( return "" bot_uid = self._team_bot_user_ids.get(team_id, self._bot_user_id) text = (parent.get("text") or "").strip() + # App-posted parents (e.g. an Alertmanager alert) carry their content + # in blocks/attachments with an empty ``text`` — fall back to those. + if not text: + text = _extract_text_from_slack_blocks(parent.get("blocks")).strip() + if not text: + text = _extract_text_from_slack_attachments( + parent.get("attachments") + ).strip() if bot_uid: text = text.replace(f"<@{bot_uid}>", "").strip() return text diff --git a/tests/gateway/test_slack.py b/tests/gateway/test_slack.py index dd61bd95f5bb..8b827fbca29a 100644 --- a/tests/gateway/test_slack.py +++ b/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 From 6235afe97fa656cc54623ce38909b51b9c2c1517 Mon Sep 17 00:00:00 2001 From: Benjamin Ross Date: Wed, 22 Jul 2026 03:54:43 -0700 Subject: [PATCH 3/4] fix(slack): surface Block Kit content in fetched thread context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bot-posted alerts (Honeycomb, PagerDuty, Datadog, GitHub bot, etc.) carry their actionable content — section text, button URLs — in Block Kit blocks, while the plain text field holds only the alert title. _fetch_thread_context and _fetch_thread_parent_text only read msg.get('text'), so that content never reached the agent. Add a _render_message_text helper that merges top-level text with readable block content, section/header/context text, actionable URLs, and (folded in from #61261 during conflict resolution) legacy attachment fields, and use it for thread-context and parent-text rendering. Salvaged from #29541. --- plugins/platforms/slack/adapter.py | 121 ++++++++++++---- tests/gateway/test_slack_approval_buttons.py | 140 ++++++++++++++++++- 2 files changed, 233 insertions(+), 28 deletions(-) diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index 578b256714e2..b08135c3b1dc 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -373,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"): @@ -4400,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, @@ -4502,25 +4594,10 @@ async def _fetch_thread_context( ): continue - msg_text = (msg.get("text") or "").strip() - # Apps (Alertmanager, Grafana, CI bots) often post with an empty - # ``text`` and the content in blocks/attachments — fall back so - # messages that started or populate the thread aren't dropped. - if not msg_text: - msg_text = _extract_text_from_slack_blocks( - msg.get("blocks") - ).strip() - if not msg_text: - msg_text = _extract_text_from_slack_attachments( - msg.get("attachments") - ).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. @@ -4618,17 +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() - # App-posted parents (e.g. an Alertmanager alert) carry their content - # in blocks/attachments with an empty ``text`` — fall back to those. - if not text: - text = _extract_text_from_slack_blocks(parent.get("blocks")).strip() - if not text: - text = _extract_text_from_slack_attachments( - parent.get("attachments") - ).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) diff --git a/tests/gateway/test_slack_approval_buttons.py b/tests/gateway/test_slack_approval_buttons.py index 13c62ee1aef0..a1b49806e57d 100644 --- a/tests/gateway/test_slack_approval_buttons.py +++ b/tests/gateway/test_slack_approval_buttons.py @@ -547,7 +547,145 @@ async def test_fetch_thread_context_includes_bot_parent(self): assert "メール要約: 本日の新着3件" in context @pytest.mark.asyncio - async def test_fetch_thread_context_excludes_self_bot_replies(self): + async def test_fetch_thread_context_extracts_block_kit_parent(self): + """Bot-posted parents that put their content in ``blocks`` (Honeycomb, + PagerDuty, Datadog, GitHub bot, etc.) used to be reduced to just the + ``text`` field — typically only the alert title — which dropped the + URL/button payload that makes the alert useful to an agent replying + in the thread. The fetched context must now include bounded display + text and actionable URLs so section text and button URLs survive.""" + adapter = _make_adapter() + mock_client = adapter._team_clients["T1"] + mock_client.conversations_replies = AsyncMock(return_value={ + "messages": [ + # Bot-posted alert: title in `text`, URL only in `blocks`. + # Mirrors what Honeycomb, PagerDuty, etc. actually send. + { + "ts": "1000.0", + "bot_id": "B_ALERT", + "subtype": "bot_message", + "username": "alertbot", + "text": "low_alerts (checkout)", + "blocks": [ + { + "type": "section", + "text": { + "type": "mrkdwn", + "text": "*Trigger fired:* low_alerts", + }, + }, + { + "type": "actions", + "elements": [ + { + "type": "button", + "text": {"type": "plain_text", "text": "View graph"}, + "url": "https://example.example/view/abc123", + }, + ], + }, + ], + }, + # User reply that triggered the fetch. + {"ts": "1000.1", "user": "U1", "text": "what's going on?"}, + ] + }) + adapter._user_name_cache = {"U1": "Alice"} + + context = await adapter._fetch_thread_context( + channel_id="C1", + thread_ts="1000.0", + current_ts="1000.1", + team_id="T1", + ) + + # Title still present. + assert "low_alerts (checkout)" in context + # URL from the action button must now surface. + assert "https://example.example/view/abc123" in context + # Marked as the thread parent. + assert "[thread parent]" in context + + @pytest.mark.asyncio + async def test_fetch_thread_context_includes_blocks_only_parent(self): + """A parent message with empty ``text`` but non-empty ``blocks`` must + still be included — without this, alerts that put *everything* in + ``blocks`` (some webhook integrations do this) are silently dropped + because the ``if not msg_text: continue`` guard fires.""" + adapter = _make_adapter() + mock_client = adapter._team_clients["T1"] + mock_client.conversations_replies = AsyncMock(return_value={ + "messages": [ + { + "ts": "1000.0", + "bot_id": "B_ALERT", + "subtype": "bot_message", + "username": "alertbot", + "text": "", + "blocks": [ + { + "type": "section", + "text": { + "type": "mrkdwn", + "text": "Build failed: ", + }, + }, + ], + }, + {"ts": "1000.1", "user": "U1", "text": "looking"}, + ] + }) + adapter._user_name_cache = {"U1": "Alice"} + + context = await adapter._fetch_thread_context( + channel_id="C1", + thread_ts="1000.0", + current_ts="1000.1", + team_id="T1", + ) + + assert "[thread parent]" in context + assert "https://example.example/build/9" in context + + @pytest.mark.asyncio + async def test_fetch_thread_parent_text_surfaces_block_urls(self): + """Cold-cache _fetch_thread_parent_text must use the same renderer as + _fetch_thread_context so a bot-posted parent with a URL only in + ``blocks`` surfaces it in reply_to_text, not just in thread context.""" + adapter = _make_adapter() + mock_client = adapter._team_clients["T1"] + mock_client.conversations_replies = AsyncMock(return_value={ + "messages": [ + { + "ts": "1000.0", + "bot_id": "B_ALERT", + "subtype": "bot_message", + "username": "alertbot", + "text": "Incident triggered", + "blocks": [ + { + "type": "actions", + "elements": [ + { + "type": "button", + "text": {"type": "plain_text", "text": "View incident"}, + "url": "https://example.example/incident/42", + }, + ], + }, + ], + }, + ] + }) + + text = await adapter._fetch_thread_parent_text( + channel_id="C1", + thread_ts="1000.0", + team_id="T1", + ) + + assert "Incident triggered" in text + assert "https://example.example/incident/42" in text """Parent (non-self bot) is kept, self-bot child replies are dropped, user replies are kept.""" adapter = _make_adapter() From 5906b801d86efb39cb6c4eb812f9e58b5ee1f842 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 22 Jul 2026 03:57:10 -0700 Subject: [PATCH 4/4] chore(contributors): map emails for slack block-text salvage (#29541, #61261, #52390) --- contributors/emails/ben.ross@moov.io | 2 ++ contributors/emails/pan.luo@ubc.ca | 2 ++ 2 files changed, 4 insertions(+) create mode 100644 contributors/emails/ben.ross@moov.io create mode 100644 contributors/emails/pan.luo@ubc.ca diff --git a/contributors/emails/ben.ross@moov.io b/contributors/emails/ben.ross@moov.io new file mode 100644 index 000000000000..7d782fdf75e7 --- /dev/null +++ b/contributors/emails/ben.ross@moov.io @@ -0,0 +1,2 @@ +bpross +# PR #29541 salvage diff --git a/contributors/emails/pan.luo@ubc.ca b/contributors/emails/pan.luo@ubc.ca new file mode 100644 index 000000000000..98feead6765f --- /dev/null +++ b/contributors/emails/pan.luo@ubc.ca @@ -0,0 +1,2 @@ +xcompass +# PR #61261 salvage