diff --git a/gateway/platforms/slack.py b/gateway/platforms/slack.py index 443f684e4b..26282b134d 100644 --- a/gateway/platforms/slack.py +++ b/gateway/platforms/slack.py @@ -15,7 +15,7 @@ import os import re import time from dataclasses import dataclass, field -from typing import Dict, Optional, Any, Tuple +from typing import Dict, Optional, Any, Tuple, List try: from slack_bolt.async_app import AsyncApp @@ -121,6 +121,63 @@ class SlackAdapter(BasePlatformAdapter): # clear them (chat_id → thread_ts). self._active_status_threads: Dict[str, str] = {} + 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.""" + if response is None or not hasattr(response, "get"): + return None + + error = str(response.get("error", "") or "").strip() + if not error: + return None + + file_label = str((file_obj or {}).get("name") or (file_obj or {}).get("id") or "this attachment") + needed = str(response.get("needed", "") or "").strip() + provided = str(response.get("provided", "") or "").strip() + reinstall_hint = " Update the Slack app scopes/settings and reinstall the app to the workspace." + provided_hint = f" Current bot scopes: {provided}." if provided else "" + + if error == "missing_scope": + needed_hint = f"Missing scope: {needed}." if needed else "Missing required Slack scope." + return f"Slack attachment access failed for {file_label}. {needed_hint}{provided_hint}{reinstall_hint}" + if error in {"not_authed", "invalid_auth", "account_inactive", "token_revoked"}: + return f"Slack attachment access failed for {file_label} because the bot token is not authorized ({error}). Refresh the token/reinstall the app." + if error in {"file_not_found", "file_deleted"}: + return f"Slack attachment {file_label} is no longer available ({error})." + if error in {"access_denied", "file_access_denied", "no_permission", "not_allowed_token_type", "restricted_action"}: + return f"Slack attachment access failed for {file_label} because the bot does not have permission ({error}). Check workspace permissions/scopes and reinstall if needed." + return None + + def _describe_slack_download_failure(self, exc: Exception, *, file_obj: Optional[Dict[str, Any]] = None) -> Optional[str]: + """Translate Slack download exceptions into user-facing attachment diagnostics.""" + file_label = str((file_obj or {}).get("name") or (file_obj or {}).get("id") or "this attachment") + + response = getattr(exc, "response", None) + api_detail = self._describe_slack_api_error(response, file_obj=file_obj) + if api_detail: + return api_detail + + try: + import httpx + except Exception: # pragma: no cover + httpx = None + + if httpx is not None and isinstance(exc, httpx.HTTPStatusError): + status = exc.response.status_code + if status == 401: + return f"Slack attachment access failed for {file_label} with HTTP 401. The bot token is not authorized for this file." + if status == 403: + return f"Slack attachment access failed for {file_label} with HTTP 403. The bot likely lacks permission or scope to read this file." + if status == 404: + return f"Slack attachment {file_label} returned HTTP 404 and is no longer reachable." + + message = str(exc) + if "Slack returned HTML instead of media" in message or "non-image data" in message: + return ( + f"Slack attachment access failed for {file_label}: Slack returned an HTML/login or non-media response. " + "This usually means a scope, auth, or file-permission problem." + ) + return None + async def connect(self) -> bool: """Connect to Slack via Socket Mode.""" if not SLACK_AVAILABLE: @@ -1193,6 +1250,7 @@ class SlackAdapter(BasePlatformAdapter): # Handle file attachments media_urls = [] media_types = [] + attachment_notices: List[str] = [] files = event.get("files", []) for f in files: # Slack Connect channels return stub file objects with @@ -1209,13 +1267,24 @@ class SlackAdapter(BasePlatformAdapter): if info_resp.get("ok"): f = info_resp["file"] else: - logger.warning( - "[Slack] files.info failed for %s: %s", - file_id, info_resp.get("error"), - ) + detail = self._describe_slack_api_error(info_resp, file_obj=f) + if detail: + attachment_notices.append(detail) + logger.warning("[Slack] %s", detail) + else: + logger.warning( + "[Slack] files.info failed for %s: %s", + file_id, info_resp.get("error"), + ) continue except Exception as e: - logger.warning("[Slack] files.info error for %s: %s", file_id, e, exc_info=True) + response = getattr(e, "response", None) + detail = self._describe_slack_api_error(response, file_obj=f) + if detail: + attachment_notices.append(detail) + logger.warning("[Slack] %s", detail) + else: + logger.warning("[Slack] files.info error for %s: %s", file_id, e, exc_info=True) continue mimetype = f.get("mimetype", "unknown") @@ -1231,7 +1300,12 @@ class SlackAdapter(BasePlatformAdapter): media_types.append(mimetype) msg_type = MessageType.PHOTO except Exception as e: # pragma: no cover - defensive logging - logger.warning("[Slack] Failed to cache image from %s: %s", url, e, exc_info=True) + detail = self._describe_slack_download_failure(e, file_obj=f) + if detail: + attachment_notices.append(detail) + logger.warning("[Slack] %s", detail) + else: + logger.warning("[Slack] Failed to cache image from %s: %s", url, e, exc_info=True) elif mimetype.startswith("audio/") and url: try: ext = "." + mimetype.split("/")[-1].split(";")[0] @@ -1242,7 +1316,12 @@ class SlackAdapter(BasePlatformAdapter): media_types.append(mimetype) msg_type = MessageType.VOICE except Exception as e: # pragma: no cover - defensive logging - logger.warning("[Slack] Failed to cache audio from %s: %s", url, e, exc_info=True) + detail = self._describe_slack_download_failure(e, file_obj=f) + if detail: + attachment_notices.append(detail) + logger.warning("[Slack] %s", detail) + else: + logger.warning("[Slack] Failed to cache audio from %s: %s", url, e, exc_info=True) elif url: # Try to handle as a document attachment try: @@ -1294,7 +1373,16 @@ class SlackAdapter(BasePlatformAdapter): pass # Binary content, skip injection except Exception as e: # pragma: no cover - defensive logging - logger.warning("[Slack] Failed to cache document from %s: %s", url, e, exc_info=True) + detail = self._describe_slack_download_failure(e, file_obj=f) + if detail: + attachment_notices.append(detail) + logger.warning("[Slack] %s", detail) + else: + logger.warning("[Slack] Failed to cache document from %s: %s", url, e, exc_info=True) + + if attachment_notices: + notice_block = "[Slack attachment notice]\n" + "\n".join(f"- {n}" for n in attachment_notices) + text = f"{notice_block}\n\n{text}" if text else notice_block # Resolve user display name (cached after first lookup) user_name = await self._resolve_user_name(user_id, chat_id=channel_id) diff --git a/tests/gateway/test_media_download_retry.py b/tests/gateway/test_media_download_retry.py index 5b5add26c2..373ced1017 100644 --- a/tests/gateway/test_media_download_retry.py +++ b/tests/gateway/test_media_download_retry.py @@ -540,7 +540,7 @@ from gateway.config import Platform, PlatformConfig # noqa: E402 def _make_slack_adapter(): - config = PlatformConfig(enabled=True, token="xoxb-fake-token") + config = PlatformConfig(enabled=True, token="***") adapter = SlackAdapter(config) adapter._app = MagicMock() adapter._app.client = AsyncMock() @@ -549,6 +549,39 @@ def _make_slack_adapter(): return adapter +# --------------------------------------------------------------------------- +# SlackAdapter diagnostics helpers +# --------------------------------------------------------------------------- + +class TestSlackAttachmentDiagnostics: + def test_missing_scope_error_returns_actionable_notice(self): + """_describe_slack_api_error translates a missing_scope response into + a user-facing notice mentioning the needed scope and the reinstall + step. This is the helper used by every files.info call site (Slack + Connect stubs + post-download failures) to surface scope problems + without making an extra probe call per attachment. + """ + adapter = _make_slack_adapter() + + response = { + "error": "missing_scope", + "needed": "files:read", + "provided": "chat:write,files:write", + } + detail = adapter._describe_slack_api_error(response, file_obj={"id": "F123", "name": "photo.jpg"}) + assert detail is not None + assert "files:read" in detail + assert "reinstall" in detail.lower() + assert "chat:write,files:write" in detail + + def test_download_failure_403_returns_permission_notice(self): + adapter = _make_slack_adapter() + exc = _make_http_status_error(403) + detail = adapter._describe_slack_download_failure(exc, file_obj={"name": "report.pdf"}) + assert "403" in detail + assert "permission or scope" in detail + + # --------------------------------------------------------------------------- # SlackAdapter._download_slack_file # --------------------------------------------------------------------------- diff --git a/tests/gateway/test_slack.py b/tests/gateway/test_slack.py index de570173a2..e578006186 100644 --- a/tests/gateway/test_slack.py +++ b/tests/gateway/test_slack.py @@ -511,6 +511,35 @@ class TestIncomingDocumentHandling: msg_event = adapter.handle_message.call_args[0][0] assert msg_event.message_type == MessageType.PHOTO + @pytest.mark.asyncio + async def test_download_failure_is_surfaced_in_message_text(self, adapter): + """Attachment download failures (401/403/HTML-body/etc.) should be + translated into a user-facing `[Slack attachment notice]` block so + the agent can tell the user what to fix (e.g. missing files:read + scope). No proactive files.info probe is made — the diagnostic + runs only when the download actually fails. + """ + import httpx + req = httpx.Request("GET", "https://files.slack.com/photo.jpg") + resp = httpx.Response(403, request=req) + + with patch.object(adapter, "_download_slack_file", new_callable=AsyncMock) as dl: + dl.side_effect = httpx.HTTPStatusError("403", request=req, response=resp) + event = self._make_event(text="what's in this?", files=[{ + "id": "F123", + "mimetype": "image/jpeg", + "name": "photo.jpg", + "url_private_download": "https://files.slack.com/photo.jpg", + "size": 1024, + }]) + await adapter._handle_slack_message(event) + + msg_event = adapter.handle_message.call_args[0][0] + assert msg_event.message_type == MessageType.TEXT + assert "[Slack attachment notice]" in msg_event.text + assert "403" in msg_event.text + assert "what's in this?" in msg_event.text + # --------------------------------------------------------------------------- # TestMessageRouting diff --git a/website/docs/user-guide/messaging/slack.md b/website/docs/user-guide/messaging/slack.md index 2f598fcfe9..696f4e065e 100644 --- a/website/docs/user-guide/messaging/slack.md +++ b/website/docs/user-guide/messaging/slack.md @@ -82,7 +82,8 @@ Navigate to **Features → OAuth & Permissions** in the sidebar. Scroll to **Sco :::caution Missing scopes = missing features Without `channels:history` and `groups:history`, the bot **will not receive messages in channels** — -it will only work in DMs. These are the most commonly missed scopes. +it will only work in DMs. Without `files:read`, Hermes can chat but **cannot reliably read user-uploaded attachments**. +These are the most commonly missed scopes. ::: **Optional scopes:** @@ -520,7 +521,8 @@ Keys are Slack channel IDs (find them via channel details → "About" → scroll | "Sending messages to this app has been turned off" in DMs | Enable the **Messages Tab** in App Home settings (see Step 5) | | "not_authed" or "invalid_auth" errors | Regenerate your Bot Token and App Token, update `.env` | | Bot responds but can't post in a channel | Invite the bot to the channel with `/invite @Hermes Agent` | -| "missing_scope" error | Add the required scope in OAuth & Permissions, then **reinstall** the app | +| Bot can chat but can't read uploaded images/files | Add `files:read`, then **reinstall** the app. Hermes now surfaces attachment access diagnostics in-chat when Slack returns scope/auth/permission failures. | +| `missing_scope` error | Add the required scope in OAuth & Permissions, then **reinstall** the app | | Socket disconnects frequently | Check your network; Bolt auto-reconnects but unstable connections cause lag | | Changed scopes/events but nothing changed | You **must reinstall** the app to your workspace after any scope or event subscription change |