diff --git a/plugins/platforms/discord/adapter.py b/plugins/platforms/discord/adapter.py index 196564dd14f95..3efd1aa3cd0bc 100644 --- a/plugins/platforms/discord/adapter.py +++ b/plugins/platforms/discord/adapter.py @@ -595,6 +595,73 @@ def _read_dm_role_auth_guild() -> Optional[int]: return guild_id if guild_id > 0 else None +# Default timeout for Discord interactive button views (exec approval, slash +# confirm, update prompt, clarify choice). Used when the user has not set +# ``approvals.discord_prompt_timeout`` in config.yaml. 300s (5 min) matches +# the previous hardcoded value; users who walk away from a long-running +# session can raise it. Bounded to a sane range — Discord interactions +# expire from the API's side at ~15 minutes anyway, so 900s is the +# practical ceiling. +_DISCORD_PROMPT_TIMEOUT_DEFAULT = 300 +_DISCORD_PROMPT_TIMEOUT_MIN = 30 +_DISCORD_PROMPT_TIMEOUT_MAX = 900 + + +def _read_discord_prompt_timeout() -> int: + """Return the timeout (in seconds) for Discord button views. + + Reads ``approvals.discord_prompt_timeout`` from config.yaml. Falls back + to the historical 300s default for any missing / malformed value, and + clamps the result to ``[_DISCORD_PROMPT_TIMEOUT_MIN, + _DISCORD_PROMPT_TIMEOUT_MAX]`` so a typo can't accidentally make + interactive prompts disappear (too short) or invalidate them via + Discord's own 15-min interaction-token expiry (too long). + """ + raw: Any = None + try: + from hermes_cli.config import read_raw_config + cfg = read_raw_config() or {} + approvals_cfg = cfg.get("approvals", {}) or {} + raw = approvals_cfg.get("discord_prompt_timeout") + except Exception: + return _DISCORD_PROMPT_TIMEOUT_DEFAULT + if raw is None or raw == "": + return _DISCORD_PROMPT_TIMEOUT_DEFAULT + try: + seconds = int(raw) + except (TypeError, ValueError): + return _DISCORD_PROMPT_TIMEOUT_DEFAULT + if seconds < _DISCORD_PROMPT_TIMEOUT_MIN: + return _DISCORD_PROMPT_TIMEOUT_MIN + if seconds > _DISCORD_PROMPT_TIMEOUT_MAX: + return _DISCORD_PROMPT_TIMEOUT_MAX + return seconds + + +def _extract_embed_text(obj: Any) -> str: + """Flatten a message's (or snapshot's) embeds into plain text. + + Bot-to-bot traffic (error relays, CI alerts, log streams) often carries + its entire payload in embeds with empty ``content``, which would + otherwise be invisible to the agent's reply/backfill context. + """ + parts = [] + for embed in getattr(obj, "embeds", None) or []: + for attr in ("title", "description"): + val = getattr(embed, attr, None) + if val: + parts.append(str(val)) + for field in getattr(embed, "fields", None) or []: + fname = getattr(field, "name", None) or "" + fval = getattr(field, "value", None) or "" + if fname or fval: + parts.append(f"{fname}: {fval}".strip(": ").strip()) + footer_text = getattr(getattr(embed, "footer", None), "text", None) + if footer_text: + parts.append(str(footer_text)) + return "\n".join(parts).strip() + + class DiscordAdapter(BasePlatformAdapter): """ Discord bot adapter. @@ -4159,6 +4226,7 @@ async def _fetch_channel_context( try: collected = [] + hit_partition = False # IMPORTANT: pass oldest_first=False explicitly. discord.py 2.x # silently flips the default to True when `after=` is supplied, # which would select the *earliest* N messages after our last @@ -4176,6 +4244,7 @@ async def _fetch_channel_context( # Everything before this is already in the session transcript. # (Redundant when _after_obj is set, but needed for cold start.) if msg.author == self._client.user: + hit_partition = True break # Skip system messages (pins, joins, thread renames, etc.) @@ -4189,6 +4258,22 @@ async def _fetch_channel_context( continue content = getattr(msg, "clean_content", msg.content) or "" + if not content: + # Forwarded messages: the wrapper has empty content; the + # real payload lives in message_snapshots. + snap_parts = [] + for snap in getattr(msg, "message_snapshots", None) or []: + snap_content = getattr(snap, "content", None) + if snap_content: + snap_parts.append(snap_content.strip()) + snap_embed_text = _extract_embed_text(snap) + if snap_embed_text: + snap_parts.append(snap_embed_text) + if snap_parts: + content = "[Forwarded] " + "\n".join(snap_parts) + if not content: + # Embed-only messages (error relays, CI alerts). + content = _extract_embed_text(msg) if not content and msg.attachments: content = "(attachment)" if not content: @@ -4199,6 +4284,24 @@ async def _fetch_channel_context( name = f"{name} [bot]" collected.append(f"[{name}] {content}") + # Threads spun off a channel message contain only a + # thread_starter_message system pointer — the real content lives + # in the parent channel under the same ID as the thread. Without + # it, a thread created from another bot's message backfills + # empty. Only fetch on a cold scan (no partition hit and no + # cached window): if the bot already spoke in the thread, the + # starter was surfaced when it first engaged. + if ( + isinstance(channel, discord.Thread) + and not hit_partition + and _after_obj is None + ): + starter_line = await self._fetch_thread_starter_line(channel) + if starter_line: + # Appended last: collected is newest-first here, and the + # reverse below puts the starter at the chronological top. + collected.append(starter_line) + if not collected: return "" @@ -4217,6 +4320,39 @@ def _thread_parent_channel(self, channel: Any) -> Any: """Return the parent text channel when invoked from a thread.""" return getattr(channel, "parent", None) or channel + async def _fetch_thread_starter_line(self, thread: Any) -> str: + """Return the thread's starter message as a formatted context line. + + For threads created from a message, the starter lives in the parent + channel with the same ID as the thread. Returns "" for standalone + threads (the fetch 404s) or when the starter has no readable content. + + Deliberately does not apply DISCORD_ALLOW_BOTS filtering: the starter + is what the thread is *about* — a user asking about it in the thread + is an explicit request for that content regardless of its author. + """ + starter = getattr(thread, "starter_message", None) + if starter is None: + parent = getattr(thread, "parent", None) + if parent is None or not hasattr(parent, "fetch_message"): + return "" + try: + starter = await parent.fetch_message(thread.id) + except Exception: + return "" + content = getattr(starter, "clean_content", None) or getattr(starter, "content", None) or "" + embed_text = _extract_embed_text(starter) + if embed_text: + content = f"{content}\n{embed_text}".strip() + if not content and getattr(starter, "attachments", None): + content = "(attachment)" + if not content: + return "" + name = starter.author.display_name + if getattr(starter.author, "bot", False): + name = f"{name} [bot]" + return f"[{name} — thread starter] {content}" + async def _resolve_interaction_channel(self, interaction: discord.Interaction) -> Optional[Any]: """Return the interaction channel, fetching it if the payload is partial.""" channel = getattr(interaction, "channel", None) @@ -4894,6 +5030,11 @@ async def _handle_message(self, message: DiscordMessage, role_authorized: bool = for snap in message.message_snapshots: if getattr(snap, "content", None): snapshot_text_parts.append(snap.content.strip()) + # Embed-only forwards (bot alerts, error relays) carry their + # payload in embeds with empty content. + snap_embed_text = _extract_embed_text(snap) + if snap_embed_text: + snapshot_text_parts.append(snap_embed_text) snapshot_attachments.extend(getattr(snap, "attachments", []) or []) if snapshot_text_parts and not raw_content: raw_content = "\n".join(snapshot_text_parts) @@ -4973,7 +5114,40 @@ async def _handle_message(self, message: DiscordMessage, role_authorized: bool = auto_threaded_channel = thread self._threads.mark(thread_id) - all_attachments = list(message.attachments) + snapshot_attachments + # Resolve the reply reference early so the referenced message's + # attachments can join the media pipeline below (vision routing for + # "what's this?" replies to screenshots) and so embed-only targets + # (bot error messages) still yield reply context. + reply_to_id = None + reply_to_text = None + reference_attachments: list = [] + _ref = message.reference + # Forwards also populate message.reference (type=forward) pointing at + # the original — skip those; their content is handled via + # message_snapshots above. + _is_forward_ref = getattr(getattr(_ref, "type", None), "name", "") == "forward" + if _ref and _ref.message_id and not _is_forward_ref: + reply_to_id = str(_ref.message_id) + ref_msg = _ref.resolved + if ref_msg is None: + # Not in discord.py's cache and not inlined in the gateway + # payload — fetch it (same channel by definition for replies). + try: + ref_msg = await message.channel.fetch_message(_ref.message_id) + except Exception: + ref_msg = None + if ref_msg is not None: + reply_to_text = getattr(ref_msg, "content", None) or None + _ref_embed_text = _extract_embed_text(ref_msg) + if _ref_embed_text: + reply_to_text = ( + f"{reply_to_text}\n{_ref_embed_text}".strip() + if reply_to_text + else _ref_embed_text + ) + reference_attachments = list(getattr(ref_msg, "attachments", None) or []) + + all_attachments = list(message.attachments) + snapshot_attachments + reference_attachments # Determine message type msg_type = MessageType.TEXT @@ -5219,13 +5393,6 @@ async def _handle_message(self, message: DiscordMessage, role_authorized: bool = _skills = self._resolve_channel_skills(_chan_id, _parent_id or None) _channel_prompt = self._resolve_channel_prompt(_chan_id, _parent_id or None) - reply_to_id = None - reply_to_text = None - if message.reference: - reply_to_id = str(message.reference.message_id) - if message.reference.resolved: - reply_to_text = getattr(message.reference.resolved, "content", None) or None - event = MessageEvent( text=event_text, message_type=msg_type, @@ -5435,7 +5602,7 @@ def __init__( allowed_user_ids: set, allowed_role_ids: Optional[set] = None, ): - super().__init__(timeout=300) # 5-minute timeout + super().__init__(timeout=_read_discord_prompt_timeout()) self.session_key = session_key self.allowed_user_ids = allowed_user_ids self.allowed_role_ids = allowed_role_ids or set() @@ -5555,7 +5722,7 @@ def __init__( allowed_user_ids: set, allowed_role_ids: Optional[set] = None, ): - super().__init__(timeout=300) + super().__init__(timeout=_read_discord_prompt_timeout()) self.session_key = session_key self.confirm_id = confirm_id self.allowed_user_ids = allowed_user_ids @@ -5660,7 +5827,7 @@ def __init__( allowed_user_ids: set, allowed_role_ids: Optional[set] = None, ): - super().__init__(timeout=300) + super().__init__(timeout=_read_discord_prompt_timeout()) self.session_key = session_key self.allowed_user_ids = allowed_user_ids self.allowed_role_ids = allowed_role_ids or set() @@ -6084,7 +6251,7 @@ def __init__( allowed_user_ids: set, allowed_role_ids: Optional[set] = None, ): - super().__init__(timeout=300) # 5-minute timeout + super().__init__(timeout=_read_discord_prompt_timeout()) self.choices = list(choices)[:24] self.clarify_id = clarify_id self.allowed_user_ids = allowed_user_ids diff --git a/tests/gateway/test_discord_prompt_timeout_config.py b/tests/gateway/test_discord_prompt_timeout_config.py new file mode 100644 index 0000000000000..735359fff1815 --- /dev/null +++ b/tests/gateway/test_discord_prompt_timeout_config.py @@ -0,0 +1,150 @@ +"""Tests for the configurable Discord interactive-view timeout. + +Previously hardcoded to 300s on ExecApprovalView, SlashConfirmView, +UpdatePromptView, and ClarifyChoiceView. Now reads +``approvals.discord_prompt_timeout`` with the same 300s default, clamped to +``[_DISCORD_PROMPT_TIMEOUT_MIN, _DISCORD_PROMPT_TIMEOUT_MAX]`` so a typo +can't make prompts disappear (too short) or outlive Discord's 15-min +interaction-token expiry (too long). +""" + +import sys +from types import SimpleNamespace +from unittest.mock import MagicMock + +import pytest + + +def _ensure_discord_mock(): + if "discord" in sys.modules and hasattr(sys.modules["discord"], "__file__"): + return + + discord_mod = MagicMock() + discord_mod.Intents.default.return_value = MagicMock() + discord_mod.Client = MagicMock + discord_mod.File = MagicMock + discord_mod.DMChannel = type("DMChannel", (), {}) + discord_mod.Thread = type("Thread", (), {}) + discord_mod.ForumChannel = type("ForumChannel", (), {}) + discord_mod.ui = SimpleNamespace(View=object, button=lambda *a, **k: (lambda fn: fn), Button=object) + discord_mod.ButtonStyle = SimpleNamespace(success=1, primary=2, secondary=2, danger=3, green=1, grey=2, blurple=2, red=3) + discord_mod.Color = SimpleNamespace(orange=lambda: 1, green=lambda: 2, blue=lambda: 3, red=lambda: 4, purple=lambda: 5) + discord_mod.Interaction = object + discord_mod.Embed = MagicMock + discord_mod.app_commands = SimpleNamespace( + describe=lambda **kwargs: (lambda fn: fn), + choices=lambda **kwargs: (lambda fn: fn), + Choice=lambda **kwargs: SimpleNamespace(**kwargs), + ) + + ext_mod = MagicMock() + commands_mod = MagicMock() + commands_mod.Bot = MagicMock + ext_mod.commands = commands_mod + + sys.modules.setdefault("discord", discord_mod) + sys.modules.setdefault("discord.ext", ext_mod) + sys.modules.setdefault("discord.ext.commands", commands_mod) + + +_ensure_discord_mock() + +from plugins.platforms.discord.adapter import ( # noqa: E402 + _DISCORD_PROMPT_TIMEOUT_DEFAULT, + _DISCORD_PROMPT_TIMEOUT_MAX, + _DISCORD_PROMPT_TIMEOUT_MIN, + _read_discord_prompt_timeout, +) + + +def _patch_config(monkeypatch, cfg): + """Stub ``hermes_cli.config.read_raw_config`` to return ``cfg``.""" + import hermes_cli.config + monkeypatch.setattr(hermes_cli.config, "read_raw_config", lambda: cfg) + + +def test_default_when_config_absent(monkeypatch): + _patch_config(monkeypatch, {}) + assert _read_discord_prompt_timeout() == _DISCORD_PROMPT_TIMEOUT_DEFAULT + + +def test_default_when_approvals_block_missing(monkeypatch): + _patch_config(monkeypatch, {"other": {}}) + assert _read_discord_prompt_timeout() == _DISCORD_PROMPT_TIMEOUT_DEFAULT + + +def test_default_when_key_missing(monkeypatch): + _patch_config(monkeypatch, {"approvals": {"mode": "manual"}}) + assert _read_discord_prompt_timeout() == _DISCORD_PROMPT_TIMEOUT_DEFAULT + + +def test_explicit_int_value(monkeypatch): + _patch_config(monkeypatch, {"approvals": {"discord_prompt_timeout": 600}}) + assert _read_discord_prompt_timeout() == 600 + + +def test_numeric_string_accepted(monkeypatch): + """YAML parsers occasionally return numbers as strings; tolerate it.""" + _patch_config(monkeypatch, {"approvals": {"discord_prompt_timeout": "450"}}) + assert _read_discord_prompt_timeout() == 450 + + +def test_malformed_value_falls_back_to_default(monkeypatch): + _patch_config( + monkeypatch, + {"approvals": {"discord_prompt_timeout": "five minutes"}}, + ) + assert _read_discord_prompt_timeout() == _DISCORD_PROMPT_TIMEOUT_DEFAULT + + +def test_value_clamped_to_minimum(monkeypatch): + """A typo of e.g. 5 seconds must not make prompts disappear.""" + _patch_config(monkeypatch, {"approvals": {"discord_prompt_timeout": 5}}) + assert _read_discord_prompt_timeout() == _DISCORD_PROMPT_TIMEOUT_MIN + + +def test_value_clamped_to_maximum(monkeypatch): + """Discord interaction tokens expire at ~15 min — clamp larger values.""" + _patch_config(monkeypatch, {"approvals": {"discord_prompt_timeout": 99999}}) + assert _read_discord_prompt_timeout() == _DISCORD_PROMPT_TIMEOUT_MAX + + +def test_zero_clamped_to_minimum(monkeypatch): + _patch_config(monkeypatch, {"approvals": {"discord_prompt_timeout": 0}}) + assert _read_discord_prompt_timeout() == _DISCORD_PROMPT_TIMEOUT_MIN + + +def test_negative_clamped_to_minimum(monkeypatch): + _patch_config(monkeypatch, {"approvals": {"discord_prompt_timeout": -300}}) + assert _read_discord_prompt_timeout() == _DISCORD_PROMPT_TIMEOUT_MIN + + +def test_empty_string_falls_back_to_default(monkeypatch): + _patch_config(monkeypatch, {"approvals": {"discord_prompt_timeout": ""}}) + assert _read_discord_prompt_timeout() == _DISCORD_PROMPT_TIMEOUT_DEFAULT + + +def test_config_read_exception_falls_back_to_default(monkeypatch): + """A crashing read_raw_config must not bring down view construction — + falling back to the historical 300s default preserves existing behavior. + """ + import hermes_cli.config + def _boom(): + raise RuntimeError("config file corrupt") + monkeypatch.setattr(hermes_cli.config, "read_raw_config", _boom) + assert _read_discord_prompt_timeout() == _DISCORD_PROMPT_TIMEOUT_DEFAULT + + +def test_default_matches_previous_hardcoded_value(): + """Behavioral parity assertion: existing installs (no new config) must + see exactly the 300s timeout the views were hardcoded to before this + change. Guards against the default drifting in a future refactor. + """ + assert _DISCORD_PROMPT_TIMEOUT_DEFAULT == 300 + + +def test_clamp_range_includes_default(): + """Sanity: the default must lie inside the clamp range, or every fresh + install would hit the clamp on its very first read. + """ + assert _DISCORD_PROMPT_TIMEOUT_MIN <= _DISCORD_PROMPT_TIMEOUT_DEFAULT <= _DISCORD_PROMPT_TIMEOUT_MAX