fix(slack): stop duplicating one authored message on entities, unfurls and the Block Kit dump

plugins/platforms/slack/adapter.py

@@ -519,6 +519,12 @@ def _extract_text_from_slack_attachments(attachments: list) -> str:
     for att in attachments:
         if not isinstance(att, dict):
             continue
+        # Slack permalink unfurls (``is_msg_unfurl``) carry the *linked*
+        # message's own body. The live inbound path already skips them; doing
+        # the same here keeps thread/parent hydration from appending a second
+        # copy of a message the agent is already reading.
+        if att.get("is_msg_unfurl"):
+            continue
         got: list[str] = [
             str(att[key]) for key in ("pretext", "title", "text") if att.get(key)
         ]
@@ -547,6 +553,21 @@ def _extract_text_from_slack_attachments(attachments: list) -> str:
 )
 _SLACK_INLINE_CODE_RE = re.compile(r"`([^`\n]+)`")
 _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 _unescape_slack_entities(text: str) -> str:
+    """Undo Slack's HTML escaping of ``&``/``<``/``>`` in flat message text.
+
+    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:
@@ -559,6 +580,10 @@ def _link(match: re.Match) -> str:
     canonical = text or ""
     if bot_uid:
         canonical = canonical.replace(f"<@{bot_uid}>", "")
+    # Unescape BEFORE link canonicalization so both sides of the comparison
+    # see the same angle-bracket forms; otherwise a link with query
+    # parameters reads as new content and gets appended a second time.
+    canonical = _unescape_slack_entities(canonical)
     canonical = _SLACK_MRKDWN_LINK_RE.sub(_link, canonical)
     canonical = _SLACK_FENCED_CODE_RE.sub(r"\1", canonical)
     canonical = _SLACK_INLINE_CODE_RE.sub(r"\1", canonical)
@@ -603,11 +628,21 @@ def _extract_additional_text_from_slack_blocks(
 
 
 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 ""
-
-    if all((block or {}).get("type") == "rich_text" for block in blocks):
+    """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 = {
@@ -659,9 +694,9 @@ def _sanitize(value):
         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]"
@@ -7437,7 +7472,15 @@ def _render_message_text(msg: dict, bot_uid: str = "") -> str:
             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

tests/gateway/test_slack.py

@@ -5417,3 +5417,244 @@ def test_hermes_slack_user_agent_prefix_format(self):
         elsewhere in the codebase for platform-partner attribution."""
         assert _slack_mod._HERMES_SLACK_USER_AGENT_PREFIX.startswith("HermesAgent/")
 
+
+# ---------------------------------------------------------------------------
+# 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("&", "&")
+
+
+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&amp;T outage", [{"type": "text", "text": "AT&T outage"}]),
+            # Literal angle brackets the user typed.
+            ("use &lt;div&gt; here", [{"type": "text", "text": "use <div> here"}]),
+            # Labelled link whose label carries an ampersand.
+            (
+                "see <https://x.example|AT&amp;T>",
+                [
+                    {"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
+
+    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