diff --git a/gateway/config.py b/gateway/config.py index ae149f81dc0b..38f1ef552f12 100644 --- a/gateway/config.py +++ b/gateway/config.py @@ -943,6 +943,8 @@ def _merge_platform_map(source_platforms: Any) -> None: bridged["reply_in_thread"] = platform_cfg["reply_in_thread"] if "require_mention" in platform_cfg: bridged["require_mention"] = platform_cfg["require_mention"] + if plat == Platform.SLACK and "strip_bot_mentions" in platform_cfg: + bridged["strip_bot_mentions"] = platform_cfg["strip_bot_mentions"] if plat == Platform.TELEGRAM and "allowed_chats" in platform_cfg: bridged["allowed_chats"] = platform_cfg["allowed_chats"] if plat == Platform.TELEGRAM and "group_allowed_chats" in platform_cfg: @@ -1037,6 +1039,8 @@ def _merge_platform_map(source_platforms: Any) -> None: os.environ["SLACK_REQUIRE_MENTION"] = str(slack_cfg["require_mention"]).lower() if "strict_mention" in slack_cfg and not os.getenv("SLACK_STRICT_MENTION"): os.environ["SLACK_STRICT_MENTION"] = str(slack_cfg["strict_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/gateway/platforms/slack.py b/gateway/platforms/slack.py index ad1de2a25a1a..9125b071f666 100644 --- a/gateway/platforms/slack.py +++ b/gateway/platforms/slack.py @@ -2522,9 +2522,10 @@ async def _handle_slack_message(self, event: dict) -> None: ): return - if is_mentioned: + if is_mentioned and self._slack_strip_bot_mentions(): # Strip the bot mention from the text text = text.replace(f"<@{bot_uid}>", "").strip() + if is_mentioned: # Register this thread so all future messages auto-trigger the bot. # Skipped in strict mode: strict_mention=true bots must be # re-mentioned every turn, so remembering the thread would @@ -3410,7 +3411,7 @@ async def _fetch_thread_context( continue # Strip bot mentions from context messages - if bot_uid: + if bot_uid and self._slack_strip_bot_mentions(): msg_text = msg_text.replace(f"<@{bot_uid}>", "").strip() prefix = "[thread parent] " if is_parent else "" @@ -3480,7 +3481,7 @@ 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() - if bot_uid: + if bot_uid and self._slack_strip_bot_mentions(): text = text.replace(f"<@{bot_uid}>", "").strip() return text except Exception as exc: # pragma: no cover - defensive @@ -3780,6 +3781,24 @@ def _slack_strict_mention(self) -> bool: "on", } + def _slack_strip_bot_mentions(self) -> bool: + """Return whether Slack bot mention tokens are removed from prompt text. + + Defaults to True to preserve existing behavior. Explicit false values + keep the raw ``<@U...>`` tokens in current and fetched thread text. + """ + 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_free_response_channels(self) -> set: """Return channel IDs where no @mention is required.""" raw = self.config.extra.get("free_response_channels") diff --git a/tests/conftest.py b/tests/conftest.py index 2da7d4a1eb4f..cc2ca7190e06 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -312,6 +312,7 @@ def _looks_like_credential(name: str) -> bool: # Force-clear on every test setup so the leak can't happen. "SLACK_REQUIRE_MENTION", "SLACK_STRICT_MENTION", + "SLACK_STRIP_BOT_MENTIONS", "SLACK_FREE_RESPONSE_CHANNELS", "SLACK_ALLOW_BOTS", "SLACK_REACTIONS", diff --git a/tests/gateway/test_slack.py b/tests/gateway/test_slack.py index 5f8a3b62348e..444a5b18e2d3 100644 --- a/tests/gateway/test_slack.py +++ b/tests/gateway/test_slack.py @@ -1801,6 +1801,21 @@ async def test_channel_mention_strips_bot_id(self, adapter): assert msg_event.text == "what's the weather?" assert "<@U_BOT>" not in msg_event.text + @pytest.mark.asyncio + async def test_channel_mention_preserves_bot_id_when_config_false(self, adapter): + """strip_bot_mentions=false keeps the mention token in MessageEvent text.""" + adapter.config.extra["strip_bot_mentions"] = False + event = { + "text": "<@U_BOT> what's the weather?", + "user": "U_USER", + "channel": "C123", + "channel_type": "channel", + "ts": "1234567890.000001", + } + await adapter._handle_slack_message(event) + msg_event = adapter.handle_message.call_args[0][0] + assert msg_event.text == "<@U_BOT> what's the weather?" + @pytest.mark.asyncio async def test_bot_messages_ignored(self, adapter): """Messages from bots should be ignored.""" @@ -3474,7 +3489,7 @@ async def test_slack_reply_to_text_set_on_thread_reply(self, adapter): { "ts": "1000.0", "bot_id": "B_CRON", - "text": "メール要約: 新着メール3件あります", + "text": "<@U_BOT> メール要約: 新着メール3件あります", }, {"ts": "1000.5", "user": "U_USER", "text": "詳細を教えて"}, ] @@ -3505,6 +3520,45 @@ async def test_slack_reply_to_text_set_on_thread_reply(self, adapter): # gateway can inject it when not already in the session history. assert msg_event.reply_to_text is not None assert "メール要約" in msg_event.reply_to_text + assert "<@U_BOT>" not in msg_event.reply_to_text + + @pytest.mark.asyncio + async def test_slack_reply_to_text_preserves_bot_mention_when_config_false( + self, adapter + ): + adapter.config.extra["strip_bot_mentions"] = False + adapter._channel_team = {} + adapter._team_bot_user_ids = {} + + adapter._app.client.conversations_replies = AsyncMock( + return_value={ + "messages": [ + { + "ts": "1000.0", + "bot_id": "B_CRON", + "text": "<@U_BOT> Parent summary", + }, + {"ts": "1000.5", "user": "U_USER", "text": "details"}, + ] + } + ) + + event = { + "text": "details", + "user": "U_USER", + "channel": "D123", + "channel_type": "im", + "ts": "1000.5", + "thread_ts": "1000.0", + } + + with patch.object( + adapter, "_resolve_user_name", new=AsyncMock(return_value="Alice") + ): + await adapter._handle_slack_message(event) + + msg_event = adapter.handle_message.call_args[0][0] + assert msg_event.reply_to_text == "<@U_BOT> Parent summary" @pytest.mark.asyncio async def test_slack_reply_to_text_none_for_top_level_message(self, adapter): diff --git a/tests/gateway/test_slack_approval_buttons.py b/tests/gateway/test_slack_approval_buttons.py index e09b3406c6df..9041f6d54065 100644 --- a/tests/gateway/test_slack_approval_buttons.py +++ b/tests/gateway/test_slack_approval_buttons.py @@ -361,7 +361,7 @@ async def test_fetches_and_formats_context(self): mock_client.conversations_replies = AsyncMock(return_value={ "messages": [ {"ts": "1000.0", "user": "U1", "text": "This is the parent message"}, - {"ts": "1000.1", "user": "U2", "text": "I think we should refactor"}, + {"ts": "1000.1", "user": "U2", "text": "<@U_BOT> I think we should refactor"}, {"ts": "1000.2", "user": "U1", "text": "Good idea, <@U_BOT> what do you think?"}, ] }) @@ -384,6 +384,31 @@ async def test_fetches_and_formats_context(self): # Bot mention should be stripped from context assert "<@U_BOT>" not in context + @pytest.mark.asyncio + async def test_fetch_thread_context_preserves_bot_mentions_when_config_false(self): + adapter = _make_adapter() + adapter.config.extra["strip_bot_mentions"] = False + mock_client = adapter._team_clients["T1"] + mock_client.conversations_replies = AsyncMock(return_value={ + "messages": [ + {"ts": "1000.0", "user": "U1", "text": "<@U_BOT> parent"}, + {"ts": "1000.1", "user": "U2", "text": "prior <@U_BOT> reply"}, + {"ts": "1000.2", "user": "U1", "text": "current <@U_BOT>"}, + ] + }) + adapter._user_name_cache = {"U1": "Alice", "U2": "Bob"} + + context = await adapter._fetch_thread_context( + channel_id="C1", + thread_ts="1000.0", + current_ts="1000.2", + team_id="T1", + ) + + assert "[thread parent] Alice: <@U_BOT> parent" in context + assert "Bob: prior <@U_BOT> reply" in context + assert "current <@U_BOT>" not in context + @pytest.mark.asyncio async def test_skips_bot_messages(self): """Self-bot child replies are skipped to avoid circular context, @@ -607,6 +632,39 @@ async def test_fetch_thread_parent_text_from_cache(self): # No additional API call assert mock_client.conversations_replies.await_count == 1 + @pytest.mark.asyncio + async def test_fetch_thread_parent_text_strips_bot_mention_by_default(self): + adapter = _make_adapter() + mock_client = adapter._team_clients["T1"] + mock_client.conversations_replies = AsyncMock(return_value={ + "messages": [ + {"ts": "1000.0", "user": "U1", "text": "<@U_BOT> Parent summary"}, + ] + }) + + parent = await adapter._fetch_thread_parent_text( + channel_id="C1", thread_ts="1000.0", team_id="T1" + ) + + assert parent == "Parent summary" + + @pytest.mark.asyncio + async def test_fetch_thread_parent_text_preserves_bot_mention_when_config_false(self): + adapter = _make_adapter() + adapter.config.extra["strip_bot_mentions"] = False + mock_client = adapter._team_clients["T1"] + mock_client.conversations_replies = AsyncMock(return_value={ + "messages": [ + {"ts": "1000.0", "user": "U1", "text": "<@U_BOT> Parent summary"}, + ] + }) + + parent = await adapter._fetch_thread_parent_text( + channel_id="C1", thread_ts="1000.0", team_id="T1" + ) + + assert parent == "<@U_BOT> Parent summary" + # =========================================================================== # _has_active_session_for_thread — session key fix (#5833) diff --git a/tests/gateway/test_slack_mention.py b/tests/gateway/test_slack_mention.py index 23aa2f15454d..e73760566e91 100644 --- a/tests/gateway/test_slack_mention.py +++ b/tests/gateway/test_slack_mention.py @@ -55,7 +55,13 @@ def _ensure_slack_mock(): OTHER_CHANNEL_ID = "C9999999999" -def _make_adapter(require_mention=None, strict_mention=None, free_response_channels=None, allowed_channels=None): +def _make_adapter( + require_mention=None, + strict_mention=None, + free_response_channels=None, + allowed_channels=None, + strip_bot_mentions=None, +): extra = {} if require_mention is not None: extra["require_mention"] = require_mention @@ -65,6 +71,8 @@ def _make_adapter(require_mention=None, strict_mention=None, free_response_chann extra["free_response_channels"] = free_response_channels if allowed_channels is not None: extra["allowed_channels"] = allowed_channels + if strip_bot_mentions is not None: + extra["strip_bot_mentions"] = strip_bot_mentions adapter = object.__new__(SlackAdapter) adapter.platform = Platform.SLACK @@ -180,6 +188,32 @@ def test_strict_mention_env_var_fallback(monkeypatch): assert adapter._slack_strict_mention() is True +# --------------------------------------------------------------------------- +# Tests: _slack_strip_bot_mentions +# --------------------------------------------------------------------------- + +def test_strip_bot_mentions_defaults_to_true(monkeypatch): + monkeypatch.delenv("SLACK_STRIP_BOT_MENTIONS", raising=False) + adapter = _make_adapter() + assert adapter._slack_strip_bot_mentions() is True + + +def test_strip_bot_mentions_false(): + adapter = _make_adapter(strip_bot_mentions=False) + assert adapter._slack_strip_bot_mentions() is False + + +def test_strip_bot_mentions_string_off(): + adapter = _make_adapter(strip_bot_mentions="off") + assert adapter._slack_strip_bot_mentions() is False + + +def test_strip_bot_mentions_env_var_fallback(monkeypatch): + monkeypatch.setenv("SLACK_STRIP_BOT_MENTIONS", "false") + adapter = _make_adapter() + assert adapter._slack_strip_bot_mentions() is False + + # --------------------------------------------------------------------------- # Tests: _slack_free_response_channels # --------------------------------------------------------------------------- @@ -516,6 +550,28 @@ def test_config_bridges_slack_strict_mention(monkeypatch, tmp_path): assert _os.environ["SLACK_STRICT_MENTION"] == "true" +def test_config_bridges_slack_strip_bot_mentions(monkeypatch, tmp_path): + from gateway.config import load_gateway_config + + hermes_home = tmp_path / ".hermes" + hermes_home.mkdir() + (hermes_home / "config.yaml").write_text( + "slack:\n" + " strip_bot_mentions: false\n", + encoding="utf-8", + ) + + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.delenv("SLACK_STRIP_BOT_MENTIONS", raising=False) + + config = load_gateway_config() + + assert config is not None + assert config.platforms[Platform.SLACK].extra.get("strip_bot_mentions") is False + import os as _os + assert _os.environ["SLACK_STRIP_BOT_MENTIONS"] == "false" + + # --------------------------------------------------------------------------- # Regression: strict mode must NOT persist mentions into _mentioned_threads # ---------------------------------------------------------------------------