From 0eb5cb5e07b7cdb6f3c1092d973a9b96eff60679 Mon Sep 17 00:00:00 2001 From: haran2001 <56040092+haran2001@users.noreply.github.com> Date: Fri, 22 May 2026 08:17:19 +0530 Subject: [PATCH] fix(slack): surface bot-event arrival and allow_bots interop diagnostics (#30091) --- plugins/platforms/slack/adapter.py | 55 ++++++++++++++++++++++++++++++ tests/gateway/test_slack.py | 44 ++++++++++++++++++++++++ 2 files changed, 99 insertions(+) diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index afe1756cd2d3..73f7116dbe1e 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -1889,6 +1889,39 @@ class SlackAdapter(BasePlatformAdapter): "[Slack] Socket Mode connected (%d workspace(s))", len(self._team_clients), ) + + # Bot-event interop diagnostic. When the user has opted into + # bot messages via ``slack.allow_bots`` / ``SLACK_ALLOW_BOTS``, + # surface the additional plumbing they almost certainly also + # need so bot-to-bot interop doesn't silently fail. + # + # See #30091: a user reported that with ``allow_bots: all`` + # configured, bot messages in shared threads were still + # dropped. Two things upstream of this code can swallow them: + # 1. The Slack app's event subscriptions in the manifest — + # Socket Mode does not deliver events the app hasn't + # subscribed to (``message.channels`` for public + # channels, ``message.groups`` for private channels, + # ``message.im`` for DMs). + # 2. The SLACK_ALLOWED_USERS / GATEWAY_ALLOWED_USERS + # per-user allowlists — the other bot's user id must be + # present (or GATEWAY_ALLOW_ALL_USERS=true). + # + # Logging once at INFO keeps the startup line discoverable + # without requiring DEBUG to enable. + _allow_bots_cfg = self._slack_allow_bots() + if _allow_bots_cfg != "none": + logger.info( + "[Slack] allow_bots=%s — for bot-to-bot interop also ensure: " + "(a) the Slack app manifest subscribes to message.channels / " + "message.groups / message.im as appropriate (run " + "'hermes slack manifest' if unsure), and (b) the other bot's " + "Slack user id is in SLACK_ALLOWED_USERS or " + "GATEWAY_ALLOW_ALL_USERS=true. Without these, bot events are " + "silently dropped upstream of the allow_bots gate.", + _allow_bots_cfg, + ) + return True except Exception as e: # pragma: no cover - defensive logging @@ -4199,6 +4232,28 @@ class SlackAdapter(BasePlatformAdapter): self, event: dict, payload: Optional[dict] = None ) -> None: """Handle an incoming Slack message event.""" + # DEBUG entry log — fires BEFORE any filtering so users debugging + # bot-to-bot interop, allow_bots config, or SLACK_ALLOWED_USERS + # drops can confirm whether the event actually arrived from Slack + # (vs. being silently filtered upstream by the app's event + # subscriptions — Socket Mode will not deliver events the app + # manifest hasn't subscribed to). See #30091. Metadata only — never + # the message text. + if logger.isEnabledFor(logging.DEBUG): + _bot_profile = event.get("bot_profile") or {} + _bot_name = (_bot_profile.get("name") if isinstance(_bot_profile, dict) else "") or "" + logger.debug( + "[Slack] event received type=%s subtype=%s user=%s bot_id=%s bot_name=%s " + "channel=%s ts=%s thread_ts=%s", + event.get("type"), + event.get("subtype"), + event.get("user", "") or "", + event.get("bot_id", "") or "", + _bot_name, + event.get("channel", ""), + event.get("ts", ""), + event.get("thread_ts", ""), + ) if event.get("subtype") == "message_changed": updated_message = event.get("message") if not isinstance(updated_message, dict): diff --git a/tests/gateway/test_slack.py b/tests/gateway/test_slack.py index fb7dd1e149eb..4b43d9d54604 100644 --- a/tests/gateway/test_slack.py +++ b/tests/gateway/test_slack.py @@ -138,6 +138,50 @@ def _redirect_cache(tmp_path, monkeypatch): ) +class TestBotEventDiagnostics: + """#30091 — surface upstream filters that drop bot events.""" + + @pytest.mark.asyncio + async def test_handler_emits_debug_for_bot_event(self, adapter, caplog): + import logging + caplog.set_level(logging.DEBUG, logger="plugins.platforms.slack.adapter") + # Stub dedup so the debug log is hit even on a bot subtype. + adapter._dedup = MagicMock() + adapter._dedup.is_duplicate.return_value = True # short-circuit after debug log + event = { + "type": "message", + "subtype": "bot_message", + "user": "U_OTHER_BOT", + "bot_id": "B_OTHER", + "bot_profile": {"name": "Liatrio Brain"}, + "ts": "12345.6789", + "channel": "C_SHARED", + "thread_ts": "12300.0", + } + await adapter._handle_slack_message(event) + debug_lines = [r.getMessage() for r in caplog.records if r.levelno == logging.DEBUG] + assert any( + "event received" in line + and "bot_id=B_OTHER" in line + and "user=U_OTHER_BOT" in line + and "Liatrio Brain" in line + for line in debug_lines + ), debug_lines + + def test_allow_bots_startup_diagnostic_extra(self): + """When allow_bots is configured via PlatformConfig.extra, the connect + path must surface the SLACK_ALLOWED_USERS + manifest-subscription + requirement so bot-to-bot interop doesn't fail silently.""" + # We can't easily run connect() end-to-end, but the diagnostic block + # reads from self.config.extra / SLACK_ALLOW_BOTS in isolation; we + # verify the read path here. + cfg = PlatformConfig(enabled=True, token="***", extra={"allow_bots": "all"}) + a = SlackAdapter(cfg) + # The connect-time diagnostic gates on the adapter's normalized + # allow_bots policy helper. + assert a._slack_allow_bots() == "all" + + # --------------------------------------------------------------------------- # TestSlashCommandSessionIsolation # ---------------------------------------------------------------------------