mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(slack): forward reaction_added events to the message pipeline
Slack reaction_added events were explicitly acked and dropped, so a user reacting to a bot message (👍 to approve, ✅ to acknowledge) produced nothing. Forward them through the normal message pipeline as synthesized MessageEvents whose text is the reaction emoji (translated to unicode for common names), keeping the downstream auth gate, thread-context fetch, dedup, and skill routing unchanged. - Self-reactions and non-message items are dropped; reactions on messages not sent by this bot are dropped (Feishu-adapter parity). - The reacted-to message's thread parent becomes the synthesized thread_ts so the reaction lands in the same session as a reply would. - Manifest gains reactions:read scope + reaction_added bot event. Salvaged from PR #29916 by @bpross. Related: #33111, #44508, #45265 (same cluster).
This commit is contained in:
parent
9ddcb58a21
commit
9c9b057b73
4 changed files with 329 additions and 6 deletions
|
|
@ -86,6 +86,7 @@ def _build_full_manifest(
|
|||
"im:write",
|
||||
"mpim:history",
|
||||
"mpim:read",
|
||||
"reactions:read",
|
||||
"users:read",
|
||||
]
|
||||
|
||||
|
|
@ -95,6 +96,7 @@ def _build_full_manifest(
|
|||
"message.groups",
|
||||
"message.im",
|
||||
"message.mpim",
|
||||
"reaction_added",
|
||||
]
|
||||
|
||||
if messaging_experience == "assistant":
|
||||
|
|
|
|||
|
|
@ -17,7 +17,7 @@ import os
|
|||
import re
|
||||
import time
|
||||
from dataclasses import dataclass, field
|
||||
from typing import Callable, Dict, Optional, Any, Tuple, List
|
||||
from typing import Callable, ClassVar, Dict, Optional, Any, Tuple, List
|
||||
|
||||
import aiohttp
|
||||
|
||||
|
|
@ -1773,13 +1773,15 @@ class SlackAdapter(BasePlatformAdapter):
|
|||
async def handle_file_change(event, say):
|
||||
pass
|
||||
|
||||
# Reactions are useful lightweight acknowledgements in Slack, but
|
||||
# Hermes does not currently need to route them into the agent loop.
|
||||
# Ack the events explicitly so high-traffic channels do not fill
|
||||
# gateway.error.log with Slack Bolt "Unhandled request" warnings.
|
||||
# Forward reaction_added events through the normal message
|
||||
# pipeline (see _handle_slack_reaction). Skills that present
|
||||
# confirmation-style proposals ("react 👍 to proceed") then work
|
||||
# end-to-end. Registered explicitly so high-traffic channels do
|
||||
# not fill gateway.error.log with Slack Bolt "Unhandled request"
|
||||
# warnings.
|
||||
@self._app.event("reaction_added")
|
||||
async def handle_reaction_added(event, say):
|
||||
pass
|
||||
await self._handle_slack_reaction(event)
|
||||
|
||||
@self._app.event("reaction_removed")
|
||||
async def handle_reaction_removed(event, say):
|
||||
|
|
@ -4282,6 +4284,127 @@ class SlackAdapter(BasePlatformAdapter):
|
|||
team_id=metadata["team_id"],
|
||||
)
|
||||
|
||||
# Common reaction names → unicode emoji. Used by ``_handle_slack_reaction``
|
||||
# so skills that match on ``text`` see the same character whether the user
|
||||
# typed it or reacted with it.
|
||||
_REACTION_EMOJI_MAP: ClassVar[Dict[str, str]] = {
|
||||
"thumbsup": "👍",
|
||||
"+1": "👍",
|
||||
"thumbsdown": "👎",
|
||||
"-1": "👎",
|
||||
"white_check_mark": "✅",
|
||||
"heavy_check_mark": "✅",
|
||||
"x": "❌",
|
||||
"no_entry": "⛔",
|
||||
"warning": "⚠️",
|
||||
"rotating_light": "🚨",
|
||||
"eyes": "👀",
|
||||
"rocket": "🚀",
|
||||
"tada": "🎉",
|
||||
"fire": "🔥",
|
||||
"wave": "👋",
|
||||
}
|
||||
|
||||
async def _handle_slack_reaction(self, event: dict) -> None:
|
||||
"""Forward ``reaction_added`` events through the normal message pipeline.
|
||||
|
||||
The reactor's user_id becomes the synthesized message's user, so the
|
||||
downstream auth gate (``_is_user_authorized``) applies as it does for
|
||||
any other message. The reacted-to message's ``thread_ts`` becomes
|
||||
the synthesized message's ``thread_ts`` so the reaction lands in the
|
||||
same thread as a regular reply would, letting skills that present
|
||||
confirmation-style proposals (``react 👍 to proceed``) treat
|
||||
reactions as real responses.
|
||||
|
||||
Common reactions are translated to unicode emoji in the text field
|
||||
(👍, 👎, ✅, etc.) so skill bodies that match on the typed character
|
||||
also fire on the equivalent reaction without needing to know about
|
||||
Slack-specific names.
|
||||
|
||||
Self-reactions (the bot reacting to its own messages, e.g. the
|
||||
:eyes: lifecycle reaction) are dropped here to prevent feedback
|
||||
loops. file-targeted reactions are ignored — only ``item.type ==
|
||||
"message"`` is forwarded. Reactions on messages not sent by this
|
||||
bot are dropped so a reaction on an unrelated human message can't
|
||||
enter the agent loop.
|
||||
"""
|
||||
item = event.get("item") or {}
|
||||
if item.get("type") != "message":
|
||||
return
|
||||
channel_id = item.get("channel")
|
||||
msg_ts = item.get("ts")
|
||||
reaction_name = event.get("reaction") or ""
|
||||
user_id = event.get("user")
|
||||
if not channel_id or not msg_ts or not user_id or not reaction_name:
|
||||
return
|
||||
# Drop self-reactions (lifecycle markers like :eyes: on incoming msgs).
|
||||
if self._bot_user_id and user_id == self._bot_user_id:
|
||||
return
|
||||
team_id = self._channel_team.get(channel_id) or ""
|
||||
if not team_id and self._team_clients:
|
||||
team_id = next(iter(self._team_clients))
|
||||
client = self._team_clients.get(team_id) if team_id else None
|
||||
|
||||
# Look up the reacted-to message so we can route the synthesized
|
||||
# event into the right thread and verify the target belongs to this
|
||||
# bot (matching the Feishu adapter's target-sender check). If the
|
||||
# lookup fails, fall back to treating the reacted-to message as the
|
||||
# thread parent — that's correct for top-level messages and
|
||||
# degrades gracefully for in-thread reactions where we lose the
|
||||
# parent linkage.
|
||||
thread_ts: Optional[str] = msg_ts
|
||||
if client is not None:
|
||||
try:
|
||||
history = await client.conversations_replies(
|
||||
channel=channel_id, ts=msg_ts, limit=1, inclusive=True,
|
||||
)
|
||||
messages = (history or {}).get("messages") or []
|
||||
if messages:
|
||||
first = messages[0]
|
||||
thread_ts = first.get("thread_ts") or first.get("ts") or msg_ts
|
||||
# Verify the reacted-to message was sent by this bot
|
||||
# (matching the Feishu adapter's target-sender check).
|
||||
# ``item_user`` on the event is the author of the
|
||||
# reacted-to message; if absent, fall back to the
|
||||
# fetched message's ``user`` field.
|
||||
item_user = event.get("item_user") or first.get("user") or ""
|
||||
bot_uid = self._team_bot_user_ids.get(team_id) or self._bot_user_id
|
||||
if item_user and bot_uid and item_user != bot_uid:
|
||||
return
|
||||
except Exception as e: # pragma: no cover - network path
|
||||
logger.debug(
|
||||
"[Slack] reaction thread_ts lookup failed for %s: %s",
|
||||
msg_ts, e,
|
||||
)
|
||||
|
||||
emoji_text = self._REACTION_EMOJI_MAP.get(reaction_name, f":{reaction_name}:")
|
||||
|
||||
# Use the reaction's own event_ts as the synthesized message ts so
|
||||
# the deduplicator in _handle_slack_message treats this reaction
|
||||
# as a distinct event (it has nothing to do with the reacted-to
|
||||
# message's ts).
|
||||
synthetic_ts = event.get("event_ts") or f"reaction-{msg_ts}-{reaction_name}-{user_id}"
|
||||
synthetic: dict = {
|
||||
"type": "message",
|
||||
"user": user_id,
|
||||
"text": emoji_text,
|
||||
"channel": channel_id,
|
||||
"ts": synthetic_ts,
|
||||
"thread_ts": thread_ts,
|
||||
# Surfaced for any downstream code that wants to know this was a
|
||||
# reaction rather than a typed message; not used by the default
|
||||
# pipeline.
|
||||
"_hermes_reaction": {
|
||||
"name": reaction_name,
|
||||
"reacted_to_ts": msg_ts,
|
||||
"event_ts": event.get("event_ts"),
|
||||
},
|
||||
}
|
||||
if team_id:
|
||||
synthetic["team"] = team_id
|
||||
|
||||
await self._handle_slack_message(synthetic)
|
||||
|
||||
async def _handle_slack_file_shared(
|
||||
self, event: dict, body: Optional[dict] = None
|
||||
) -> None:
|
||||
|
|
|
|||
|
|
@ -1153,3 +1153,182 @@ class TestThreadEngagement:
|
|||
"1000.000003",
|
||||
"1000.000004",
|
||||
}
|
||||
|
||||
|
||||
# ===========================================================================
|
||||
# _handle_slack_reaction — reaction_added forwarding
|
||||
# ===========================================================================
|
||||
|
||||
class TestSlackReactionForwarding:
|
||||
"""Reactions should flow through the same pipeline as typed messages."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reaction_synthesizes_message_in_thread(self):
|
||||
"""A 👍 reaction on a message in a thread should produce a synthesized
|
||||
MessageEvent that lands in that thread with text ``👍``, going through
|
||||
_handle_slack_message so the auth gate, thread-context fetch, and
|
||||
skill routing all apply unchanged."""
|
||||
adapter = _make_adapter()
|
||||
mock_client = adapter._team_clients["T1"]
|
||||
# Reacted-to message is itself a reply inside a thread; its thread_ts
|
||||
# points at the thread parent.
|
||||
mock_client.conversations_replies = AsyncMock(return_value={
|
||||
"messages": [
|
||||
{"ts": "2000.0", "thread_ts": "1000.0", "user": "U_BOT", "text": "Proposal"}
|
||||
]
|
||||
})
|
||||
forwarded: list[dict] = []
|
||||
|
||||
async def _capture(event):
|
||||
forwarded.append(event)
|
||||
|
||||
with patch.object(adapter, "_handle_slack_message", new=_capture):
|
||||
await adapter._handle_slack_reaction({
|
||||
"type": "reaction_added",
|
||||
"user": "U1",
|
||||
"reaction": "thumbsup",
|
||||
"item": {"type": "message", "channel": "C1", "ts": "2000.0"},
|
||||
"item_user": "U_BOT",
|
||||
"event_ts": "3000.0",
|
||||
})
|
||||
|
||||
assert len(forwarded) == 1
|
||||
synth = forwarded[0]
|
||||
assert synth["type"] == "message"
|
||||
assert synth["user"] == "U1"
|
||||
assert synth["text"] == "👍"
|
||||
assert synth["channel"] == "C1"
|
||||
# Threaded back to the parent of the reacted-to message, not to the
|
||||
# reacted-to message itself.
|
||||
assert synth["thread_ts"] == "1000.0"
|
||||
# Distinct synthetic ts so dedup doesn't merge with anything else.
|
||||
assert synth["ts"] == "3000.0"
|
||||
# Reaction metadata preserved for downstream introspection.
|
||||
assert synth["_hermes_reaction"]["name"] == "thumbsup"
|
||||
assert synth["_hermes_reaction"]["reacted_to_ts"] == "2000.0"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_self_reaction_dropped(self):
|
||||
"""The bot's own reactions (e.g. the :eyes: lifecycle marker on
|
||||
incoming messages) must not feed back into the pipeline."""
|
||||
adapter = _make_adapter()
|
||||
forwarded: list[dict] = []
|
||||
|
||||
async def _capture(event):
|
||||
forwarded.append(event)
|
||||
|
||||
with patch.object(adapter, "_handle_slack_message", new=_capture):
|
||||
await adapter._handle_slack_reaction({
|
||||
"type": "reaction_added",
|
||||
"user": "U_BOT", # matches adapter._bot_user_id
|
||||
"reaction": "eyes",
|
||||
"item": {"type": "message", "channel": "C1", "ts": "1000.0"},
|
||||
"item_user": "U1",
|
||||
"event_ts": "3000.0",
|
||||
})
|
||||
|
||||
assert forwarded == []
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unknown_reaction_uses_colon_name(self):
|
||||
"""Reactions outside the unicode emoji map still forward, with text
|
||||
set to the Slack short name in colons. Skills can match on those."""
|
||||
adapter = _make_adapter()
|
||||
mock_client = adapter._team_clients["T1"]
|
||||
mock_client.conversations_replies = AsyncMock(return_value={
|
||||
"messages": [{"ts": "1000.0", "user": "U_BOT", "text": "Parent"}]
|
||||
})
|
||||
forwarded: list[dict] = []
|
||||
|
||||
async def _capture(event):
|
||||
forwarded.append(event)
|
||||
|
||||
with patch.object(adapter, "_handle_slack_message", new=_capture):
|
||||
await adapter._handle_slack_reaction({
|
||||
"type": "reaction_added",
|
||||
"user": "U1",
|
||||
"reaction": "moov-rocket", # custom workspace emoji
|
||||
"item": {"type": "message", "channel": "C1", "ts": "1000.0"},
|
||||
"item_user": "U_BOT",
|
||||
"event_ts": "3000.0",
|
||||
})
|
||||
|
||||
assert len(forwarded) == 1
|
||||
assert forwarded[0]["text"] == ":moov-rocket:"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_non_message_reaction_ignored(self):
|
||||
"""File reactions and other non-message item types are dropped — we
|
||||
only forward message reactions."""
|
||||
adapter = _make_adapter()
|
||||
forwarded: list[dict] = []
|
||||
|
||||
async def _capture(event):
|
||||
forwarded.append(event)
|
||||
|
||||
with patch.object(adapter, "_handle_slack_message", new=_capture):
|
||||
await adapter._handle_slack_reaction({
|
||||
"type": "reaction_added",
|
||||
"user": "U1",
|
||||
"reaction": "thumbsup",
|
||||
"item": {"type": "file", "file": "F123"},
|
||||
"event_ts": "3000.0",
|
||||
})
|
||||
|
||||
assert forwarded == []
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_top_level_message_threads_to_self(self):
|
||||
"""When the reacted-to message is itself the thread parent (no
|
||||
thread_ts of its own), the synthesized event uses the message ts
|
||||
as thread_ts."""
|
||||
adapter = _make_adapter()
|
||||
mock_client = adapter._team_clients["T1"]
|
||||
mock_client.conversations_replies = AsyncMock(return_value={
|
||||
"messages": [{"ts": "1000.0", "user": "U_BOT", "text": "Parent"}]
|
||||
})
|
||||
forwarded: list[dict] = []
|
||||
|
||||
async def _capture(event):
|
||||
forwarded.append(event)
|
||||
|
||||
with patch.object(adapter, "_handle_slack_message", new=_capture):
|
||||
await adapter._handle_slack_reaction({
|
||||
"type": "reaction_added",
|
||||
"user": "U1",
|
||||
"reaction": "+1", # alias for thumbsup
|
||||
"item": {"type": "message", "channel": "C1", "ts": "1000.0"},
|
||||
"item_user": "U_BOT",
|
||||
"event_ts": "3000.0",
|
||||
})
|
||||
|
||||
assert len(forwarded) == 1
|
||||
assert forwarded[0]["text"] == "👍"
|
||||
assert forwarded[0]["thread_ts"] == "1000.0"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reaction_on_non_bot_message_dropped(self):
|
||||
"""A reaction on a message not sent by this bot must not enter the
|
||||
agent loop — matching the Feishu adapter's target-sender check."""
|
||||
adapter = _make_adapter()
|
||||
mock_client = adapter._team_clients["T1"]
|
||||
mock_client.conversations_replies = AsyncMock(return_value={
|
||||
"messages": [{"ts": "1000.0", "user": "U_OTHER", "text": "Not our bot"}]
|
||||
})
|
||||
forwarded: list[dict] = []
|
||||
|
||||
async def _capture(event):
|
||||
forwarded.append(event)
|
||||
|
||||
with patch.object(adapter, "_handle_slack_message", new=_capture):
|
||||
await adapter._handle_slack_reaction({
|
||||
"type": "reaction_added",
|
||||
"user": "U1",
|
||||
"reaction": "thumbsup",
|
||||
"item": {"type": "message", "channel": "C1", "ts": "1000.0"},
|
||||
"item_user": "U_OTHER", # not our bot
|
||||
"event_ts": "3000.0",
|
||||
})
|
||||
|
||||
assert forwarded == []
|
||||
|
||||
|
|
|
|||
|
|
@ -148,3 +148,22 @@ class TestSlackFullManifest:
|
|||
bot_events = manifest["settings"]["event_subscriptions"]["bot_events"]
|
||||
for event in ("message.im", "message.channels", "message.groups", "app_mention"):
|
||||
assert event in bot_events
|
||||
|
||||
def test_reaction_scope_and_event_included(self):
|
||||
"""reaction_added event + reactions:read scope must be in the manifest
|
||||
so the adapter can forward reactions into the message pipeline."""
|
||||
manifest = _build_full_manifest("Hermes", "Your Hermes agent on Slack")
|
||||
|
||||
bot_scopes = manifest["oauth_config"]["scopes"]["bot"]
|
||||
bot_events = manifest["settings"]["event_subscriptions"]["bot_events"]
|
||||
assert "reactions:read" in bot_scopes
|
||||
assert "reaction_added" in bot_events
|
||||
|
||||
def test_reaction_scope_survives_no_assistant(self):
|
||||
manifest = _build_full_manifest(
|
||||
"Hermes", "Your Hermes agent on Slack", include_assistant=False
|
||||
)
|
||||
bot_scopes = manifest["oauth_config"]["scopes"]["bot"]
|
||||
bot_events = manifest["settings"]["event_subscriptions"]["bot_events"]
|
||||
assert "reactions:read" in bot_scopes
|
||||
assert "reaction_added" in bot_events
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue