KrustyPlanet/hermes-agent
6
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

fix(slack): MPIMs (group DMs) obey shared-surface mention gating + reaction guard

Browse source 
Group DMs (MPIMs) were classified as DMs and thereby exempted from every
operator control that shared surfaces are supposed to honor: allowed_channels,
require_mention, strict_mention, free_response_channels, and the reaction
guard. Symptom: the bot added 👀/✅ to unmentioned MPIM
messages and still invoked the agent (which then returned NO_REPLY) instead of
the gateway dropping the event before model execution. Removing an MPIM from
allowed_channels did not disable it.

Root cause is the DM classification at adapter.py:
    is_dm = channel_type in {"im", "mpim"}
used for BOTH routing exemptions and reaction gating. An MPIM is a shared
surface (multiple humans can see and trigger the bot), not a private 1:1 DM,
so it must be gated like a channel.

This behavior was introduced/reinforced by a trail of Slack group-DM PRs:
- #4633  fix(slack): treat group DMs (mpim) like DMs + reaction guard
- #54632 fix(slack): subscribe to message.mpim + mpim scopes so group DMs work
- #54663 fix(slack): group DMs work OOTB + reinstall nudge
#54632/#54663 correctly made MPIM messages *reachable*; #4633 over-reached by
giving them the DM mention/reaction *exemptions*. This corrects only that
over-reach.

Fix (minimal): introduce `is_one_to_one_dm = channel_type == "im"` and key the
two EXEMPTION sites off it instead of `is_dm`:
- mention/allowlist gating block (`if not is_one_to_one_dm and bot_uid:`)
- reaction guard (`(is_one_to_one_dm or is_mentioned)`)
`is_dm` is intentionally retained for session/thread scoping and chat_type
labeling, where treating an MPIM as a persistent multi-party conversation is
correct — only the mention/reaction exemptions were wrong.

Docs: slack.md now distinguishes 1:1 DMs (mention-exempt) from group DMs
(shared surface; obey require_mention/strict_mention/allowed_channels/
free_response_channels; reactions only when @mentioned).

Tests: +7 in test_slack_mention.py (MPIM unmentioned dropped under
require_mention and strict_mention; MPIM mentioned processed; MPIM off
allowed_channels dropped; MPIM in free_response opted in; 1:1 IM still exempt;
reaction guard drops unmentioned MPIM). Updated _would_process to model the
is_one_to_one_dm gating + strict_mention. 72 passed.
This commit is contained in:
Victor Kyriazakos 2026-07-02 20:36:51 +00:00 • committed by kshitij
parent 42bc07d107
commit accd672054
3 changed files with 96 additions and 9 deletions
Download patch file Download diff file Expand all files Collapse all files

20
plugins/platforms/slack/adapter.py
View file

@ -2761,6 +2761,15 @@ class SlackAdapter(BasePlatformAdapter):
if not channel_type and channel_id.startswith("D"):
channel_type = "im"
is_dm = channel_type in {"im", "mpim"} # Both 1:1 and group DMs
# A 1:1 IM is a private conversation with a single human — mention-exempt
# and safe to react to unconditionally, like any DM. An MPIM (group DM)
# is a SHARED surface: multiple humans can see and trigger the bot, so it
# must obey the same operator controls as a channel (allowed_channels /
# require_mention / strict_mention / free_response_channels) and must not
# get reaction noise on messages that don't address the bot. Only the 1:1
# case earns the DM exemptions; session/thread scoping below still treats
# both as DM-style persistent conversations.
is_one_to_one_dm = channel_type == "im"
# Build thread_ts for session keying.
# In channels: fall back to ts so each top-level @mention starts a
@ -2823,7 +2832,7 @@ class SlackAdapter(BasePlatformAdapter):
event_thread_ts = event.get("thread_ts")
is_thread_reply = bool(event_thread_ts and event_thread_ts != ts)
if not is_dm and bot_uid:
if not is_one_to_one_dm and bot_uid:
# Check allowed channels — if set, only respond in these channels (whitelist)
allowed_channels = self._slack_allowed_channels()
if allowed_channels and channel_id not in allowed_channels:
@ -3216,10 +3225,11 @@ class SlackAdapter(BasePlatformAdapter):
auto_skill=_auto_skill,
)
# Only react when bot is directly addressed (DM or @mention).
# In listen-all channels (require_mention=false), reacting to every
# casual message would be noisy.
_should_react = (is_dm or is_mentioned) and self._reactions_enabled()
# Only react when bot is directly addressed (1:1 DM or @mention).
# MPIMs are shared surfaces: reacting to every group-DM message (even
# when unmentioned) is visible noise to the whole group, so they must
# be @mentioned to earn a reaction — same as any channel.
_should_react = (is_one_to_one_dm or is_mentioned) and self._reactions_enabled()
if _should_react:
self._reacting_message_ids.add(ts)

77
tests/gateway/test_slack_mention.py
View file

@ -243,12 +243,22 @@ def test_free_response_channels_int_list():
def _would_process(adapter, *, is_dm=False, channel_id=CHANNEL_ID,
text="hello", mentioned=False, thread_reply=False,
active_session=False):
active_session=False, channel_type=None):
"""Simulate the mention gating logic from _handle_slack_message.
Returns True if the message would be processed, False if it would be
skipped (returned early).
``channel_type`` mirrors the real Slack payload ("im" = 1:1 DM,
"mpim" = group DM, "" = channel). When omitted it is derived from the
legacy ``is_dm`` flag as a 1:1 IM, preserving existing callers. Gating
keys off ``is_one_to_one_dm`` (only a true 1:1 IM is exempt); MPIMs are
shared surfaces and go through the same gating as channels.
"""
if channel_type is None:
channel_type = "im" if is_dm else ""
is_one_to_one_dm = channel_type == "im"
bot_uid = adapter._team_bot_user_ids.get("T1", adapter._bot_user_id)
if mentioned:
text = f"<@{bot_uid}> {text}"
@ -257,7 +267,7 @@ def _would_process(adapter, *, is_dm=False, channel_id=CHANNEL_ID,
or adapter._slack_message_matches_mention_patterns(text)
)
if not is_dm and bot_uid:
if not is_one_to_one_dm and bot_uid:
# allowed_channels check (whitelist — must pass before other gating)
allowed = adapter._slack_allowed_channels()
if allowed and channel_id not in allowed:
@ -267,6 +277,8 @@ def _would_process(adapter, *, is_dm=False, channel_id=CHANNEL_ID,
return True
elif not adapter._slack_require_mention():
return True
elif adapter._slack_strict_mention() and not is_mentioned:
return False
elif not is_mentioned:
if thread_reply and active_session:
return True
@ -306,6 +318,67 @@ def test_dm_always_processed_regardless_of_setting():
assert _would_process(adapter, is_dm=True, text="hello") is True
# ---------------------------------------------------------------------------
# Tests: MPIM / group-DM shared-surface gating (regression for the group-DM
# routing bug introduced by PRs #4633 / #54632 / #54663, which classified
# mpim as a DM and thereby exempted it from mention gating + reaction guards).
# ---------------------------------------------------------------------------
def test_mpim_unmentioned_strict_mention_ignored():
"""MPIM, not mentioned, strict_mention on -> dropped (shared surface)."""
adapter = _make_adapter(require_mention=True, strict_mention=True,
free_response_channels=[])
assert _would_process(adapter, channel_type="mpim", text="hello") is False
def test_mpim_unmentioned_require_mention_ignored():
"""MPIM, not mentioned, require_mention on (non-strict) -> dropped."""
adapter = _make_adapter(require_mention=True)
assert _would_process(adapter, channel_type="mpim", text="hello") is False
def test_mpim_mentioned_processed():
"""MPIM with an @mention is processed like any addressed message."""
adapter = _make_adapter(require_mention=True, strict_mention=True)
assert _would_process(adapter, channel_type="mpim", mentioned=True,
text="hello") is True
def test_mpim_not_in_allowed_channels_dropped():
"""MPIM absent from a non-empty allowed_channels whitelist is dropped,
even when mentioned."""
adapter = _make_adapter(require_mention=True, allowed_channels=["C_ALLOWED"])
assert _would_process(adapter, channel_type="mpim", channel_id="C_BLOCKED",
mentioned=True, text="hello") is False
def test_mpim_in_free_response_processed_without_mention():
"""An MPIM explicitly listed in free_response_channels still opts in."""
adapter = _make_adapter(require_mention=True,
free_response_channels=["G_MPIM"])
assert _would_process(adapter, channel_type="mpim", channel_id="G_MPIM",
text="hello") is True
def test_one_to_one_im_still_exempt():
"""1:1 IM behavior is preserved: mention-exempt regardless of settings."""
adapter = _make_adapter(require_mention=True, strict_mention=True)
assert _would_process(adapter, channel_type="im", text="hello") is True
def test_mpim_unmentioned_does_not_react():
"""Reaction guard: only a 1:1 IM or an @mention earns a reaction. An
unmentioned MPIM message must NOT get :eyes:/:white_check_mark: noise."""
def _should_react(channel_type, is_mentioned):
is_one_to_one_dm = channel_type == "im"
return is_one_to_one_dm or is_mentioned
assert _should_react("mpim", False) is False # the reported spam case
assert _should_react("mpim", True) is True # addressed -> ok
assert _should_react("im", False) is True # 1:1 DM -> ok
assert _should_react("", False) is False # channel, unmentioned
def test_mentioned_message_always_processed():
adapter = _make_adapter(require_mention=True)
assert _would_process(adapter, mentioned=True, text="what's up") is True

8
website/docs/user-guide/messaging/slack.md
View file

@ -412,14 +412,18 @@ Set this to `true` in busy workspaces where Slack's default "the bot remembers t
:::
:::info
Slack supports both patterns: `@mention` required to start a conversation by default, but you can opt specific channels out via `SLACK_FREE_RESPONSE_CHANNELS` (comma-separated channel IDs) or `slack.free_response_channels` in `config.yaml`. Once the bot has an active session in a thread, subsequent thread replies do not require a mention. In DMs the bot always responds without needing a mention.
Slack supports both patterns: `@mention` required to start a conversation by default, but you can opt specific channels out via `SLACK_FREE_RESPONSE_CHANNELS` (comma-separated channel IDs) or `slack.free_response_channels` in `config.yaml`. Once the bot has an active session in a thread, subsequent thread replies do not require a mention. In **1:1 DMs** the bot always responds without needing a mention.
:::
:::caution Group DMs (MPIMs) are shared surfaces, not 1:1 DMs
A **1:1 direct message** is a private conversation with one person, so it is mention-exempt. A **group DM (MPIM / multi-person DM)** is a *shared surface* — multiple people can see and trigger the bot — so it obeys the same operator controls as a channel: `require_mention`, `strict_mention`, `free_response_channels`, and `allowed_channels` all apply, and the bot only adds `:eyes:`/`:white_check_mark:` reactions when it is actually `@mentioned`. To let the bot respond freely in a specific group DM, add its channel ID (starts with `G`) to `free_response_channels`.
:::
### Channel allowlist (`allowed_channels`)
Restrict the bot to a fixed set of Slack channels — useful when the bot is invited to many channels but should only respond in a few. When set, messages from channels NOT in this list are **silently ignored**, even if the bot is `@mentioned`.
**DMs are exempt** from this filter, so authorized users can always reach the bot in a direct message.
**1:1 DMs are exempt** from this filter, so authorized users can always reach the bot in a direct message. **Group DMs (MPIMs) are not exempt** — like channels, an MPIM must be on the allowlist (its ID starts with `G`) or its messages are dropped.
```yaml
slack: