diff --git a/hermes_cli/config.py b/hermes_cli/config.py index de2d9effd207..1dab57c413e1 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -2520,6 +2520,10 @@ DEFAULT_CONFIG = { "require_mention": True, # Require @mention to respond in channels "free_response_channels": "", # Comma-separated channel IDs where bot responds without mention "allowed_channels": "", # If set, bot ONLY responds in these channel IDs (whitelist) + # Ignore a channel/thread message addressed to another user (first token + # @mentions someone other than the bot) unless the bot is also mentioned. + # Opt-in; default off keeps existing behaviour. Env: SLACK_IGNORE_OTHER_USER_MENTIONS. + "ignore_other_user_mentions": False, "channel_prompts": {}, # Per-channel ephemeral system prompts }, diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index 4d7b0c434006..5946ff180f39 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -4392,6 +4392,9 @@ class SlackAdapter(BasePlatformAdapter): thread_ts = None # In channels, respond if: + # (unless ignore_other_user_mentions is on and the message opens by + # @mentioning another user without also mentioning the bot — then + # stay silent regardless of the rules below) # 0. Channel is in free_response_channels, OR require_mention is # disabled — always process regardless of mention. # 1. The bot is @mentioned in this message, OR @@ -4442,6 +4445,22 @@ class SlackAdapter(BasePlatformAdapter): ) return + # A message that opens by @mentioning another user is directed at + # that person. Stay silent unless we are also mentioned — this + # overrides free-response and mentioned-thread auto-follow so the + # bot does not butt in on chatter aimed at someone else. + self_uids = {u for u in (bot_uid, self._bot_user_id) if u} + if ( + self._slack_ignore_other_user_mentions() + and not is_mentioned + and self._slack_message_addressed_to_other_user(routing_text, self_uids) + ): + logger.debug( + "[Slack] Ignoring message addressed to another user in channel %s", + channel_id, + ) + return + if channel_id in self._slack_free_response_channels(): pass # Free-response channel — always process elif not self._slack_require_mention(): @@ -6708,6 +6727,45 @@ class SlackAdapter(BasePlatformAdapter): "on", } + def _slack_ignore_other_user_mentions(self) -> bool: + """When true, ignore channel/thread messages addressed to another user. + + A message whose first token @-mentions someone other than this bot is + treated as directed at that person; the bot stays silent unless it is + also mentioned. Defaults to False (opt-in) so existing behaviour is + unchanged until enabled. Mirrors Discord's ``ignore_other_user_mentions`` + (PR #33501), adapted to Slack's thread model: the trigger is a *leading* + mention ("addressed to"), so a message that merely references another + user mid-sentence still reaches the bot. + """ + configured = self.config.extra.get("ignore_other_user_mentions") + if configured is not None: + if isinstance(configured, str): + return configured.lower() in {"true", "1", "yes", "on"} + return bool(configured) + return os.getenv("SLACK_IGNORE_OTHER_USER_MENTIONS", "false").lower() in { + "true", + "1", + "yes", + "on", + } + + 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. + + Slack renders a user mention as ``<@U123>`` (or ``<@U123|name>``). A + message whose first token is such a mention is addressed to that user. + Returns False when the leading mention is the bot itself (``self_uids``), + when there is no leading user mention, or for channel/broadcast tokens + (````, ``<#C…>``) which address the room rather than a person. + """ + if not text: + return False + match = re.match(r"\s*<@([^>|\s]+)(?:\|[^>]*)?>", text) + if not match: + return False + return match.group(1) not in self_uids + def _slack_free_response_channels(self) -> set: """Return channel IDs where no @mention is required.""" raw = self.config.extra.get("free_response_channels") @@ -7151,6 +7209,10 @@ def _apply_yaml_config(yaml_cfg: dict, slack_cfg: dict) -> dict | 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 "ignore_other_user_mentions" in slack_cfg and not os.getenv("SLACK_IGNORE_OTHER_USER_MENTIONS"): + os.environ["SLACK_IGNORE_OTHER_USER_MENTIONS"] = str( + slack_cfg["ignore_other_user_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") @@ -7200,9 +7262,9 @@ def register(ctx) -> None: # and the static _PLATFORMS["slack"] dict in hermes_cli/gateway.py. setup_fn=interactive_setup, # YAML→env config bridge — owns the translation of config.yaml slack: - # keys (require_mention, strict_mention, allow_bots, - # free_response_channels, reactions, allowed_channels) into SLACK_* - # env vars that the adapter reads via os.getenv(). Replaces the + # keys (require_mention, strict_mention, ignore_other_user_mentions, + # allow_bots, free_response_channels, reactions, allowed_channels) into + # SLACK_* env vars that the adapter reads via os.getenv(). Replaces the # hardcoded block in gateway/config.py. Hook contract: #24849. apply_yaml_config_fn=_apply_yaml_config, # Auth env vars for _is_user_authorized() integration diff --git a/tests/gateway/test_slack_ignore_other_user_mentions.py b/tests/gateway/test_slack_ignore_other_user_mentions.py new file mode 100644 index 000000000000..d103c0bc3955 --- /dev/null +++ b/tests/gateway/test_slack_ignore_other_user_mentions.py @@ -0,0 +1,320 @@ +"""Tests for the Slack ``ignore_other_user_mentions`` option. + +When enabled, the bot stays silent on a channel/thread message that opens by +@-mentioning someone other than itself (a message *addressed to* another user), +unless the bot is also mentioned. This is Slack parity for the Discord option +of the same name (PR #33501), adapted to Slack's thread model: the trigger is a +*leading* mention, so a message that merely references another user mid-sentence +(e.g. "loop in @rasha") still reaches the bot. + +Helper-level tests exercise the real config/parse methods directly; the +integration tests drive the real ``SlackAdapter._handle_slack_message`` so the +gate is verified end-to-end rather than against a re-implementation. +""" + +import sys +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from gateway.config import Platform, PlatformConfig + + +# --------------------------------------------------------------------------- +# Mock slack-bolt if not installed (same pattern as test_slack_mention.py) +# --------------------------------------------------------------------------- + +def _ensure_slack_mock(): + if "slack_bolt" in sys.modules and hasattr(sys.modules["slack_bolt"], "__file__"): + return + + slack_bolt = MagicMock() + slack_bolt.async_app.AsyncApp = MagicMock + slack_bolt.adapter.socket_mode.async_handler.AsyncSocketModeHandler = MagicMock + + slack_sdk = MagicMock() + slack_sdk.web.async_client.AsyncWebClient = MagicMock + + for name, mod in [ + ("slack_bolt", slack_bolt), + ("slack_bolt.async_app", slack_bolt.async_app), + ("slack_bolt.adapter", slack_bolt.adapter), + ("slack_bolt.adapter.socket_mode", slack_bolt.adapter.socket_mode), + ("slack_bolt.adapter.socket_mode.async_handler", slack_bolt.adapter.socket_mode.async_handler), + ("slack_sdk", slack_sdk), + ("slack_sdk.web", slack_sdk.web), + ("slack_sdk.web.async_client", slack_sdk.web.async_client), + ]: + sys.modules.setdefault(name, mod) + + +_ensure_slack_mock() + +import plugins.platforms.slack.adapter as _slack_mod # noqa: E402 + +_slack_mod.SLACK_AVAILABLE = True + +from plugins.platforms.slack.adapter import SlackAdapter # noqa: E402 + + +BOT_USER_ID = "U_BOT_123" +OTHER_USER_ID = "U_OTHER_456" +CHANNEL_ID = "C0AQWDLHY9M" + + +def _make_adapter(**extra): + adapter = object.__new__(SlackAdapter) + adapter.platform = Platform.SLACK + adapter.config = PlatformConfig(enabled=True, extra=dict(extra)) + adapter._bot_user_id = BOT_USER_ID + adapter._team_bot_user_ids = {} + return adapter + + +# --------------------------------------------------------------------------- +# _slack_ignore_other_user_mentions() +# --------------------------------------------------------------------------- + +def test_ignore_other_user_mentions_defaults_off(monkeypatch): + monkeypatch.delenv("SLACK_IGNORE_OTHER_USER_MENTIONS", raising=False) + adapter = _make_adapter() + assert adapter._slack_ignore_other_user_mentions() is False + + +def test_ignore_other_user_mentions_extra_true(): + adapter = _make_adapter(ignore_other_user_mentions=True) + assert adapter._slack_ignore_other_user_mentions() is True + + +def test_ignore_other_user_mentions_extra_false(): + adapter = _make_adapter(ignore_other_user_mentions=False) + assert adapter._slack_ignore_other_user_mentions() is False + + +def test_ignore_other_user_mentions_extra_string_forms(): + assert _make_adapter(ignore_other_user_mentions="on")._slack_ignore_other_user_mentions() is True + assert _make_adapter(ignore_other_user_mentions="true")._slack_ignore_other_user_mentions() is True + assert _make_adapter(ignore_other_user_mentions="off")._slack_ignore_other_user_mentions() is False + + +def test_ignore_other_user_mentions_env_fallback(monkeypatch): + monkeypatch.setenv("SLACK_IGNORE_OTHER_USER_MENTIONS", "true") + adapter = _make_adapter() + assert adapter._slack_ignore_other_user_mentions() is True + + +def test_ignore_other_user_mentions_extra_overrides_env(monkeypatch): + monkeypatch.setenv("SLACK_IGNORE_OTHER_USER_MENTIONS", "true") + adapter = _make_adapter(ignore_other_user_mentions=False) + assert adapter._slack_ignore_other_user_mentions() is False + + +# --------------------------------------------------------------------------- +# _slack_message_addressed_to_other_user() +# --------------------------------------------------------------------------- + +SELF_UIDS = {BOT_USER_ID} + + +def _addressed(text): + return _make_adapter()._slack_message_addressed_to_other_user(text, SELF_UIDS) + + +def test_addressed_leading_other_user_mention(): + assert _addressed(f"<@{OTHER_USER_ID}> check this out") is True + + +def test_addressed_leading_bot_mention_is_not_other(): + assert _addressed(f"<@{BOT_USER_ID}> hello") is False + + +def test_addressed_pipe_form_other_user(): + assert _addressed(f"<@{OTHER_USER_ID}|rasha> check this out") is True + + +def test_addressed_pipe_form_bot_is_not_other(): + assert _addressed(f"<@{BOT_USER_ID}|hermes> hello") is False + + +def test_addressed_no_mention(): + assert _addressed("hello there, thanks for that") is False + + +def test_addressed_mid_text_mention_not_leading(): + assert _addressed(f"can you loop in <@{OTHER_USER_ID}> on this?") is False + + +def test_addressed_leading_whitespace_then_other_mention(): + assert _addressed(f" <@{OTHER_USER_ID}> take a look") is True + + +def test_addressed_empty_and_blank(): + assert _addressed("") is False + assert _addressed(" ") is False + + +def test_addressed_channel_and_broadcast_tokens_are_not_users(): + # , , and channel refs address the room, not a person. + assert _addressed(" standup in 5") is False + assert _addressed("<#C0000000000|general> see here") is False + + +# --------------------------------------------------------------------------- +# Integration: real _handle_slack_message +# --------------------------------------------------------------------------- + +@pytest.fixture +def adapter(): + config = PlatformConfig(enabled=True, token="xoxb-fake-token") + a = SlackAdapter(config) + a._app = MagicMock() + a._app.client = AsyncMock() + a._bot_user_id = BOT_USER_ID + a._running = True + a.handle_message = AsyncMock() + return a + + +@pytest.fixture(autouse=True) +def _redirect_cache(tmp_path, monkeypatch): + monkeypatch.setattr( + "gateway.platforms.base.DOCUMENT_CACHE_DIR", tmp_path / "doc_cache" + ) + # Keep gating driven by config.extra, not ambient env. + monkeypatch.delenv("SLACK_IGNORE_OTHER_USER_MENTIONS", raising=False) + monkeypatch.delenv("SLACK_FREE_RESPONSE_CHANNELS", raising=False) + monkeypatch.delenv("SLACK_REQUIRE_MENTION", raising=False) + + +def _event(text, ts, thread_ts=None): + event = { + "channel": CHANNEL_ID, + "channel_type": "channel", + "user": "U_HUMAN", + "text": text, + "ts": ts, + } + if thread_ts is not None: + event["thread_ts"] = thread_ts + return event + + +async def _run(adapter, event): + with patch.object( + adapter, "_resolve_user_name", new=AsyncMock(return_value="human") + ), patch.object( + adapter, "_fetch_thread_context", new=AsyncMock(return_value=None) + ), patch.object( + adapter, "_fetch_thread_parent_text", new=AsyncMock(return_value="") + ), patch.object( + adapter, "_has_active_session_for_thread", return_value=False + ): + await adapter._handle_slack_message(event) + + +@pytest.mark.asyncio +async def test_free_response_ignores_message_addressed_to_other_user(adapter): + adapter.config.extra["free_response_channels"] = CHANNEL_ID + adapter.config.extra["ignore_other_user_mentions"] = True + + await _run(adapter, _event(f"<@{OTHER_USER_ID}> this is for you", ts="1700000000.000001")) + + adapter.handle_message.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_free_response_can_opt_out_of_ignoring_other_user_mentions(adapter): + adapter.config.extra["free_response_channels"] = CHANNEL_ID + adapter.config.extra["ignore_other_user_mentions"] = False + + await _run(adapter, _event(f"<@{OTHER_USER_ID}> still ambient chatter", ts="1700000000.000002")) + + adapter.handle_message.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_free_response_replies_when_bot_also_mentioned(adapter): + adapter.config.extra["free_response_channels"] = CHANNEL_ID + adapter.config.extra["ignore_other_user_mentions"] = True + + await _run( + adapter, + _event(f"<@{OTHER_USER_ID}> and <@{BOT_USER_ID}> please compare", ts="1700000000.000003"), + ) + + adapter.handle_message.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_mentioned_thread_ignores_followup_addressed_to_other_user(adapter): + """Ben's case: once the bot has been mentioned in a thread it auto-follows, + but a follow-up addressed to another human should not wake it.""" + thread_ts = "1700000000.000010" + adapter._mentioned_threads.add(thread_ts) + adapter.config.extra["ignore_other_user_mentions"] = True + + await _run( + adapter, + _event(f"<@{OTHER_USER_ID}> check this out", ts="1700000000.000011", thread_ts=thread_ts), + ) + + adapter.handle_message.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_mentioned_thread_still_answers_plain_followup(adapter): + """No over-suppression: a plain follow-up (no leading mention) in a + mentioned thread is still answered when the option is on.""" + thread_ts = "1700000000.000020" + adapter._mentioned_threads.add(thread_ts) + adapter.config.extra["ignore_other_user_mentions"] = True + + await _run( + adapter, + _event("thanks, that makes sense", ts="1700000000.000021", thread_ts=thread_ts), + ) + + adapter.handle_message.assert_awaited_once() + + +# --------------------------------------------------------------------------- +# Config bridge: config.yaml slack.ignore_other_user_mentions → extra + env +# --------------------------------------------------------------------------- + +def test_config_bridges_slack_ignore_other_user_mentions(monkeypatch, tmp_path): + """config.yaml ``slack.ignore_other_user_mentions`` reaches the runtime env + var the adapter reads (same wiring as ``strict_mention`` — via the plugin's + apply_yaml_config_fn bridge, not the generic shared-key allowlist).""" + from gateway.config import load_gateway_config + + hermes_home = tmp_path / ".hermes" + hermes_home.mkdir() + (hermes_home / "config.yaml").write_text( + "slack:\n ignore_other_user_mentions: true\n", + encoding="utf-8", + ) + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.delenv("SLACK_IGNORE_OTHER_USER_MENTIONS", raising=False) + + load_gateway_config() + + import os as _os + assert _os.environ["SLACK_IGNORE_OTHER_USER_MENTIONS"] == "true" + + +def test_ignore_other_user_mentions_env_wins_over_yaml(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 ignore_other_user_mentions: true\n", + encoding="utf-8", + ) + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.setenv("SLACK_IGNORE_OTHER_USER_MENTIONS", "false") + + load_gateway_config() + + import os as _os + assert _os.environ["SLACK_IGNORE_OTHER_USER_MENTIONS"] == "false"