From 34e616e778d065acea1477ddc6933c30378def89 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sun, 28 Jun 2026 23:02:41 -0700 Subject: [PATCH] feat(slack): nudge stale installs to add mpim scopes; mark message.mpim required Follow-up to the group-DM manifest fix. The manifest change only helps NEW installs; existing apps keep their old (mpim-less) scopes until the admin reinstalls. Since a missing message.mpim event delivers nothing (no runtime API error to catch), detect stale installs at connect time from the auth.test x-oauth-scopes header and log an actionable reinstall nudge when im:history is granted but mpim:history is not. Also promote message.mpim from Recommended to Required in the docs event tables so the default setup path can't drop it. --- plugins/platforms/slack/adapter.py | 47 ++++++++ .../test_slack_group_dm_scope_warning.py | 108 ++++++++++++++++++ website/docs/user-guide/messaging/slack.md | 2 +- .../current/user-guide/messaging/slack.md | 2 +- 4 files changed, 157 insertions(+), 2 deletions(-) create mode 100644 tests/gateway/test_slack_group_dm_scope_warning.py diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index 60f510d7070..7515a85f289 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -829,6 +829,51 @@ class SlackAdapter(BasePlatformAdapter): # Non-fatal — the user saw the initial ack already. return SendResult(success=True, message_id=None) + def _warn_if_missing_group_dm_scopes(self, auth_response, team_name: str) -> None: + """Nudge existing installs to reinstall when group-DM scopes are absent. + + Group DMs only reach the bot when the app is subscribed to + ``message.mpim`` and granted ``mpim:history`` (see slack_cli.py + manifest). A missing event delivers *nothing* — there is no runtime + API error to catch — so the only place we can detect a stale install + is at connect time, by inspecting the ``x-oauth-scopes`` header the + Slack ``auth.test`` response carries. If the app clearly handles 1:1 + DMs (``im:history`` present) but lacks ``mpim:history``, it predates + this fix; log exactly what to add and that a reinstall is required. + """ + try: + # Track warned workspaces so the nudge fires once per process per + # team, not on every reconnect. getattr-default keeps bare + # object.__new__ test instances (no __init__) from crashing. + warned = getattr(self, "_group_dm_scope_warned", None) + if warned is None: + warned = set() + self._group_dm_scope_warned = warned + headers = getattr(auth_response, "headers", None) or {} + raw = headers.get("x-oauth-scopes") or headers.get("X-OAuth-Scopes") or "" + if not raw: + return # Header absent (e.g. some proxies) — don't guess. + granted = {s.strip() for s in raw.split(",") if s.strip()} + team_key = team_name or "" + if team_key in warned: + return + # Only nudge real DM-capable installs; "im:history" present but + # "mpim:history" missing == stale manifest from before the fix. + if "im:history" in granted and "mpim:history" not in granted: + warned.add(team_key) + logger.warning( + "[Slack] Group DMs (multi-person DMs) will not work in " + "workspace %s: the app is missing the 'mpim:history' scope " + "and 'message.mpim' event. Add 'mpim:history' (and " + "'mpim:read') to bot scopes, add 'message.mpim' to event " + "subscriptions, then REINSTALL the app to the workspace. " + "Regenerating the app from `hermes slack` produces a " + "manifest with these already included.", + team_key or "this workspace", + ) + except Exception: # pragma: no cover - diagnostics must never break connect + pass + async def connect(self, *, is_reconnect: bool = False) -> bool: """Connect to Slack via Socket Mode.""" if not SLACK_AVAILABLE: @@ -957,6 +1002,8 @@ class SlackAdapter(BasePlatformAdapter): team_id, ) + self._warn_if_missing_group_dm_scopes(auth_response, team_name) + # Register message event handler @self._app.event("message") async def handle_message_event(event, say): diff --git a/tests/gateway/test_slack_group_dm_scope_warning.py b/tests/gateway/test_slack_group_dm_scope_warning.py new file mode 100644 index 00000000000..1ead07bbbd7 --- /dev/null +++ b/tests/gateway/test_slack_group_dm_scope_warning.py @@ -0,0 +1,108 @@ +""" +Tests for the connect-time group-DM scope nudge. + +When a Slack app handles 1:1 DMs (``im:history`` granted) but is missing +``mpim:history``, group DMs are silently dropped by Slack before the adapter +ever sees them. ``_warn_if_missing_group_dm_scopes`` inspects the +``x-oauth-scopes`` header from ``auth.test`` at connect time and logs an +actionable reinstall nudge — the only point where a stale install is +detectable, since a missing event produces no runtime API error. +""" + +import logging +import sys +from unittest.mock import MagicMock + + +# --------------------------------------------------------------------------- +# 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 + + +class _FakeAuthResponse: + """Mimics slack_sdk's AsyncSlackResponse — a .headers dict carrying scopes.""" + + def __init__(self, scopes_csv): + self.headers = {"x-oauth-scopes": scopes_csv} + + +def _make_adapter(): + # object.__new__ skips __init__ (heavy setup) — established slack-test pattern. + return object.__new__(SlackAdapter) + + +def test_warns_when_mpim_history_missing(caplog): + adapter = _make_adapter() + resp = _FakeAuthResponse("chat:write,im:history,im:read,channels:history") + with caplog.at_level(logging.WARNING): + adapter._warn_if_missing_group_dm_scopes(resp, "Acme") + assert any("Group DMs" in r.message and "mpim:history" in r.message + for r in caplog.records) + + +def test_no_warning_when_mpim_history_present(caplog): + adapter = _make_adapter() + resp = _FakeAuthResponse("chat:write,im:history,mpim:history,mpim:read") + with caplog.at_level(logging.WARNING): + adapter._warn_if_missing_group_dm_scopes(resp, "Acme") + assert not any("Group DMs" in r.message for r in caplog.records) + + +def test_no_warning_when_no_dm_scopes_at_all(caplog): + # A channel-only app (no im:history) shouldn't be nudged about group DMs. + adapter = _make_adapter() + resp = _FakeAuthResponse("chat:write,channels:history") + with caplog.at_level(logging.WARNING): + adapter._warn_if_missing_group_dm_scopes(resp, "Acme") + assert not any("Group DMs" in r.message for r in caplog.records) + + +def test_warns_only_once_per_workspace(caplog): + adapter = _make_adapter() + resp = _FakeAuthResponse("im:history") + with caplog.at_level(logging.WARNING): + adapter._warn_if_missing_group_dm_scopes(resp, "Acme") + adapter._warn_if_missing_group_dm_scopes(resp, "Acme") + warnings = [r for r in caplog.records if "Group DMs" in r.message] + assert len(warnings) == 1 + + +def test_missing_header_does_not_warn(caplog): + # Header absent (e.g. some proxies strip it) — don't guess, stay silent. + adapter = _make_adapter() + resp = _FakeAuthResponse("") + with caplog.at_level(logging.WARNING): + adapter._warn_if_missing_group_dm_scopes(resp, "Acme") + assert not any("Group DMs" in r.message for r in caplog.records) diff --git a/website/docs/user-guide/messaging/slack.md b/website/docs/user-guide/messaging/slack.md index b1b65c57ee9..67ccb661733 100644 --- a/website/docs/user-guide/messaging/slack.md +++ b/website/docs/user-guide/messaging/slack.md @@ -126,7 +126,7 @@ This step is critical — it controls what messages the bot can see. | Event | Required? | Purpose | |-------|-----------|---------| | `message.im` | **Yes** | Bot receives direct messages | -| `message.mpim` | **Recommended** | Bot receives messages in **group DMs** (multi-person DMs) it's added to | +| `message.mpim` | **Yes** | Bot receives messages in **group DMs** (multi-person DMs) it's added to | | `message.channels` | **Yes** | Bot receives messages in **public** channels it's added to | | `message.groups` | **Recommended** | Bot receives messages in **private** channels it's invited to | | `app_mention` | **Yes** | Prevents Bolt SDK errors when bot is @mentioned | diff --git a/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/user-guide/messaging/slack.md b/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/user-guide/messaging/slack.md index 2b6dbb21225..1fb03a1dc5f 100644 --- a/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/user-guide/messaging/slack.md +++ b/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/user-guide/messaging/slack.md @@ -112,7 +112,7 @@ Socket Mode 让机器人通过 WebSocket 连接，无需公开 URL。 | 事件 | 是否必需 | 用途 | |-------|-----------|---------| | `message.im` | **必需** | 机器人接收私信 | -| `message.mpim` | **推荐** | 机器人接收其加入的**群组私信**（多人私信）消息 | +| `message.mpim` | **必需** | 机器人接收其加入的**群组私信**（多人私信）消息 | | `message.channels` | **必需** | 机器人接收其加入的**公开**频道消息 | | `message.groups` | **推荐** | 机器人接收被邀请加入的**私有**频道消息 | | `app_mention` | **必需** | 防止机器人被 @ 提及时出现 Bolt SDK 错误 |