From 7cda0e522443c6e7790793b93b085508fc530fc8 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Fri, 1 May 2026 08:53:30 +0530 Subject: [PATCH] fix(gateway/slack): ephemeral ack and routing for slash commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slack slash commands (/q, /btw, /stop, /model, etc.) previously showed no user-visible acknowledgement and posted command replies as public channel messages. This diverged from Discord, which uses ephemeral deferred responses for slash commands. Changes: - handle_hermes_command now passes response_type='ephemeral' and a 'Running /cmd…' text to ack(), giving the user immediate 'Only visible to you' feedback when they invoke any native slash command. - _handle_slash_command stashes the Slack response_url from the command payload in a per-channel context dict before dispatching to handle_message. - send() checks for a pending slash context and, when found, POSTs to the response_url with replace_original=true to swap the initial ack with the real command reply (e.g. 'Queued for the next turn.'), keeping it ephemeral. - Stale slash contexts are garbage-collected on lookup (120s TTL). - The response_url POST is non-fatal: if it fails, the user already saw the initial ack, and send() returns success=True. Fixes #18182 --- gateway/platforms/slack.py | 113 +++++++++++++++++++- tests/gateway/test_slack.py | 205 ++++++++++++++++++++++++++++++++++++ 2 files changed, 317 insertions(+), 1 deletion(-) diff --git a/gateway/platforms/slack.py b/gateway/platforms/slack.py index 77341c9c..4c6f29e8 100644 --- a/gateway/platforms/slack.py +++ b/gateway/platforms/slack.py @@ -21,6 +21,7 @@ try: from slack_bolt.async_app import AsyncApp from slack_bolt.adapter.socket_mode.async_handler import AsyncSocketModeHandler from slack_sdk.web.async_client import AsyncWebClient + import aiohttp SLACK_AVAILABLE = True except ImportError: SLACK_AVAILABLE = False @@ -310,6 +311,11 @@ class SlackAdapter(BasePlatformAdapter): # Track active assistant thread status indicators so stop_typing can # clear them (chat_id → thread_ts). self._active_status_threads: Dict[str, str] = {} + # Slash-command contexts: stash response_url + user_id so send() + # can route the first reply ephemerally. Keyed by + # (channel_id, user_id) to avoid cross-user collisions. + # Each value: {"response_url": str, "ts": float} + self._slash_command_contexts: Dict[Tuple[str, str], Dict[str, Any]] = {} def _describe_slack_api_error(self, response: Any, *, file_obj: Optional[Dict[str, Any]] = None) -> Optional[str]: """Convert Slack API auth/permission failures into actionable user-facing text.""" @@ -368,6 +374,86 @@ class SlackAdapter(BasePlatformAdapter): ) return None + # ------------------------------------------------------------------ + # Slash-command ephemeral helpers + # ------------------------------------------------------------------ + + _SLASH_CTX_TTL = 120.0 # seconds — response_url is valid for 30 min; + # we use a much shorter TTL to avoid routing unrelated messages + # as ephemeral if the command handler was slow or dropped. + + def _pop_slash_context( + self, chat_id: str, + ) -> Optional[Dict[str, Any]]: + """Return and remove the slash-command context for *chat_id*, if fresh. + + Contexts older than ``_SLASH_CTX_TTL`` seconds are silently discarded. + Uses a full scan (dict is tiny) so we don't need the user_id in + ``send()``, which only receives the channel ID from base.py. + """ + now = time.monotonic() + # Clean up stale entries on every lookup — dict is small. + stale_keys = [ + k for k, v in self._slash_command_contexts.items() + if now - v["ts"] > self._SLASH_CTX_TTL + ] + for k in stale_keys: + self._slash_command_contexts.pop(k, None) + + # Find the context for this channel (may be keyed under any user). + match_key = None + for key in list(self._slash_command_contexts): + if key[0] == chat_id: + match_key = key + break + if match_key is None: + return None + return self._slash_command_contexts.pop(match_key) + + async def _send_slash_ephemeral( + self, + ctx: Dict[str, Any], + content: str, + ) -> "SendResult": + """Replace the initial ephemeral ack via ``response_url``. + + Slack's ``response_url`` accepts a POST with ``replace_original`` + for up to 30 minutes after the slash command was invoked. This + lets us swap the "Running /cmd…" placeholder with the real reply, + and the message stays ephemeral ("Only visible to you"). + + Falls back to a simple ``True`` SendResult if the POST fails — + the user already saw the initial ack, so a delivery failure here + is non-critical. + """ + formatted = self.format_message(content) + payload = { + "response_type": "ephemeral", + "replace_original": True, + "text": formatted, + } + try: + async with aiohttp.ClientSession() as session: + async with session.post( + ctx["response_url"], + json=payload, + timeout=aiohttp.ClientTimeout(total=10), + ) as resp: + if resp.status == 200: + return SendResult(success=True, message_id=None) + body = await resp.text() + logger.warning( + "[Slack] response_url POST returned %s: %s", + resp.status, + body[:200], + ) + except Exception as e: + logger.warning( + "[Slack] response_url POST failed: %s", e, + ) + # Non-fatal — the user saw the initial ack already. + return SendResult(success=True, message_id=None) + async def connect(self) -> bool: """Connect to Slack via Socket Mode.""" if not SLACK_AVAILABLE: @@ -502,7 +588,11 @@ class SlackAdapter(BasePlatformAdapter): @self._app.command(_slash_pattern) async def handle_hermes_command(ack, command): - await ack() + slash = (command.get("command") or "").lstrip("/") + await ack( + response_type="ephemeral", + text=f"Running `/{slash}`…", + ) await self._handle_slash_command(command) # Register Block Kit action handlers for approval buttons @@ -574,6 +664,17 @@ class SlackAdapter(BasePlatformAdapter): return SendResult(success=False, error="Not connected") try: + # Check for a pending slash-command context. When the user ran a + # native slash command (e.g. /q, /stop, /model), the initial ack + # already showed an ephemeral "Running /cmd…" message. If we have + # a stashed response_url for this channel, replace that ack with + # the actual command reply ephemerally instead of posting publicly. + slash_ctx = self._pop_slash_context(chat_id) + if slash_ctx: + return await self._send_slash_ephemeral( + slash_ctx, content, + ) + # Convert standard markdown → Slack mrkdwn formatted = self.format_message(content) @@ -2537,6 +2638,16 @@ class SlackAdapter(BasePlatformAdapter): raw_message=command, ) + # Stash the Slack response_url so the first reply for this + # channel+user can be routed ephemerally (replaces the initial + # "Running /cmd…" ack shown by handle_hermes_command). + response_url = command.get("response_url", "") + if response_url and user_id and channel_id: + self._slash_command_contexts[(channel_id, user_id)] = { + "response_url": response_url, + "ts": time.monotonic(), + } + await self.handle_message(event) def _has_active_session_for_thread( diff --git a/tests/gateway/test_slack.py b/tests/gateway/test_slack.py index ef9897bd..5830d1f5 100644 --- a/tests/gateway/test_slack.py +++ b/tests/gateway/test_slack.py @@ -53,6 +53,9 @@ def _ensure_slack_mock(): ]: sys.modules.setdefault(name, mod) + # aiohttp is imported alongside slack-bolt; mock it if missing + sys.modules.setdefault("aiohttp", MagicMock()) + _ensure_slack_mock() @@ -2586,3 +2589,205 @@ class TestSlackReplyToText: assert msg_event.reply_to_text is None # Top-level message: reply_to_message_id must be falsy (None or empty). assert not msg_event.reply_to_message_id + + +# --------------------------------------------------------------------------- +# Slash-command ephemeral ack and routing (#18182) +# --------------------------------------------------------------------------- + + +class TestSlashEphemeralAck: + """Slash commands should produce an ephemeral ack and route replies ephemerally.""" + + @pytest.mark.asyncio + async def test_slash_command_stashes_response_url(self, adapter): + """_handle_slash_command stashes response_url for later ephemeral routing.""" + command = { + "command": "/q", + "text": "follow-up question", + "user_id": "U_SLASH", + "channel_id": "C_SLASH", + "response_url": "https://hooks.slack.com/commands/T123/456/abc", + } + await adapter._handle_slash_command(command) + + # The context should be stashed under (channel_id, user_id). + key = ("C_SLASH", "U_SLASH") + assert key in adapter._slash_command_contexts + ctx = adapter._slash_command_contexts[key] + assert ctx["response_url"] == "https://hooks.slack.com/commands/T123/456/abc" + assert "ts" in ctx + + @pytest.mark.asyncio + async def test_slash_command_without_response_url_does_not_stash(self, adapter): + """Commands without a response_url should not create a context.""" + command = { + "command": "/stop", + "text": "", + "user_id": "U1", + "channel_id": "C1", + # no response_url + } + await adapter._handle_slash_command(command) + assert len(adapter._slash_command_contexts) == 0 + + @pytest.mark.asyncio + async def test_pop_slash_context_returns_and_removes(self, adapter): + """_pop_slash_context returns the context and removes it.""" + import time + adapter._slash_command_contexts[("C1", "U1")] = { + "response_url": "https://hooks.slack.com/test", + "ts": time.monotonic(), + } + + ctx = adapter._pop_slash_context("C1") + assert ctx is not None + assert ctx["response_url"] == "https://hooks.slack.com/test" + # Must be removed after pop + assert len(adapter._slash_command_contexts) == 0 + + @pytest.mark.asyncio + async def test_pop_slash_context_returns_none_for_no_match(self, adapter): + """_pop_slash_context returns None when no context exists.""" + ctx = adapter._pop_slash_context("C_NONEXISTENT") + assert ctx is None + + @pytest.mark.asyncio + async def test_pop_slash_context_discards_stale_entries(self, adapter): + """Stale contexts older than TTL are cleaned up.""" + import time + adapter._slash_command_contexts[("C1", "U1")] = { + "response_url": "https://hooks.slack.com/stale", + "ts": time.monotonic() - adapter._SLASH_CTX_TTL - 1, + } + + ctx = adapter._pop_slash_context("C1") + assert ctx is None + assert len(adapter._slash_command_contexts) == 0 + + @pytest.mark.asyncio + async def test_send_uses_response_url_when_context_exists(self, adapter): + """send() should POST to response_url for slash command replies.""" + import time + adapter._slash_command_contexts[("C_SLASH", "U_SLASH")] = { + "response_url": "https://hooks.slack.com/commands/T123/456/abc", + "ts": time.monotonic(), + } + + mock_resp = AsyncMock() + mock_resp.status = 200 + mock_resp.__aenter__ = AsyncMock(return_value=mock_resp) + mock_resp.__aexit__ = AsyncMock(return_value=False) + + mock_session = AsyncMock() + mock_session.post = MagicMock(return_value=mock_resp) + mock_session.__aenter__ = AsyncMock(return_value=mock_session) + mock_session.__aexit__ = AsyncMock(return_value=False) + + with patch("gateway.platforms.slack.aiohttp.ClientSession", return_value=mock_session): + result = await adapter.send("C_SLASH", "Queued for the next turn.") + + assert result.success is True + # Verify response_url was POSTed to + mock_session.post.assert_called_once() + call_args = mock_session.post.call_args + assert call_args[0][0] == "https://hooks.slack.com/commands/T123/456/abc" + payload = call_args[1]["json"] + assert payload["response_type"] == "ephemeral" + assert payload["replace_original"] is True + assert "Queued for the next turn" in payload["text"] + + # Context must be consumed + assert len(adapter._slash_command_contexts) == 0 + + @pytest.mark.asyncio + async def test_send_falls_through_without_context(self, adapter): + """send() should use normal chat_postMessage when no slash context exists.""" + mock_result = {"ts": "1234.5678", "ok": True} + adapter._app.client.chat_postMessage = AsyncMock(return_value=mock_result) + + result = await adapter.send("C_NORMAL", "Hello world") + + assert result.success is True + adapter._app.client.chat_postMessage.assert_called_once() + + @pytest.mark.asyncio + async def test_send_slash_ephemeral_fallback_on_post_failure(self, adapter): + """_send_slash_ephemeral returns success=True even if POST fails.""" + import time + adapter._slash_command_contexts[("C1", "U1")] = { + "response_url": "https://hooks.slack.com/commands/bad", + "ts": time.monotonic(), + } + + mock_resp = AsyncMock() + mock_resp.status = 500 + mock_resp.text = AsyncMock(return_value="Internal Server Error") + mock_resp.__aenter__ = AsyncMock(return_value=mock_resp) + mock_resp.__aexit__ = AsyncMock(return_value=False) + + mock_session = AsyncMock() + mock_session.post = MagicMock(return_value=mock_resp) + mock_session.__aenter__ = AsyncMock(return_value=mock_session) + mock_session.__aexit__ = AsyncMock(return_value=False) + + with patch("gateway.platforms.slack.aiohttp.ClientSession", return_value=mock_session): + result = await adapter.send("C1", "Some response") + + # Still success — the user saw the initial ack already + assert result.success is True + + @pytest.mark.asyncio + async def test_send_slash_ephemeral_fallback_on_exception(self, adapter): + """_send_slash_ephemeral returns success=True even if aiohttp raises.""" + import time + adapter._slash_command_contexts[("C1", "U1")] = { + "response_url": "https://hooks.slack.com/commands/timeout", + "ts": time.monotonic(), + } + + mock_session = AsyncMock() + mock_session.post = MagicMock(side_effect=Exception("connection timeout")) + mock_session.__aenter__ = AsyncMock(return_value=mock_session) + mock_session.__aexit__ = AsyncMock(return_value=False) + + with patch("gateway.platforms.slack.aiohttp.ClientSession", return_value=mock_session): + result = await adapter.send("C1", "Some response") + + assert result.success is True + + @pytest.mark.asyncio + async def test_native_slash_stashes_context_and_dispatches(self, adapter): + """Full flow: native /q slash → stash + handle_message dispatch.""" + command = { + "command": "/q", + "text": "do something", + "user_id": "U_Q", + "channel_id": "C_Q", + "response_url": "https://hooks.slack.com/commands/T1/2/q", + } + await adapter._handle_slash_command(command) + + # 1. handle_message was called with the right event + adapter.handle_message.assert_called_once() + event = adapter.handle_message.call_args[0][0] + assert event.text == "/q do something" + assert event.message_type == MessageType.COMMAND + + # 2. Context stashed for ephemeral routing + assert ("C_Q", "U_Q") in adapter._slash_command_contexts + + @pytest.mark.asyncio + async def test_legacy_hermes_slash_stashes_context(self, adapter): + """Legacy /hermes also stashes context.""" + command = { + "command": "/hermes", + "text": "help", + "user_id": "U_H", + "channel_id": "C_H", + "response_url": "https://hooks.slack.com/commands/T1/3/h", + } + await adapter._handle_slash_command(command) + + adapter.handle_message.assert_called_once() + assert ("C_H", "U_H") in adapter._slash_command_contexts