Skip to content

feat(discord): add react/unreact actions to the discord tool - #67008

Open
Kyzcreig wants to merge 1 commit into
NousResearch:mainfrom
ANG-Ventures:upstream/discord-react-tool
Open

feat(discord): add react/unreact actions to the discord tool#67008
Kyzcreig wants to merge 1 commit into
NousResearch:mainfrom
ANG-Ventures:upstream/discord-react-tool

Conversation

@Kyzcreig

Copy link
Copy Markdown
Contributor

What

The discord REST tool exposed pin_message / unpin_message / delete_message but no way for an agent to add or remove an emoji reaction. The only reaction path was the Discord adapter's automatic 👀/✅/❌ processing markers, which are unreachable from a tool call — so an agent could not react to a Discord message at all. (send_message action="react" only supports adapters exposing a public add_reaction(chat_id, emoji, message_id) coroutine — Signal/iMessage — not Discord.)

Fix

Add react / unreact as core discord actions via pure REST:

PUT    /channels/{channel_id}/messages/{message_id}/reactions/{emoji}/@me   (add, 204)
DELETE /channels/{channel_id}/messages/{message_id}/reactions/{emoji}/@me   (remove own, 204)
  • The emoji is URL-encoded (urllib.parse.quote(emoji, safe="")) — Unicode percent-encoded, custom emoji as name:idname%3Aid, #%23 — so : / # / multibyte sequences never corrupt the path.
  • unreact removes only the bot's own reaction — stated in the action manifest so an agent doesn't read the idempotent 204 as "cleared everyone's reaction."
  • Per-action 403 hints: react needs ADD_REACTIONS + READ_MESSAGE_HISTORY; unreact needs only READ_MESSAGE_HISTORY (removing an own reaction doesn't need ADD_REACTIONS), so operators aren't told to over-grant.
  • _discord_request raises DiscordAPIError on every non-2xx, so the success result is emitted only on a genuine 204 — a 404/400/429 surfaces an error, never a fabricated success.
  • react/unreact inherit the existing discord.server_actions allowlist exactly (available by default when unset, same as pin_message/delete_message; gated when the key is a list).

Tests

tests/tools/test_discord_react.py: encoding (Unicode ✅, custom name:id, reserved #), PUT vs DELETE, missing-emoji error, non-403 (404) does not fabricate success, 403 actionable hint, allowlist gate, default-unset availability, schema/manifest wiring. Updated two core/admin schema-split tests to derive from _CORE_ACTION_NAMES instead of a hardcoded literal (behavior-contract, not change-detector). 109 tests pass.

Verified live: added + removed + re-added ✅ on a real Discord message via the runtime handler.

The discord REST tool exposed pin/unpin/delete_message but no way for an
agent to add or remove an emoji reaction — the only reaction path was the
adapter's automatic 👀/✅/❌ processing markers, unreachable from a tool call.
So an agent could not react to a Discord message at all.

Add react/unreact as core discord actions via pure REST:
  PUT/DELETE /channels/{cid}/messages/{mid}/reactions/{emoji}/@me
The emoji is URL-encoded (Unicode percent-encoded; custom emoji as name:id),
so ':' / '#' / multibyte sequences never corrupt the path. unreact removes
only the bot's OWN reaction (documented in the manifest so an agent doesn't
read the idempotent 204 as 'cleared everyone's'). Per-action 403 hints:
react needs ADD_REACTIONS+READ_MESSAGE_HISTORY, unreact only the latter.

_discord_request raises DiscordAPIError on every non-2xx, so the success
result is emitted only on a genuine 204 — a 404/400/429 surfaces an error,
never a fabricated success. react/unreact inherit the existing
discord.server_actions allowlist exactly (available by default, same as
pin/delete).

Tests: tests/tools/test_discord_react.py (encoding incl. custom + '#',
PUT/DELETE, missing-emoji, non-403 no-fake-success, 403 hint, allowlist
gate, default-unset availability, schema/manifest). Update two core/admin
schema-split tests to derive from _CORE_ACTION_NAMES instead of a hardcoded
literal (behavior-contract, not change-detector). Verified live: added +
removed + re-added ✅ on a real Discord message via the runtime handler.

Co-authored-by: Kyzcreig <9063726+Kyzcreig@users.noreply.github.com>
@alt-glitch alt-glitch added type/feature New feature or request comp/tools Tool registry, model_tools, toolsets platform/discord Discord bot adapter P3 Low — cosmetic, nice to have labels Jul 18, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #29026: this adds agent-initiated Discord REST reactions through the Discord tool; it is distinct from inbound reaction-event routing.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused Discord-tool addition. The premise is verified on current main: tools/discord_tool.py:633-655 has no react or unreact action, while tools/send_message_tool.py:263-352 requires public adapter reaction methods that Discord does not provide. The REST implementation in 5c0d9d89 follows the existing request/error path, preserves discord.server_actions, and tests encoding plus error and permission cases.

The branch is behind current main, but the current-main blobs for both modified existing files still match the PR base, so this appears mechanically salvageable.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 19, 2026
@Kyzcreig

Copy link
Copy Markdown
Contributor Author

Thanks for the review, and for verifying the premise against current main rather than taking the PR description's word for it.

On the branch being behind main: happy to rebase onto current main whenever that's useful — just say the word and I'll push it. I've deliberately left it alone so far to avoid churning the diff while it's queued for review, but if a fresh merge base makes it easier to assess I'll do it immediately.

No other changes pending on my side. Ping me if anything in the REST implementation or the error/permission test coverage wants reworking.

@eyeonall

Copy link
Copy Markdown

Cross-linking the convergence work: #89405 has been updated to incorporate the Discord reaction goals from this PR while keeping the final surface plugin-owned rather than exposing direct core Discord react / unreact tool actions.

Attribution for this contribution is included in #89405: #67008 by @Kyzcreig.

@Kyzcreig

Copy link
Copy Markdown
Contributor Author

Thanks @eyeonall — both for folding this in and for carrying the attribution over to #89405; appreciated.

keeping the final surface plugin-owned rather than exposing direct core Discord react / unreact tool actions

I read #89405 and agree this is the better shape for the repo: the outbound goals here (bot-applied reactions, REST v10 request building, error/permission handling) are covered by the manifest + delivery helpers, and plugin-owned semantics fits the "conservative at the waist" bar better than two new core tool actions ever did.

One scope note for the record, not a blocker: #89405 intentionally doesn't cover the ad-hoc case this PR exposed — a model/agent deciding at runtime to react to an arbitrary inbound message (the manifest path decorates the bot's own outbound sends; on_discord_reaction_add fires on reaction events). If that capability is ever wanted, it can ride the same plugin-owned machinery later — no reason to hold two heads open for it now.

Ceding right-of-way: #89405 supersedes this PR. I'll close this one when it lands (and happy to close sooner if a maintainer prefers). No further changes coming on this branch.

@alt-glitch alt-glitch removed the sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades label Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have platform/discord Discord bot adapter sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants