From ac62c19c34072a968d01845edbfc4550cb95c3f2 Mon Sep 17 00:00:00 2001 From: Nikita Barkov Date: Tue, 11 Aug 2026 15:11:44 +0200 Subject: [PATCH] feat(slack): make stripping the bot's own mention configurable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The adapter deletes the bot's own `<@U…>` token from the text before the agent reads it, and reports nothing in its place. A thread keeps waking the bot after the first mention, so every delivered turn then looks identical: the agent cannot tell "someone tagged me" from "I was woken by thread routing" — the distinction an agent needs to decide for itself whether a turn deserves an answer. Add `slack.strip_bot_mentions` (default `true` — today's behavior byte for byte, so nothing changes for anyone who does not opt in). With `false` the mention stays where the author put it, rendered as `@BotName`, the same shape `_humanize_user_mentions()` gives mentions of other participants; its absence then means the bot was woken by channel or thread routing. The asymmetry is the signal — no marker text is injected. The name comes from `_team_bot_names` / `_bot_display_name`, both resolved at connect time, so there is no extra Slack call; an unresolved name leaves the raw token rather than deleting it. Routing is untouched (`is_mentioned`, `_mentioned_threads`, `require_mention`, `strict_mention`, `thread_require_mention` are all evaluated before this), and so is command parsing, which runs off the separate `mention_stripped` variable. - `_slack_strip_bot_mentions()` resolves `config.extra` → `SLACK_STRIP_BOT_MENTIONS` → default `true`, like its four siblings. - Thread history follows the same policy (`_render_message_text`, `_format_thread_context`), so past turns don't read as "nobody ever tagged me"; block content is compared against the text as written, since the blocks carry the raw token. - `_fetch_thread_parent_text()` forwards the caller's `strip_bot_mention` into the render on a cache miss. That path is the root-mention wake check (#24848), which greps the parent for the raw `<@id>`, and the render deleted it unconditionally — so on a cold cache the check could never match, in either flag state. - Documented in the Slack guide and the environment-variable reference; `config.yaml` is the canonical place, the env var is a mirror. - 51 tests over both flag states. Co-authored-by: Junie --- hermes_cli/config_defaults.py | 6 + plugins/platforms/slack/adapter.py | 152 +++++- plugins/platforms/slack/plugin.yaml | 8 + .../gateway/test_slack_strip_bot_mentions.py | 508 ++++++++++++++++++ .../docs/reference/environment-variables.md | 1 + website/docs/user-guide/messaging/slack.md | 29 + 6 files changed, 682 insertions(+), 22 deletions(-) create mode 100644 tests/gateway/test_slack_strip_bot_mentions.py diff --git a/hermes_cli/config_defaults.py b/hermes_cli/config_defaults.py index 4738aa42a941..7685bb329688 100644 --- a/hermes_cli/config_defaults.py +++ b/hermes_cli/config_defaults.py @@ -2034,6 +2034,12 @@ "ignore_other_user_mentions": False, # If True, require @mention in Slack thread replies too. "thread_require_mention": False, + # If False, the bot's own <@id> mention is NOT deleted from the text the + # agent receives — it is rendered as @BotName (like mentions of other + # participants) so the agent can tell an explicit tag from a + # thread-routed wake-up. Default True keeps the historical strip. + # Env: SLACK_STRIP_BOT_MENTIONS. + "strip_bot_mentions": True, "channel_prompts": {}, # Per-channel ephemeral system prompts }, diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index 50d215ea65e3..34fd1374aab0 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -4432,6 +4432,41 @@ async def _humanize_user_mentions( ) return text + def _own_bot_name(self, team_id: str = "") -> str: + """This bot's display name in ``team_id`` (empty when not resolved yet). + + Workspace-scoped: :attr:`_team_bot_names` is filled per team at connect + time, with :attr:`_bot_display_name` as the primary-workspace fallback. + No Slack call — both are already in memory. + """ + return ( + (team_id and self._team_bot_names.get(team_id)) + or self._bot_display_name + or "" + ).strip() + + @staticmethod + def _render_own_mention(text: str, bot_uid: str, bot_name: str) -> str: + """Render this bot's own ``<@id>`` tokens as ``@DisplayName``, in place. + + Used when ``strip_bot_mentions`` is off: the mention stays where the + author put it, in the same shape :meth:`_humanize_user_mentions` gives + mentions of other participants, so the agent reads an ordinary tag + rather than a synthetic marker. Handles the labelled ``<@U123|name>`` + form too. With no known display name the raw token is left untouched — + an unresolved id still beats a deleted mention. + """ + if not text or not bot_uid or not bot_name: + return text + # Callable replacement: a display name is arbitrary user data, and a + # backslash in it would be read as a group reference and raise + # re.error, dropping the message. + return re.sub( + rf"<@{re.escape(bot_uid)}(?:\|[^>]*)?>", + lambda _m: f"@{bot_name}", + text, + ).strip() + def _build_identity_prompt(self, team_id: str = "") -> str: """Return an ephemeral system-prompt line grounding the bot's identity. @@ -4445,11 +4480,7 @@ def _build_identity_prompt(self, team_id: str = "") -> str: (see :meth:`_humanize_user_mentions`), so naming the bot's own display name here gives the agent a positive anchor for "that's me." """ - name = ( - (team_id and self._team_bot_names.get(team_id)) - or self._bot_display_name - or "" - ).strip() + name = self._own_bot_name(team_id) if not name: return "" return ( @@ -6238,8 +6269,17 @@ async def _handle_slack_message( return if is_mentioned: - # Strip the bot mention from the text - text = text.replace(f"<@{bot_uid}>", "").strip() + if self._slack_strip_bot_mentions(): + # Strip the bot mention from the text + text = text.replace(f"<@{bot_uid}>", "").strip() + else: + # Keep it, rendered like any other participant's mention + # (@DisplayName), so the agent can tell an explicit tag from a + # channel/thread-routed wake-up. Routing already happened above + # and is unaffected. + text = self._render_own_mention( + text, bot_uid, self._own_bot_name(team_id) + ) # Re-run command normalization against the canonical Slack text, # not the block-augmented agent text. Otherwise quoted/forwarded # rich-text payload can become accidental command arguments. @@ -6787,10 +6827,12 @@ async def _handle_slack_message( reply_to_text = None # Humanize remaining user mentions: the bot's own mention was already - # stripped above, so any ``<@UID>`` left in the trigger text refers to - # OTHER participants. Render them as ``@DisplayName`` so the agent can - # tell who is being addressed and never mistakes a human's mention for - # a mention of itself (the "bot thinks it's @someone-else" bug). + # stripped (or rendered as ``@DisplayName``) above, so any ``<@UID>`` + # left in the trigger text refers to OTHER participants — except the + # bot's own token when its display name is not resolved yet. Render + # them as ``@DisplayName`` so the agent can tell who is being addressed + # and never mistakes a human's mention for a mention of itself (the + # "bot thinks it's @someone-else" bug). # Mirrors Discord's clean_content. channel_context (thread backfill) # already renders senders by display name via _format_thread_context. text = await self._humanize_user_mentions( @@ -7638,7 +7680,12 @@ async def _handle_clarify_action(self, ack, body, action) -> None: # ----- Thread context fetching ----- @staticmethod - def _render_message_text(msg: dict, bot_uid: str = "") -> str: + def _render_message_text( + msg: dict, + bot_uid: str = "", + strip_bot_mention: bool = True, + bot_name: str = "", + ) -> str: """Return bounded display text for a Slack message, surfacing Block Kit content. Starts with ``text``, strips bot mentions, then appends rich-text @@ -7648,16 +7695,27 @@ def _render_message_text(msg: dict, bot_uid: str = "") -> str: readable text and URL list needed by thread-context and parent- text rendering — bounded by what the blocks actually contain, not a JSON dump. + + With ``strip_bot_mention=False`` the bot's own mention is rendered as + ``@bot_name`` instead of deleted. Block content is then compared + against the text as written, since the blocks carry the raw token. """ - msg_text = (msg.get("text") or "").strip() + raw_text = (msg.get("text") or "").strip() + msg_text = raw_text if bot_uid: - msg_text = msg_text.replace(f"<@{bot_uid}>", "").strip() + if strip_bot_mention: + msg_text = msg_text.replace(f"<@{bot_uid}>", "").strip() + else: + msg_text = SlackAdapter._render_own_mention( + msg_text, bot_uid, bot_name + ) + dedupe_text = msg_text if strip_bot_mention else raw_text 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: + if rich_text and rich_text not in dedupe_text: extras.append(rich_text) for block in blocks: block_type = (block or {}).get("type", "") @@ -7665,7 +7723,7 @@ def _render_message_text(msg: dict, bot_uid: str = "") -> str: 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): + if section_text and section_text not in dedupe_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 @@ -7673,13 +7731,13 @@ def _render_message_text(msg: dict, bot_uid: str = "") -> str: attachments_text = _extract_text_from_slack_attachments( msg.get("attachments") or [] ).strip() - if attachments_text and attachments_text not in msg_text and all( + if attachments_text and attachments_text not in dedupe_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)] + new_urls = [u for u in urls if u not in dedupe_text 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 @@ -7869,6 +7927,10 @@ async def _format_thread_context( from gateway.session import neutralize_untrusted_inline_text bot_uid = self._team_bot_user_ids.get(team_id, self._bot_user_id) + # Thread history follows the same mention policy as the current + # message, otherwise past turns read as "nobody ever tagged me". + strip_bot_mention = self._slack_strip_bot_mentions() + bot_name = "" if strip_bot_mention else self._own_bot_name(team_id) context_parts = [] parent_text = "" for msg in messages: @@ -7912,13 +7974,22 @@ async def _format_thread_context( and msg_user == self_bot_uid ) - msg_text = self._render_message_text(msg, bot_uid=bot_uid) + msg_text = self._render_message_text( + msg, + bot_uid=bot_uid, + strip_bot_mention=strip_bot_mention, + bot_name=bot_name, + ) if not msg_text: continue - # Strip bot mentions from context messages + # Strip bot mentions from context messages (Block Kit extras can + # still carry the raw token past the render above). if bot_uid: - msg_text = msg_text.replace(f"<@{bot_uid}>", "").strip() + if strip_bot_mention: + msg_text = msg_text.replace(f"<@{bot_uid}>", "").strip() + else: + msg_text = self._render_own_mention(msg_text, bot_uid, bot_name) if is_parent: parent_text = msg_text @@ -8012,6 +8083,8 @@ async def _fetch_thread_parent_text( Used for reply_to_text injection (mention stripped) and for the parent-mentioned-bot wake check (#24848 — pass ``strip_bot_mention=False`` so the ``<@bot>`` token is preserved). + That caller searches for the raw ``<@id>``, so ``strip_bot_mentions: + false`` must not rewrite it into ``@BotName`` here either. Uses the same per-thread cache as :meth:`_fetch_thread_context` to avoid hitting ``conversations.replies`` twice. Falls back to a cheap single- @@ -8048,7 +8121,13 @@ 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 = self._render_message_text(parent, bot_uid=bot_uid or "") + # Forward the caller's intent, so a cold cache still yields the + # raw token the root-mention check looks for. + text = self._render_message_text( + parent, + bot_uid=bot_uid or "", + strip_bot_mention=strip_bot_mention, + ) if strip_bot_mention and bot_uid: text = text.replace(f"<@{bot_uid}>", "").strip() return text @@ -8829,6 +8908,29 @@ def _slack_thread_require_mention(self) -> bool: "on", } + def _slack_strip_bot_mentions(self) -> bool: + """Whether the bot's own ``<@id>`` is deleted from the agent-visible text. + + Default True is the historical behaviour. False keeps the mention, + rendered as ``@BotName`` like any other participant's, so the agent can + tell an explicit tag from a channel/thread-routed wake-up. Routing is + unaffected either way — this changes only the text. + + Explicit-false parsing (like :meth:`_slack_require_mention`), since the + safe default is True; unrecognised or empty values keep stripping on. + """ + configured = self.config.extra.get("strip_bot_mentions") + if configured is not None: + if isinstance(configured, str): + return configured.lower() not in {"false", "0", "no", "off"} + return bool(configured) + return os.getenv("SLACK_STRIP_BOT_MENTIONS", "true").lower() not in { + "false", + "0", + "no", + "off", + } + def _slack_message_addressed_to_other_user(self, text: str, self_uids: set) -> bool: """Return True when ``text`` opens by @-mentioning a non-bot user. @@ -9514,6 +9616,12 @@ def _apply_yaml_config(yaml_cfg: dict, slack_cfg: dict) -> dict | None: os.environ["SLACK_THREAD_REQUIRE_MENTION"] = str( slack_cfg["thread_require_mention"] ).lower() + if "strip_bot_mentions" in slack_cfg and not os.getenv( + "SLACK_STRIP_BOT_MENTIONS" + ): + os.environ["SLACK_STRIP_BOT_MENTIONS"] = str( + slack_cfg["strip_bot_mentions"] + ).lower() if "allow_bots" in slack_cfg and not os.getenv("SLACK_ALLOW_BOTS"): os.environ["SLACK_ALLOW_BOTS"] = str(slack_cfg["allow_bots"]).lower() frc = slack_cfg.get("free_response_channels") diff --git a/plugins/platforms/slack/plugin.yaml b/plugins/platforms/slack/plugin.yaml index b04ac8b2ca18..3f57bfb07977 100644 --- a/plugins/platforms/slack/plugin.yaml +++ b/plugins/platforms/slack/plugin.yaml @@ -43,3 +43,11 @@ optional_env: top-level free response channels prompt: "Require mentions in Slack threads? (true/false)" password: false + - name: SLACK_STRIP_BOT_MENTIONS + description: >- + Delete the bot's own @mention from the text the agent sees (default true). + Set false to keep it, rendered as @BotName, so the agent can tell an + explicit tag from a thread-routed wake-up. Canonical setting: + slack.strip_bot_mentions in config.yaml + prompt: "Strip the bot's own mention from Slack text? (true/false)" + password: false diff --git a/tests/gateway/test_slack_strip_bot_mentions.py b/tests/gateway/test_slack_strip_bot_mentions.py new file mode 100644 index 000000000000..c5a096fa1e7d --- /dev/null +++ b/tests/gateway/test_slack_strip_bot_mentions.py @@ -0,0 +1,508 @@ +"""``slack.strip_bot_mentions``: whether the agent sees its own mention. + +Default ``True`` deletes the bot's own ``<@U…>`` token before the agent reads +the message (the historical behaviour, pinned by +``test_slack.py::test_channel_mention_strips_bot_id``). ``False`` keeps it, +rendered as ``@BotName`` like any other participant's, so an explicit tag is +distinguishable from a thread-routed wake-up. Routing and command parsing are +unaffected either way. +""" + +import os +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from gateway.config import PlatformConfig +from plugins.platforms.slack.adapter import ( + SlackAdapter, + _apply_yaml_config, + _ThreadContextCache, +) + + +@pytest.fixture(autouse=True) +def _clean_env(): + """``_apply_yaml_config`` writes ``os.environ`` directly — restore it.""" + saved = os.environ.get("SLACK_STRIP_BOT_MENTIONS") + os.environ.pop("SLACK_STRIP_BOT_MENTIONS", None) + yield + os.environ.pop("SLACK_STRIP_BOT_MENTIONS", None) + if saved is not None: + os.environ["SLACK_STRIP_BOT_MENTIONS"] = saved + + +def make_adapter(extra=None): + return SlackAdapter(PlatformConfig(enabled=True, token="***", extra=extra or {})) + + +def delivery_adapter(strip, bot_name="TestBot", team_names=None): + """Adapter wired to capture what ``_handle_slack_message`` delivers. + + Everything the trigger path would fetch from Slack is stubbed; the routing + and text-building code under test is the real thing. + """ + adapter = make_adapter({"strip_bot_mentions": strip}) + adapter._app = MagicMock() + adapter._app.client = AsyncMock() + adapter._bot_user_id = "U_BOT" + adapter._team_bot_user_ids["T123"] = "U_BOT" + adapter._bot_display_name = bot_name + adapter._team_bot_names = dict(team_names or {}) + adapter._running = True + adapter.handle_message = AsyncMock() + adapter._has_active_session_for_thread = lambda **_: False + + async def _no_thread_context(**_): + return "" + + async def _no_parent_text(**_): + return "" + + async def _no_thread_images(**_): + return [], [] + + async def _resolve_user_name(user_id, chat_id="", team_id=""): + return {"U_USER": "Nikita"}.get(user_id, "") + + adapter._fetch_thread_context = _no_thread_context + adapter._fetch_thread_parent_text = _no_parent_text + adapter._collect_thread_root_images = _no_thread_images + adapter._resolve_user_name = _resolve_user_name + return adapter + + +def thread_adapter(strip): + """Like :func:`delivery_adapter`, but with the real parent-text lookup. + + The thread tests exercise ``_fetch_thread_parent_text`` itself, so its + delivery-path stub is dropped (the cache is primed instead of Slack). + """ + adapter = delivery_adapter(strip=strip) + del adapter._fetch_thread_parent_text + return adapter + + +def slack_event(text, ts="1234567890.000001", thread_ts=None, team="T123", **extra): + event = { + "type": "message", + "channel": "C123", + "channel_type": "channel", + "team": team, + "user": "U_USER", + "text": text, + "ts": ts, + } + if thread_ts is not None: + event["thread_ts"] = thread_ts + event.update(extra) + return event + + +def delivered(adapter): + adapter.handle_message.assert_awaited_once() + return adapter.handle_message.await_args.args[0] + + +# The Slack composer mirrors the flat text in a rich_text block on every +# message; for a bare mention that mirror is the mention alone. +MENTION_ONLY_BLOCKS = [ + { + "type": "rich_text", + "elements": [ + { + "type": "rich_text_section", + "elements": [{"type": "user", "user_id": "U_BOT"}], + } + ], + } +] + + +class TestFlagResolution: + """``config.extra`` → ``SLACK_STRIP_BOT_MENTIONS`` → default ``True``.""" + + def test_defaults_to_stripping(self): + assert make_adapter()._slack_strip_bot_mentions() is True + + @pytest.mark.parametrize("value", ["false", "False", "0", "no", "off", "OFF"]) + def test_env_turns_stripping_off(self, value, monkeypatch): + monkeypatch.setenv("SLACK_STRIP_BOT_MENTIONS", value) + assert make_adapter()._slack_strip_bot_mentions() is False + + @pytest.mark.parametrize("value", ["true", "1", "yes", "", "maybe"]) + def test_unrecognised_env_keeps_stripping(self, value, monkeypatch): + """Explicit-false parsing: only a known negative disables the strip.""" + monkeypatch.setenv("SLACK_STRIP_BOT_MENTIONS", value) + assert make_adapter()._slack_strip_bot_mentions() is True + + def test_config_extra_beats_env(self, monkeypatch): + monkeypatch.setenv("SLACK_STRIP_BOT_MENTIONS", "false") + assert make_adapter({"strip_bot_mentions": True})._slack_strip_bot_mentions() is True + + def test_config_extra_string_forms(self): + assert ( + make_adapter({"strip_bot_mentions": "false"})._slack_strip_bot_mentions() + is False + ) + assert ( + make_adapter({"strip_bot_mentions": "true"})._slack_strip_bot_mentions() + is True + ) + + def test_config_extra_bool_false(self): + assert ( + make_adapter({"strip_bot_mentions": False})._slack_strip_bot_mentions() + is False + ) + + +class TestYamlEnvBridge: + """``config.yaml`` is the canonical surface; the env var is its mirror.""" + + def test_bridges_config_yaml_to_env(self): + _apply_yaml_config({}, {"strip_bot_mentions": False}) + assert os.environ["SLACK_STRIP_BOT_MENTIONS"] == "false" + assert make_adapter()._slack_strip_bot_mentions() is False + + def test_does_not_overwrite_an_explicit_env_var(self, monkeypatch): + monkeypatch.setenv("SLACK_STRIP_BOT_MENTIONS", "false") + _apply_yaml_config({}, {"strip_bot_mentions": True}) + assert os.environ["SLACK_STRIP_BOT_MENTIONS"] == "false" + + def test_absent_key_leaves_env_alone(self): + """A config.yaml without the key must not pin the mirror either way.""" + _apply_yaml_config({}, {}) + assert "SLACK_STRIP_BOT_MENTIONS" not in os.environ + + +class TestRenderOwnMention: + """``_render_own_mention``: the token becomes ``@Name`` where it stood.""" + + def test_renders_in_place(self): + assert ( + SlackAdapter._render_own_mention("hey <@U_BOT> look", "U_BOT", "TestBot") + == "hey @TestBot look" + ) + + def test_renders_the_labelled_form(self): + """Slack also delivers ``<@U123|name>`` (legacy / some clients).""" + assert ( + SlackAdapter._render_own_mention("<@U_BOT|yana> hi", "U_BOT", "TestBot") + == "@TestBot hi" + ) + + def test_mention_only_text_is_not_emptied(self): + assert ( + SlackAdapter._render_own_mention("<@U_BOT>", "U_BOT", "TestBot") + == "@TestBot" + ) + + def test_unknown_name_leaves_the_raw_token(self): + """An unresolved id still beats a deleted mention.""" + assert ( + SlackAdapter._render_own_mention("<@U_BOT> ping", "U_BOT", "") + == "<@U_BOT> ping" + ) + + def test_other_participants_are_untouched(self): + assert ( + SlackAdapter._render_own_mention("<@U_OTHER> ping", "U_BOT", "TestBot") + == "<@U_OTHER> ping" + ) + + @pytest.mark.parametrize("name", ["Te\\st", "Bot\\1", "\\g<0>"]) + def test_regex_metacharacters_in_the_name_are_literal(self, name): + """A display name is user data — it must never be read as a template.""" + assert ( + SlackAdapter._render_own_mention("<@U_BOT> ping", "U_BOT", name) + == f"@{name} ping" + ) + + +class TestTriggerMessageText: + """What the agent actually receives for an explicitly mentioned message.""" + + @pytest.mark.asyncio + async def test_default_strips_the_mention(self): + adapter = delivery_adapter(strip=True) + + await adapter._handle_slack_message(slack_event("<@U_BOT> what's up?")) + + assert delivered(adapter).text == "what's up?" + + @pytest.mark.asyncio + async def test_flag_off_keeps_the_mention_in_place(self): + adapter = delivery_adapter(strip=False) + + await adapter._handle_slack_message(slack_event("hey <@U_BOT> look")) + + event = delivered(adapter) + assert event.text == "hey @TestBot look" + assert "<@U_BOT>" not in event.text + + @pytest.mark.asyncio + async def test_flag_off_mention_only_message(self): + adapter = delivery_adapter(strip=False) + + await adapter._handle_slack_message(slack_event("<@U_BOT>")) + + assert delivered(adapter).text == "@TestBot" + + @pytest.mark.asyncio + @pytest.mark.parametrize("strip,expected", [(True, ""), (False, "@TestBot")]) + async def test_mention_only_message_with_blocks_is_not_duplicated( + self, strip, expected + ): + """Real Slack messages carry a rich_text mirror of their own text. + + Block dedupe deletes ``<@bot_uid>`` from both sides to bridge the strip + below, which reduces a mention-only block to the empty string — read as + new content and appended, so the mention would arrive twice. + """ + adapter = delivery_adapter(strip=strip) + + await adapter._handle_slack_message( + slack_event("<@U_BOT>", blocks=MENTION_ONLY_BLOCKS) + ) + + assert delivered(adapter).text == expected + + @pytest.mark.asyncio + async def test_flag_off_uses_the_per_workspace_name(self): + """Multi-workspace: the bot's handle is team-scoped, not global.""" + adapter = delivery_adapter( + strip=False, bot_name="TestBot", team_names={"T123": "WorkspaceBot"} + ) + + await adapter._handle_slack_message(slack_event("<@U_BOT> ping")) + + assert delivered(adapter).text == "@WorkspaceBot ping" + + @pytest.mark.asyncio + async def test_flag_off_without_a_known_name_keeps_the_mention(self): + """Before connect resolves a handle: never delete, never crash.""" + adapter = delivery_adapter(strip=False, bot_name=None) + + await adapter._handle_slack_message(slack_event("<@U_BOT> ping")) + + text = delivered(adapter).text + assert "ping" in text + assert text != "ping", "the mention must not be dropped" + + @pytest.mark.asyncio + async def test_flag_off_edited_message_follows_the_flag(self): + """``message_changed`` is normalized into the same trigger path.""" + adapter = delivery_adapter(strip=False) + + await adapter._handle_slack_message( + { + "subtype": "message_changed", + "channel": "C123", + "channel_type": "channel", + "team": "T123", + "ts": "1234567890.000001", + "message": { + "text": "<@U_BOT> take another look", + "user": "U_USER", + "channel": "C123", + "ts": "1234567890.000001", + "edited": {"user": "U_USER", "ts": "1234567899.000001"}, + }, + } + ) + + assert delivered(adapter).text == "@TestBot take another look" + + @pytest.mark.asyncio + @pytest.mark.parametrize("strip", [True, False]) + async def test_unmentioned_thread_wakeup_is_delivered_unchanged(self, strip): + """The asymmetry is the signal: no tag ⇒ nothing added to the text.""" + adapter = delivery_adapter(strip=strip) + adapter._register_mentioned_thread("100.000", team_id="T123") + + await adapter._handle_slack_message( + slack_event("and then we ship", ts="101.000", thread_ts="100.000") + ) + + assert delivered(adapter).text == "and then we ship" + + +class TestThreadContext: + """History must read the same way as the message that woke the bot.""" + + _PARENT = {"ts": "100.000", "user": "U_USER", "text": "<@U_BOT> check this"} + + @pytest.mark.asyncio + @pytest.mark.parametrize("strip,expected", [(True, ""), (False, "@TestBot")]) + async def test_mention_only_parent_with_blocks_is_not_duplicated( + self, strip, expected + ): + """Same dedupe contract on the thread-history path.""" + adapter = delivery_adapter(strip=strip) + + _content, parent_text = await adapter._format_thread_context( + [ + { + "ts": "100.000", + "user": "U_USER", + "text": "<@U_BOT>", + "blocks": MENTION_ONLY_BLOCKS, + } + ], + thread_ts="100.000", + current_ts="101.000", + team_id="T123", + channel_id="C123", + ) + + assert parent_text == expected + + @pytest.mark.asyncio + @pytest.mark.parametrize( + "strip,expected", [(True, "check this"), (False, "@TestBot check this")] + ) + async def test_context_lines_follow_the_flag(self, strip, expected): + adapter = delivery_adapter(strip=strip) + + content, parent_text = await adapter._format_thread_context( + [dict(self._PARENT)], + thread_ts="100.000", + current_ts="101.000", + team_id="T123", + channel_id="C123", + ) + + assert parent_text == expected + assert expected in content + assert "<@U_BOT>" not in content + + @pytest.mark.asyncio + async def test_keeping_the_mention_does_not_duplicate_the_message(self): + """Block content is compared against the text as written. + + Slack mirrors an authored message into a ``rich_text`` block carrying + the raw token, so comparing it against the rendered ``@BotName`` would + append the same message a second time. + """ + adapter = delivery_adapter(strip=False) + parent = { + "ts": "100.000", + "user": "U_USER", + "text": "hey <@U_BOT> look at this", + "blocks": [ + { + "type": "rich_text", + "elements": [ + { + "type": "rich_text_section", + "elements": [ + {"type": "text", "text": "hey "}, + {"type": "user", "user_id": "U_BOT"}, + {"type": "text", "text": " look at this"}, + ], + } + ], + } + ], + } + + _content, parent_text = await adapter._format_thread_context( + [parent], + thread_ts="100.000", + current_ts="101.000", + team_id="T123", + channel_id="C123", + ) + + assert parent_text == "hey @TestBot look at this" + assert "<@U_BOT>" not in parent_text + + @pytest.mark.asyncio + @pytest.mark.parametrize("strip", [True, False]) + async def test_root_mention_detection_still_sees_the_raw_token(self, strip): + """The wake check greps for ``<@id>`` — the flag must not rewrite it.""" + adapter = thread_adapter(strip) + adapter._thread_context_cache["C123:100.000:T123"] = _ThreadContextCache( + content="", + parent_text="already rendered", + messages=[dict(self._PARENT)], + ) + + text = await adapter._fetch_thread_parent_text( + channel_id="C123", + thread_ts="100.000", + team_id="T123", + strip_bot_mention=False, + ) + + assert "<@U_BOT>" in text + + @pytest.mark.asyncio + @pytest.mark.parametrize("strip", [True, False]) + async def test_root_mention_detection_survives_a_cold_cache(self, strip): + """Same contract on the fetch path — that's the #24848 case. + + The wake check runs during routing, before any thread-context fetch has + primed the cache, so after a restart the detector always lands here. + """ + adapter = thread_adapter(strip) + client = AsyncMock() + client.conversations_replies.return_value = { + "messages": [dict(self._PARENT)] + } + adapter._get_client = lambda *_a, **_kw: client + + text = await adapter._fetch_thread_parent_text( + channel_id="C123", + thread_ts="100.000", + team_id="T123", + strip_bot_mention=False, + ) + + assert "<@U_BOT>" in text + + @pytest.mark.asyncio + @pytest.mark.parametrize("strip", [True, False]) + async def test_parent_wake_check_wakes_the_bot_in_both_states(self, strip): + """End of the chain: a mention in the root still wakes on a plain reply.""" + adapter = thread_adapter(strip) + adapter._thread_context_cache["C123:100.000:T123"] = _ThreadContextCache( + content="", + parent_text="already rendered", + messages=[dict(self._PARENT)], + ) + + assert await adapter._should_wake_on_unmentioned_message( + event_thread_ts="100.000", + channel_id="C123", + user_id="U_USER", + is_thread_reply=True, + team_id="T123", + ) + + +class TestCommandsUnaffected: + """Command parsing runs off its own variable — both flag states dispatch.""" + + @pytest.mark.asyncio + @pytest.mark.parametrize("strip", [True, False]) + async def test_slash_command_behind_a_mention(self, strip): + adapter = delivery_adapter(strip=strip) + + await adapter._handle_slack_message(slack_event("<@U_BOT> /status")) + + event = delivered(adapter) + assert event.text == "/status" + assert event.is_command() + + @pytest.mark.asyncio + @pytest.mark.parametrize("strip", [True, False]) + async def test_bang_command_behind_a_mention(self, strip): + adapter = delivery_adapter(strip=strip) + + await adapter._handle_slack_message(slack_event("<@U_BOT> !new")) + + event = delivered(adapter) + assert event.text == "/new" + assert event.is_command() diff --git a/website/docs/reference/environment-variables.md b/website/docs/reference/environment-variables.md index 8bdeb8b7369f..35f53df79390 100644 --- a/website/docs/reference/environment-variables.md +++ b/website/docs/reference/environment-variables.md @@ -346,6 +346,7 @@ These are set automatically by the Docker terminal backend when `proxy.enabled: | `SLACK_ALLOW_ALL_USERS` | Allow any Slack user to trigger the bot (dev only). | | `SLACK_ALLOW_BOTS` | Accept messages from other Slack bots: `none` (default), `mentions`, or `all`. The bot always ignores its own messages. | | `SLACK_THREAD_REQUIRE_MENTION` | Require an explicit @mention for Slack thread replies while preserving top-level free-response channels | +| `SLACK_STRIP_BOT_MENTIONS` | Delete the bot's own @mention from the text the agent sees (default `true`). `false` keeps it, rendered as `@BotName`, so the agent can tell an explicit tag from a thread-routed wake-up; routing and commands are unaffected. Canonical setting: `slack.strip_bot_mentions` in `config.yaml`. | | `SLACK_HOME_CHANNEL` | Default Slack channel for cron delivery | | `SLACK_HOME_CHANNEL_NAME` | Display name for the Slack home channel | | `GOOGLE_CHAT_PROJECT_ID` | GCP project hosting the Pub/Sub topic (falls back to `GOOGLE_CLOUD_PROJECT`) | diff --git a/website/docs/user-guide/messaging/slack.md b/website/docs/user-guide/messaging/slack.md index 544ed727e914..b7a86bbbea48 100644 --- a/website/docs/user-guide/messaging/slack.md +++ b/website/docs/user-guide/messaging/slack.md @@ -625,6 +625,14 @@ slack: # Env: SLACK_REQUIRE_MENTION_CHANNELS. require_mention_channels: "" + # Delete the bot's own @mention from the text the agent receives. + # Default true (historical behavior): "@bot what's up?" arrives as + # "what's up?". Set false to keep it, rendered as @BotName like any + # other participant's mention, so the agent can tell an explicit tag + # from a thread-routed wake-up. Text only — routing and commands are + # unchanged. Env: SLACK_STRIP_BOT_MENTIONS. + strip_bot_mentions: true + # Custom mention patterns that trigger the bot # (in addition to the default @mention detection) mention_patterns: @@ -666,6 +674,27 @@ The gating options compose — each answers a different question: Rules of thumb: `strict_mention` is the broadest hammer; `thread_require_mention` quiets busy threads without touching top-level gating; `require_mention_channels` re-tightens individual channels on an otherwise free-response bot; `ignore_other_user_mentions` only skips messages explicitly addressed to another person. 1:1 DMs always respond and are unaffected by all of these. +#### Letting the agent see its own mention (`strip_bot_mentions`) + +The options above decide **who wakes the bot**. `strip_bot_mentions` decides something different: whether the agent can see that it was tagged. + +By default the adapter deletes the bot's own mention before the agent reads the message — `@hermes what's up?` arrives as `what's up?`. Because a thread keeps waking the bot after the first mention, every delivered turn then looks the same, and an agent that decides for itself whether to answer has nothing to go on. + +Set `strip_bot_mentions: false` to keep the mention, rendered as `@BotName` in its original position — the same shape mentions of other participants already get: + +```yaml +slack: + strip_bot_mentions: false +``` + +| | `true` (default) | `false` | +|---|---|---| +| `hey @hermes look` | `hey look` | `hey @Hermes look` | +| thread reply with no mention | unchanged | unchanged — the absence *is* the signal | +| thread context / thread parent | mention removed | mention shown, matching the current message | + +What it does **not** change: who wakes the bot (all the gating options above keep their meaning), and command handling — `@hermes /status` and `@hermes !status` still dispatch as `/status` in both states. Env mirror: `SLACK_STRIP_BOT_MENTIONS`; `config.yaml` is the canonical place. + ### Accepting messages from other bots (`allow_bots`) By default Hermes ignores every message authored by another Slack bot or app (including Workflow Builder posts). For multi-agent workspaces — several Hermes instances or peer bots collaborating in one channel — opt in with `allow_bots`: