diff --git a/gateway/platforms/discord.py b/gateway/platforms/discord.py index bcca80c5b5f..4793df35c7c 100644 --- a/gateway/platforms/discord.py +++ b/gateway/platforms/discord.py @@ -3896,6 +3896,84 @@ class DiscordAdapter(BasePlatformAdapter): except Exception as e: return SendResult(success=False, error=str(e)) + async def send_clarify( + self, + chat_id: str, + question: str, + choices: Optional[list], + clarify_id: str, + session_key: str, + metadata: Optional[Dict[str, Any]] = None, + ) -> SendResult: + """Render a clarify prompt with one Discord button per choice. + + Multi-choice mode (``choices`` non-empty): renders a button per option + plus a final "✏️ Other (type answer)" button. Picking "Other" flips + the clarify entry into text-capture mode so the next user message in + the session becomes the response. Numeric clicks resolve immediately + via ``resolve_gateway_clarify(clarify_id, choice_text)``. + + Open-ended mode (``choices`` empty/None): renders the question as + plain embed text — no buttons. The gateway's text-intercept captures + the next message in this session and resolves the clarify. + """ + if not self._client or not DISCORD_AVAILABLE: + return SendResult(success=False, error="Not connected") + + try: + target_id = chat_id + if metadata and metadata.get("thread_id"): + target_id = metadata["thread_id"] + + channel = self._client.get_channel(int(target_id)) + if not channel: + channel = await self._client.fetch_channel(int(target_id)) + + # Discord embed description limit is 4096; trim conservatively. + max_desc = 4088 + body = str(question or "").strip() + if len(body) > max_desc: + body = body[: max_desc - 3] + "..." + + embed = discord.Embed( + title="❓ Hermes needs your input", + description=body, + color=discord.Color.orange(), + ) + + clean_choices = [ + str(c).strip() for c in (choices or []) if c is not None and str(c).strip() + ] + # Discord allows up to 5 buttons per row, 5 rows per view = 25. + # We reserve one slot for the "Other" button, so cap at 24 choices. + clean_choices = clean_choices[:24] + + if clean_choices: + embed.add_field( + name="Choices", + value="Pick one below, or click ✏️ Other to type a custom answer.", + inline=False, + ) + view = ClarifyChoiceView( + choices=clean_choices, + clarify_id=clarify_id, + allowed_user_ids=self._allowed_user_ids, + allowed_role_ids=self._allowed_role_ids, + ) + else: + embed.add_field( + name="Reply", + value="Reply in this channel with your answer.", + inline=False, + ) + view = None + + msg = await channel.send(embed=embed, view=view) if view else await channel.send(embed=embed) + return SendResult(success=True, message_id=str(msg.id)) + except Exception as e: + logger.warning("[%s] send_clarify failed: %s", self.name, e) + return SendResult(success=False, error=str(e)) + async def send_update_prompt( self, chat_id: str, prompt: str, default: str = "", session_key: str = "", @@ -5138,3 +5216,188 @@ if DISCORD_AVAILABLE: async def on_timeout(self): self.resolved = True self.clear_items() + + + class ClarifyChoiceView(discord.ui.View): + """Interactive button view for the clarify tool's multiple-choice prompts. + + Renders one button per choice (max 24) plus a final ``✏️ Other`` button. + Picking a numeric choice resolves the gateway clarify entry immediately; + picking ``Other`` flips the entry into text-capture mode so the next + user message in the session becomes the response (the gateway's + text-intercept handles the resolution). + + Auth gating mirrors ``ExecApprovalView`` — only users/roles in the + Discord adapter's allowlist may answer. Single-use: after the first + valid click all buttons disable and the embed updates to show who + answered and what they chose. + """ + + def __init__( + self, + choices: List[str], + clarify_id: str, + allowed_user_ids: set, + allowed_role_ids: Optional[set] = None, + ): + super().__init__(timeout=300) # 5-minute timeout + self.choices = list(choices)[:24] + self.clarify_id = clarify_id + self.allowed_user_ids = allowed_user_ids + self.allowed_role_ids = allowed_role_ids or set() + self.resolved = False + + for index, choice in enumerate(self.choices): + # Discord button labels are capped at 80 chars. + label_body = choice if len(choice) <= 75 else choice[:72] + "..." + button = discord.ui.Button( + label=f"{index + 1}. {label_body}", + style=discord.ButtonStyle.primary, + custom_id=f"clarify:{clarify_id}:{index}", + ) + button.callback = self._make_choice_callback(index, choice) + self.add_item(button) + + other_btn = discord.ui.Button( + label="✏️ Other (type answer)", + style=discord.ButtonStyle.secondary, + custom_id=f"clarify:{clarify_id}:other", + ) + other_btn.callback = self._on_other + self.add_item(other_btn) + + def _check_auth(self, interaction: "discord.Interaction") -> bool: + return _component_check_auth( + interaction, self.allowed_user_ids, self.allowed_role_ids, + ) + + def _make_choice_callback(self, index: int, choice: str): + async def _callback(interaction: "discord.Interaction"): + await self._resolve_choice(interaction, index, choice) + return _callback + + async def _resolve_choice( + self, + interaction: "discord.Interaction", + index: int, + choice: str, + ) -> None: + """Resolve the clarify with a chosen option.""" + if self.resolved: + await interaction.response.send_message( + "This prompt has already been answered~", ephemeral=True, + ) + return + if not self._check_auth(interaction): + await interaction.response.send_message( + "You're not authorized to answer this prompt~", ephemeral=True, + ) + return + + self.resolved = True + for child in self.children: + child.disabled = True + + embed = interaction.message.embeds[0] if ( + interaction.message and interaction.message.embeds + ) else None + if embed: + user = getattr(interaction, "user", None) + display_name = getattr(user, "display_name", "user") + embed.color = discord.Color.green() + embed.set_footer(text=f"Answered by {display_name}: {choice}") + + try: + await interaction.response.edit_message(embed=embed, view=self) + except Exception: + logger.debug( + "Discord clarify edit_message failed for %s", + self.clarify_id, + exc_info=True, + ) + try: + await interaction.response.defer() + except Exception: + pass + + # Resolve via the gateway clarify primitive — same mechanism as + # Telegram. Look up the canonical choice text from the entry so + # we round-trip the original value, not a button-label variant. + resolved_text: Optional[str] = None + try: + from tools.clarify_gateway import _entries as _clarify_entries # type: ignore + entry = _clarify_entries.get(self.clarify_id) + if entry and entry.choices and 0 <= index < len(entry.choices): + resolved_text = entry.choices[index] + except Exception: + resolved_text = None + if resolved_text is None: + resolved_text = choice + + try: + from tools.clarify_gateway import resolve_gateway_clarify + resolved = resolve_gateway_clarify(self.clarify_id, resolved_text) + logger.info( + "Discord clarify button resolved (id=%s, choice=%r, user=%s, ok=%s)", + self.clarify_id, resolved_text, + getattr(getattr(interaction, "user", None), "display_name", "?"), + resolved, + ) + except Exception as exc: + logger.error( + "Discord clarify resolve_gateway_clarify failed (id=%s): %s", + self.clarify_id, exc, + ) + + async def _on_other(self, interaction: "discord.Interaction") -> None: + """Flip the clarify entry into text-capture mode.""" + if self.resolved: + await interaction.response.send_message( + "This prompt has already been answered~", ephemeral=True, + ) + return + if not self._check_auth(interaction): + await interaction.response.send_message( + "You're not authorized to answer this prompt~", ephemeral=True, + ) + return + + # Don't pop the entry — the gateway's text-intercept needs it + # until the user actually types. Just mark it as awaiting text + # and disable the buttons so the user can't double-click. + try: + from tools.clarify_gateway import mark_awaiting_text + mark_awaiting_text(self.clarify_id) + except Exception as exc: + logger.warning( + "Discord clarify mark_awaiting_text failed (id=%s): %s", + self.clarify_id, exc, + ) + + self.resolved = True + for child in self.children: + child.disabled = True + + embed = interaction.message.embeds[0] if ( + interaction.message and interaction.message.embeds + ) else None + if embed: + user = getattr(interaction, "user", None) + display_name = getattr(user, "display_name", "user") + embed.color = discord.Color.blue() + embed.set_footer( + text=f"Awaiting typed response from {display_name}…", + ) + + try: + await interaction.response.edit_message(embed=embed, view=self) + except Exception: + try: + await interaction.response.defer() + except Exception: + pass + + async def on_timeout(self): + self.resolved = True + for child in self.children: + child.disabled = True diff --git a/tests/gateway/conftest.py b/tests/gateway/conftest.py index da8a2d33641..b6bcc28c506 100644 --- a/tests/gateway/conftest.py +++ b/tests/gateway/conftest.py @@ -119,6 +119,14 @@ def _ensure_discord_mock() -> None: self.title = title self.description = description self.color = color + self.fields = [] + self.footer = None + def add_field(self, *, name=None, value=None, inline=False, **_): + self.fields.append({"name": name, "value": value, "inline": inline}) + return self + def set_footer(self, *, text=None, icon_url=None, **_): + self.footer = {"text": text, "icon_url": icon_url} + return self discord_mod.Embed = _FakeEmbed # ui.View / ui.Select / ui.Button: real classes (not MagicMock) so diff --git a/tests/gateway/test_discord_clarify_buttons.py b/tests/gateway/test_discord_clarify_buttons.py new file mode 100644 index 00000000000..b6e21f1f44b --- /dev/null +++ b/tests/gateway/test_discord_clarify_buttons.py @@ -0,0 +1,408 @@ +"""Tests for Discord clarify button rendering and resolution. + +Mirrors test_telegram_clarify_buttons.py for the Discord ``send_clarify`` +override and the ``ClarifyChoiceView`` callbacks. Discord uses ``discord.ui.View`` +button callbacks (closures) rather than a string-prefixed callback_query +dispatcher like Telegram — the auth + resolution path is the same: + + · numeric choice → resolve_gateway_clarify(clarify_id, choice_text) + · "Other" button → mark_awaiting_text(clarify_id) so the text-intercept + captures the next user message in this session + · already-resolved or unauthorized → ephemeral "this prompt..." reply +""" + +import asyncio +import sys +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock + +import pytest + +# Repo root importable +_repo = str(Path(__file__).resolve().parents[2]) +if _repo not in sys.path: + sys.path.insert(0, _repo) + +# Triggers the shared discord mock from tests/gateway/conftest.py before +# importing the production module. +from gateway.platforms.discord import ( # noqa: E402 + ClarifyChoiceView, + DiscordAdapter, +) +from gateway.config import PlatformConfig # noqa: E402 + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _make_adapter(*, allowed_users=None, allowed_roles=None): + config = PlatformConfig(enabled=True, token="test-token", extra={}) + adapter = DiscordAdapter(config) + adapter._client = MagicMock() + adapter._allowed_user_ids = set(allowed_users or []) + adapter._allowed_role_ids = set(allowed_roles or []) + return adapter + + +def _clear_clarify_state(): + from tools import clarify_gateway as cm + with cm._lock: + cm._entries.clear() + cm._session_index.clear() + cm._notify_cbs.clear() + + +def _make_interaction(*, user_id="42", display_name="Tester", roles=None, + include_message=True): + """Build a mock discord.Interaction with response.edit_message / + send_message / defer all coroutine-callable.""" + user = SimpleNamespace( + id=user_id, + display_name=display_name, + roles=[SimpleNamespace(id=r) for r in (roles or [])], + ) + response = SimpleNamespace( + edit_message=AsyncMock(), + send_message=AsyncMock(), + defer=AsyncMock(), + ) + if include_message: + embed = MagicMock() + embed.color = None + embed.set_footer = MagicMock() + message = SimpleNamespace(embeds=[embed]) + else: + message = None + return SimpleNamespace(user=user, response=response, message=message) + + +# =========================================================================== +# ClarifyChoiceView construction +# =========================================================================== + +class TestClarifyChoiceViewConstruction: + """The view should build numeric buttons plus an Other button.""" + + def test_renders_n_choice_buttons_plus_other(self): + view = ClarifyChoiceView( + choices=["apple", "banana", "cherry"], + clarify_id="cidX", + allowed_user_ids={"42"}, + ) + # 3 numeric + 1 "Other" + assert len(view.children) == 4 + labels = [b.label for b in view.children] + assert labels[0].startswith("1. apple") + assert labels[1].startswith("2. banana") + assert labels[2].startswith("3. cherry") + assert "Other" in labels[3] + # custom_ids encode clarify_id + index/other + ids = [b.custom_id for b in view.children] + assert ids[0] == "clarify:cidX:0" + assert ids[1] == "clarify:cidX:1" + assert ids[2] == "clarify:cidX:2" + assert ids[3] == "clarify:cidX:other" + + def test_caps_at_24_choices_plus_other(self): + choices = [f"choice-{i}" for i in range(50)] + view = ClarifyChoiceView( + choices=choices, + clarify_id="cidY", + allowed_user_ids=set(), + ) + # Discord limit is 25 components; we cap choices at 24 + 1 Other = 25 + assert len(view.children) == 25 + assert "Other" in view.children[-1].label + + def test_truncates_long_choice_label(self): + long_choice = "x" * 200 + view = ClarifyChoiceView( + choices=[long_choice], + clarify_id="cidZ", + allowed_user_ids=set(), + ) + # 75 chars + 3 ellipsis chars in the body, plus "1. " prefix + first_label = view.children[0].label + assert first_label.startswith("1. ") + assert first_label.endswith("...") + # Final label total <= 80 (Discord cap on button labels) + assert len(first_label) <= 80 + + +# =========================================================================== +# Choice callback → resolve_gateway_clarify +# =========================================================================== + +class TestClarifyChoiceResolve: + """Clicking a numeric button should resolve the clarify entry.""" + + def setup_method(self): + _clear_clarify_state() + + @pytest.mark.asyncio + async def test_choice_resolves_with_canonical_choice_text(self): + from tools import clarify_gateway as cm + cm.register("cidA", "sk-A", "Pick", ["red", "green", "blue"]) + + view = ClarifyChoiceView( + choices=["red", "green", "blue"], + clarify_id="cidA", + allowed_user_ids={"42"}, + ) + + interaction = _make_interaction(user_id="42") + await view._resolve_choice(interaction, index=1, choice="green") + + # Resolved through clarify primitive + with cm._lock: + entry = cm._entries.get("cidA") + assert entry is not None + assert entry.response == "green" + assert entry.event.is_set() + # Buttons disabled + assert all(b.disabled for b in view.children) + # Embed updated + edit_message called + interaction.response.edit_message.assert_called_once() + + @pytest.mark.asyncio + async def test_choice_falls_back_to_label_text_when_entry_missing(self): + """If the gateway entry vanished (race / stale view), the button's + own choice text is used as the response.""" + from tools import clarify_gateway as cm + # Note: no cm.register() — entry intentionally absent + + view = ClarifyChoiceView( + choices=["alpha"], + clarify_id="cidGone", + allowed_user_ids=set(), + ) + interaction = _make_interaction() + # Doesn't raise; resolve_gateway_clarify returns False quietly + await view._resolve_choice(interaction, index=0, choice="alpha") + # Still marks the view resolved + disables buttons + assert view.resolved is True + assert all(b.disabled for b in view.children) + + @pytest.mark.asyncio + async def test_already_resolved_sends_ephemeral_reply(self): + view = ClarifyChoiceView( + choices=["a", "b"], + clarify_id="cidB", + allowed_user_ids=set(), + ) + view.resolved = True + + interaction = _make_interaction() + await view._resolve_choice(interaction, index=0, choice="a") + + interaction.response.send_message.assert_called_once() + kwargs = interaction.response.send_message.call_args.kwargs + assert kwargs.get("ephemeral") is True + # No resolve was called + interaction.response.edit_message.assert_not_called() + + @pytest.mark.asyncio + async def test_unauthorized_user_rejected(self): + from tools import clarify_gateway as cm + cm.register("cidC", "sk-C", "Pick", ["x"]) + + # Allowlist set, user not in it + view = ClarifyChoiceView( + choices=["x"], + clarify_id="cidC", + allowed_user_ids={"99999"}, # not 42 + ) + + interaction = _make_interaction(user_id="42") + await view._resolve_choice(interaction, index=0, choice="x") + + # Ephemeral rejection, no resolution, no edit + interaction.response.send_message.assert_called_once() + kwargs = interaction.response.send_message.call_args.kwargs + assert kwargs.get("ephemeral") is True + interaction.response.edit_message.assert_not_called() + with cm._lock: + entry = cm._entries.get("cidC") + assert entry is not None + assert not entry.event.is_set() + + +# =========================================================================== +# "Other" button → mark_awaiting_text +# =========================================================================== + +class TestClarifyOtherButton: + """Clicking Other should flip the entry into text-capture mode.""" + + def setup_method(self): + _clear_clarify_state() + + @pytest.mark.asyncio + async def test_other_flips_entry_to_awaiting_text(self): + from tools import clarify_gateway as cm + cm.register("cidD", "sk-D", "Pick", ["x", "y"]) + + view = ClarifyChoiceView( + choices=["x", "y"], + clarify_id="cidD", + allowed_user_ids=set(), + ) + + interaction = _make_interaction() + await view._on_other(interaction) + + # Entry awaiting_text now + pending = cm.get_pending_for_session("sk-D") + assert pending is not None + assert pending.clarify_id == "cidD" + assert pending.awaiting_text is True + # Entry still pending (not resolved) + with cm._lock: + entry = cm._entries.get("cidD") + assert entry is not None + assert not entry.event.is_set() + # View locked + buttons disabled + assert view.resolved is True + assert all(b.disabled for b in view.children) + interaction.response.edit_message.assert_called_once() + + @pytest.mark.asyncio + async def test_other_unauthorized_user_rejected(self): + from tools import clarify_gateway as cm + cm.register("cidE", "sk-E", "Pick", ["x"]) + + view = ClarifyChoiceView( + choices=["x"], + clarify_id="cidE", + allowed_user_ids={"99999"}, + ) + + interaction = _make_interaction(user_id="42") + await view._on_other(interaction) + + # Rejected; entry NOT awaiting text + interaction.response.send_message.assert_called_once() + pending = cm.get_pending_for_session("sk-E") + assert pending is None or pending.awaiting_text is False + + +# =========================================================================== +# DiscordAdapter.send_clarify integration +# =========================================================================== + +class TestDiscordSendClarify: + """Verify send_clarify renders an embed and (optionally) attaches the view.""" + + def setup_method(self): + _clear_clarify_state() + + @pytest.mark.asyncio + async def test_multi_choice_attaches_view(self): + adapter = _make_adapter(allowed_users={"42"}) + channel = MagicMock() + sent_msg = MagicMock() + sent_msg.id = 123456 + channel.send = AsyncMock(return_value=sent_msg) + adapter._client.get_channel = MagicMock(return_value=channel) + + result = await adapter.send_clarify( + chat_id="9001", + question="Pick a color", + choices=["red", "green", "blue"], + clarify_id="cidM", + session_key="sk-M", + ) + + assert result.success is True + assert result.message_id == "123456" + # Verify channel.send was called with embed + view kwargs + channel.send.assert_called_once() + kwargs = channel.send.call_args.kwargs + assert "embed" in kwargs + assert "view" in kwargs + assert isinstance(kwargs["view"], ClarifyChoiceView) + # 3 choice buttons + 1 Other + assert len(kwargs["view"].children) == 4 + + @pytest.mark.asyncio + async def test_open_ended_omits_view(self): + adapter = _make_adapter() + channel = MagicMock() + sent_msg = MagicMock() + sent_msg.id = 222 + channel.send = AsyncMock(return_value=sent_msg) + adapter._client.get_channel = MagicMock(return_value=channel) + + result = await adapter.send_clarify( + chat_id="9001", + question="What is your name?", + choices=None, + clarify_id="cidOE", + session_key="sk-OE", + ) + + assert result.success is True + channel.send.assert_called_once() + kwargs = channel.send.call_args.kwargs + # Open-ended path renders embed but no view (text-capture handles reply) + assert "embed" in kwargs + assert "view" not in kwargs + + @pytest.mark.asyncio + async def test_routes_to_thread_when_metadata_thread_id_set(self): + adapter = _make_adapter() + channel = MagicMock() + sent_msg = MagicMock() + sent_msg.id = 333 + channel.send = AsyncMock(return_value=sent_msg) + adapter._client.get_channel = MagicMock(return_value=channel) + + await adapter.send_clarify( + chat_id="9001", + question="?", + choices=["a"], + clarify_id="cidT", + session_key="sk-T", + metadata={"thread_id": "7777"}, + ) + + # Channel lookup should resolve to thread id, not chat_id + adapter._client.get_channel.assert_called_once_with(7777) + + @pytest.mark.asyncio + async def test_not_connected_returns_failure(self): + adapter = _make_adapter() + adapter._client = None + result = await adapter.send_clarify( + chat_id="9001", + question="?", + choices=["a"], + clarify_id="cidNC", + session_key="sk-NC", + ) + assert result.success is False + assert "Not connected" in (result.error or "") + + @pytest.mark.asyncio + async def test_filters_empty_and_whitespace_choices(self): + adapter = _make_adapter() + channel = MagicMock() + sent_msg = MagicMock() + sent_msg.id = 444 + channel.send = AsyncMock(return_value=sent_msg) + adapter._client.get_channel = MagicMock(return_value=channel) + + await adapter.send_clarify( + chat_id="9001", + question="?", + choices=["", " ", "real-choice", None], + clarify_id="cidF", + session_key="sk-F", + ) + kwargs = channel.send.call_args.kwargs + view = kwargs["view"] + # Only 1 real choice + 1 Other = 2 children + assert len(view.children) == 2 + assert "real-choice" in view.children[0].label