diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index 54dd79bb134..4eaacfdbf62 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -10,6 +10,7 @@ Uses slack-bolt (Python) with Socket Mode for: import asyncio import contextvars +import inspect import json import logging import os @@ -704,6 +705,31 @@ class SlackAdapter(BasePlatformAdapter): # first ping/pong as evidence of a wedged transport. self._socket_first_ping_grace_s = 60.0 + async def _close_workspace_clients(self) -> None: + """Close any Slack SDK clients that may own aiohttp sessions.""" + clients: List[Any] = [] + if self._app is not None: + primary_client = getattr(self._app, "client", None) + if primary_client is not None: + clients.append(primary_client) + clients.extend(self._team_clients.values()) + + seen_ids: set[int] = set() + for client in clients: + ident = id(client) + if ident in seen_ids: + continue + seen_ids.add(ident) + + for method_name in ("close", "aclose"): + closer = getattr(client, method_name, None) + if not callable(closer): + continue + result = closer() + if inspect.isawaitable(result): + await result + break + def _start_socket_mode_handler(self) -> None: """Start the Slack Socket Mode background task.""" if not self._app or not self._app_token: @@ -1334,6 +1360,7 @@ class SlackAdapter(BasePlatformAdapter): # receive every Slack event and dispatch it twice, producing double # responses — the same bug that affected DiscordAdapter (#18187). await self._stop_socket_mode_handler() + await self._close_workspace_clients() self._app = None self._app_token = app_token self._proxy_url = proxy_url @@ -1651,9 +1678,14 @@ class SlackAdapter(BasePlatformAdapter): ) await self._stop_socket_mode_handler() + await self._close_workspace_clients() self._app = None self._app_token = None self._proxy_url = None + self._bot_user_id = None + self._team_clients = {} + self._team_bot_user_ids = {} + self._channel_team = {} self._release_platform_lock() diff --git a/tests/gateway/test_slack.py b/tests/gateway/test_slack.py index 61fc14512fa..b5946e8b968 100644 --- a/tests/gateway/test_slack.py +++ b/tests/gateway/test_slack.py @@ -468,6 +468,7 @@ class TestSlackConnectCleanup: ) second_handler = MagicMock() + second_handler.close_async = AsyncMock(return_value=None) # _start_socket_mode_handler awaits the result of start_async via # asyncio.create_task — so the stub must return a real coroutine, not a # bare MagicMock. @@ -490,6 +491,62 @@ class TestSlackConnectCleanup: first_handler.close_async.assert_awaited_once_with() assert adapter._handler is second_handler + with patch("gateway.status.release_scoped_lock"): + await adapter.disconnect() + + @pytest.mark.asyncio + async def test_disconnect_closes_workspace_clients_and_clears_runtime_state(self): + """Regression for #51465: shutdown must close Slack WebClients. + + ``hermes gateway run --replace`` takes the old process through the + normal adapter.disconnect() path. If Slack leaves AsyncWebClient + instances open there, aiohttp logs ``Unclosed client session`` while + the old gateway exits after SIGTERM. + """ + config = PlatformConfig(enabled=True, token="xoxb-fake") + adapter = SlackAdapter(config) + + socket_task = asyncio.create_task(_pending_for_fake_task()) + handler = MagicMock() + handler.close_async = AsyncMock(return_value=None) + + primary_client = MagicMock() + primary_client.close = AsyncMock(return_value=None) + team_client = MagicMock() + team_client.close = AsyncMock(return_value=None) + + adapter._running = True + adapter._handler = handler + adapter._socket_mode_task = socket_task + adapter._app = MagicMock() + adapter._app.client = primary_client + adapter._team_clients = {"T_FAKE": team_client} + adapter._team_bot_user_ids = {"T_FAKE": "U_BOT"} + adapter._channel_team = {"C_FAKE": "T_FAKE"} + adapter._platform_lock_scope = "slack-app-token" + adapter._platform_lock_identity = "xapp-fake" + adapter._app_token = "xapp-fake" + adapter._proxy_url = "http://proxy.example.com:3128" + adapter._bot_user_id = "U_BOT" + + with patch("gateway.status.release_scoped_lock") as mock_release: + await adapter.disconnect() + + handler.close_async.assert_awaited_once_with() + primary_client.close.assert_awaited_once_with() + team_client.close.assert_awaited_once_with() + assert socket_task.cancelled() + assert adapter._app is None + assert adapter._handler is None + assert adapter._socket_mode_task is None + assert adapter._team_clients == {} + assert adapter._team_bot_user_ids == {} + assert adapter._channel_team == {} + assert adapter._bot_user_id is None + assert adapter._app_token is None + assert adapter._proxy_url is None + mock_release.assert_called_once_with("slack-app-token", "xapp-fake") + # --------------------------------------------------------------------------- # TestSlackSocketWatchdog