From 3c5c389f189dc335dabef6869f9d9e3b95068253 Mon Sep 17 00:00:00 2001 From: Matt Ferguson Date: Wed, 22 Jul 2026 03:58:33 -0700 Subject: [PATCH] fix(slack): widen Socket Mode dedup TTL to cover reconnect redelivery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slack buffers un-acked Socket Mode events and replays them when the websocket reconnects; the replay can arrive several minutes later — past the 300s default dedup TTL — producing a duplicate bot reply. Default the Slack dedup window to 1 hour (memory stays bounded by the deduplicator's max_size LRU pruning) and allow overriding via SLACK_DEDUP_TTL_SECONDS. Salvaged from PR #40064 by @MrAbsaroka (reapplied onto the plugin-migrated adapter path). Fixes #4777. --- plugins/platforms/slack/adapter.py | 32 +++++++++- tests/gateway/test_slack_dedup_ttl.py | 92 +++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 tests/gateway/test_slack_dedup_ttl.py diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index 4eaacfdbf6..72f90124cd 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -498,6 +498,30 @@ def _resolve_slack_proxy_url() -> Optional[str]: return proxy_url +def _slack_dedup_ttl_seconds() -> float: + """Dedup window for redelivered Socket Mode events (#4777). + + Slack buffers un-acked Socket Mode events and replays them when the + websocket reconnects. The replay can arrive several minutes after the + original — well past the 300s default TTL — which would otherwise be + treated as a new message and produce a duplicate bot reply. Memory is + bounded by ``MessageDeduplicator(max_size=...)`` (LRU pruning), not by + the TTL, so the window can safely span the worst-case reconnect gap. + Override with ``SLACK_DEDUP_TTL_SECONDS``. + """ + raw = os.getenv("SLACK_DEDUP_TTL_SECONDS", "") + if raw: + try: + value = float(raw) + if value > 0: + return value + except ValueError: + logger.warning( + "[Slack] Invalid SLACK_DEDUP_TTL_SECONDS=%r; using default", raw + ) + return 3600.0 # 1 hour — covers Slack reconnect redelivery windows + + # Map Slack audio mimetypes to the file extension that matches the actual # container bytes. Critically, Slack's in-app "record a clip" voice messages # arrive as MP4/AAC containers (``audio/mp4``, filename ``audio_message*.mp4``), @@ -643,8 +667,12 @@ class SlackAdapter(BasePlatformAdapter): self._team_bot_user_ids: Dict[str, str] = {} # team_id → bot_user_id self._channel_team: Dict[str, str] = {} # channel_id → team_id # Dedup cache: prevents duplicate bot responses when Socket Mode - # reconnects redeliver events. - self._dedup = MessageDeduplicator() + # reconnects redeliver events (#4777). The TTL must outlast Slack's + # worst-case reconnect-redelivery gap, not just a few seconds — the + # 300s default let replays that landed >5 min later slip through and + # produce a second reply. max_size bounds memory, so the long window + # is safe. + self._dedup = MessageDeduplicator(ttl_seconds=_slack_dedup_ttl_seconds()) # Track pending approval message_ts → resolved flag to prevent # double-clicks on approval buttons. self._approval_resolved: Dict[str, bool] = {} diff --git a/tests/gateway/test_slack_dedup_ttl.py b/tests/gateway/test_slack_dedup_ttl.py new file mode 100644 index 0000000000..f32a829b6c --- /dev/null +++ b/tests/gateway/test_slack_dedup_ttl.py @@ -0,0 +1,92 @@ +""" +Tests for Slack Socket Mode dedup TTL (#4777). + +Slack replays un-acked Socket Mode events when the websocket reconnects. +The replay can land several minutes after the original; the dedup window +must outlast that gap so the redelivered event is suppressed instead of +producing a second bot reply. Regression for the 300s-default bug where +replays >5 min later slipped through. + +Follows the slack-bolt mocking pattern from test_slack_mention.py. +""" + +import os +import sys +import time +from unittest.mock import MagicMock, patch + + +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 gateway.platforms.helpers import MessageDeduplicator # noqa: E402 +from plugins.platforms.slack.adapter import _slack_dedup_ttl_seconds # noqa: E402 + + +def test_default_ttl_outlasts_slack_reconnect_redelivery_window(): + # The whole point of the fix: the window must be much longer than the + # ~6 min reconnect-redelivery gap that caused the duplicate reply. + with patch.dict(os.environ, {}, clear=True): + assert _slack_dedup_ttl_seconds() >= 1800.0 + + +def test_env_override_is_respected(): + with patch.dict(os.environ, {"SLACK_DEDUP_TTL_SECONDS": "120"}, clear=True): + assert _slack_dedup_ttl_seconds() == 120.0 + + +def test_invalid_env_falls_back_to_default(): + with patch.dict(os.environ, {"SLACK_DEDUP_TTL_SECONDS": "not-a-number"}, clear=True): + assert _slack_dedup_ttl_seconds() >= 1800.0 + with patch.dict(os.environ, {"SLACK_DEDUP_TTL_SECONDS": "0"}, clear=True): + assert _slack_dedup_ttl_seconds() >= 1800.0 + + +def test_redelivery_six_minutes_later_is_suppressed(): + """A replay 6 min after first processing must be treated as duplicate.""" + dedup = MessageDeduplicator(ttl_seconds=_slack_dedup_ttl_seconds()) + event_ts = "1733382960.001500" + + # First delivery — recorded now. + assert dedup.is_duplicate(event_ts) is False + # Simulate the entry being stamped 6 minutes ago (reconnect redelivery gap). + dedup._seen[event_ts] = time.time() - 360 + # Redelivery of the SAME event must still be caught. + assert dedup.is_duplicate(event_ts) is True + + +def test_old_default_300s_would_have_missed_it(): + """Pins the regression: the prior 300s window let the replay through.""" + dedup = MessageDeduplicator(ttl_seconds=300) + event_ts = "1733382960.001500" + assert dedup.is_duplicate(event_ts) is False + dedup._seen[event_ts] = time.time() - 360 # 6 min ago, past 300s TTL + # Demonstrates the bug: replay treated as new → second reply. + assert dedup.is_duplicate(event_ts) is False