Skip to content

fix(slack): forward reaction_added events to the message pipeline - #29916

Closed
bpross wants to merge 2 commits into
NousResearch:mainfrom
bpross:slack-reaction-forwarding
Closed

fix(slack): forward reaction_added events to the message pipeline#29916
bpross wants to merge 2 commits into
NousResearch:mainfrom
bpross:slack-reaction-forwarding

Conversation

@bpross

@bpross bpross commented May 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Skills that present confirmation-style proposals — "react 👍 to proceed, 👎 to drop" — couldn't work end-to-end because the Slack adapter never registered a handler for reaction_added. Events arrived at slack_bolt, were logged as Unhandled request, and discarded before reaching the agent. Users had to fall back to retyping their intent, which defeats the reaction UX.

This change adds the missing handler and synthesizes the reaction into the existing message pipeline, so reactions become first-class confirmation responses without any per-skill plumbing.

What changes

A new @app.event(\"reaction_added\") registration that calls a new _handle_slack_reaction method. The handler:

  • Uses the reactor's user_id as the synthesized message's user, so the existing _is_user_authorized gate still applies — non-allowlisted users' reactions are dropped exactly like their typed messages would be.
  • Looks up the reacted-to message via conversations.replies to find its thread_ts, so the synthesized event lands in the right thread. Skills waiting on a clarify in that thread receive the reaction as the next user response.
  • Translates common reactions to their unicode emoji in the text field (thumbsup👍, +1👍, white_check_mark, thumbsdown👎, etc.). Skill bodies that match on the typed character also fire on the equivalent reaction without needing to know Slack short-names. Custom workspace emoji fall through as :name:.
  • Drops self-reactions (the bot's own :eyes: lifecycle marker, or any reaction added by the bot user_id) so they can't feed back into the pipeline.
  • Ignores file-targeted reactions — only item.type == \"message\" is forwarded.
  • Preserves the reaction name and reacted-to ts in a _hermes_reaction key on the synthesized event for any downstream code that wants to know it was a reaction rather than a typed message.

The reactor's response goes through _handle_slack_message unchanged — dedup, auth, thread-context, and skill routing all reuse the existing paths.

Tests

New TestSlackReactionForwarding class with five cases:

  • test_reaction_synthesizes_message_in_thread — 👍 reaction on an in-thread bot message produces a synthesized event with the right thread_ts, user, and text.
  • test_self_reaction_dropped — bot's own :eyes: reaction is not forwarded.
  • test_unknown_reaction_uses_colon_name — custom workspace emoji falls through as :name:.
  • test_non_message_reaction_ignored — file reactions are dropped.
  • test_top_level_message_threads_to_self — reaction on a top-level message threads to that message's ts.

Full slack test sweep run locally:

```
python -m pytest tests/gateway/test_slack_approval_buttons.py tests/gateway/test_slack.py tests/gateway/test_slack_mention.py tests/gateway/test_slack_channel_skills.py
```

280 passed, 0 regressions.

Why this is a fix, not a feature

The README and skill-authoring docs encourage skills like "react 👍 to confirm" as a natural UX. The current adapter silently breaks that contract. Users see WARNING slack_bolt.AsyncApp: Unhandled request ({'type': 'event_callback', 'event': {'type': 'reaction_added'}}) in logs but no clue why their confirmation flow doesn't fire.

Test plan

  • Synthesized event preserves user, text, channel, thread_ts
  • Self-reactions don't feedback-loop
  • Unknown reactions fall through with the short-name
  • File reactions don't crash or forward
  • Top-level reactions thread to themselves
  • No regressions in the existing slack test modules

Skills that present confirmation-style proposals -- "react 👍 to
proceed, 👎 to drop" -- could not work end-to-end because the Slack
adapter never registered a handler for ``reaction_added``. The event
arrived at slack_bolt, was logged as "Unhandled request", and discarded
before reaching the agent. Users could only respond by re-typing the
intent, which defeats the confirmation UX.

This change adds ``@app.event("reaction_added")`` and synthesizes a
MessageEvent into the existing message pipeline:

- The reactor's user_id becomes the synthesized message's ``user``, so
  ``_is_user_authorized`` still gates non-allowlisted reactors.
- The reacted-to message is looked up via ``conversations.replies`` so
  the synthesized event's ``thread_ts`` points at the right thread.
  Skills waiting on a clarify in a specific thread receive the
  reaction as the next user response, exactly like a typed reply.
- The text field is set to the unicode form for common emoji
  (👍 → 👍, ✅ → ✅, etc.)
  so skill bodies that match on the typed character also match the
  reaction without needing to know Slack short-names.
- Custom workspace emoji fall through as ``:name:`` so skills can
  still match them.

Self-reactions (the bot's own 👀 lifecycle marker, or any
self-reaction by the bot user_id) are dropped here so they can't feed
back into the pipeline.

Tests (TestSlackReactionForwarding, 5 cases):
- in-thread reaction synthesizes a message in the right thread,
  preserves user/text/channel, and includes _hermes_reaction metadata
- self-reactions (bot reacting to its own message) are dropped
- unknown reactions fall back to ``:name:`` text
- file-targeted reactions are ignored (item.type != "message")
- top-level reactions thread to themselves when there is no parent

Full slack test sweep: 280 passed, 0 regressions.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/slack Slack app adapter labels May 21, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for identifying a real gap: current main still acknowledges reaction_added without forwarding it (plugins/platforms/slack/adapter.py:1117-1127). The submitted implementation needs a rework before it can be salvaged.

Problems

  • The patch targets the deleted gateway/platforms/slack.py; Slack now runs from plugins/platforms/slack/adapter.py after migration commit 560010547.
  • The proposed handler does not verify that the reacted-to message was sent by Hermes. It reads event.item but ignores event.item_user, so a reaction on an unrelated human message can enter _handle_slack_message. Feishu performs this target-sender verification before routing reactions (plugins/platforms/feishu/adapter.py:2901-2915).
  • Default generated manifests omit both reaction_added and reactions:read (hermes_cli/slack_cli.py:66-90), while this PR changes no manifest code.

Suggested changes

  • Port the work to plugins/platforms/slack/adapter.py, verify the target belongs to this bot, and cover human/peer-bot targets plus multi-workspace routing.
  • Update the generated manifest and its tests alongside runtime registration.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
Port to plugins/platforms/slack/adapter.py after migration commit
5600105 deleted gateway/platforms/slack.py. Merge main.

Address hermes-sweeper review feedback:

- Port the handler from the deleted gateway/platforms/slack.py to
  plugins/platforms/slack/adapter.py.
- Add target-sender verification: check item_user / fetched message
  user against this bot's user_id for the workspace, matching the
  Feishu adapter's target-sender check. Reactions on messages not
  sent by this bot are dropped so a reaction on an unrelated human
  message can't enter _handle_slack_message.
- Update generated manifest (hermes_cli/slack_cli.py) to include
  reaction_added event and reactions:read scope.
- Add manifest tests for reactions:read and reaction_added.
- Add test_reaction_on_non_bot_message_dropped for target-sender
  verification.

Tests: 344 passed across slack test modules + manifest tests, 0 regressions.
teknium1 pushed a commit that referenced this pull request Jul 23, 2026
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).
teknium1 added a commit that referenced this pull request Jul 23, 2026
… handoff

Build the full reaction pipeline on top of the #29916 base:

- Opt-in gate: slack.reaction_triggers (default OFF — reaction events
  stay acked-and-dropped so busy channels don't wake the agent on every
  emoji). 'true' routes reactions on the bot's OWN messages; an explicit
  emoji-name list routes those emojis from any message (handoff flows).
- reaction_removed events now route too, distinguished by the
  cross-platform text convention reaction:added:<emoji> /
  reaction:removed:<emoji> (matches the Feishu and Photon adapters, so
  agents and skills see one shape everywhere).
- Authorization: the reactor becomes the synthesized message's user, so
  the early _is_user_authorized gate and allowed_channels whitelist
  apply exactly as for typed messages. _hermes_force_process only skips
  the mention requirement (a reaction on the bot's own message is
  definitionally addressed to the bot), mirroring Feishu/Photon.
- Gateway hooks (#33111 by @johnkattenhorn): every human reaction on a
  message item fires reaction:added / reaction:removed through the new
  BasePlatformAdapter.set_reaction_handler → GatewayRunner
  ._handle_reaction_event → HookRegistry.emit, independent of the
  routing opt-in. Documented in hooks.md.
- Channel handoff (#45265 by @Kev-fs): slack.reaction_trigger_target
  routes the reaction turn to a configured channel (top-level via
  _hermes_no_thread_response + reply-anchor suppression in
  gateway/platforms/base.py) or C123:<ts> thread.
- Manifest: reaction_removed event subscription added alongside
  reaction_added/reactions:read.
- Docs: slack.md Reaction Triggers section; hooks.md event table rows.

Also credits #44508 by @harrisonmedmedmetrics (inbound reaction_added
handling — same plumbing class, superseded by this consolidated shape).

Co-authored-by: johnkattenhorn <john.kattenhorn.personal@gmail.com>
Co-authored-by: Kev-fs <kevin@fleetsmarts.net>
Co-authored-by: harrisonmedmedmetrics <harrison@medmetricsrx.com>
teknium1 pushed a commit that referenced this pull request Jul 23, 2026
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).
teknium1 added a commit that referenced this pull request Jul 23, 2026
… handoff

Build the full reaction pipeline on top of the #29916 base:

- Opt-in gate: slack.reaction_triggers (default OFF — reaction events
  stay acked-and-dropped so busy channels don't wake the agent on every
  emoji). 'true' routes reactions on the bot's OWN messages; an explicit
  emoji-name list routes those emojis from any message (handoff flows).
- reaction_removed events now route too, distinguished by the
  cross-platform text convention reaction:added:<emoji> /
  reaction:removed:<emoji> (matches the Feishu and Photon adapters, so
  agents and skills see one shape everywhere).
- Authorization: the reactor becomes the synthesized message's user, so
  the early _is_user_authorized gate and allowed_channels whitelist
  apply exactly as for typed messages. _hermes_force_process only skips
  the mention requirement (a reaction on the bot's own message is
  definitionally addressed to the bot), mirroring Feishu/Photon.
- Gateway hooks (#33111 by @johnkattenhorn): every human reaction on a
  message item fires reaction:added / reaction:removed through the new
  BasePlatformAdapter.set_reaction_handler → GatewayRunner
  ._handle_reaction_event → HookRegistry.emit, independent of the
  routing opt-in. Documented in hooks.md.
- Channel handoff (#45265 by @Kev-fs): slack.reaction_trigger_target
  routes the reaction turn to a configured channel (top-level via
  _hermes_no_thread_response + reply-anchor suppression in
  gateway/platforms/base.py) or C123:<ts> thread.
- Manifest: reaction_removed event subscription added alongside
  reaction_added/reactions:read.
- Docs: slack.md Reaction Triggers section; hooks.md event table rows.

Also credits #44508 by @harrisonmedmedmetrics (inbound reaction_added
handling — same plumbing class, superseded by this consolidated shape).

Co-authored-by: johnkattenhorn <john.kattenhorn.personal@gmail.com>
Co-authored-by: Kev-fs <kevin@fleetsmarts.net>
Co-authored-by: harrisonmedmedmetrics <harrison@medmetricsrx.com>
teknium1 pushed a commit that referenced this pull request Jul 23, 2026
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).
teknium1 added a commit that referenced this pull request Jul 23, 2026
… handoff

Build the full reaction pipeline on top of the #29916 base:

- Opt-in gate: slack.reaction_triggers (default OFF — reaction events
  stay acked-and-dropped so busy channels don't wake the agent on every
  emoji). 'true' routes reactions on the bot's OWN messages; an explicit
  emoji-name list routes those emojis from any message (handoff flows).
- reaction_removed events now route too, distinguished by the
  cross-platform text convention reaction:added:<emoji> /
  reaction:removed:<emoji> (matches the Feishu and Photon adapters, so
  agents and skills see one shape everywhere).
- Authorization: the reactor becomes the synthesized message's user, so
  the early _is_user_authorized gate and allowed_channels whitelist
  apply exactly as for typed messages. _hermes_force_process only skips
  the mention requirement (a reaction on the bot's own message is
  definitionally addressed to the bot), mirroring Feishu/Photon.
- Gateway hooks (#33111 by @johnkattenhorn): every human reaction on a
  message item fires reaction:added / reaction:removed through the new
  BasePlatformAdapter.set_reaction_handler → GatewayRunner
  ._handle_reaction_event → HookRegistry.emit, independent of the
  routing opt-in. Documented in hooks.md.
- Channel handoff (#45265 by @Kev-fs): slack.reaction_trigger_target
  routes the reaction turn to a configured channel (top-level via
  _hermes_no_thread_response + reply-anchor suppression in
  gateway/platforms/base.py) or C123:<ts> thread.
- Manifest: reaction_removed event subscription added alongside
  reaction_added/reactions:read.
- Docs: slack.md Reaction Triggers section; hooks.md event table rows.

Also credits #44508 by @harrisonmedmedmetrics (inbound reaction_added
handling — same plumbing class, superseded by this consolidated shape).

Co-authored-by: johnkattenhorn <john.kattenhorn.personal@gmail.com>
Co-authored-by: Kev-fs <kevin@fleetsmarts.net>
Co-authored-by: harrisonmedmedmetrics <harrison@medmetricsrx.com>
teknium1 pushed a commit that referenced this pull request Jul 23, 2026
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).
teknium1 added a commit that referenced this pull request Jul 23, 2026
… handoff

Build the full reaction pipeline on top of the #29916 base:

- Opt-in gate: slack.reaction_triggers (default OFF — reaction events
  stay acked-and-dropped so busy channels don't wake the agent on every
  emoji). 'true' routes reactions on the bot's OWN messages; an explicit
  emoji-name list routes those emojis from any message (handoff flows).
- reaction_removed events now route too, distinguished by the
  cross-platform text convention reaction:added:<emoji> /
  reaction:removed:<emoji> (matches the Feishu and Photon adapters, so
  agents and skills see one shape everywhere).
- Authorization: the reactor becomes the synthesized message's user, so
  the early _is_user_authorized gate and allowed_channels whitelist
  apply exactly as for typed messages. _hermes_force_process only skips
  the mention requirement (a reaction on the bot's own message is
  definitionally addressed to the bot), mirroring Feishu/Photon.
- Gateway hooks (#33111 by @johnkattenhorn): every human reaction on a
  message item fires reaction:added / reaction:removed through the new
  BasePlatformAdapter.set_reaction_handler → GatewayRunner
  ._handle_reaction_event → HookRegistry.emit, independent of the
  routing opt-in. Documented in hooks.md.
- Channel handoff (#45265 by @Kev-fs): slack.reaction_trigger_target
  routes the reaction turn to a configured channel (top-level via
  _hermes_no_thread_response + reply-anchor suppression in
  gateway/platforms/base.py) or C123:<ts> thread.
- Manifest: reaction_removed event subscription added alongside
  reaction_added/reactions:read.
- Docs: slack.md Reaction Triggers section; hooks.md event table rows.

Also credits #44508 by @harrisonmedmedmetrics (inbound reaction_added
handling — same plumbing class, superseded by this consolidated shape).

Co-authored-by: johnkattenhorn <john.kattenhorn.personal@gmail.com>
Co-authored-by: Kev-fs <kevin@fleetsmarts.net>
Co-authored-by: harrisonmedmedmetrics <harrison@medmetricsrx.com>
@teknium1

Copy link
Copy Markdown
Contributor

Merged via #70195 — your commit was cherry-picked/reapplied onto current main with your authorship preserved in git history: your reaction-pipeline forwarding (earliest, May 21) is the base commit with your authorship.

Thanks for the contribution!

@teknium1 teknium1 closed this Jul 23, 2026
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
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 NousResearch#29916 by @bpross.
Related: NousResearch#33111, NousResearch#44508, NousResearch#45265 (same cluster).
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
… handoff

Build the full reaction pipeline on top of the NousResearch#29916 base:

- Opt-in gate: slack.reaction_triggers (default OFF — reaction events
  stay acked-and-dropped so busy channels don't wake the agent on every
  emoji). 'true' routes reactions on the bot's OWN messages; an explicit
  emoji-name list routes those emojis from any message (handoff flows).
- reaction_removed events now route too, distinguished by the
  cross-platform text convention reaction:added:<emoji> /
  reaction:removed:<emoji> (matches the Feishu and Photon adapters, so
  agents and skills see one shape everywhere).
- Authorization: the reactor becomes the synthesized message's user, so
  the early _is_user_authorized gate and allowed_channels whitelist
  apply exactly as for typed messages. _hermes_force_process only skips
  the mention requirement (a reaction on the bot's own message is
  definitionally addressed to the bot), mirroring Feishu/Photon.
- Gateway hooks (NousResearch#33111 by @johnkattenhorn): every human reaction on a
  message item fires reaction:added / reaction:removed through the new
  BasePlatformAdapter.set_reaction_handler → GatewayRunner
  ._handle_reaction_event → HookRegistry.emit, independent of the
  routing opt-in. Documented in hooks.md.
- Channel handoff (NousResearch#45265 by @Kev-fs): slack.reaction_trigger_target
  routes the reaction turn to a configured channel (top-level via
  _hermes_no_thread_response + reply-anchor suppression in
  gateway/platforms/base.py) or C123:<ts> thread.
- Manifest: reaction_removed event subscription added alongside
  reaction_added/reactions:read.
- Docs: slack.md Reaction Triggers section; hooks.md event table rows.

Also credits NousResearch#44508 by @harrisonmedmedmetrics (inbound reaction_added
handling — same plumbing class, superseded by this consolidated shape).

Co-authored-by: johnkattenhorn <john.kattenhorn.personal@gmail.com>
Co-authored-by: Kev-fs <kevin@fleetsmarts.net>
Co-authored-by: harrisonmedmedmetrics <harrison@medmetricsrx.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/slack Slack app adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants