diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index e017838fce..c8bde60aa6 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -424,6 +424,68 @@ def _rewrite_known_bang_command(text: str) -> str: return text +def _slack_permalink_path(channel_id: str | None, message_ts: str | None) -> str: + """The workspace-independent tail of a Slack message permalink. + + A permalink is ``https://.slack.com/archives//p``; + only the tail can be rebuilt from a payload, so both sides of a dedupe + comparison are reduced to it. + """ + if not channel_id or not message_ts: + return "" + return f"archives/{channel_id}/p{str(message_ts).replace('.', '')}" + + +def _slack_str_field(el: dict, name: str) -> str: + """Read a string field of a Block Kit element. + + Block Kit carries text as an object in many places, and a non-string would + raise in ``str.join`` below and cost the whole message. + """ + value = el.get(name) + return value if isinstance(value, str) else "" + + +def _render_slack_inline_element(el: dict) -> str: + """Render one Block Kit inline element, empty when it carries nothing. + + Slack adds inline types without notice, so unknown ones fall back to + whatever human-readable field they carry rather than rendering as nothing. + """ + el_type = el.get("type", "") + if el_type == "text": + return _slack_str_field(el, "text") + if el_type == "channel": + return f"<#{el.get('channel_id', '')}>" + if el_type == "user": + return f"<@{el.get('user_id', '')}>" + if el_type == "usergroup": + return f"" + if el_type == "team": + return f"" + if el_type == "emoji": + return f":{el.get('name', '')}:" + if el_type == "broadcast": + return f"" + if el_type == "color": + return _slack_str_field(el, "value") + if el_type == "date": + fallback = _slack_str_field(el, "fallback") + if fallback: + return fallback + # ``link``, ``message_mention``, a ``date`` without a ``fallback`` and any + # unknown type: a URL plus an optional label. + url = _slack_str_field(el, "url") + label = _slack_str_field(el, "text") or _slack_str_field(el, "fallback") + if not url and el_type == "message_mention": + # ``url`` is optional here; ``channel_id`` and ``message_ts`` are not, + # and they are the permalink's own components. + url = _slack_permalink_path(el.get("channel_id"), el.get("message_ts")) + if url: + return f"{label} ({url})" if label and label != url else url + return label + + def _extract_text_from_slack_blocks(blocks: list) -> str: """Extract readable text from Slack Block Kit blocks, including quoted/forwarded content. @@ -443,28 +505,7 @@ def _extract_text_from_slack_blocks(blocks: list) -> str: def _render_inline_elements(elements: list) -> str: """Render inline elements (text, link, channel, user, emoji, etc.).""" - pieces: list[str] = [] - for el in elements: - el_type = el.get("type", "") - if el_type == "text": - pieces.append(el.get("text", "")) - elif el_type == "link": - url = el.get("url", "") - text = el.get("text", "") - pieces.append(f"{text} ({url})" if text and text != url else url) - elif el_type == "channel": - pieces.append(f"<#{el.get('channel_id', '')}>") - elif el_type == "user": - pieces.append(f"<@{el.get('user_id', '')}>") - elif el_type == "usergroup": - pieces.append(f"") - elif el_type == "emoji": - pieces.append(f":{el.get('name', '')}:") - elif el_type == "broadcast": - pieces.append(f"") - elif el_type == "date": - pieces.append(el.get("fallback", "")) - return "".join(pieces) + return "".join(_render_slack_inline_element(el) for el in elements) def _append_line(text: str, quote_depth: int = 0, bullet: str = "") -> None: if not text or not text.strip(): @@ -540,6 +581,10 @@ def _extract_text_from_slack_attachments(attachments: list) -> str: for att in attachments: if not isinstance(att, dict): continue + # A permalink unfurl carries the linked message's own body, which the + # agent is already reading. The live inbound path skips these too. + if att.get("is_msg_unfurl"): + continue got: list[str] = [ str(att[key]) for key in ("pretext", "title", "text") if att.get(key) ] @@ -560,34 +605,132 @@ def _extract_text_from_slack_attachments(attachments: list) -> str: return "\n".join(line for line in lines if line).strip() +#: Any ```` autolink; Slack is not limited to ``https`` +#: and ``mailto``. _SLACK_MRKDWN_LINK_RE = re.compile( - r"<((?:https?|mailto):[^>|]+)(?:\|([^>]+))?>" + r"<([a-zA-Z][a-zA-Z0-9+.\-]*:[^>|]+)(?:\|([^>]+))?>" ) +#: The optional label Slack may attach to a mention in the flat text, while +#: the blocks carry the bare id: ``<@U…|name>``, ``<#C…|general>``, +#: ````, ````. +_SLACK_ENTITY_LABEL_RE = re.compile(r"<([@#!][^>|]*)\|[^>]*>") +_SLACK_FENCED_CODE_RE = re.compile( + r"(?|]*)(?:\|([^>]*))?>") +#: A message permalink, reduced to the tail :func:`_slack_permalink_path` +#: rebuilds: the workspace host and the thread query differ between the flat +#: text and a payload that carries only ``channel_id``/``message_ts``. +_SLACK_PERMALINK_RE = re.compile( + r"https?://[^\s/]+/(archives/[A-Za-z0-9]+/p\d+)(?:\?[^\s)]*)?" +) +_SLACK_INLINE_STYLE_RE = re.compile(r"([*_~])([^\n]+?)\1") +_SLACK_HTML_ENTITY_RE = re.compile(r"&(amp|lt|gt);") +_SLACK_HTML_ENTITIES = {"amp": "&", "lt": "<", "gt": ">"} -def _normalize_slack_text_for_dedupe(text: str) -> str: - """Canonicalize equivalent Slack plain-text and rich-block link forms. +def _unescape_slack_entities(text: str) -> str: + """Undo Slack's HTML escaping of ``&``/``<``/``>`` in flat message text. - Slack serializes the same authored link as ```` in the event's - plain ``text`` field and as a structured ``link`` element in ``blocks``. - Comparing those raw strings makes a normal rich-text message look like - additional quoted content and appends the whole message a second time. + Slack escapes those three characters in the flat ``text`` field but leaves + the ``blocks`` payload raw, so any comparison between the two must run on + a common form. Thread permalinks make this load-bearing: every "Copy link" + URL carries ``?thread_ts=…&cid=…``. """ + return _SLACK_HTML_ENTITY_RE.sub( + lambda match: _SLACK_HTML_ENTITIES[match.group(1)], text or "" + ) + + +def _normalize_slack_text_for_dedupe(text: str, bot_uid: str = "") -> str: + """Normalize Slack text for comparison with rendered rich text.""" def _link(match: re.Match) -> str: url, label = match.group(1), match.group(2) return f"{label} ({url})" if label and label != url else url - canonical = _SLACK_MRKDWN_LINK_RE.sub(_link, text or "") + def _date(match: re.Match) -> str: + # ````, read down to what the rich-text + # side renders: the fallback, or the URL when there is no fallback. + fallback = match.group(2) + if fallback: + return fallback + parts = match.group(1).split("^") + return parts[2] if len(parts) > 2 else "" + + canonical = text or "" + # Before link canonicalization, so both sides see the same angle brackets + # and the same ``&`` in query parameters. + canonical = _unescape_slack_entities(canonical) + canonical = _SLACK_MRKDWN_LINK_RE.sub(_link, canonical) + canonical = _SLACK_DATE_RE.sub(_date, canonical) + # After the link form, so that a pasted permalink is already a bare URL. + canonical = _SLACK_PERMALINK_RE.sub(r"\1", canonical) + # After the date form, which carries a label of its own. + canonical = _SLACK_ENTITY_LABEL_RE.sub(r"<\1>", canonical) + # After the label, so that ``<@U…|hermes>`` is stripped like ``<@U…>``. + if bot_uid: + canonical = canonical.replace(f"<@{bot_uid}>", "") + canonical = _SLACK_FENCED_CODE_RE.sub(r"\1", canonical) + canonical = _SLACK_INLINE_CODE_RE.sub(r"\1", canonical) + while True: + unstyled = _SLACK_INLINE_STYLE_RE.sub(r"\2", canonical) + if unstyled == canonical: + break + canonical = unstyled return re.sub(r"\s+", " ", canonical).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: - return "" +def _extract_additional_text_from_slack_blocks( + blocks: list, primary_text: str, bot_uid: str = "" +) -> str: + """Render rich-text content not already represented by primary_text.""" + primary = _normalize_slack_text_for_dedupe(primary_text, bot_uid) + primary_fenced = { + _normalize_slack_text_for_dedupe(match.group(0), bot_uid) + for match in _SLACK_FENCED_CODE_RE.finditer(primary_text or "") + } + parts: list[str] = [] - if all((block or {}).get("type") == "rich_text" for block in blocks): + for block in blocks or []: + if (block or {}).get("type") != "rich_text": + continue + for element in block.get("elements", []): + element_type = element.get("type", "") + rendered = _extract_text_from_slack_blocks( + [{"type": "rich_text", "elements": [element]}] + ).strip() + if not rendered: + continue + normalized = _normalize_slack_text_for_dedupe(rendered, bot_uid) + if element_type == "rich_text_preformatted": + is_duplicate = normalized in primary_fenced + else: + is_duplicate = normalized == primary or normalized in primary + if normalized and is_duplicate: + continue + parts.append(rendered) + + return "\n".join(parts) + + +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. + + Only blocks the agent cannot already read are serialized. ``rich_text`` + blocks are the authored message itself and are rendered into the message + text by :func:`_extract_text_from_slack_blocks`; dumping them here would + repeat the author's own words — and, because the allowlist below drops + ``url``, the repeat reads as the same sentence with every link silently + removed. This view exists for the UI-heavy blocks bots post (``section``, + ``actions``, ``accessory``, …), so a single such block must not drag the + authored text along with it. + """ + inspectable = [ + block for block in (blocks or []) if (block or {}).get("type") != "rich_text" + ] + if not inspectable: return "" scalar_allowlist = { @@ -639,9 +782,9 @@ def _serialize_slack_blocks_for_agent(blocks: list, max_chars: int = 6000) -> st return repr(value) try: - payload = json.dumps(_sanitize(blocks), ensure_ascii=False, indent=2) + payload = json.dumps(_sanitize(inspectable), ensure_ascii=False, indent=2) except Exception: - payload = repr(blocks) + payload = repr(inspectable) if len(payload) > max_chars: payload = payload[: max_chars - 18].rstrip() + "\n... [truncated]" @@ -5898,17 +6041,17 @@ class SlackAdapter(BasePlatformAdapter): # model name appears to contain spaces). blocks = event.get("blocks") if blocks and not is_command_text: - blocks_text = _extract_text_from_slack_blocks(blocks) - if blocks_text: - # Only append if the blocks contain text not already present - # in the plain text field (avoids duplication). - stripped_blocks = blocks_text.strip() - block_text_is_duplicate = ( - stripped_blocks in text.strip() - or _normalize_slack_text_for_dedupe(stripped_blocks) - == _normalize_slack_text_for_dedupe(text) + blocks_text = _extract_additional_text_from_slack_blocks( + blocks, + text, + bot_uid=self._team_bot_user_ids.get( + dedup_team_id, self._bot_user_id ) - if stripped_blocks and not block_text_is_duplicate: + or "", + ) + if blocks_text: + stripped_blocks = blocks_text.strip() + if stripped_blocks: logger.debug( "Slack: extracted additional text from blocks " "(likely quoted/forwarded content; chars=%d)", @@ -7656,8 +7799,10 @@ class SlackAdapter(BasePlatformAdapter): 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: + rich_text = _extract_additional_text_from_slack_blocks( + blocks, msg_text, bot_uid=bot_uid + ).strip() + if rich_text: extras.append(rich_text) for block in blocks: block_type = (block or {}).get("type", "") @@ -7679,7 +7824,15 @@ class SlackAdapter(BasePlatformAdapter): 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)] + # ``msg.text`` escapes ``&`` inside URLs while the block payload + # keeps it raw, so a plain substring check re-lists a URL the + # message already shows. + msg_text_raw = _unescape_slack_entities(msg_text) + new_urls = [ + u + for u in urls + if u not in msg_text_raw and all(u not in e for e in extras) + ] if new_urls: extras.append("URLs: " + ", ".join(new_urls)) # Surface file/image attachments as compact text markers. The diff --git a/tests/gateway/test_slack.py b/tests/gateway/test_slack.py index 1f0d03e57c..12af58009d 100644 --- a/tests/gateway/test_slack.py +++ b/tests/gateway/test_slack.py @@ -100,6 +100,14 @@ _slack_mod.SLACK_AVAILABLE = True from plugins.platforms.slack.adapter import SlackAdapter # noqa: E402 +def _rich_text_blocks(*elements): + return [{"type": "rich_text", "elements": list(elements)}] + + +def _rich_text_section(*elements): + return {"type": "rich_text_section", "elements": list(elements)} + + def test_slack_mock_bootstrap_preserves_installed_packages(): """Installed Slack dependencies must remain importable as real packages.""" for package in ("slack_sdk", "aiohttp"): @@ -1739,6 +1747,210 @@ class TestIncomingDocumentHandling: assert "• First bullet" in msg_event.text assert "• Second bullet" in msg_event.text + @pytest.mark.parametrize( + ("text", "section_elements"), + [ + ( + "update the path to `src/app`", + [ + {"type": "text", "text": "update the path to "}, + {"type": "text", "text": "src/app", "style": {"code": True}}, + ], + ), + ( + "use *bold* and _italic_ text", + [ + {"type": "text", "text": "use "}, + {"type": "text", "text": "bold", "style": {"bold": True}}, + {"type": "text", "text": " and "}, + {"type": "text", "text": "italic", "style": {"italic": True}}, + {"type": "text", "text": " text"}, + ], + ), + ( + "use *_~styled~_* text", + [ + {"type": "text", "text": "use "}, + { + "type": "text", + "text": "styled", + "style": {"bold": True, "italic": True, "strike": True}, + }, + {"type": "text", "text": " text"}, + ], + ), + ( + "read ", + [ + {"type": "text", "text": "read "}, + { + "type": "link", + "url": "https://example.com/docs", + "text": "the docs", + }, + ], + ), + ], + ids=("inline-code", "inline-styles", "nested-inline-styles", "link"), + ) + @pytest.mark.asyncio + async def test_equivalent_rich_text_is_not_duplicated( + self, adapter, text, section_elements + ): + event = self._make_event( + text=text, + blocks=_rich_text_blocks(_rich_text_section(*section_elements)), + ) + + await adapter._handle_slack_message(event) + + assert adapter.handle_message.call_args[0][0].text == text + + @pytest.mark.parametrize( + "text", + ( + "run ```echo ok```", + "run\n\n```\necho ok\n```\n", + ), + ids=("compact-fence", "fence-with-surrounding-newlines"), + ) + @pytest.mark.asyncio + async def test_equivalent_preformatted_text_is_not_duplicated( + self, adapter, text + ): + event = self._make_event( + text=text, + blocks=_rich_text_blocks( + _rich_text_section({"type": "text", "text": "run"}), + { + "type": "rich_text_preformatted", + "elements": [{"type": "text", "text": "echo ok"}], + }, + ), + ) + + await adapter._handle_slack_message(event) + + assert adapter.handle_message.call_args[0][0].text == text + + @pytest.mark.asyncio + async def test_preformatted_text_also_mentioned_in_prose_is_preserved(self, adapter): + event = self._make_event( + text="run echo ok to verify the command", + blocks=_rich_text_blocks( + _rich_text_section( + {"type": "text", "text": "run echo ok to verify the command"} + ), + { + "type": "rich_text_preformatted", + "elements": [{"type": "text", "text": "echo ok"}], + }, + ), + ) + + await adapter._handle_slack_message(event) + + assert adapter.handle_message.call_args[0][0].text == ( + "run echo ok to verify the command\n```\necho ok\n```" + ) + + @pytest.mark.asyncio + async def test_block_only_bot_mention_does_not_duplicate_rich_text(self, adapter): + event = self._make_event( + text="update the path", + blocks=_rich_text_blocks( + _rich_text_section( + {"type": "user", "user_id": "U_BOT"}, + {"type": "text", "text": " update the path"}, + ) + ), + ) + + await adapter._handle_slack_message(event) + + assert adapter.handle_message.call_args[0][0].text == "update the path" + + @pytest.mark.asyncio + async def test_secondary_workspace_bot_mention_does_not_duplicate_rich_text( + self, adapter + ): + adapter._team_bot_user_ids["T_SECONDARY"] = "U_SECONDARY_BOT" + event = self._make_event( + text="update the path", + blocks=_rich_text_blocks( + _rich_text_section( + {"type": "user", "user_id": "U_SECONDARY_BOT"}, + {"type": "text", "text": " update the path"}, + ) + ), + ) + + await adapter._handle_slack_message(event, {"team_id": "T_SECONDARY"}) + + assert adapter.handle_message.call_args[0][0].text == "update the path" + + @pytest.mark.asyncio + async def test_rich_text_list_already_in_text_is_not_duplicated(self, adapter): + event = self._make_event( + text="• first\n• second", + blocks=_rich_text_blocks( + { + "type": "rich_text_list", + "style": "bullet", + "elements": [ + _rich_text_section({"type": "text", "text": "first"}), + _rich_text_section({"type": "text", "text": "second"}), + ], + } + ), + ) + + await adapter._handle_slack_message(event) + + msg_event = adapter.handle_message.call_args[0][0] + assert msg_event.text == "• first\n• second" + + @pytest.mark.asyncio + async def test_rich_text_different_section_is_preserved(self, adapter): + event = self._make_event( + text="review `test/yana`", + blocks=_rich_text_blocks( + _rich_text_section( + {"type": "text", "text": "also review test/prod"} + ) + ), + ) + + await adapter._handle_slack_message(event) + + msg_event = adapter.handle_message.call_args[0][0] + assert msg_event.text == "review `test/yana`\nalso review test/prod" + + @pytest.mark.asyncio + async def test_rich_text_duplicate_section_keeps_quote(self, adapter): + event = self._make_event( + text="review `test/yana`", + blocks=_rich_text_blocks( + _rich_text_section( + {"type": "text", "text": "review "}, + {"type": "text", "text": "test/yana", "style": {"code": True}}, + ), + { + "type": "rich_text_quote", + "elements": [ + _rich_text_section( + {"type": "text", "text": "quoted context"} + ) + ], + }, + ), + ) + + await adapter._handle_slack_message(event) + + msg_event = adapter.handle_message.call_args[0][0] + assert msg_event.text == "review `test/yana`\n> quoted context" + # --------------------------------------------------------------------------- # TestIncomingAudioHandling — Slack voice messages (regression) @@ -4286,6 +4498,45 @@ class TestThreadImageContext: assert "[image: shelf.jpg]" in rendered assert "[file: specs.pdf (application/pdf)]" in rendered + def test_render_message_text_deduplicates_main_section_and_keeps_quote( + self, adapter + ): + msg = { + "text": "<@U_BOT> review `src/app`", + "blocks": _rich_text_blocks( + _rich_text_section( + {"type": "user", "user_id": "U_BOT"}, + {"type": "text", "text": " review "}, + {"type": "text", "text": "src/app", "style": {"code": True}}, + ), + { + "type": "rich_text_quote", + "elements": [ + _rich_text_section( + {"type": "text", "text": "quoted context"} + ) + ], + }, + ), + } + + assert adapter._render_message_text(msg, bot_uid="U_BOT") == ( + "review `src/app`\n> quoted context" + ) + + def test_render_message_text_deduplicates_compact_fenced_code(self, adapter): + msg = { + "text": "run ```echo ok```", + "blocks": _rich_text_blocks( + _rich_text_section({"type": "text", "text": "run"}), + { + "type": "rich_text_preformatted", + "elements": [{"type": "text", "text": "echo ok"}], + }, + ), + } + + assert adapter._render_message_text(msg) == "run ```echo ok```" # -- integration: cold-start thread hydrate ---------------------------- @@ -4687,3 +4938,726 @@ class TestNativeTaskCardProgress: "chat.stopStream", ] assert adapter._native_task_card_streams == {} + + +# --------------------------------------------------------------------------- +# TestSlackAuthoredTextDeduplication +# --------------------------------------------------------------------------- + + +# A "Copy link" URL for a Slack thread always carries query parameters, so +# Slack HTML-escapes the ``&`` in ``event.text`` while leaving the same URL +# raw inside ``blocks[].link.url``. +_THREAD_PERMALINK = ( + "https://example.slack.com/archives/C0BCDG3H66P/p1786102118226679" + "?thread_ts=1786102118.226679&cid=C0BCDG3H66P" +) +_THREAD_PERMALINK_ESCAPED = _THREAD_PERMALINK.replace("&", "&") + +# A permalink as the Slack client pastes it — no query parameters, delivered as +# a ``message_mention`` element rather than a plain ``link``. +_PERMALINK = "https://example.slack.com/archives/C0BCDG3H66P/p1786102118226679" + + +class TestSlackAuthoredTextDeduplication: + """One authored Slack message must never be appended to itself. + + Slack delivers the same authored text twice — flat in ``event.text`` and + structurally in ``event.blocks`` — and HTML-escapes ``&``/``<``/``>`` in + the flat copy only. Whenever the two representations fail to compare + equal, the block rendering is mistaken for additional content and the + user sees their own message twice. Both merge sites are covered: + ``_handle_slack_message`` (live inbound) and ``_render_message_text`` + (thread/parent hydration). + """ + + @staticmethod + def _thread_link_blocks(*trailing): + return _rich_text_blocks( + _rich_text_section( + {"type": "user", "user_id": "U_BOT"}, + {"type": "text", "text": " do you see "}, + {"type": "link", "url": _THREAD_PERMALINK}, + {"type": "text", "text": " ?"}, + ), + *trailing, + ) + + @staticmethod + def _thread_link_text(): + return f"<@U_BOT> do you see <{_THREAD_PERMALINK_ESCAPED}> ?" + + # -- helper-level equivalence ----------------------------------------- + + @pytest.mark.parametrize( + "flat_text,elements", + [ + # Thread permalink: query params make Slack escape ``&`` in text + # while ``blocks[].link.url`` stays raw. The reported bug. + ( + f"look <{_THREAD_PERMALINK_ESCAPED}> here", + [ + {"type": "text", "text": "look "}, + {"type": "link", "url": _THREAD_PERMALINK}, + {"type": "text", "text": " here"}, + ], + ), + # Bare ampersand in prose. + ("AT&T outage", [{"type": "text", "text": "AT&T outage"}]), + # Literal angle brackets the user typed. + ("use <div> here", [{"type": "text", "text": "use
here"}]), + # Labelled link whose label carries an ampersand. + ( + "see ", + [ + {"type": "text", "text": "see "}, + {"type": "link", "url": "https://x.example", "text": "AT&T"}, + ], + ), + ], + ) + def test_escaped_entities_compare_equal(self, flat_text, elements): + assert ( + _slack_mod._extract_additional_text_from_slack_blocks( + _rich_text_blocks(_rich_text_section(*elements)), flat_text + ) + == "" + ) + + def test_genuine_quote_still_appended_next_to_escaped_link(self): + """Negative case: the fix must not swallow real structured content.""" + blocks = self._thread_link_blocks( + { + "type": "rich_text_quote", + "elements": [ + _rich_text_section({"type": "text", "text": "quoted context"}) + ], + } + ) + + assert ( + _slack_mod._extract_additional_text_from_slack_blocks( + blocks, self._thread_link_text(), bot_uid="U_BOT" + ) + == "> quoted context" + ) + + # -- live inbound path ------------------------------------------------- + + @pytest.mark.asyncio + async def test_live_inbound_thread_permalink_not_duplicated(self, adapter): + await adapter._handle_slack_message( + { + "text": self._thread_link_text(), + "blocks": self._thread_link_blocks(), + "user": "U_USER", + "client_msg_id": "cm-1", + "channel": "D_DM", + "channel_type": "im", + "ts": "123.456", + "team": "T_TEAM", + } + ) + + adapter.handle_message.assert_awaited_once() + text = adapter.handle_message.await_args.args[0].text + assert text.count("p1786102118226679") == 1 + assert text.count("do you see") == 1 + + # -- thread/parent hydration path -------------------------------------- + + def test_hydration_thread_permalink_not_duplicated(self, adapter): + rendered = adapter._render_message_text( + {"text": self._thread_link_text(), "blocks": self._thread_link_blocks()}, + bot_uid="U_BOT", + ) + + assert rendered.count("p1786102118226679") == 1 + assert rendered.count("do you see") == 1 + + def test_hydration_skips_message_unfurl_attachment(self, adapter): + """A permalink unfurl echoes the *linked* message — the live path + already skips it, so hydration must not re-append it either.""" + rendered = adapter._render_message_text( + { + "text": f"<{_THREAD_PERMALINK_ESCAPED}>", + "attachments": [ + { + "is_msg_unfurl": True, + "text": "the linked message body", + "fallback": "linked message fallback", + } + ], + } + ) + + assert "the linked message body" not in rendered + assert "linked message fallback" not in rendered + + def test_hydration_still_surfaces_regular_attachments(self, adapter): + """Alert-bot content lives only in attachments — keep surfacing it.""" + rendered = adapter._render_message_text( + { + "text": "", + "attachments": [ + {"is_msg_unfurl": True, "text": "echoed message body"}, + {"title": "FiringAlert", "text": "disk usage 95%"}, + ], + } + ) + + assert "echoed message body" not in rendered + assert "FiringAlert" in rendered + assert "disk usage 95%" in rendered + + # -- Block Kit payload dump -------------------------------------------- + + @pytest.mark.asyncio + async def test_block_kit_dump_leaves_out_the_authored_rich_text(self, adapter): + """A single non-rich_text block must not drag the message in with it. + + The dump exists for the interactive blocks bots post, and its + allowlist deliberately drops ``url``. Serializing the authored + ``rich_text`` alongside them therefore repeats the user's own + sentence with its links deleted — the "second copy without the + link" a reporter sees. + """ + await adapter._handle_slack_message( + { + "text": self._thread_link_text(), + "blocks": self._thread_link_blocks() + + [{"type": "section", "text": {"type": "mrkdwn", "text": "extra"}}], + "user": "U_USER", + "client_msg_id": "cm-2", + "channel": "D_DM", + "channel_type": "im", + "ts": "123.457", + "team": "T_TEAM", + } + ) + + text = adapter.handle_message.await_args.args[0].text + assert text.count("do you see") == 1 + assert text.count("p1786102118226679") == 1 + # The block the agent cannot otherwise read is still surfaced. + assert "extra" in text + + @pytest.mark.asyncio + async def test_no_block_kit_dump_for_a_plain_authored_message(self, adapter): + await adapter._handle_slack_message( + { + "text": self._thread_link_text(), + "blocks": self._thread_link_blocks(), + "user": "U_USER", + "client_msg_id": "cm-3", + "channel": "D_DM", + "channel_type": "im", + "ts": "123.458", + "team": "T_TEAM", + } + ) + + text = adapter.handle_message.await_args.args[0].text + assert "[Slack Block Kit payload for this message]" not in text + + # -- inline elements the renderer does not know ------------------------ + + @staticmethod + def _mention_blocks(element, *trailing): + """The blocks Slack sends for ``@bot do you see ?``.""" + return _rich_text_blocks( + _rich_text_section( + {"type": "user", "user_id": "U_BOT"}, + {"type": "text", "text": " do you see "}, + element, + {"type": "text", "text": " ?"}, + ), + *trailing, + ) + + @staticmethod + def _mention_text(): + """``event.text`` for a pasted permalink: label equals the URL.""" + return f"<@U_BOT> do you see <{_PERMALINK}|{_PERMALINK}> ?" + + @pytest.mark.parametrize( + "element", + [ + # Slack's own element for a pasted message permalink, as the + # client sends it: required ids plus an optional url/label. + { + "type": "message_mention", + "channel_id": "C0BCDG3H66P", + "message_ts": "1786102118.226679", + "url": _PERMALINK, + "text": _PERMALINK, + }, + # Same element with the optional label omitted. + { + "type": "message_mention", + "channel_id": "C0BCDG3H66P", + "message_ts": "1786102118.226679", + "url": _PERMALINK, + }, + # Slack adds inline element types without notice; one that carries + # a url must render rather than vanish. + {"type": "an_element_slack_adds_later", "url": _PERMALINK}, + # ... and one that carries only a label. + {"type": "an_element_slack_adds_later", "text": _PERMALINK}, + ], + ) + def test_url_bearing_inline_elements_render_instead_of_vanishing(self, element): + assert ( + _slack_mod._extract_additional_text_from_slack_blocks( + self._mention_blocks(element), self._mention_text(), bot_uid="U_BOT" + ) + == "" + ) + assert _PERMALINK in _slack_mod._extract_text_from_slack_blocks( + self._mention_blocks(element) + ) + + @pytest.mark.parametrize( + "element,rendered", + [ + # Block Kit carries text as an object in many places, so an unknown + # element may hold one where a string belongs. + ( + { + "type": "an_element_slack_adds_later", + "text": {"type": "plain_text", "text": "oops"}, + }, + "", + ), + # A string field next to it is still read. + ( + { + "type": "an_element_slack_adds_later", + "text": {"type": "plain_text", "text": "oops"}, + "fallback": _PERMALINK, + }, + _PERMALINK, + ), + # A known type reading a field of its own is no different. + ({"type": "color", "value": {"type": "plain_text", "text": "#fff"}}, ""), + ( + { + "type": "date", + "timestamp": 1786102118, + "fallback": {"type": "plain_text", "text": "Aug 7th"}, + }, + "", + ), + ({"type": "text", "text": {"type": "plain_text", "text": "oops"}}, ""), + ], + ) + def test_inline_element_with_an_object_field_keeps_the_message( + self, element, rendered + ): + """A non-string field must not reach the caller's ``str.join``.""" + blocks = self._mention_blocks(element) + flat_text = f"<@U_BOT> do you see {rendered} ?" + + assert _slack_mod._extract_text_from_slack_blocks(blocks) == flat_text + assert ( + _slack_mod._extract_additional_text_from_slack_blocks( + blocks, flat_text, bot_uid="U_BOT" + ) + == "" + ) + + @pytest.mark.parametrize( + "flat_text", + [ + # The permalink as pasted... + f"<@U_BOT> do you see <{_PERMALINK}|{_PERMALINK}> ?", + # ...and its "Copy link" form, whose query parameters the element + # cannot rebuild. + f"<@U_BOT> do you see <{_THREAD_PERMALINK_ESCAPED}> ?", + ], + ) + def test_url_less_message_mention_is_not_duplicated(self, flat_text): + """``url`` is optional on this element; ``channel_id`` and + ``message_ts`` are not, and they rebuild the permalink's tail.""" + blocks = self._mention_blocks( + { + "type": "message_mention", + "channel_id": "C0BCDG3H66P", + "message_ts": "1786102118.226679", + } + ) + + assert ( + _slack_mod._extract_additional_text_from_slack_blocks( + blocks, flat_text, bot_uid="U_BOT" + ) + == "" + ) + assert ( + "archives/C0BCDG3H66P/p1786102118226679" + in _slack_mod._extract_text_from_slack_blocks(blocks) + ) + + @pytest.mark.parametrize( + "element", + [ + # The element's own ``url`` never carries the query parameters the + # flat text has... + { + "type": "message_mention", + "channel_id": "C0BCDG3H66P", + "message_ts": "1786102118.226679", + "url": _PERMALINK, + "text": "Custom label", + }, + # ...and it may not carry a ``url`` at all. + { + "type": "message_mention", + "channel_id": "C0BCDG3H66P", + "message_ts": "1786102118.226679", + "text": "Custom label", + }, + ], + ) + def test_labelled_permalink_with_query_params_is_not_duplicated(self, element): + """A labelled link is canonicalized to ``label (url)``, so reducing the + permalink must stop at the query and leave the closing parenthesis.""" + blocks = self._mention_blocks(element) + + assert ( + _slack_mod._extract_additional_text_from_slack_blocks( + blocks, + f"<@U_BOT> do you see <{_THREAD_PERMALINK_ESCAPED}|Custom label> ?", + bot_uid="U_BOT", + ) + == "" + ) + + def test_quote_beside_a_url_less_message_mention_appended_alone(self): + """The quote is the only addition: the sentence around the permalink + must not come back as a second copy with the link blanked.""" + blocks = self._mention_blocks( + { + "type": "message_mention", + "channel_id": "C0BCDG3H66P", + "message_ts": "1786102118.226679", + }, + { + "type": "rich_text_quote", + "elements": [ + _rich_text_section({"type": "text", "text": "quoted context"}) + ], + }, + ) + + assert ( + _slack_mod._extract_additional_text_from_slack_blocks( + blocks, self._mention_text(), bot_uid="U_BOT" + ) + == "> quoted context" + ) + + def test_quote_containing_an_unrenderable_element_is_still_appended(self): + """Negative case: a quote is absent from ``event.text`` by construction, + so it is never a duplicate of it.""" + blocks = _rich_text_blocks( + _rich_text_section({"type": "text", "text": "look at this"}), + { + "type": "rich_text_quote", + "elements": [ + {"type": "text", "text": "see "}, + {"type": "an_element_slack_adds_later"}, + {"type": "text", "text": " please"}, + ], + }, + ) + + additional = _slack_mod._extract_additional_text_from_slack_blocks( + blocks, "look at this", bot_uid="U_BOT" + ) + + assert "see" in additional + assert "please" in additional + + @pytest.mark.parametrize( + ("element", "flat"), + [ + # ``fallback`` and ``url`` are both optional on the rich-text date + # element, so an element with neither renders as nothing. + ({}, ""), + ({"fallback": "Aug 7"}, ""), + ( + {"url": "https://cal/x", "fallback": "Aug 7"}, + "", + ), + ( + {"url": "https://cal/x"}, + "", + ), + ], + ) + def test_date_element_is_not_read_as_new_content(self, element, flat): + """The flat field carries ```` while the rich text renders the + fallback or the url, so both sides need reading down to one value.""" + blocks = _rich_text_blocks( + _rich_text_section( + {"type": "user", "user_id": "U_BOT"}, + {"type": "text", "text": " meet at "}, + { + "type": "date", + "timestamp": 1786102118, + "format": "{date_short}", + **element, + }, + {"type": "text", "text": " ok?"}, + ) + ) + + assert ( + _slack_mod._extract_additional_text_from_slack_blocks( + blocks, f"<@U_BOT> meet at {flat} ok?", bot_uid="U_BOT" + ) + == "" + ) + + @pytest.mark.parametrize("flat_text", ["", "New alert"]) + def test_app_message_keeps_its_body(self, flat_text): + """Negative case: an app posts its body in the blocks, with a flat + ``text`` field that is empty or a short notification of its own.""" + blocks = _rich_text_blocks( + _rich_text_section( + {"type": "text", "text": "Build failed on "}, + # ``team`` carries neither a url nor a label. + {"type": "team", "team_id": "T123"}, + {"type": "text", "text": " see logs"}, + ) + ) + + additional = _slack_mod._extract_additional_text_from_slack_blocks( + blocks, flat_text, bot_uid="U_BOT" + ) + + assert "Build failed on" in additional + assert "see logs" in additional + + def test_hydrated_app_message_without_flat_text_keeps_its_body(self, adapter): + rendered = adapter._render_message_text( + { + "text": "", + "blocks": _rich_text_blocks( + _rich_text_section( + {"type": "text", "text": "Build failed on "}, + {"type": "team", "team_id": "T123"}, + {"type": "text", "text": " see logs"}, + ) + ), + }, + bot_uid="U_BOT", + ) + + assert "Build failed on" in rendered + assert "see logs" in rendered + + def test_workspace_mention_is_not_read_as_new_content(self): + """A workspace mention renders into the flat form Slack sends.""" + blocks = _rich_text_blocks( + _rich_text_section( + {"type": "user", "user_id": "U_BOT"}, + {"type": "text", "text": " ping "}, + {"type": "team", "team_id": "T123"}, + {"type": "text", "text": " now"}, + ) + ) + + assert ( + _slack_mod._extract_additional_text_from_slack_blocks( + blocks, "<@U_BOT> ping now", bot_uid="U_BOT" + ) + == "" + ) + + def test_color_element_is_not_read_as_new_content(self): + """The composer keeps the typed hex code in the flat text.""" + blocks = _rich_text_blocks( + _rich_text_section( + {"type": "user", "user_id": "U_BOT"}, + {"type": "text", "text": " brand is "}, + {"type": "color", "value": "#FF0000"}, + {"type": "text", "text": " ok?"}, + ) + ) + + assert ( + _slack_mod._extract_additional_text_from_slack_blocks( + blocks, "<@U_BOT> brand is #FF0000 ok?", bot_uid="U_BOT" + ) + == "" + ) + + @pytest.mark.parametrize( + ("element", "flat"), + [ + ( + {"type": "channel", "channel_id": "C024BE7LR"}, + "<@U_BOT> see <#C024BE7LR|general> please", + ), + ( + {"type": "usergroup", "usergroup_id": "SAZ94GDB8"}, + "<@U_BOT> see please", + ), + ( + {"type": "user", "user_id": "U024BE7LH"}, + "<@U_BOT> see <@U024BE7LH|nikita> please", + ), + ( + {"type": "broadcast", "range": "here"}, + "<@U_BOT> see please", + ), + ], + ) + def test_labelled_mention_is_not_read_as_new_content(self, element, flat): + """Slack may label any mention in the flat text while the blocks carry + the bare id.""" + blocks = _rich_text_blocks( + _rich_text_section( + {"type": "user", "user_id": "U_BOT"}, + {"type": "text", "text": " see "}, + element, + {"type": "text", "text": " please"}, + ) + ) + + assert ( + _slack_mod._extract_additional_text_from_slack_blocks( + blocks, flat, bot_uid="U_BOT" + ) + == "" + ) + + def test_section_of_a_single_untrusted_element_is_still_delivered(self): + """Negative case: a mismatch is never a reason to drop content.""" + blocks = _rich_text_blocks( + _rich_text_section({"type": "team", "team_id": "T123"}) + ) + + assert _slack_mod._extract_additional_text_from_slack_blocks( + blocks, "New alert", bot_uid="U_BOT" + ) + + @pytest.mark.parametrize( + "flat", + [ + "hey <@U_BOT|hermes> please look", + "hey <@U_BOT> please look", + "hey <@U_BOT> please look", + ], + ) + def test_labelled_bot_mention_is_not_read_as_new_content(self, flat): + """The render drops the bot mention, so every flat form of it must be + dropped from the flat text too.""" + blocks = _rich_text_blocks( + _rich_text_section( + {"type": "text", "text": "hey "}, + {"type": "user", "user_id": "U_BOT"}, + {"type": "text", "text": " please look"}, + ) + ) + + assert ( + _slack_mod._extract_additional_text_from_slack_blocks( + blocks, flat, bot_uid="U_BOT" + ) + == "" + ) + + def test_non_http_scheme_link_is_not_read_as_new_content(self): + """Autolinks are not limited to the schemes we happened to list.""" + blocks = _rich_text_blocks( + _rich_text_section( + {"type": "user", "user_id": "U_BOT"}, + {"type": "text", "text": " call "}, + {"type": "link", "url": "tel:+15551234567"}, + {"type": "text", "text": " now"}, + ) + ) + + assert ( + _slack_mod._extract_additional_text_from_slack_blocks( + blocks, "<@U_BOT> call now", bot_uid="U_BOT" + ) + == "" + ) + + @pytest.mark.asyncio + async def test_live_inbound_pasted_permalink_not_duplicated(self, adapter): + await adapter._handle_slack_message( + { + "text": self._mention_text(), + "blocks": self._mention_blocks( + { + "type": "message_mention", + "channel_id": "C0BCDG3H66P", + "message_ts": "1786102118.226679", + "url": _PERMALINK, + "text": _PERMALINK, + } + ), + "user": "U_USER", + "client_msg_id": "cm-4", + "channel": "D_DM", + "channel_type": "im", + "ts": "123.459", + "team": "T_TEAM", + } + ) + + text = adapter.handle_message.await_args.args[0].text + # One line, and no second copy with the permalink blanked out. + assert text.count("do you see") == 1 + assert "\n" not in text + assert _PERMALINK in text + + def test_hydration_pasted_permalink_not_duplicated(self, adapter): + rendered = adapter._render_message_text( + { + "text": self._mention_text(), + "blocks": self._mention_blocks( + { + "type": "message_mention", + "channel_id": "C0BCDG3H66P", + "message_ts": "1786102118.226679", + "url": _PERMALINK, + } + ), + }, + bot_uid="U_BOT", + ) + + assert rendered.count("do you see") == 1 + assert "\n" not in rendered + assert _PERMALINK in rendered + + def test_block_kit_dump_still_describes_bot_ui_blocks(self): + """Negative case: UI-heavy bot blocks are why the dump exists.""" + payload = _slack_mod._serialize_slack_blocks_for_agent( + [ + { + "type": "section", + "text": {"type": "mrkdwn", "text": "Deploy failed"}, + }, + { + "type": "actions", + "elements": [ + { + "type": "button", + "action_id": "rollback", + "text": {"type": "plain_text", "text": "Roll back"}, + } + ], + }, + ] + ) + + assert "Deploy failed" in payload + assert "rollback" in payload + assert "Roll back" in payload