fix: route Slack reaction triggers to target channel - #45265
Conversation
26d34ad to
9c1ba91
Compare
|
A few notes on org-specific constants in the shared Slack adapter: 1. Hardcoded default trigger reactions ( 2. Org-specific workflow note ( if reaction == "karen":
source_lines.append(
"Workflow note: :karen: on #fleetsmarts-dev or #karen-train "
"is a request to start the code-change workflow..."
)This hardcodes 3. Default empty means safer: The Overall the implementation is solid — dedup, allowlists, target channel routing, and tests are well done. The concern is purely about shared-core defaults. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for implementing a concrete reaction-handoff path. The feature premise remains valid: current main still acknowledges reaction_added without routing it (plugins/platforms/slack/adapter.py:1121-1123).
Problems
- The implementation is stale after 5600105: Slack now lives in
plugins/platforms/slack/adapter.py, and Slack YAML bridging is plugin-owned atplugins/platforms/slack/adapter.py:4485-4519; this diff changes the former inline adapter/core bridge instead. gateway/platforms/slack.py:1724-1727defaults every workspace toplankton,karen. Triggering agent work must be opt-in, not an organization-specific global default.gateway/platforms/slack.py:1962-1970injects FleetSmarts channel names and workflow policy into the shared adapter. Please remove that tenant-specific branch or make any note neutral and explicitly configured.- The PR adds a reaction-trigger path but does not document the necessary Slack event subscription. The current setup list is
website/docs/user-guide/messaging/slack.md:122-133and does not includereaction_added.
Suggested changes
- Port the implementation and config bridge into the Slack bundled plugin, make triggers disabled until configured, document the event/configuration, and add tests for the disabled default and YAML bridge.
Automated hermes-sweeper review.
| """Return reaction names that should route the reacted-to message.""" | ||
| raw = self.config.extra.get("trigger_reactions") or os.getenv( | ||
| "SLACK_TRIGGER_REACTIONS", | ||
| "plankton,karen", |
There was a problem hiding this comment.
Please do not make plankton,karen a shared default. This enables agent-triggering behavior for every Slack workspace; use an empty default and require explicit operator configuration.
| source_lines.append(f"Original Slack message: {permalink}") | ||
| if reaction == "karen": | ||
| source_lines.append( | ||
| "Workflow note: :karen: on #fleetsmarts-dev or #karen-train " |
There was a problem hiding this comment.
This injects FleetSmarts-specific channel names and workflow policy into Hermes for every :karen: trigger. Remove it from the shared adapter or make a neutral equivalent explicitly configurable.
| @self._app.event("reaction_added") | ||
| async def handle_reaction_added(event, say): | ||
| pass | ||
| await self._handle_slack_reaction_added(event) |
There was a problem hiding this comment.
Please add the corresponding Slack setup/docs update for subscribing to reaction_added; current user guidance lists bot events at website/docs/user-guide/messaging/slack.md:122-133 but does not include it.
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).
… 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>
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).
… 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>
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).
… 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>
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).
… 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>
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).
… 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>
- john.kattenhorn.personal@gmail.com -> johnkattenhorn (NousResearch#33111) - harrison@medmetricsrx.com -> harrisonmedmedmetrics (NousResearch#44508) - kevin@fleetsmarts.net -> Kev-fs (NousResearch#45265)
Summary
Test Plan