fix(slack): prefer live send adapter and try multi-workspace tokens individually
Two related Slack delivery fixes for send_message text sends: - Route Slack text delivery through _send_via_adapter so the live in-process gateway adapter (multi-workspace aware, channel→client mapping, adapter-side gates) is preferred, with the plugin's _standalone_send as the out-of-process fallback — matching how the media path already behaves. - _standalone_send: SLACK_BOT_TOKEN can be a comma-separated list in multi-workspace installs and slack_tokens.json carries OAuth per-workspace tokens; the standalone Web-API path used to send the literal comma-joined string, which Slack rejects as invalid_auth. Try each token individually, retrying on token-scoped errors (invalid_auth / not_in_channel / channel_not_found …) and stopping on terminal ones. User-DM resolution (U…/W… targets) also tries each token. Adapted from #47547 by @replygirl — the original patched the legacy tools/send_message_tool.py::_send_slack helper, which moved to the Slack plugin's _standalone_send in #41112. Salvaged from #47547
This commit is contained in:
replygirl
Teknium
parent
8685fea0ce
commit
d9fe008db8
|
|
@ -8606,9 +8606,29 @@ async def _standalone_send(
|
|||
``chat.postMessage``.
|
||||
"""
|
||||
del force_document # signature parity with other standalone senders
|
||||
token = getattr(pconfig, "token", None) or os.getenv("SLACK_BOT_TOKEN", "")
|
||||
if not token:
|
||||
raw_token = getattr(pconfig, "token", None) or os.getenv("SLACK_BOT_TOKEN", "")
|
||||
|
||||
# ``SLACK_BOT_TOKEN`` can be a comma-separated list in multi-workspace
|
||||
# gateways, and OAuth installs persist per-workspace tokens in
|
||||
# slack_tokens.json. The standalone path has no team→client map, so try
|
||||
# each token individually instead of sending the literal comma-joined
|
||||
# string, which Slack rejects as ``invalid_auth`` (#47547).
|
||||
tokens = [t.strip() for t in str(raw_token or "").split(",") if t.strip()]
|
||||
try:
|
||||
from hermes_constants import get_hermes_home
|
||||
|
||||
_tokens_file = get_hermes_home() / "slack_tokens.json"
|
||||
if _tokens_file.exists():
|
||||
_saved = json.loads(_tokens_file.read_text(encoding="utf-8"))
|
||||
for _entry in _saved.values():
|
||||
_tok = _entry.get("token", "") if isinstance(_entry, dict) else ""
|
||||
if _tok and _tok not in tokens:
|
||||
tokens.append(_tok)
|
||||
except Exception:
|
||||
pass
|
||||
if not tokens:
|
||||
return {"error": "Slack send failed: SLACK_BOT_TOKEN not configured"}
|
||||
token = tokens[0]
|
||||
|
||||
# User-targeted delivery: chat.postMessage / files_upload_v2 reject bare
|
||||
# user IDs (U.../W...) — resolve to a DM conversation ID (D...) first via
|
||||
|
|
@ -8616,7 +8636,12 @@ async def _standalone_send(
|
|||
# instead of failing with channel_not_found (#17444).
|
||||
chat_id = str(chat_id or "")
|
||||
if chat_id[:1] in ("U", "W"):
|
||||
resolved = await _resolve_slack_user_dm(token, chat_id)
|
||||
resolved = None
|
||||
for _tok in tokens:
|
||||
resolved = await _resolve_slack_user_dm(_tok, chat_id)
|
||||
if resolved is not None:
|
||||
token = _tok
|
||||
break
|
||||
if resolved is None:
|
||||
return {
|
||||
"error": (
|
||||
|
|
@ -8771,20 +8796,32 @@ async def _standalone_send(
|
|||
_proxy = resolve_proxy_url()
|
||||
_sess_kw, _req_kw = proxy_kwargs_for_aiohttp(_proxy)
|
||||
url = "https://slack.com/api/chat.postMessage"
|
||||
headers = {
|
||||
"Authorization": f"Bearer {token}",
|
||||
"Content-Type": "application/json",
|
||||
# Errors that mean "wrong workspace token for this channel" — worth
|
||||
# retrying with the next token. Anything else is terminal.
|
||||
retryable_token_errors = {
|
||||
"invalid_auth",
|
||||
"not_authed",
|
||||
"token_revoked",
|
||||
"account_inactive",
|
||||
"not_in_channel",
|
||||
"channel_not_found",
|
||||
}
|
||||
last_error = "unknown"
|
||||
async with aiohttp.ClientSession(
|
||||
timeout=aiohttp.ClientTimeout(total=30), **_sess_kw
|
||||
) as session:
|
||||
payload = {"channel": chat_id, "text": formatted, "mrkdwn": True}
|
||||
if thread_id:
|
||||
payload["thread_ts"] = thread_id
|
||||
async with session.post(
|
||||
url, headers=headers, json=payload, **_req_kw
|
||||
) as resp:
|
||||
data = await resp.json()
|
||||
for tok in tokens:
|
||||
headers = {
|
||||
"Authorization": f"Bearer {tok}",
|
||||
"Content-Type": "application/json",
|
||||
}
|
||||
async with session.post(
|
||||
url, headers=headers, json=payload, **_req_kw
|
||||
) as resp:
|
||||
data = await resp.json()
|
||||
if data.get("ok"):
|
||||
return {
|
||||
"success": True,
|
||||
|
|
@ -8792,7 +8829,10 @@ async def _standalone_send(
|
|||
"chat_id": chat_id,
|
||||
"message_id": data.get("ts"),
|
||||
}
|
||||
return {"error": f"Slack API error: {data.get('error', 'unknown')}"}
|
||||
last_error = data.get("error", "unknown")
|
||||
if last_error not in retryable_token_errors:
|
||||
break
|
||||
return {"error": f"Slack API error: {last_error}"}
|
||||
except Exception as e:
|
||||
return {"error": f"Slack send failed: {e}"}
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,163 @@
|
|||
"""Slack-specific send_message delivery regressions.
|
||||
|
||||
Salvaged from #47547 and adapted to the post-#41112 plugin layout: the legacy
|
||||
``_send_slack`` helper moved to ``plugins/platforms/slack/adapter.py::
|
||||
_standalone_send`` and text sends now route through ``_send_via_adapter``
|
||||
(live adapter first, registry standalone fallback).
|
||||
"""
|
||||
|
||||
import asyncio
|
||||
import sys
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from gateway.config import Platform
|
||||
from tools.send_message_tool import _send_to_platform
|
||||
|
||||
|
||||
def _ensure_slack_mock(monkeypatch):
|
||||
"""Install lightweight Slack modules when optional Slack deps are absent."""
|
||||
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),
|
||||
]:
|
||||
monkeypatch.setitem(sys.modules, name, mod)
|
||||
|
||||
|
||||
def test_slack_send_to_platform_routes_through_send_via_adapter(monkeypatch):
|
||||
"""Slack text sends go through _send_via_adapter (live adapter first)."""
|
||||
_ensure_slack_mock(monkeypatch)
|
||||
|
||||
live_send = AsyncMock(return_value={"success": True, "message_id": "live-ts"})
|
||||
|
||||
with patch("tools.send_message_tool._send_via_adapter", live_send):
|
||||
result = asyncio.run(
|
||||
_send_to_platform(
|
||||
Platform.SLACK,
|
||||
SimpleNamespace(enabled=True, token="bad-token,good-token", extra={}),
|
||||
"C123",
|
||||
"**hello** from Hermes",
|
||||
thread_id="171.1",
|
||||
)
|
||||
)
|
||||
|
||||
assert result == {"success": True, "message_id": "live-ts"}
|
||||
live_send.assert_awaited_once()
|
||||
call = live_send.await_args
|
||||
assert call.args[0] == Platform.SLACK
|
||||
assert call.args[2] == "C123"
|
||||
assert call.kwargs["thread_id"] == "171.1"
|
||||
|
||||
|
||||
class _SlackResponse:
|
||||
def __init__(self, payload):
|
||||
self._payload = payload
|
||||
|
||||
async def json(self):
|
||||
return self._payload
|
||||
|
||||
|
||||
class _SlackPostContext:
|
||||
def __init__(self, response):
|
||||
self._response = response
|
||||
|
||||
async def __aenter__(self):
|
||||
return self._response
|
||||
|
||||
async def __aexit__(self, exc_type, exc, tb):
|
||||
return False
|
||||
|
||||
|
||||
class _SlackSession:
|
||||
"""Fake aiohttp session whose good-token posts succeed."""
|
||||
|
||||
def __init__(self):
|
||||
self.calls = []
|
||||
|
||||
async def __aenter__(self):
|
||||
return self
|
||||
|
||||
async def __aexit__(self, exc_type, exc, tb):
|
||||
return False
|
||||
|
||||
def post(self, url, *, headers, json, **kwargs):
|
||||
token = headers["Authorization"].removeprefix("Bearer ")
|
||||
self.calls.append((token, json))
|
||||
if token == "good-token":
|
||||
payload = {"ok": True, "ts": "171.123"}
|
||||
else:
|
||||
payload = {"ok": False, "error": "invalid_auth"}
|
||||
return _SlackPostContext(_SlackResponse(payload))
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def _standalone_send(monkeypatch):
|
||||
_ensure_slack_mock(monkeypatch)
|
||||
from plugins.platforms.slack import adapter as slack_adapter
|
||||
|
||||
return slack_adapter._standalone_send
|
||||
|
||||
|
||||
def test_standalone_send_tries_comma_separated_tokens_individually(
|
||||
monkeypatch, _standalone_send
|
||||
):
|
||||
"""Multi-workspace token lists must not be sent as one literal token."""
|
||||
fake_session = _SlackSession()
|
||||
monkeypatch.setattr(
|
||||
"aiohttp.ClientSession", lambda *args, **kwargs: fake_session
|
||||
)
|
||||
|
||||
pconfig = SimpleNamespace(enabled=True, token="bad-token, good-token", extra={})
|
||||
result = asyncio.run(_standalone_send(pconfig, "C123", "hello"))
|
||||
|
||||
assert result == {
|
||||
"success": True,
|
||||
"platform": "slack",
|
||||
"chat_id": "C123",
|
||||
"message_id": "171.123",
|
||||
}
|
||||
assert [token for token, _payload in fake_session.calls] == [
|
||||
"bad-token",
|
||||
"good-token",
|
||||
]
|
||||
|
||||
|
||||
def test_standalone_send_stops_on_non_token_error(monkeypatch, _standalone_send):
|
||||
"""Terminal errors (not token-scoped) must not burn the remaining tokens."""
|
||||
|
||||
class _FatalSession(_SlackSession):
|
||||
def post(self, url, *, headers, json, **kwargs):
|
||||
token = headers["Authorization"].removeprefix("Bearer ")
|
||||
self.calls.append((token, json))
|
||||
return _SlackPostContext(
|
||||
_SlackResponse({"ok": False, "error": "msg_too_long"})
|
||||
)
|
||||
|
||||
fake_session = _FatalSession()
|
||||
monkeypatch.setattr(
|
||||
"aiohttp.ClientSession", lambda *args, **kwargs: fake_session
|
||||
)
|
||||
|
||||
pconfig = SimpleNamespace(enabled=True, token="tok-a,tok-b", extra={})
|
||||
result = asyncio.run(_standalone_send(pconfig, "C123", "hello"))
|
||||
|
||||
assert result == {"error": "Slack API error: msg_too_long"}
|
||||
assert len(fake_session.calls) == 1
|
||||
|
|
@ -1071,20 +1071,20 @@ async def _send_to_platform(platform, pconfig, chat_id, message, thread_id=None,
|
|||
last_result = result
|
||||
return last_result
|
||||
|
||||
# --- Slack: route both text and native files through the plugin's
|
||||
# standalone sender. This path is used by out-of-process cron runs where
|
||||
# no live gateway adapter is available; dropping ``media_files`` here made
|
||||
# MEDIA directives disappear while the text delivery still reported
|
||||
# success.
|
||||
# --- Slack: prefer the live gateway adapter, then the plugin's
|
||||
# standalone sender. The live adapter is multi-workspace aware (it maps
|
||||
# channels to the workspace client that owns them) and honors adapter-side
|
||||
# gates like ignored_channels; the standalone Web-API path may only have a
|
||||
# comma-separated token list. ``_send_via_adapter`` tries the in-process
|
||||
# adapter first and falls back to the registry standalone sender for
|
||||
# out-of-process cron runs, preserving MEDIA delivery on the fallback
|
||||
# (media-bearing sends were already intercepted by the branch above).
|
||||
if platform == Platform.SLACK:
|
||||
from gateway.platform_registry import platform_registry
|
||||
entry = platform_registry.get("slack")
|
||||
if entry is None or entry.standalone_sender_fn is None:
|
||||
return {"error": "Slack plugin not registered or missing standalone_sender_fn"}
|
||||
last_result = None
|
||||
for i, chunk in enumerate(chunks):
|
||||
is_last = i == len(chunks) - 1
|
||||
result = await entry.standalone_sender_fn(
|
||||
result = await _send_via_adapter(
|
||||
platform,
|
||||
pconfig,
|
||||
chat_id,
|
||||
chunk,
|
||||
|
|
|
|||
Loading…
Reference in New Issue