From f159e86d1bea7169df52162def69ec84dbf5bfe7 Mon Sep 17 00:00:00 2001 From: Ritvik Gautam Date: Tue, 29 Sep 2026 09:05:54 +0530 Subject: [PATCH] fix(discord): notify owners on blocking prompts Extend the existing opt-in approval mention policy to slash confirmations, clarification questions, and update prompts. Keep mentions profile-scoped under multiplexing and bounded within Discord's content limit. The opt-in stays on the shared profile-scoped flag reader (extra_or_secret via _extra_or_env_flag) and main's YAML bridge, so a secondary profile never inherits the process env and env-over-YAML matches the other Discord flags. AllowedMentions names the exact numeric owners on all four prompts. Rebased onto main's localized prompt headers: the owner ping line now prefixes the catalog-driven slash-confirm, clarify, and update titles instead of the old English literals. Ritvik Gautam made this change with Doryani AI. Co-authored-by: nanobro <38958450+nanobro@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 --- hermes_cli/config_defaults.py | 3 +- plugins/platforms/discord/adapter.py | 86 +++++-- .../gateway/test_discord_approval_mentions.py | 235 ++++++++++++++++-- website/docs/user-guide/messaging/discord.md | 23 ++ 4 files changed, 309 insertions(+), 38 deletions(-) diff --git a/hermes_cli/config_defaults.py b/hermes_cli/config_defaults.py index db159a52e8d1f..3f7d413e43335 100644 --- a/hermes_cli/config_defaults.py +++ b/hermes_cli/config_defaults.py @@ -1593,7 +1593,8 @@ def _aux(timeout, *, reasoning_effort=True, **extra): # Max bytes per cached attachment (held in memory while written); 0 = no cap. Env: # DISCORD_MAX_ATTACHMENT_BYTES. "max_attachment_bytes": 33554432, - # Mention allowed users on approval prompts so owners notice them in shared channels. Env: + # Mention numeric allowed users on blocking prompts (command approvals, slash confirmations, + # clarify questions, update prompts) so owners notice them in shared channels. Env: # DISCORD_APPROVAL_MENTIONS. "approval_mentions": False, # Voice-channel inactivity timeout (seconds); 0 = stay until `/voice leave`. diff --git a/plugins/platforms/discord/adapter.py b/plugins/platforms/discord/adapter.py index f77f812d09385..76697bba44fd1 100644 --- a/plugins/platforms/discord/adapter.py +++ b/plugins/platforms/discord/adapter.py @@ -5515,14 +5515,50 @@ def _self_contained_prompt_content( return f"{prefix}{body}{suffix}" def _approval_mention_content(self) -> Optional[str]: - """User mentions for approval prompts, gated on ``discord.approval_mentions`` - (``DISCORD_APPROVAL_MENTIONS``). Only numeric allowlist entries; default off.""" + """Owner mentions for blocking prompts (exec approvals, slash confirmations, clarify, update + prompts), gated on ``discord.approval_mentions`` (``DISCORD_APPROVAL_MENTIONS``) through the + profile-scoped flag reader. Only numeric allowlist entries; default off.""" if not self._extra_or_env_flag("approval_mentions", "DISCORD_APPROVAL_MENTIONS", "false", truthy=True): return None user_ids = sorted(uid for uid in self._allowed_user_ids if str(uid).isdigit()) if not user_ids: return None - return " ".join(f"<@{uid}>" for uid in user_ids) + # Keep enough room for the actual blocking prompt. Large enterprise + # allowlists can otherwise make the mention line exceed Discord's + # 2,000-character message limit before the prompt body is added. + mention_budget = self.MAX_MESSAGE_LENGTH // 4 + mentions: list[str] = [] + used = 0 + for uid in user_ids: + token = f"<@{uid}>" + added = len(token) + (1 if mentions else 0) + if used + added > mention_budget: + break + mentions.append(token) + used += added + return " ".join(mentions) or None + + @staticmethod + def _interactive_prompt_send_kwargs( + *, content: str, embed: Any, view: Any = None, + mention_content: Optional[str] = None, + ) -> Dict[str, Any]: + """Build safe Discord kwargs for a blocking interactive prompt.""" + send_kwargs: Dict[str, Any] = {"content": content, "embed": embed} + if view is not None: + send_kwargs["view"] = view + if mention_content: + allowed_mentions_cls = getattr(discord, "AllowedMentions", None) + object_cls = getattr(discord, "Object", None) + if allowed_mentions_cls is not None and object_cls is not None: + owner_ids = [int(uid) for uid in re.findall(r"<@(\d+)>", mention_content)] + send_kwargs["allowed_mentions"] = allowed_mentions_cls( + users=[object_cls(id=uid) for uid in owner_ids], + roles=False, + everyone=False, + replied_user=False, + ) + return send_kwargs async def _send_prompt( self, chat_id: str, metadata: Optional[dict], build, *, fail_log: Optional[str] = None, @@ -5594,13 +5630,9 @@ def _build(_channel): admin_user_ids=admin_user_ids, allow_permanent="always" in choices, allow_session="session" in choices, smart_denied=prompt.smart_denied, ) - send_kwargs: Dict[str, Any] = {"content": content, "embed": embed, "view": view} - if mention_content: - allowed_mentions_cls = getattr(discord, "AllowedMentions", None) - if allowed_mentions_cls is not None: - send_kwargs["allowed_mentions"] = allowed_mentions_cls( - users=True, roles=False, everyone=False, replied_user=False, - ) + send_kwargs = self._interactive_prompt_send_kwargs( + content=content, embed=embed, view=view, mention_content=mention_content, + ) return send_kwargs, view return await self._send_prompt(prompt.chat_id, prompt.metadata, _build) @@ -5614,12 +5646,19 @@ def _build(_channel): # content only, so embed-rendering clients don't see it twice (#114693). header = title or t("platform.discord.approval.confirm_title") embed = discord.Embed(title=_truncate_discord_component_text(header, _DISCORD_EMBED_TITLE_LIMIT), color=discord.Color.orange()) - content = self._self_contained_prompt_content(f"**{header}**", message) + mention_content = self._approval_mention_content() + prompt_header = f"**{header}**" + if mention_content: + prompt_header = f"{mention_content}\n{prompt_header}" + content = self._self_contained_prompt_content(prompt_header, message) view = SlashConfirmView( session_key=session_key, confirm_id=confirm_id, allowed_user_ids=self._allowed_user_ids, allowed_role_ids=self._allowed_role_ids, ) - return {"content": content, "embed": embed, "view": view}, view + send_kwargs = self._interactive_prompt_send_kwargs( + content=content, embed=embed, view=view, mention_content=mention_content, + ) + return send_kwargs, view return await self._send_prompt(chat_id, metadata, _build) async def send_clarify( @@ -5663,12 +5702,16 @@ def _build(_channel): else: hint = t("platform.discord.prompt.clarify_hint_text") view = None + mention_content = self._approval_mention_content() + prompt_header = f"❓ **{clarify_title}**" + if mention_content: + prompt_header = f"{mention_content}\n{prompt_header}" content = self._self_contained_prompt_content( - f"❓ **{clarify_title}**", str(question or "").strip(), tail=f"\n\n{hint}", + prompt_header, str(question or "").strip(), tail=f"\n\n{hint}", + ) + send_kwargs = self._interactive_prompt_send_kwargs( + content=content, embed=embed, view=view, mention_content=mention_content, ) - send_kwargs = {"content": content, "embed": embed} - if view: - send_kwargs["view"] = view return send_kwargs, view return await self._send_prompt(chat_id, metadata, _build, fail_log="send_clarify") @@ -5688,8 +5731,15 @@ def _build(_channel): session_key=session_key, allowed_user_ids=self._allowed_user_ids, allowed_role_ids=self._allowed_role_ids, ) - content = self._self_contained_prompt_content(f"☤ **{update_title}**", f"{prompt}{default_hint}") - return {"content": content, "embed": embed, "view": view}, view + mention_content = self._approval_mention_content() + prompt_header = f"☤ **{update_title}**" + if mention_content: + prompt_header = f"{mention_content}\n{prompt_header}" + content = self._self_contained_prompt_content(prompt_header, f"{prompt}{default_hint}") + send_kwargs = self._interactive_prompt_send_kwargs( + content=content, embed=embed, view=view, mention_content=mention_content, + ) + return send_kwargs, view result = await self._send_prompt(chat_id, metadata, _build) if result.success and _metadata_marks_nonconversational(metadata): await self._nonconversational_messages.mark_many([result.message_id]) diff --git a/tests/gateway/test_discord_approval_mentions.py b/tests/gateway/test_discord_approval_mentions.py index 690afbb0c0e9c..6ad71d65315a9 100644 --- a/tests/gateway/test_discord_approval_mentions.py +++ b/tests/gateway/test_discord_approval_mentions.py @@ -1,14 +1,27 @@ -"""Discord approval prompts can opt into owner mentions.""" +"""Discord blocking prompts can opt into owner mentions.""" +import contextlib import os from types import SimpleNamespace import pytest -from plugins.platforms.discord.adapter import ( - DiscordAdapter, - _apply_yaml_config, -) +from gateway.config import PlatformConfig +from plugins.platforms.discord import adapter as discord_adapter +from plugins.platforms.discord.adapter import DiscordAdapter, _apply_yaml_config + + +class _FakeObject: + def __init__(self, *, id): + self.id = id + + +class _FakeAllowedMentions: + def __init__(self, *, users, roles, everyone, replied_user): + self.users = users + self.roles = roles + self.everyone = everyone + self.replied_user = replied_user class _FakeChannel: @@ -28,28 +41,214 @@ def get_channel(self, channel_id): return self.channel -@pytest.mark.asyncio -async def test_exec_approval_mentions_allowed_users_when_enabled(monkeypatch): - monkeypatch.setenv("DISCORD_APPROVAL_MENTIONS", "true") +@pytest.fixture(autouse=True) +def _stable_discord_types(monkeypatch): + monkeypatch.setattr(discord_adapter.discord, "Object", _FakeObject) + monkeypatch.setattr(discord_adapter.discord, "AllowedMentions", _FakeAllowedMentions) + + +def _make_adapter(*, extra=None, allowed_user_ids=None): channel = _FakeChannel() adapter = object.__new__(DiscordAdapter) adapter._client = _FakeClient(channel) - adapter._allowed_user_ids = {"222", "111", "alice"} + adapter._allowed_user_ids = allowed_user_ids or {"222", "111", "alice"} adapter._allowed_role_ids = set() - adapter.config = SimpleNamespace(extra=None) + adapter.config = PlatformConfig(enabled=True, extra=extra or {}) + return adapter, channel + + +# Every prompt that blocks the agent on an owner's answer. A non-owner mention in the body +# must stay inert: only the owners named in the ping line may be notified. +_PROMPTS = { + "exec_approval": lambda a: a.send_exec_approval( + chat_id="99", command="make check <@999>", session_key="session-1", description="dangerous command", + ), + "slash_confirm": lambda a: a.send_slash_confirm( + chat_id="99", title="Confirm reset", message="Reset session? <@999>", + session_key="session-1", confirm_id="confirm-1", + ), + "clarify_open": lambda a: a.send_clarify( + chat_id="99", question="Which environment? <@999>", choices=None, + clarify_id="clarify-1", session_key="session-1", + ), + "clarify_choices": lambda a: a.send_clarify( + chat_id="99", question="Which environment? <@999>", choices=["staging", "production"], + clarify_id="clarify-1", session_key="session-1", + ), + "update_prompt": lambda a: a.send_update_prompt( + chat_id="99", prompt="Restore stashed changes? <@999>", session_key="session-1", + ), +} + + +async def _send(prompt, adapter, channel): + result = await _PROMPTS[prompt](adapter) + assert result.success is True + assert channel.sent_kwargs is not None + return channel.sent_kwargs + + +def _assert_owner_ping(sent): + assert sent["content"].startswith("<@111> <@222>\n") + assert "<@alice>" not in sent["content"] + assert "<@999>" in sent["content"] + allowed_mentions = sent["allowed_mentions"] + assert [user.id for user in allowed_mentions.users] == [111, 222] + assert allowed_mentions.roles is False + assert allowed_mentions.everyone is False + assert allowed_mentions.replied_user is False + assert len(sent["content"]) <= DiscordAdapter.MAX_MESSAGE_LENGTH + + +@pytest.mark.asyncio +@pytest.mark.parametrize("prompt", sorted(_PROMPTS)) +async def test_blocking_prompts_ping_only_numeric_owners_when_enabled(monkeypatch, prompt): + monkeypatch.setenv("DISCORD_APPROVAL_MENTIONS", "true") + adapter, channel = _make_adapter() + + _assert_owner_ping(await _send(prompt, adapter, channel)) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("prompt", sorted(_PROMPTS)) +async def test_blocking_prompts_do_not_ping_by_default(monkeypatch, prompt): + monkeypatch.delenv("DISCORD_APPROVAL_MENTIONS", raising=False) + adapter, channel = _make_adapter() + + sent = await _send(prompt, adapter, channel) + + assert not sent["content"].startswith("<@") + assert "allowed_mentions" not in sent + + +_KEY = "DISCORD_APPROVAL_MENTIONS" - result = await adapter.send_exec_approval( + +@pytest.fixture +def multiplexed(monkeypatch): + """Multiplexed gateway with the process env var recorded, so the YAML bridge's writes are undone.""" + from agent import secret_scope + + monkeypatch.setattr(secret_scope, "_MULTIPLEX_ACTIVE", True) + monkeypatch.setenv(_KEY, "") + + def set_process_env(value): + if value is None: + monkeypatch.delenv(_KEY) + else: + monkeypatch.setenv(_KEY, value) + + return set_process_env + + +@contextlib.contextmanager +def _profile(scoped_env): + """Bind a secondary profile's secret scope; ``None`` is the unscoped default profile.""" + from agent import secret_scope + + if scoped_env is None: + yield + return + token = secret_scope.set_secret_scope(scoped_env) + try: + yield + finally: + secret_scope.reset_secret_scope(token) + + +def _profile_adapter(yaml_cfg, scoped_env): + """Load the profile's ``discord:`` YAML through the real bridge under its own scope.""" + with _profile(scoped_env): + return _make_adapter(extra=_apply_yaml_config({}, yaml_cfg)) + + +async def _pings(profile_adapter, scoped_env): + adapter, channel = profile_adapter + with _profile(scoped_env): + sent = await _send("clarify_open", adapter, channel) + if "allowed_mentions" not in sent: + assert not sent["content"].startswith("<@") + return False + _assert_owner_ping(sent) + return True + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "process_env,scoped_env,yaml_cfg,expected", + [ + # Default profile (unscoped): its own explicit env beats its YAML, either way. + ("true", None, {"approval_mentions": False}, True), + ("false", None, {"approval_mentions": True}, False), + (None, None, {"approval_mentions": True}, True), + (None, None, {"approval_mentions": False}, False), + # Secondary profile: its scoped env beats its YAML; the process env is never consulted. + ("false", {_KEY: "true"}, {"approval_mentions": False}, True), + ("true", {_KEY: "false"}, {"approval_mentions": True}, False), + ("true", {}, {"approval_mentions": False}, False), + ("false", {}, {"approval_mentions": True}, True), + ("true", {}, {}, False), + ], + ids=[ + "default-env-true-beats-yaml-false", "default-env-false-beats-yaml-true", + "default-yaml-true", "default-yaml-false", + "secondary-scoped-true-beats-yaml-false", "secondary-scoped-false-beats-yaml-true", + "secondary-yaml-false-ignores-process-true", "secondary-yaml-true-ignores-process-false", + "secondary-unset-ignores-process-true", + ], +) +async def test_mention_opt_in_resolves_env_then_own_yaml_per_profile( + multiplexed, process_env, scoped_env, yaml_cfg, expected, +): + multiplexed(process_env) + + assert await _pings(_profile_adapter(yaml_cfg, scoped_env), scoped_env) is expected + if scoped_env is not None: + assert os.environ.get(_KEY) == process_env # a secondary's YAML never reaches the process env + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "default_yaml,secondary_scope,secondary_yaml", + [(False, "true", False), (True, "false", True)], + ids=["secondary-opts-in", "secondary-opts-out"], +) +async def test_mention_opt_in_stays_with_the_owning_profile( + multiplexed, default_yaml, secondary_scope, secondary_yaml, +): + """Default → secondary → default → secondary on long-lived adapters with conflicting settings: + neither profile's opt-in (bridged env, scoped env or YAML) leaks into the other's prompts.""" + multiplexed(None) + scoped_env = {_KEY: secondary_scope} + default = _profile_adapter({"approval_mentions": default_yaml}, None) + secondary = _profile_adapter({"approval_mentions": secondary_yaml}, scoped_env) + + assert os.environ[_KEY] == str(default_yaml).lower() + for _ in range(2): + assert await _pings(default, None) is default_yaml + assert await _pings(secondary, scoped_env) is (secondary_scope == "true") + + +@pytest.mark.asyncio +async def test_large_allowlist_keeps_prompt_within_discord_limit(monkeypatch): + monkeypatch.setenv("DISCORD_APPROVAL_MENTIONS", "true") + allowed_user_ids = {str(100_000_000_000_000_000 + index) for index in range(120)} + adapter, channel = _make_adapter(allowed_user_ids=allowed_user_ids) + + result = await adapter.send_clarify( chat_id="99", - command="make check", + question="Q" * 4_000, + choices=None, + clarify_id="clarify-1", session_key="session-1", - description="dangerous command", ) assert result.success is True - # Mentions are prepended to the (always present) content mirror. - assert channel.sent_kwargs["content"].startswith("<@111> <@222>\n") - assert "make check" in channel.sent_kwargs["content"] - assert "allowed_mentions" in channel.sent_kwargs + content = channel.sent_kwargs["content"] + assert content.startswith("<@") + assert len(content) <= adapter.MAX_MESSAGE_LENGTH + assert content.count("<@") < len(allowed_user_ids) + assert len(channel.sent_kwargs["allowed_mentions"].users) == content.count("<@") def test_yaml_config_seeds_websocket_health_with_primary_precedence(monkeypatch): @@ -78,5 +277,3 @@ def test_yaml_config_seeds_websocket_health_with_primary_precedence(monkeypatch) "websocket_heartbeat_ack_max_age_seconds": 75, "websocket_max_latency_seconds": 30, } - - diff --git a/website/docs/user-guide/messaging/discord.md b/website/docs/user-guide/messaging/discord.md index d639cea111b76..6997b4a35596d 100644 --- a/website/docs/user-guide/messaging/discord.md +++ b/website/docs/user-guide/messaging/discord.md @@ -774,6 +774,29 @@ The buttons disable themselves once a choice is made so duplicate clicks don't d Interactive prompts (command approvals, `clarify` questions, and slash-command confirmations) share one layout: the **plain message** carries the full payload — the command and why it was flagged plus the approval deadline, or the question and reply hint — the **embed card** underneath is a header only, and the buttons sit below the card. Everything you need to decide is in the plain text, so the prompt reads correctly on clients that hide or detach embeds, and nothing is shown twice on clients that render them. +### Notify owners when input is waiting + +Discord does not notify channel members for an ordinary bot message. To ping the +numeric users in `discord.allow_from` whenever Hermes is blocked on input, enable: + +```yaml +discord: + approval_mentions: true +``` + +This covers dangerous-command approvals, slash-command confirmations, `clarify` +questions, and update prompts. `DISCORD_APPROVAL_MENTIONS=true` is the equivalent +environment override. Mentions are opt-in to avoid surprise notifications; only +numeric allowlisted users are pinged, while role, `@everyone`, and reply-reference +notifications remain disabled. A very large allowlist is cut to the first owners +(by ID) that fit a quarter of Discord's 2,000-character message, so the prompt +itself always fits. + +A nonempty environment setting overrides YAML, as with the other Discord flags. +Each profile's gateway bot follows only its own profile's `.env` and +`config.yaml`: one profile opting in never pings owners for another profile's +prompts. + ## Home Channel You can designate a "home channel" where the bot sends proactive messages (such as cron job output, reminders, and notifications). There are two ways to set it: