fix(slack): MPIMs (group DMs) obey shared-surface mention gating + reaction guard - #57564
Merged
kshitijk4poor merged 2 commits intoJul 3, 2026
Conversation
…action guard
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:
- NousResearch#4633 fix(slack): treat group DMs (mpim) like DMs + reaction guard
- NousResearch#54632 fix(slack): subscribe to message.mpim + mpim scopes so group DMs work
- NousResearch#54663 fix(slack): group DMs work OOTB + reinstall nudge
NousResearch#54632/NousResearch#54663 correctly made MPIM messages *reachable*; NousResearch#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.
alt-glitch
added
type/bug
Collaborator
Related: #57339 (earlier OPEN PR by @victor-kyriazakos with a byte-identical fix; this is a salvage of it), #7294 (OPEN issue requesting exactly this mpim mention-gating), #4633 (closed PR that introduced the DM over-reach). This salvage and #57339 are competing fixes for the same bug via the same mechanism (is_one_to_one_dm split) — a maintainer should pick one and close the other. |
The reaction-guard regression test defined a local _should_react lambda and asserted it against itself — a tautology that would stay green even if the production guard at _handle_slack_message reverted to (is_dm or is_mentioned), re-introducing the unmentioned-MPIM reaction spam this PR fixes. Replace it with a shared _reaction_guard helper plus a source-introspection test that pins the production expression: asserts (is_one_to_one_dm or is_mentioned) is present and (is_dm or is_mentioned) is absent. Mutation-checked — reverting the adapter guard now fails the test. Follow-up self-review finding on the salvage of NousResearch#57339.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Slack group DMs (MPIMs) now obey the same operator controls as channels —
require_mention,strict_mention,allowed_channels,free_response_channels— and only get:eyes:/:white_check_mark:reactions when@mentioned. Previously they were classified as 1:1 DMs and skipped all gating, reacting to (and running the agent on) every unmentioned message — visible noise to the whole group.Root cause: the adapter classified MPIMs as DMs (
is_dm = channel_type in {"im","mpim"}), and that single flag drove both the mention/allowlist gate (if not is_dm) and the reaction guard (is_dm or is_mentioned). A 1:1 IM is a private conversation with one human, so it's legitimately mention-exempt; an MPIM is a shared surface and must obey channel-style controls.Changes
plugins/platforms/slack/adapter.py: addis_one_to_one_dm = channel_type == "im"and key the two exemption sites off it.is_dm(im + mpim) is intentionally retained for session/thread scoping andchat_typelabeling, where treating an MPIM as a persistent multi-party conversation is correct.tests/gateway/test_slack_mention.py: extend_would_processto modelis_one_to_one_dmgating + thestrict_mentionbranch; 7 regression tests.website/docs/user-guide/messaging/slack.md: distinguish 1:1 DMs (mention-exempt) from group DMs (shared surface).Validation
im)mpim)main(both gate + reaction guard driven byis_dm).is_dm/is_imsites are session scoping / chat_type labeling and correctly left alone.test_slack.py+test_slack_mention.py= 288 passed; lint clean.mpim-exempt behavior flips the 3 MPIM-drop regression tests red, confirming they're genuine (not tautologies).Salvaged from #57339 by @victor-kyriazakos; authorship preserved via cherry-pick. Closes #57339.