mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-22 16:25:58 +00:00
feat(slack): require_mention_channels per-channel force-mention override
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).
This commit is contained in:
parent
03b18d86c3
commit
7a0cfebf74
3 changed files with 244 additions and 3 deletions
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
209
tests/gateway/test_slack_require_mention_channels.py
Normal file
209
tests/gateway/test_slack_require_mention_channels.py
Normal file
|
|
@ -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()
|
||||
Loading…
Add table
Add a link
Reference in a new issue