mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-23 16:36:23 +00:00
fix(slack): resolve user IDs to DM channels in standalone cron delivery
Cron jobs targeting a Slack user (deliver=slack:U…) failed with channel_not_found: chat.postMessage and files_upload_v2 require a conversation ID, and a DM must be opened first via conversations.open to obtain a D… ID. The tool-level resolution in send_message() does not cover cron's direct _send_to_platform → standalone_sender_fn path. Add _resolve_slack_user_dm to the Slack plugin: resolves U…/W… targets via conversations.open (proxy-aware, cached per token+user so repeated cron fires don't re-open the DM), wired into _standalone_send ahead of both the text and media delivery branches so every standalone entry point benefits. Adapted from #17444 by @Esther-Zhu023 — the original patched the legacy _send_slack helper in tools/send_message_tool.py, which has since moved into the plugin as _standalone_send (#41112). Closes #17444 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Esther
Teknium
parent
4f726ed467
commit
c7b9dfa961
2 changed files with 204 additions and 0 deletions
|
|
@ -6472,6 +6472,60 @@ class SlackAdapter(BasePlatformAdapter):
|
|||
# ──────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
# Cache for Slack user ID -> DM conversation ID resolution in the standalone
|
||||
# send path. Keyed by "{token}:{user_id}" to support multi-workspace setups.
|
||||
_slack_dm_cache: Dict[str, str] = {}
|
||||
|
||||
|
||||
async def _resolve_slack_user_dm(token: str, user_id: str) -> Optional[str]:
|
||||
"""Resolve a Slack user ID (U.../W...) to a DM conversation ID (D...).
|
||||
|
||||
``chat.postMessage`` and ``files_upload_v2`` require a conversation ID; a
|
||||
DM must be opened first via ``conversations.open``. Results are cached
|
||||
per (token, user_id) pair to avoid redundant API calls. Returns None if
|
||||
resolution fails (missing ``im:write`` scope, unknown user, etc.).
|
||||
"""
|
||||
cache_key = f"{token}:{user_id}"
|
||||
if cache_key in _slack_dm_cache:
|
||||
return _slack_dm_cache[cache_key]
|
||||
|
||||
try:
|
||||
import aiohttp
|
||||
except ImportError:
|
||||
return None
|
||||
try:
|
||||
from gateway.platforms.base import proxy_kwargs_for_aiohttp
|
||||
|
||||
_proxy = resolve_proxy_url()
|
||||
_sess_kw, _req_kw = proxy_kwargs_for_aiohttp(_proxy)
|
||||
url = "https://slack.com/api/conversations.open"
|
||||
headers = {
|
||||
"Authorization": f"Bearer {token}",
|
||||
"Content-Type": "application/json",
|
||||
}
|
||||
async with aiohttp.ClientSession(
|
||||
timeout=aiohttp.ClientTimeout(total=15), **_sess_kw
|
||||
) as session:
|
||||
payload = {"users": user_id}
|
||||
async with session.post(
|
||||
url, headers=headers, json=payload, **_req_kw
|
||||
) as resp:
|
||||
data = await resp.json()
|
||||
if data.get("ok") and data.get("channel", {}).get("id"):
|
||||
channel_id = data["channel"]["id"]
|
||||
_slack_dm_cache[cache_key] = channel_id
|
||||
return channel_id
|
||||
logger.warning(
|
||||
"[Slack] conversations.open failed for %s: %s",
|
||||
user_id,
|
||||
data.get("error", "unknown"),
|
||||
)
|
||||
return None
|
||||
except Exception as e:
|
||||
logger.warning("[Slack] conversations.open exception for %s: %s", user_id, e)
|
||||
return None
|
||||
|
||||
|
||||
async def _standalone_send(
|
||||
pconfig,
|
||||
chat_id,
|
||||
|
|
@ -6496,6 +6550,22 @@ async def _standalone_send(
|
|||
if not token:
|
||||
return {"error": "Slack send failed: SLACK_BOT_TOKEN not configured"}
|
||||
|
||||
# User-targeted delivery: chat.postMessage / files_upload_v2 reject bare
|
||||
# user IDs (U.../W...) — resolve to a DM conversation ID (D...) first via
|
||||
# conversations.open so `deliver=slack:U…` cron jobs reach the user's DM
|
||||
# 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)
|
||||
if resolved is None:
|
||||
return {
|
||||
"error": (
|
||||
f"Slack user ID resolution failed for {chat_id} "
|
||||
"(conversations.open — check the bot's im:write scope)"
|
||||
)
|
||||
}
|
||||
chat_id = resolved
|
||||
|
||||
formatted = message
|
||||
if message:
|
||||
try:
|
||||
|
|
|
|||
|
|
@ -1251,6 +1251,140 @@ class TestStandaloneSendMedia:
|
|||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# TestStandaloneSendUserDmResolution
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestStandaloneSendUserDmResolution:
|
||||
"""_standalone_send resolves user IDs (U.../W...) to DM channels via
|
||||
conversations.open before posting (#17444). Cron `deliver=slack:U…`
|
||||
bypasses send_message()'s tool-level resolution, so the standalone
|
||||
sender must resolve on its own."""
|
||||
|
||||
@staticmethod
|
||||
def _mock_resp(payload):
|
||||
resp = MagicMock()
|
||||
resp.status = 200
|
||||
resp.json = AsyncMock(return_value=payload)
|
||||
resp.__aenter__ = AsyncMock(return_value=resp)
|
||||
resp.__aexit__ = AsyncMock(return_value=None)
|
||||
return resp
|
||||
|
||||
def _mock_session(self, *responses):
|
||||
session = MagicMock()
|
||||
session.__aenter__ = AsyncMock(return_value=session)
|
||||
session.__aexit__ = AsyncMock(return_value=None)
|
||||
session.post = MagicMock(side_effect=list(responses))
|
||||
return session
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_user_id_target_resolves_dm_then_posts(self):
|
||||
_slack_mod._slack_dm_cache.clear()
|
||||
open_resp = self._mock_resp({"ok": True, "channel": {"id": "D999888777"}})
|
||||
post_resp = self._mock_resp({"ok": True, "ts": "123.456"})
|
||||
session = self._mock_session(open_resp, post_resp)
|
||||
config = PlatformConfig(enabled=True, token="xoxb-fake-token")
|
||||
|
||||
with patch.object(_slack_mod.aiohttp, "ClientSession", return_value=session):
|
||||
result = await _slack_mod._standalone_send(
|
||||
config, "U1234567890", "hello via DM"
|
||||
)
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["chat_id"] == "D999888777"
|
||||
open_url = session.post.call_args_list[0].args[0]
|
||||
assert "conversations.open" in open_url
|
||||
assert session.post.call_args_list[0].kwargs["json"] == {"users": "U1234567890"}
|
||||
post_url = session.post.call_args_list[1].args[0]
|
||||
assert "chat.postMessage" in post_url
|
||||
assert session.post.call_args_list[1].kwargs["json"]["channel"] == "D999888777"
|
||||
_slack_mod._slack_dm_cache.clear()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_channel_id_skips_resolution(self):
|
||||
_slack_mod._slack_dm_cache.clear()
|
||||
post_resp = self._mock_resp({"ok": True, "ts": "123.456"})
|
||||
session = self._mock_session(post_resp)
|
||||
config = PlatformConfig(enabled=True, token="xoxb-fake-token")
|
||||
|
||||
with patch.object(_slack_mod.aiohttp, "ClientSession", return_value=session):
|
||||
result = await _slack_mod._standalone_send(config, "C123", "hello channel")
|
||||
|
||||
assert result["success"] is True
|
||||
assert session.post.call_count == 1
|
||||
assert "chat.postMessage" in session.post.call_args.args[0]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_user_id_resolution_failure_returns_error(self):
|
||||
_slack_mod._slack_dm_cache.clear()
|
||||
open_resp = self._mock_resp({"ok": False, "error": "user_not_found"})
|
||||
session = self._mock_session(open_resp)
|
||||
config = PlatformConfig(enabled=True, token="xoxb-fake-token")
|
||||
|
||||
with patch.object(_slack_mod.aiohttp, "ClientSession", return_value=session):
|
||||
result = await _slack_mod._standalone_send(config, "U9999999999", "hello")
|
||||
|
||||
assert "error" in result
|
||||
assert "user ID resolution failed" in result["error"]
|
||||
assert session.post.call_count == 1
|
||||
assert "conversations.open" in session.post.call_args.args[0]
|
||||
_slack_mod._slack_dm_cache.clear()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_user_id_resolution_cached_across_sends(self):
|
||||
_slack_mod._slack_dm_cache.clear()
|
||||
open_resp = self._mock_resp({"ok": True, "channel": {"id": "D555444333"}})
|
||||
post_resp1 = self._mock_resp({"ok": True, "ts": "1.1"})
|
||||
session1 = self._mock_session(open_resp, post_resp1)
|
||||
config = PlatformConfig(enabled=True, token="xoxb-fake-token")
|
||||
|
||||
with patch.object(_slack_mod.aiohttp, "ClientSession", return_value=session1):
|
||||
r1 = await _slack_mod._standalone_send(config, "U1112223334", "first")
|
||||
assert r1["success"] is True
|
||||
assert session1.post.call_count == 2
|
||||
|
||||
post_resp2 = self._mock_resp({"ok": True, "ts": "2.2"})
|
||||
session2 = self._mock_session(post_resp2)
|
||||
with patch.object(_slack_mod.aiohttp, "ClientSession", return_value=session2):
|
||||
r2 = await _slack_mod._standalone_send(config, "U1112223334", "second")
|
||||
assert r2["success"] is True
|
||||
assert r2["chat_id"] == "D555444333"
|
||||
assert session2.post.call_count == 1 # cache hit — no conversations.open
|
||||
_slack_mod._slack_dm_cache.clear()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_user_id_media_delivery_resolves_dm_before_upload(self, tmp_path):
|
||||
"""Media path composes with DM resolution: files_upload_v2 gets D…"""
|
||||
_slack_mod._slack_dm_cache.clear()
|
||||
image = tmp_path / "report.png"
|
||||
image.write_bytes(b"\x89PNG\r\n\x1a\n")
|
||||
open_resp = self._mock_resp({"ok": True, "channel": {"id": "D777666555"}})
|
||||
session = self._mock_session(open_resp)
|
||||
client = MagicMock()
|
||||
client.files_upload_v2 = AsyncMock(return_value={"ok": True, "ts": "9.9"})
|
||||
config = PlatformConfig(enabled=True, token="xoxb-fake-token")
|
||||
|
||||
with (
|
||||
patch.object(_slack_mod.aiohttp, "ClientSession", return_value=session),
|
||||
patch.object(_slack_mod, "AsyncWebClient", return_value=client),
|
||||
patch.object(_slack_mod, "resolve_proxy_url", return_value=None),
|
||||
):
|
||||
result = await _slack_mod._standalone_send(
|
||||
config,
|
||||
"U1234509876",
|
||||
"daily report",
|
||||
thread_id=None,
|
||||
media_files=[(str(image), False)],
|
||||
)
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["chat_id"] == "D777666555"
|
||||
client.files_upload_v2.assert_awaited_once()
|
||||
assert client.files_upload_v2.await_args.kwargs["channel"] == "D777666555"
|
||||
_slack_mod._slack_dm_cache.clear()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# TestSendDocument
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue