diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index b2bebf35355b..0f08440d245f 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -105,6 +105,18 @@ def _reply_anchor_for_event(event) -> str | None: source = getattr(event, "source", None) platform = _platform_name(getattr(source, "platform", None)) thread_id = getattr(source, "thread_id", None) + raw_message = getattr(event, "raw_message", None) + if ( + platform == "slack" + and isinstance(raw_message, dict) + and raw_message.get("_hermes_no_thread_response") + ): + # Slack reaction handoffs into a configured target channel are meant + # to create a new top-level message there. Returning the synthetic + # event's message_id as reply_to would make + # SlackAdapter._resolve_thread_ts() treat it as a thread anchor and + # reply in a (nonexistent) thread anyway. + return None if platform == "telegram" and thread_id and getattr(source, "chat_type", None) == "dm": # Reply to the triggering user message. Replying to Telegram's earlier # topic seed/anchor can render the bot response outside the active lane. @@ -2443,6 +2455,11 @@ class BasePlatformAdapter(ABC): self.config = config self.platform = platform self._message_handler: Optional[MessageHandler] = None + # Optional gateway-supplied fan-out for platform-native emoji + # reaction events (see ``set_reaction_handler``). + self._reaction_handler: Optional[ + Callable[[Dict[str, Any]], Awaitable[None]] + ] = None # Optional hook (e.g. Telegram DM topic recovery) that rewrites # ``event.source.thread_id`` before session keying. Returns the # corrected thread_id or None to leave the source untouched. @@ -2988,6 +3005,25 @@ class BasePlatformAdapter(ABC): """Set an optional handler for messages arriving during active sessions.""" self._busy_session_handler = handler + def set_reaction_handler( + self, handler: Optional[Callable[[Dict[str, Any]], Awaitable[None]]] + ) -> None: + """Set the handler for emoji-reaction events on platform messages. + + Called by adapters that subscribe to platform-native reaction events + (currently the Slack adapter's ``reaction_added``/``reaction_removed``). + The handler receives a normalised event dict — ``platform``, + ``event_name`` ("reaction:added"/"reaction:removed"), ``reaction``, + ``user_id``, ``item_user_id``, ``channel_id``, ``message_ts``, + ``event_ts``, ``raw_event`` — and fans out via + ``HookRegistry.emit(event_name, ...)``. + + Adapters without reaction support simply never call the handler. + """ + # Assign defensively: subclasses initialized via ``object.__new__`` + # in tests never run ``BasePlatformAdapter.__init__``. + self._reaction_handler = handler # type: ignore[attr-defined] + def set_authorization_check( self, callback: Optional[Callable[[str, Optional[str], Optional[str]], bool]], diff --git a/gateway/run.py b/gateway/run.py index 9b3393007952..4aee7e8cc9f8 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -4438,6 +4438,22 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew except Exception: logger.debug("Failed to sync gateway session model metadata", exc_info=True) + async def _handle_reaction_event(self, ctx: Dict[str, Any]) -> None: + """Fan a normalised platform reaction event out to the HookRegistry. + + Adapters call this via ``set_reaction_handler`` for every + platform-native reaction event they surface. The adapter-supplied + ``event_name`` ("reaction:added" / "reaction:removed") becomes the + hook event so user hooks subscribe with the same name scheme as the + existing ``agent:*`` family. Errors never block the adapter's event + loop — the hook contract is non-blocking. + """ + event_name = str(ctx.get("event_name") or "reaction:added") + try: + await self.hooks.emit(event_name, ctx) + except Exception: + logger.debug("[Gateway] reaction hook emit failed", exc_info=True) + async def _handle_adapter_fatal_error(self, adapter: BasePlatformAdapter) -> None: """React to an adapter failure after startup. @@ -7892,6 +7908,7 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew adapter.set_fatal_error_handler(self._handle_adapter_fatal_error) adapter.set_session_store(self.session_store) adapter.set_busy_session_handler(self._handle_active_session_busy_message) + adapter.set_reaction_handler(self._handle_reaction_event) adapter.set_topic_recovery_fn(self._recover_telegram_topic_thread_id) adapter.set_authorization_check(self._make_adapter_auth_check(adapter.platform)) adapter._busy_text_mode = self._busy_text_mode @@ -8871,6 +8888,7 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew adapter.set_fatal_error_handler(self._handle_adapter_fatal_error) adapter.set_session_store(self.session_store) adapter.set_busy_session_handler(self._handle_active_session_busy_message) + adapter.set_reaction_handler(self._handle_reaction_event) adapter.set_topic_recovery_fn(self._recover_telegram_topic_thread_id) adapter.set_authorization_check(self._make_adapter_auth_check(adapter.platform)) adapter._busy_text_mode = self._busy_text_mode @@ -9745,6 +9763,7 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew ) adapter.set_session_store(self.session_store) adapter.set_busy_session_handler(self._handle_active_session_busy_message) + adapter.set_reaction_handler(self._handle_reaction_event) adapter.set_topic_recovery_fn(self._recover_telegram_topic_thread_id) adapter.set_authorization_check( self._make_adapter_auth_check(platform, profile_name=profile_name) diff --git a/hermes_cli/slack_cli.py b/hermes_cli/slack_cli.py index 6d9124c2c465..fa5a971ff353 100644 --- a/hermes_cli/slack_cli.py +++ b/hermes_cli/slack_cli.py @@ -97,6 +97,7 @@ def _build_full_manifest( "message.im", "message.mpim", "reaction_added", + "reaction_removed", ] if messaging_experience == "assistant": diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index 68c016bc9370..a13c8d2c54b3 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -1785,7 +1785,7 @@ class SlackAdapter(BasePlatformAdapter): @self._app.event("reaction_removed") async def handle_reaction_removed(event, say): - pass + await self._handle_slack_reaction(event, removed=True) @self._app.event("assistant_thread_started") async def handle_assistant_thread_started(event, say, body): @@ -4305,8 +4305,8 @@ class SlackAdapter(BasePlatformAdapter): "wave": "👋", } - async def _handle_slack_reaction(self, event: dict) -> None: - """Forward ``reaction_added`` events through the normal message pipeline. + async def _handle_slack_reaction(self, event: dict, removed: bool = False) -> None: + """Forward reaction 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 @@ -4316,17 +4316,29 @@ class SlackAdapter(BasePlatformAdapter): 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. + The synthesized text follows the cross-platform convention already + used by the Feishu and Photon adapters — ``reaction:added:`` / + ``reaction:removed:`` — with common Slack reaction names + translated to unicode emoji (👍, 👎, ✅, …) so agents and skills see + the same shape on every platform. Because the synthesized event is + threaded under the reacted-to message, the existing reply-context + plumbing injects the target message's text as ``reply_to_text`` and + the agent sees WHAT was reacted to. + + Message-pipeline routing is OPT-IN via ``slack.reaction_triggers`` + (default off) so busy channels don't wake the agent on every emoji. + Gateway hooks (``reaction:added`` / ``reaction:removed``) fire for + every non-self reaction on a message item regardless of the opt-in, + so hook consumers can observe reactions without enabling agent + routing. 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. + "message"`` is forwarded. Unless an explicit emoji allowlist is + configured, 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": @@ -4345,6 +4357,43 @@ class SlackAdapter(BasePlatformAdapter): team_id = next(iter(self._team_clients)) client = self._team_clients.get(team_id) if team_id else None + action = "removed" if removed else "added" + + # Fire the gateway hook surface first (reaction:added/removed) so + # hook consumers observe every human reaction even when agent + # routing below is disabled. getattr-guard: tests build adapters + # via object.__new__ without running __init__. + reaction_handler = getattr(self, "_reaction_handler", None) + if reaction_handler is not None: + try: + await reaction_handler( + { + "platform": "slack", + "event_name": f"reaction:{action}", + "reaction": reaction_name, + "user_id": user_id, + "item_user_id": event.get("item_user"), + "item_type": item.get("type"), + "channel_id": channel_id, + "message_ts": msg_ts, + "team_id": team_id, + "event_ts": event.get("event_ts"), + "raw_event": event, + } + ) + except Exception: # pragma: no cover - hook contract is non-blocking + logger.debug("[Slack] reaction hook forwarding failed", exc_info=True) + + # Opt-in gate for message-pipeline routing. None → disabled (ack + # only, the pre-existing behavior); empty set → all emoji route; + # non-empty set → only the allowlisted emoji names route. + triggers = self._slack_reaction_triggers() + if triggers is None: + return + explicit_allowlist = bool(triggers) + if explicit_allowlist and reaction_name.strip(":") not in triggers: + return + # 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 @@ -4366,18 +4415,34 @@ class SlackAdapter(BasePlatformAdapter): # (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. + # fetched message's ``user`` field. Operators that + # configure an explicit emoji allowlist deliberately + # chose trigger emojis, so those may target any + # message (emoji-handoff workflows). 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: + if ( + not explicit_allowlist + and 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, ) + elif not explicit_allowlist: + # No client to verify the target message's sender — without an + # explicit allowlist we cannot prove the reaction targets our + # own message, so verify via the event's item_user alone. + item_user = event.get("item_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 - emoji_text = self._REACTION_EMOJI_MAP.get(reaction_name, f":{reaction_name}:") + emoji_text = self._REACTION_EMOJI_MAP.get(reaction_name, reaction_name) # Use the reaction's own event_ts as the synthesized message ts so # the deduplicator in _handle_slack_message treats this reaction @@ -4387,15 +4452,21 @@ class SlackAdapter(BasePlatformAdapter): synthetic: dict = { "type": "message", "user": user_id, - "text": emoji_text, + "text": f"reaction:{action}:{emoji_text}", "channel": channel_id, "ts": synthetic_ts, "thread_ts": thread_ts, + # A reaction on the bot's own message (or an operator-allowlisted + # trigger emoji) is definitionally addressed to the bot — skip + # the mention requirement the way Feishu/Photon reaction routing + # does. User authorization and allowed_channels still apply. + "_hermes_force_process": True, # 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, + "action": action, "reacted_to_ts": msg_ts, "event_ts": event.get("event_ts"), }, @@ -4403,8 +4474,75 @@ class SlackAdapter(BasePlatformAdapter): if team_id: synthetic["team"] = team_id + # Optional handoff target (#45265): route the reaction-triggered + # turn into a configured channel (and optionally thread) instead of + # the source thread. A channel-only target is a handoff, not a + # reply — respond top-level there. + target_channel, target_thread = self._slack_reaction_trigger_target() + if target_channel: + synthetic["channel"] = target_channel + synthetic["channel_type"] = ( + "im" if target_channel.startswith("D") else "channel" + ) + synthetic["_hermes_reaction_source_channel"] = channel_id + if target_thread: + synthetic["thread_ts"] = target_thread + else: + synthetic.pop("thread_ts", None) + synthetic["_hermes_no_thread_response"] = True + await self._handle_slack_message(synthetic) + def _slack_reaction_triggers(self) -> Optional[set]: + """Return the reaction-routing opt-in state. + + ``None`` — disabled (default): reaction events are acked and + dropped, preserving the historical behavior. + empty set — enabled for all emoji (``reaction_triggers: true``), + limited to reactions on the bot's own messages. + non-empty set — enabled for exactly these emoji names, on any + message (operator-curated handoff emojis). + + Sources: ``slack.reaction_triggers`` in config.yaml (bool or list), + or the ``SLACK_REACTION_TRIGGERS`` env var (``true``/``all`` or a + comma-separated emoji-name list). + """ + raw = self.config.extra.get("reaction_triggers") + if raw is None: + raw = os.getenv("SLACK_REACTION_TRIGGERS") or None + if raw is None: + return None + if isinstance(raw, bool): + return set() if raw else None + if isinstance(raw, (list, tuple, set)): + names = {str(p).strip().strip(":") for p in raw if str(p).strip().strip(":")} + return names or set() + text = str(raw or "").strip() + if not text or text.lower() in {"false", "0", "no", "off"}: + return None + if text.lower() in {"true", "1", "yes", "on", "all", "*"}: + return set() + return {p.strip().strip(":") for p in re.split(r"[,\s]+", text) if p.strip().strip(":")} + + def _slack_reaction_trigger_target(self) -> Tuple[str, str]: + """Return the optional (channel, thread) handoff target for reactions. + + ``slack.reaction_trigger_target`` accepts ``C123`` (respond + top-level in that channel) or ``C123:1710000000.000100`` (respond + in that thread). Empty by default — reactions route into the + thread of the reacted-to message. + """ + raw = self.config.extra.get("reaction_trigger_target") + if raw is None: + raw = os.getenv("SLACK_REACTION_TRIGGER_TARGET", "") + target = str(raw or "").strip() + if not target: + return "", "" + if ":" in target: + channel, thread = target.split(":", 1) + return channel.strip(), thread.strip() + return target, "" + async def _handle_slack_file_shared( self, event: dict, body: Optional[dict] = None ) -> None: @@ -4915,6 +5053,12 @@ class SlackAdapter(BasePlatformAdapter): thread_ts = event.get("thread_ts") or assistant_meta.get("thread_ts") if not thread_ts and self._dm_top_level_threads_as_sessions(): thread_ts = ts + elif event.get("_hermes_no_thread_response"): + # Reaction handoff into a configured target channel (#45265): + # the response should be a new top-level message in the target + # channel, never a thread under the synthetic ts (which is the + # reaction's event_ts — not a real message there). + thread_ts = event.get("thread_ts") or None else: # Channel message session scoping. # @@ -4970,6 +5114,10 @@ class SlackAdapter(BasePlatformAdapter): ) event_thread_ts = event.get("thread_ts") is_thread_reply = bool(event_thread_ts and event_thread_ts != ts) + # Internal routing paths (reaction triggers) are pre-authorized as + # "addressed to the bot" — they skip the mention requirement but NOT + # the allowed_channels whitelist or user authorization above. + force_process = bool(event.get("_hermes_force_process")) # Some Slack bot posts arrive as ordinary-looking message events with a # bot *user* id but without ``bot_id``/``subtype=bot_message``. This is @@ -5022,7 +5170,9 @@ class SlackAdapter(BasePlatformAdapter): ) return - if ( + if force_process: + pass # Explicit internal routing path (reaction trigger). + elif ( channel_id not in self._slack_require_mention_channels() and ( channel_id in self._slack_free_response_channels() @@ -8253,6 +8403,14 @@ def _apply_yaml_config(yaml_cfg: dict, slack_cfg: dict) -> dict | None: os.environ["SLACK_REQUIRE_MENTION_CHANNELS"] = str(rmc) if "reactions" in slack_cfg and not os.getenv("SLACK_REACTIONS"): os.environ["SLACK_REACTIONS"] = str(slack_cfg["reactions"]).lower() + rt = slack_cfg.get("reaction_triggers") + if rt is not None and not os.getenv("SLACK_REACTION_TRIGGERS"): + if isinstance(rt, (list, tuple, set)): + rt = ",".join(str(v) for v in rt) + os.environ["SLACK_REACTION_TRIGGERS"] = str(rt) + rtt = slack_cfg.get("reaction_trigger_target") + if rtt is not None and not os.getenv("SLACK_REACTION_TRIGGER_TARGET"): + os.environ["SLACK_REACTION_TRIGGER_TARGET"] = str(rtt) ac = slack_cfg.get("allowed_channels") if ac is not None and not os.getenv("SLACK_ALLOWED_CHANNELS"): if isinstance(ac, list): diff --git a/tests/gateway/test_slack_approval_buttons.py b/tests/gateway/test_slack_approval_buttons.py index 8e622a7c2cb1..662f6948dfd2 100644 --- a/tests/gateway/test_slack_approval_buttons.py +++ b/tests/gateway/test_slack_approval_buttons.py @@ -1160,15 +1160,44 @@ class TestThreadEngagement: # =========================================================================== class TestSlackReactionForwarding: - """Reactions should flow through the same pipeline as typed messages.""" + """Reactions should flow through the same pipeline as typed messages, + gated by the ``reaction_triggers`` opt-in (default: off).""" + + @staticmethod + def _enable_triggers(adapter, value=True): + adapter.config.extra["reaction_triggers"] = value + + @pytest.mark.asyncio + async def test_default_off_reaction_acked_and_dropped(self): + """Without the reaction_triggers opt-in, reaction events are acked + and dropped — the historical behavior — so busy channels don't wake + the agent on every emoji.""" + 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": "message", "channel": "C1", "ts": "2000.0"}, + "item_user": "U_BOT", + "event_ts": "3000.0", + }) + + assert forwarded == [] @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.""" + MessageEvent that lands in that thread with text ``reaction:added:👍``, + going through _handle_slack_message so the auth gate, thread-context + fetch, and skill routing all apply unchanged.""" adapter = _make_adapter() + self._enable_triggers(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. @@ -1196,7 +1225,7 @@ class TestSlackReactionForwarding: synth = forwarded[0] assert synth["type"] == "message" assert synth["user"] == "U1" - assert synth["text"] == "👍" + assert synth["text"] == "reaction:added:👍" assert synth["channel"] == "C1" # Threaded back to the parent of the reacted-to message, not to the # reacted-to message itself. @@ -1205,13 +1234,49 @@ class TestSlackReactionForwarding: assert synth["ts"] == "3000.0" # Reaction metadata preserved for downstream introspection. assert synth["_hermes_reaction"]["name"] == "thumbsup" + assert synth["_hermes_reaction"]["action"] == "added" assert synth["_hermes_reaction"]["reacted_to_ts"] == "2000.0" + # Pre-authorized as addressed-to-the-bot (skips mention gate only). + assert synth["_hermes_force_process"] is True + + @pytest.mark.asyncio + async def test_reaction_removed_synthesizes_removed_text(self): + """reaction_removed events route with the reaction:removed: prefix so + the agent can distinguish an un-react from a react.""" + adapter = _make_adapter() + self._enable_triggers(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_removed", + "user": "U1", + "reaction": "white_check_mark", + "item": {"type": "message", "channel": "C1", "ts": "1000.0"}, + "item_user": "U_BOT", + "event_ts": "3000.0", + }, + removed=True, + ) + + assert len(forwarded) == 1 + assert forwarded[0]["text"] == "reaction:removed:✅" + assert forwarded[0]["_hermes_reaction"]["action"] == "removed" @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() + self._enable_triggers(adapter) forwarded: list[dict] = [] async def _capture(event): @@ -1230,10 +1295,11 @@ class TestSlackReactionForwarding: 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.""" + async def test_unknown_reaction_passes_name_through(self): + """Reactions outside the unicode emoji map still forward, with the + Slack short name in the text. Skills can match on those.""" adapter = _make_adapter() + self._enable_triggers(adapter) mock_client = adapter._team_clients["T1"] mock_client.conversations_replies = AsyncMock(return_value={ "messages": [{"ts": "1000.0", "user": "U_BOT", "text": "Parent"}] @@ -1254,13 +1320,14 @@ class TestSlackReactionForwarding: }) assert len(forwarded) == 1 - assert forwarded[0]["text"] == ":moov-rocket:" + assert forwarded[0]["text"] == "reaction:added: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() + self._enable_triggers(adapter) forwarded: list[dict] = [] async def _capture(event): @@ -1283,6 +1350,7 @@ class TestSlackReactionForwarding: thread_ts of its own), the synthesized event uses the message ts as thread_ts.""" adapter = _make_adapter() + self._enable_triggers(adapter) mock_client = adapter._team_clients["T1"] mock_client.conversations_replies = AsyncMock(return_value={ "messages": [{"ts": "1000.0", "user": "U_BOT", "text": "Parent"}] @@ -1303,7 +1371,7 @@ class TestSlackReactionForwarding: }) assert len(forwarded) == 1 - assert forwarded[0]["text"] == "👍" + assert forwarded[0]["text"] == "reaction:added:👍" assert forwarded[0]["thread_ts"] == "1000.0" @pytest.mark.asyncio @@ -1311,6 +1379,7 @@ class TestSlackReactionForwarding: """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() + self._enable_triggers(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"}] @@ -1332,3 +1401,252 @@ class TestSlackReactionForwarding: assert forwarded == [] + @pytest.mark.asyncio + async def test_allowlisted_emoji_routes_from_any_message(self): + """An explicit emoji allowlist deliberately targets any message + (emoji-handoff workflows), and non-listed emoji stay dropped.""" + adapter = _make_adapter() + self._enable_triggers(adapter, ["task"]) + mock_client = adapter._team_clients["T1"] + mock_client.conversations_replies = AsyncMock(return_value={ + "messages": [{"ts": "1000.0", "user": "U_OTHER", "text": "Human message"}] + }) + forwarded: list[dict] = [] + + async def _capture(event): + forwarded.append(event) + + with patch.object(adapter, "_handle_slack_message", new=_capture): + # Listed emoji on a HUMAN message → routes. + await adapter._handle_slack_reaction({ + "type": "reaction_added", + "user": "U1", + "reaction": "task", + "item": {"type": "message", "channel": "C1", "ts": "1000.0"}, + "item_user": "U_OTHER", + "event_ts": "3000.0", + }) + # Unlisted emoji → dropped even on the bot's own message. + await adapter._handle_slack_reaction({ + "type": "reaction_added", + "user": "U1", + "reaction": "thumbsup", + "item": {"type": "message", "channel": "C1", "ts": "1000.0"}, + "item_user": "U_BOT", + "event_ts": "3001.0", + }) + + assert len(forwarded) == 1 + assert forwarded[0]["text"] == "reaction:added:task" + + @pytest.mark.asyncio + async def test_target_channel_handoff_routes_top_level(self): + """reaction_trigger_target=C_TRAIN routes the synthesized turn to the + target channel as a new top-level message (#45265).""" + adapter = _make_adapter() + self._enable_triggers(adapter, ["task"]) + adapter.config.extra["reaction_trigger_target"] = "C_TRAIN" + mock_client = adapter._team_clients["T1"] + mock_client.conversations_replies = AsyncMock(return_value={ + "messages": [{"ts": "1000.0", "user": "U_OTHER", "text": "Handoff me"}] + }) + 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": "task", + "item": {"type": "message", "channel": "C1", "ts": "1000.0"}, + "item_user": "U_OTHER", + "event_ts": "3000.0", + }) + + assert len(forwarded) == 1 + synth = forwarded[0] + assert synth["channel"] == "C_TRAIN" + assert "thread_ts" not in synth + assert synth["_hermes_no_thread_response"] is True + assert synth["_hermes_reaction_source_channel"] == "C1" + + @pytest.mark.asyncio + async def test_target_channel_with_thread_routes_to_thread(self): + """A C123: target routes into that thread of the target channel.""" + adapter = _make_adapter() + self._enable_triggers(adapter, ["task"]) + adapter.config.extra["reaction_trigger_target"] = "C_TRAIN:1719.0001" + mock_client = adapter._team_clients["T1"] + mock_client.conversations_replies = AsyncMock(return_value={ + "messages": [{"ts": "1000.0", "user": "U_OTHER", "text": "Handoff me"}] + }) + 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": "task", + "item": {"type": "message", "channel": "C1", "ts": "1000.0"}, + "item_user": "U_OTHER", + "event_ts": "3000.0", + }) + + assert len(forwarded) == 1 + assert forwarded[0]["channel"] == "C_TRAIN" + assert forwarded[0]["thread_ts"] == "1719.0001" + assert "_hermes_no_thread_response" not in forwarded[0] + + @pytest.mark.asyncio + async def test_hook_fires_even_when_routing_disabled(self): + """The gateway reaction handler fires for every human reaction on a + message item, independent of the reaction_triggers opt-in.""" + adapter = _make_adapter() # opt-in NOT set + hook_events: list[dict] = [] + + async def _hook(ctx): + hook_events.append(ctx) + + adapter.set_reaction_handler(_hook) + 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_BOT", + "event_ts": "3000.0", + }) + await adapter._handle_slack_reaction( + { + "type": "reaction_removed", + "user": "U1", + "reaction": "thumbsup", + "item": {"type": "message", "channel": "C1", "ts": "1000.0"}, + "item_user": "U_BOT", + "event_ts": "3001.0", + }, + removed=True, + ) + + assert forwarded == [] # routing stays off + assert [e["event_name"] for e in hook_events] == [ + "reaction:added", + "reaction:removed", + ] + assert hook_events[0]["platform"] == "slack" + assert hook_events[0]["reaction"] == "thumbsup" + assert hook_events[0]["user_id"] == "U1" + assert hook_events[0]["channel_id"] == "C1" + assert hook_events[0]["message_ts"] == "1000.0" + + @pytest.mark.asyncio + async def test_hook_not_fired_for_self_reactions(self): + """The bot's own lifecycle reactions never reach the hook surface.""" + adapter = _make_adapter() + hook_events: list[dict] = [] + + async def _hook(ctx): + hook_events.append(ctx) + + adapter.set_reaction_handler(_hook) + await adapter._handle_slack_reaction({ + "type": "reaction_added", + "user": "U_BOT", + "reaction": "eyes", + "item": {"type": "message", "channel": "C1", "ts": "1000.0"}, + "event_ts": "3000.0", + }) + + assert hook_events == [] + + def test_trigger_config_parsing(self): + """reaction_triggers accepts bool / list / string forms.""" + adapter = _make_adapter() + assert adapter._slack_reaction_triggers() is None # default off + adapter.config.extra["reaction_triggers"] = False + assert adapter._slack_reaction_triggers() is None + adapter.config.extra["reaction_triggers"] = True + assert adapter._slack_reaction_triggers() == set() + adapter.config.extra["reaction_triggers"] = ["thumbsup", ":task:"] + assert adapter._slack_reaction_triggers() == {"thumbsup", "task"} + adapter.config.extra["reaction_triggers"] = "thumbsup, task" + assert adapter._slack_reaction_triggers() == {"thumbsup", "task"} + adapter.config.extra["reaction_triggers"] = "all" + assert adapter._slack_reaction_triggers() == set() + adapter.config.extra["reaction_triggers"] = "false" + assert adapter._slack_reaction_triggers() is None + + def test_trigger_env_fallback(self, monkeypatch): + """SLACK_REACTION_TRIGGERS env enables routing when config is unset.""" + adapter = _make_adapter() + monkeypatch.setenv("SLACK_REACTION_TRIGGERS", "true") + assert adapter._slack_reaction_triggers() == set() + monkeypatch.setenv("SLACK_REACTION_TRIGGERS", "task,karen") + assert adapter._slack_reaction_triggers() == {"task", "karen"} + + def test_target_config_parsing(self): + adapter = _make_adapter() + assert adapter._slack_reaction_trigger_target() == ("", "") + adapter.config.extra["reaction_trigger_target"] = "C123" + assert adapter._slack_reaction_trigger_target() == ("C123", "") + adapter.config.extra["reaction_trigger_target"] = "C123:1710.0001" + assert adapter._slack_reaction_trigger_target() == ("C123", "1710.0001") + + +class TestSlackReactionAuthorizationGate: + """The synthesized reaction event must pass through the same early + authorization rejection as typed messages — an unauthorized reactor + cannot wake the agent.""" + + @pytest.mark.asyncio + async def test_unauthorized_reactor_rejected_in_pipeline(self): + adapter = _make_adapter() + adapter.config.extra["reaction_triggers"] = True + mock_client = adapter._team_clients["T1"] + mock_client.conversations_replies = AsyncMock(return_value={ + "messages": [{"ts": "1000.0", "user": "U_BOT", "text": "Proposal"}] + }) + + class _Runner: + def __init__(self): + self.handled = [] + self.auth_checked = [] + + async def handle(self, event): + self.handled.append(event) + return None + + def _is_user_authorized(self, source): + self.auth_checked.append(source.user_id) + return False # reject everyone + + runner = _Runner() + adapter.set_message_handler(runner.handle) + adapter.handle_message = AsyncMock() + + await adapter._handle_slack_reaction({ + "type": "reaction_added", + "user": "U_RANDO", + "reaction": "thumbsup", + "item": {"type": "message", "channel": "C1", "ts": "1000.0"}, + "item_user": "U_BOT", + "event_ts": "3000.0", + }) + + # The auth gate saw the reactor's user id and rejected it before + # any MessageEvent reached the gateway. + assert "U_RANDO" in runner.auth_checked + assert runner.handled == [] + adapter.handle_message.assert_not_called() + diff --git a/tests/hermes_cli/test_slack_cli.py b/tests/hermes_cli/test_slack_cli.py index 70a01d211d3f..2ca462cb6bb6 100644 --- a/tests/hermes_cli/test_slack_cli.py +++ b/tests/hermes_cli/test_slack_cli.py @@ -150,14 +150,16 @@ class TestSlackFullManifest: 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.""" + """reaction_added/removed events + reactions:read scope must be in the + manifest so the adapter can forward reactions into the message + pipeline and gateway hooks.""" 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 + assert "reaction_removed" in bot_events def test_reaction_scope_survives_no_assistant(self): manifest = _build_full_manifest( diff --git a/website/docs/user-guide/features/hooks.md b/website/docs/user-guide/features/hooks.md index f38ed9343b96..cdb957e3748b 100644 --- a/website/docs/user-guide/features/hooks.md +++ b/website/docs/user-guide/features/hooks.md @@ -81,6 +81,8 @@ async def handle(event_type: str, context: dict): | `agent:start` | Agent begins processing a message | `platform`, `user_id`, `session_id`, `message` | | `agent:step` | Each iteration of the tool-calling loop | `platform`, `user_id`, `session_id`, `iteration`, `tool_names` | | `agent:end` | Agent finishes processing | `platform`, `user_id`, `session_id`, `message`, `response` | +| `reaction:added` | An emoji reaction was added to a message the bot can see (Slack adapter currently). Requires the `reactions:read` scope + the `reaction_added` bot event subscription; the bot must be a member of the channel. | `platform`, `reaction`, `user_id`, `item_user_id`, `item_type`, `channel_id`, `message_ts`, `team_id`, `event_ts`, `raw_event` | +| `reaction:removed` | An emoji reaction was removed from a message the bot can see. Requires the `reaction_removed` bot event subscription. | same shape as `reaction:added` | | `command:*` | Any slash command executed | `platform`, `user_id`, `command`, `args` | #### Wildcard Matching diff --git a/website/docs/user-guide/messaging/slack.md b/website/docs/user-guide/messaging/slack.md index cb8799c3b087..16a4e1373ee1 100644 --- a/website/docs/user-guide/messaging/slack.md +++ b/website/docs/user-guide/messaging/slack.md @@ -625,6 +625,50 @@ How `mentions` mode gates: For strict multi-bot deployments, pair with `require_mention: true` and `strict_mention: true` — see the smoke-check profile below. +### Reaction Triggers (`reaction_triggers`) + +By default, emoji reactions are acknowledged and dropped — a 👍 on a bot +message does nothing. Set `slack.reaction_triggers` to route reactions into +the agent loop (requires the `reactions:read` scope plus the +`reaction_added`/`reaction_removed` bot event subscriptions in your Slack app +manifest — regenerate with `hermes slack manifest`): + +```yaml +slack: + # Opt-in. false/absent (default) = reactions are acked and dropped. + # true = any reaction ON THE BOT'S OWN MESSAGES routes to the agent. + reaction_triggers: true + # Or an explicit emoji allowlist — only these names route, and they may + # target ANY message (emoji-handoff workflows, e.g. :task: to capture): + # reaction_triggers: [white_check_mark, thumbsup, task] + # Optional handoff target: respond in this channel (top-level) or thread + # (C123:) instead of the reacted-to message's thread. + # reaction_trigger_target: C0123456789 +``` + +Environment equivalents: `SLACK_REACTION_TRIGGERS` (`true`/`all` or a +comma-separated list) and `SLACK_REACTION_TRIGGER_TARGET`. + +Behavior: + +- The reaction arrives as a normal agent turn with text + `reaction:added:👍` / `reaction:removed:👍` (common Slack names are + translated to unicode; unknown names pass through as-is, e.g. + `reaction:added:custom-emoji`), threaded under + the reacted-to message so the agent sees what was reacted to and the + turn lands in the same session as a reply would. +- The reactor becomes the message's user, so **user authorization and + `allowed_channels` gating apply exactly as for typed messages** — a + random user's reaction cannot trigger the agent anywhere their message + couldn't. +- With `reaction_triggers: true`, only reactions on the bot's **own** + messages route (approve/acknowledge flows). With an explicit emoji + allowlist, the listed emojis route from any message. +- The bot's own lifecycle reactions (`:eyes:` etc.) never feed back. +- Independent of this opt-in, every human reaction fires the + `reaction:added`/`reaction:removed` [gateway hooks](../features/hooks.md#available-events) + for observers that don't need agent turns. + ### Peer-Agent Smoke Check For multi-bot Slack deployments that rely on strict per-turn mentions, keep the following profile: