Skip to content

fix(discord): mention user in approval/clarify prompts for notification ping - #47342

Open
nanobro wants to merge 1 commit into
NousResearch:mainfrom
nanobro:fix/discord-approval-notification
Open

fix(discord): mention user in approval/clarify prompts for notification ping#47342
nanobro wants to merge 1 commit into
NousResearch:mainfrom
nanobro:fix/discord-approval-notification

Conversation

@nanobro

@nanobro nanobro commented Jun 16, 2026

Copy link
Copy Markdown

What

When Hermes needs user approval on Discord, it now mentions the user (<@USER_ID>) so they get a native Discord notification ping instead of silently waiting in a thread.

Problem

Approval/clarify prompts were sent into threads without any mention. Users would miss requests entirely because there was no notification.

Solution

  • Added user_mention parameter to 4 Discord adapter methods:

    • send_exec_approval() — dangerous command approval
    • send_slash_confirm() — expensive slash command confirmation
    • send_clarify() — agent clarification prompts
    • send_update_prompt() — gateway update input
  • Gateway builds <@user_id> mention for Discord platform only (other platforms unaffected)

  • Mention is prepended to embed description, triggering Discord's native ping

Files Changed

File Change
gateway/run.py Build _user_mention in 3 callback functions
plugins/platforms/discord/adapter.py Accept and prepend user_mention in 4 send methods
docs/howto-discord-approval-notify.md Community how-to documentation

Testing

  • ✅ Syntax check passed (py_compile)
  • ✅ No secrets/credentials in diff
  • ✅ Gateway restarted successfully with changes

…on ping

- Add user_mention parameter to Discord adapter send methods
  (send_exec_approval, send_slash_confirm, send_clarify, send_update_prompt)
- Build <@user_id> mention in gateway callbacks for Discord platform only
- Prepends mention to embed description so user gets native Discord ping
- Adds howto doc for community sharing
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery platform/discord Discord bot adapter labels Jun 16, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for targeting a real notification gap. Current main still defaults to no exec-approval mention and does not mention users for slash confirmations, clarify prompts, or update prompts.

Problems

  • The new user_mention keyword is passed from generic gateway paths in gateway/run.py:10713, :13986, and :14112, including non-Discord adapters. Current shared adapter contracts such as gateway/platforms/base.py:3076-3084 do not accept it, so this can turn native prompt rendering into a TypeError fallback.
  • The adapter changes put mentions only in embed descriptions. Current main's accepted exec-mention implementation places the mention in plain message content and configures AllowedMentions (plugins/platforms/discord/adapter.py:5609-5611, :5647-5657). The sibling prompts now also use content mirrors (:5691-5702, :5823-5827, :5864-5869), so the patch needs to target those paths.
  • No regression tests accompany the four changed send methods or the cross-platform gateway contract.

Suggested changes

  • Keep the Discord-specific data out of generic adapter calls unless the common contract is intentionally extended and all implementations are covered.
  • Build on the current content-mirror and opt-in mention mechanism, then add focused Discord and non-Discord dispatch tests.

Automated hermes-sweeper review.

Comment thread gateway/run.py
@@ -10708,6 +10713,7 @@ async def _request_slash_confirm(
session_key=session_key,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This call is generic: non-Discord adapters are invoked too, but their shared send_slash_confirm contract has no user_mention keyword (gateway/platforms/base.py:3076-3084). Passing it even as None can raise TypeError and discard the native-button path. Scope this argument to Discord or extend the contract consistently.

# Embed description limit is 4096; message usually fits easily.
max_desc = 4088
body = message if len(message) <= max_desc else message[: max_desc - 3] + "..."
if user_mention:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current main sends interactive-prompt payloads in ordinary content as well as embeds, and its existing exec-approval mention path prepends the mention to that content with scoped AllowedMentions. Put the notification marker in the content mirror rather than only this embed description.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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 14, 2026
@doryani-ai

Copy link
Copy Markdown

Thanks for identifying the exact four-prompt notification gap, @nanobro. I reproduced it in production and opened #82982 as a current-main successor after this branch remained conflicted and the July review feedback was unresolved. The new PR credits you as co-author, preserves this PR's scope, and builds on the merged opt-in content-mirror/AllowedMentions design from #60493. It also adds multiplex-profile isolation, exact owner-only AllowedMentions, a bounded large-allowlist path, focused regression coverage, and docs. Maintainers can use whichever contribution path they prefer.

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 P3 Low — cosmetic, nice to have platform/discord Discord bot 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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants