From 7a0cfebf74b57bc80438b70c32c655ccc91f2ed0 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 22 Jul 2026 08:30:42 -0700 Subject: [PATCH] feat(slack): require_mention_channels per-channel force-mention override MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Port of the Slack half of #13855 (by @kshitijk4poor), reimplemented against the plugin adapter (the original PR targets the deleted gateway/platforms/slack.py and six other legacy adapters). Channels listed in slack.require_mention_channels (config.yaml) or SLACK_REQUIRE_MENTION_CHANNELS ALWAYS require an explicit @mention, even when require_mention is false globally or the channel is in free_response_channels — the opposite direction of free_response_channels. Instead of duplicating the PR's inline reply-to-bot-thread/mentioned-thread/ session checks, the forced channel falls through to the SAME decision chain as normal mention gating, so all five wake checks in _should_wake_on_unmentioned_message keep applying (single decision path). Credit: adapted from #13855 by @kshitijk4poor (Slack half only; the other platform halves target deleted legacy adapters and are out of scope for this cluster). --- hermes_cli/config.py | 3 + plugins/platforms/slack/adapter.py | 35 ++- .../test_slack_require_mention_channels.py | 209 ++++++++++++++++++ 3 files changed, 244 insertions(+), 3 deletions(-) create mode 100644 tests/gateway/test_slack_require_mention_channels.py diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 8737e58e77a1..caeda3341586 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -2570,6 +2570,9 @@ 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) + # Channel IDs where @mention is ALWAYS required, even when + # require_mention is false globally (per-channel force-mention override). + "require_mention_channels": "", # 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. diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index 2bba13107549..3b85ad311f05 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -4215,11 +4215,16 @@ class SlackAdapter(BasePlatformAdapter): return if ( - channel_id in self._slack_free_response_channels() - or not self._slack_require_mention() + channel_id not in self._slack_require_mention_channels() + and ( + channel_id in self._slack_free_response_channels() + or not self._slack_require_mention() + ) ): # Free-response channel, or mention requirement disabled - # globally. thread_require_mention still gates thread + # globally — unless the channel is force-mention-gated via + # require_mention_channels, which overrides both. + # thread_require_mention still gates thread # replies: top-level messages stay free-response, but a bot # must be re-mentioned to join thread follow-ups. if ( @@ -6480,6 +6485,25 @@ class SlackAdapter(BasePlatformAdapter): return {part.strip() for part in raw.split(",") if part.strip()} return set() + def _slack_require_mention_channels(self) -> set: + """Return channel IDs where a bot @mention is ALWAYS required. + + Per-channel override in the opposite direction of + ``free_response_channels``: even when ``require_mention`` is disabled + globally (or the channel would otherwise be free-response), messages + in these channels only reach the bot via an explicit mention or one + of the wake checks in :meth:`_should_wake_on_unmentioned_message`. + Empty set means no per-channel force-mention override (#13855). + """ + raw = self.config.extra.get("require_mention_channels") + if raw is None: + raw = os.getenv("SLACK_REQUIRE_MENTION_CHANNELS", "") + if isinstance(raw, list): + return {str(part).strip() for part in raw if str(part).strip()} + if isinstance(raw, str) and raw.strip(): + return {part.strip() for part in raw.split(",") if part.strip()} + return set() + def _slack_mention_patterns(self) -> List["re.Pattern"]: """Compile optional regex wake-word patterns for channel triggers. @@ -6784,6 +6808,11 @@ def _apply_yaml_config(yaml_cfg: dict, slack_cfg: dict) -> dict | None: if isinstance(frc, list): frc = ",".join(str(v) for v in frc) os.environ["SLACK_FREE_RESPONSE_CHANNELS"] = str(frc) + rmc = slack_cfg.get("require_mention_channels") + if rmc is not None and not os.getenv("SLACK_REQUIRE_MENTION_CHANNELS"): + if isinstance(rmc, list): + rmc = ",".join(str(v) for v in rmc) + os.environ["SLACK_REQUIRE_MENTION_CHANNELS"] = str(rmc) if "reactions" in slack_cfg and not os.getenv("SLACK_REACTIONS"): os.environ["SLACK_REACTIONS"] = str(slack_cfg["reactions"]).lower() ac = slack_cfg.get("allowed_channels") diff --git a/tests/gateway/test_slack_require_mention_channels.py b/tests/gateway/test_slack_require_mention_channels.py new file mode 100644 index 000000000000..5597c1c6b40b --- /dev/null +++ b/tests/gateway/test_slack_require_mention_channels.py @@ -0,0 +1,209 @@ +"""Tests for the Slack ``require_mention_channels`` per-channel override. + +Channels listed here ALWAYS require an explicit bot @mention, even when +``require_mention`` is disabled globally or the channel would otherwise be +free-response — the opposite direction of ``free_response_channels`` (#13855). +Wake checks (bot-authored thread, previously mentioned thread, active session) +still apply, so ongoing conversations are not cut off. +""" + +import sys +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from gateway.config import PlatformConfig + + +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, _apply_yaml_config # noqa: E402 + +BOT_USER_ID = "U_BOT" +CHANNEL_ID = "C_FORCED" + + +@pytest.fixture(autouse=True) +def _clean_env(monkeypatch, tmp_path): + monkeypatch.setattr( + "gateway.platforms.base.DOCUMENT_CACHE_DIR", tmp_path / "doc_cache" + ) + for var in ( + "SLACK_REQUIRE_MENTION", + "SLACK_REQUIRE_MENTION_CHANNELS", + "SLACK_FREE_RESPONSE_CHANNELS", + "SLACK_STRICT_MENTION", + "SLACK_THREAD_REQUIRE_MENTION", + ): + monkeypatch.delenv(var, raising=False) + + +@pytest.fixture() +def adapter(): + config = PlatformConfig(enabled=True, token="xoxb-fake-token") + a = SlackAdapter(config) + a._app = MagicMock() + a._app.client = AsyncMock() + a._app.client.users_info = AsyncMock( + return_value={ + "user": { + "is_bot": False, + "profile": {"display_name": "Test User"}, + "real_name": "Test User", + } + } + ) + a._bot_user_id = BOT_USER_ID + a._running = True + a.handle_message = AsyncMock() + a._fetch_thread_context = AsyncMock(return_value="") + a._fetch_thread_parent_text = AsyncMock(return_value="") + a._has_active_session_for_thread = MagicMock(return_value=False) + return a + + +def _event(text, ts="100.000", thread_ts=None, channel=CHANNEL_ID): + event = { + "type": "message", + "channel": channel, + "channel_type": "channel", + "user": "U_HUMAN", + "text": text, + "ts": ts, + } + if thread_ts is not None: + event["thread_ts"] = thread_ts + return event + + +# --------------------------------------------------------------------------- +# _slack_require_mention_channels() parsing +# --------------------------------------------------------------------------- + + +def _make(extra=None): + a = object.__new__(SlackAdapter) + a.config = PlatformConfig(enabled=True, extra=dict(extra or {})) + return a + + +def test_require_mention_channels_default_empty(): + assert _make()._slack_require_mention_channels() == set() + + +def test_require_mention_channels_csv_and_list(): + assert _make({"require_mention_channels": "C1, C2"})._slack_require_mention_channels() == { + "C1", + "C2", + } + assert _make({"require_mention_channels": ["C1", "C2"]})._slack_require_mention_channels() == { + "C1", + "C2", + } + + +def test_require_mention_channels_env_fallback(monkeypatch): + monkeypatch.setenv("SLACK_REQUIRE_MENTION_CHANNELS", "C9") + assert _make()._slack_require_mention_channels() == {"C9"} + + +def test_yaml_bridge_sets_env(monkeypatch): + monkeypatch.delenv("SLACK_REQUIRE_MENTION_CHANNELS", raising=False) + _apply_yaml_config({}, {"require_mention_channels": ["C1", "C2"]}) + import os + + assert os.environ["SLACK_REQUIRE_MENTION_CHANNELS"] == "C1,C2" + monkeypatch.delenv("SLACK_REQUIRE_MENTION_CHANNELS", raising=False) + + +# --------------------------------------------------------------------------- +# Routing behaviour +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_forced_channel_requires_mention_even_when_global_off(adapter): + adapter.config.extra["require_mention"] = False + adapter.config.extra["require_mention_channels"] = CHANNEL_ID + + await adapter._handle_slack_message(_event("ambient chatter")) + + adapter.handle_message.assert_not_called() + + +@pytest.mark.asyncio +async def test_forced_channel_overrides_free_response(adapter): + adapter.config.extra["free_response_channels"] = CHANNEL_ID + adapter.config.extra["require_mention_channels"] = CHANNEL_ID + + await adapter._handle_slack_message(_event("still ambient chatter")) + + adapter.handle_message.assert_not_called() + + +@pytest.mark.asyncio +async def test_forced_channel_mention_routes(adapter): + adapter.config.extra["require_mention"] = False + adapter.config.extra["require_mention_channels"] = CHANNEL_ID + + await adapter._handle_slack_message(_event(f"<@{BOT_USER_ID}> hello")) + + adapter.handle_message.assert_called_once() + assert adapter.handle_message.call_args[0][0].text == "hello" + + +@pytest.mark.asyncio +async def test_forced_channel_wake_checks_still_apply(adapter): + """A previously mentioned thread still auto-follows in a forced channel.""" + adapter.config.extra["require_mention"] = False + adapter.config.extra["require_mention_channels"] = CHANNEL_ID + adapter._mentioned_threads.add("100.000") + + await adapter._handle_slack_message( + _event("follow-up", ts="101.000", thread_ts="100.000") + ) + + adapter.handle_message.assert_called_once() + + +@pytest.mark.asyncio +async def test_other_channel_stays_free_response(adapter): + adapter.config.extra["require_mention"] = False + adapter.config.extra["require_mention_channels"] = CHANNEL_ID + + await adapter._handle_slack_message( + _event("no mention needed here", channel="C_OTHER") + ) + + adapter.handle_message.assert_called_once()