Skip to content

fix(discord): resolve outbound @Name mentions to real <@id> (opt-in) - #69206

Closed
tieubao wants to merge 2 commits into
NousResearch:mainfrom
dwarvesf:fix/discord-outbound-mention-resolution
Closed

fix(discord): resolve outbound @Name mentions to real <@id> (opt-in)#69206
tieubao wants to merge 2 commits into
NousResearch:mainfrom
dwarvesf:fix/discord-outbound-mention-resolution

Conversation

@tieubao

@tieubao tieubao commented Jul 22, 2026

Copy link
Copy Markdown

What

Adds opt-in outbound mention resolution to the Discord adapter. When DISCORD_RESOLVE_MENTIONS is enabled, a bot's readable @Display Name in an outgoing message is rewritten to a real <@id> mention so it actually pings.

Fixes #ISSUE.

Why

An LLM reliably writes @Name rather than the raw <@123...> Discord requires, so an agent's attempt to tag a user, or hand off to another bot, currently comes out as inert plain text. The Feishu adapter already resolves mentions on outbound; this brings Discord to parity, reusing the guild-member name→ID matching the Discord adapter already has in _resolve_allowed_usernames.

How

  • New _resolve_outbound_mentions(content, channel): matches guild members by name / display_name / global_name (case-insensitive, longest name first so @Support Bot wins over a member named Support) and rewrites @Name<@id>.
  • Called once in send(), before format_message().
  • Opt-in and inert by default: with DISCORD_RESOLVE_MENTIONS unset/false the method returns the content unchanged, so behavior is byte-identical for every existing deployment.
  • Only user mentions are produced; @everyone/roles remain governed by the existing allowed_mentions safe defaults. Already-formed <@id>, emails (me@host), and unknown names are left untouched.

Tests

Unit tests cover: name→mention, case-insensitivity, multiple names in one message, flag-off no-op, already-real mention untouched, and email / unknown-name left alone.

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins platform/discord Discord bot adapter area/config Config system, migrations, profiles sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 22, 2026
@PRATHAMESH75

Copy link
Copy Markdown
Contributor

Reviewed against #69203 — the runtime change is clean and does what the issue asks.

Behavior. Opt-in via DISCORD_RESOLVE_MENTIONS (default off → no behavior change), guild-member matching over display_name/global_name/name, longest-name-first so @Support Bot wins over a member named Support, idempotent on an already-formed <@id>, and the (?<![\w<@])@Name(?![\w]) guard correctly skips me@example.com. It mirrors the Feishu adapter's outbound resolution as intended, and os/re are already imported at adapter module scope (adapter.py:19-20), so the send() path is fine.

One thing worth changing — the test reads source. tests/gateway/test_discord_outbound_mentions.py does ast.parse(open(_ADAPTER).read()) and execs the extracted function in a hand-built namespace. That conflicts with the AGENTS.md "don't read source in tests" convention (the same ast.parse(<source>) pattern was flagged and removed on a prior PR), and because the test injects its own ns = {"os", "re", ...}, it would still pass even if the real module stopped importing re/os — i.e. it doesn't ride the real import path. Sibling tests import the symbol directly; e.g. tests/gateway/test_discord_allowed_mentions.py does from plugins.platforms.discord.adapter import _build_allowed_mentions. Suggest either exercising the method on a lightweight adapter instance, or extracting the matcher as a module-level helper (like _build_allowed_mentions) and importing it — same assertions, real import path.

Otherwise this is a solid parity fix.

@tieubao

tieubao commented Jul 22, 2026

Copy link
Copy Markdown
Author

Addressed in cc0fdb86. The test now imports DiscordAdapter and exercises _resolve_outbound_mentions on an init-bypassed instance (object.__new__), matching the sibling test_discord_allowed_mentions which imports _build_allowed_mentions directly. It rides the real import path — it would fail if adapter.py stopped importing re/os — and no longer ast.parse/execs adapter source. Same assertions; 9 passed.

tieubao added 2 commits July 22, 2026 17:56
An LLM writes @name rather than the raw <@id> Discord needs, so an agent's attempt to tag a user or hand off to another bot is inert plain text. The Feishu adapter already resolves mentions on outbound; this adds the same to Discord, reusing the guild-member name->id matching the adapter already has (_resolve_allowed_usernames).

Opt-in via DISCORD_RESOLVE_MENTIONS (unset/false = byte-identical behavior). @everyone/roles stay governed by the existing allowed_mentions safe defaults.

Fixes #69203

Signed-off-by: Han Ngo <nntruonghan@gmail.com>
@tieubao
tieubao force-pushed the fix/discord-outbound-mention-resolution branch from cc0fdb8 to b8103fb Compare July 22, 2026 10:57
@teknium1

Copy link
Copy Markdown
Contributor

Thank you for the focused Discord parity work and for addressing the earlier test-import feedback.

This automated hermes-sweeper review is closing the PR under the standing configuration policy:

  • The opt-in is a new non-secret behavioral flag, DISCORD_RESOLVE_MENTIONS (plugins/platforms/discord/adapter.py:4798 in PR head b8103fb63375). Maintainer policy requires behavioral settings and feature flags to be exposed through config.yaml, rather than a new user-facing environment variable (AGENTS.md:102-107).
  • The Discord adapter already has the established discord: YAML-to-legacy-env bridge in _apply_yaml_config (plugins/platforms/discord/adapter.py:9628-9734) and defaults in hermes_cli/config_defaults.py:1782.

A focused re-scope using a documented discord.resolve_outbound_mentions config.yaml setting (with any internal compatibility bridge kept private) would fit the project configuration model.


Closed as not-planned per standing maintainer policy (env-var-for-config). This is a design-direction decision, not a code-quality judgment — see the Contribution Rubric in AGENTS.md for what the project is looking for. If you believe this policy was misapplied to your change, comment here and a maintainer will take a look.

@teknium1 teknium1 closed this Jul 30, 2026
@teknium1 teknium1 added the sweeper:not-planned Sweeper: closed per standing maintainer policy (design direction) label Jul 30, 2026
mehmetkr-31 added a commit to mehmetkr-31/hermes-agent that referenced this pull request Aug 4, 2026
… edit + forum paths

Re-scope of NousResearch#69206 (closed under the env-var-for-config policy) to the shape
the sweeper asked for: the opt-in is the documented
``discord.resolve_outbound_mentions`` setting in config.yaml, bridged to the
internal ``DISCORD_RESOLVE_MENTIONS`` var by ``_apply_yaml_config``, exactly as
the neighbouring ``discord.approval_mentions`` works. The bridge is kept
private -- the default-config comment documents only the config key, and no
test pins the env override.

Three delivery paths, not one. The original resolved mentions in send() only:

* the forum branch returned before the resolver ran, so a forum thread's
  starter post kept the inert @name;
* edit_message() -- which is how every streamed reply is delivered -- never
  resolved at all, and neither did its _edit_overflow_split() continuation
  chunks, which re-format the same content.

The resolver now runs above the forum branch in send(), and once in
edit_message() on the FINAL edit. Mid-stream is deliberately skipped: the text
is still partial there, so a member named "Al" matches while "@alice" is only
half-written, and an edit can deliver that ping. Resolving once at finalize
means the message the user keeps is the correct one, and it also covers the
overflow-split path.

guild.members is empty unless the privileged Server Members intent is
requested, which startup asked for only for named allowlist entries or role
authorization -- so turning the setting on resolved nothing and gave no clue
why. The condition moves to a module-level _needs_members_intent() helper that
includes this opt-in and is testable without a live client. The "*" open-mode
wildcard still does not pull the intent in, so the migrate-from-OpenClaw path
is unchanged.

The Developer Portal prerequisite is documented where someone setting this up
actually reads: the intent table and its explanation in the Discord setup
guide, the discord: config block next to the other settings, and voice-mode.md,
whose "only needed if your allowlist uses usernames" claim was now incomplete.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have platform/discord Discord bot adapter sweeper:not-planned Sweeper: closed per standing maintainer policy (design direction) sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants