diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index db76034989c5a..cfbe9b1b26026 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -2440,6 +2440,9 @@ def build_source( user_id_alt: Optional[str] = None, chat_id_alt: Optional[str] = None, is_bot: bool = False, + guild_id: Optional[str] = None, + parent_chat_id: Optional[str] = None, + message_id: Optional[str] = None, ) -> SessionSource: """Helper to build a SessionSource for this platform.""" # Normalize empty topic to None @@ -2457,6 +2460,9 @@ def build_source( user_id_alt=user_id_alt, chat_id_alt=chat_id_alt, is_bot=is_bot, + guild_id=str(guild_id) if guild_id else None, + parent_chat_id=str(parent_chat_id) if parent_chat_id else None, + message_id=str(message_id) if message_id else None, ) @abstractmethod diff --git a/gateway/platforms/discord.py b/gateway/platforms/discord.py index f741d45b5f35a..fe799339328f9 100644 --- a/gateway/platforms/discord.py +++ b/gateway/platforms/discord.py @@ -3256,6 +3256,7 @@ async def _handle_message(self, message: DiscordMessage) -> None: if auto_thread and not skip_thread and not is_voice_linked_channel and not is_reply_message: thread = await self._auto_create_thread(message) if thread: + parent_channel_id = str(message.channel.id) is_thread = True thread_id = str(thread.id) auto_threaded_channel = thread @@ -3315,6 +3316,9 @@ async def _handle_message(self, message: DiscordMessage) -> None: thread_id=thread_id, chat_topic=chat_topic, is_bot=getattr(message.author, "bot", False), + guild_id=str(message.guild.id) if message.guild else None, + parent_chat_id=parent_channel_id, + message_id=str(message.id), ) # Build media URLs -- download image attachments to local cache so the diff --git a/gateway/session.py b/gateway/session.py index db90d31217273..b5d02edde0422 100644 --- a/gateway/session.py +++ b/gateway/session.py @@ -83,6 +83,9 @@ class SessionSource: user_id_alt: Optional[str] = None # Platform-specific stable alt ID (Signal UUID, Feishu union_id) chat_id_alt: Optional[str] = None # Signal group internal ID is_bot: bool = False # True when the message author is a bot/webhook (Discord) + guild_id: Optional[str] = None # Discord guild / Slack workspace / Matrix server scope + parent_chat_id: Optional[str] = None # Parent channel when chat_id refers to a thread + message_id: Optional[str] = None # ID of the triggering message (for pin/reply/react) @property def description(self) -> str: @@ -120,8 +123,14 @@ def to_dict(self) -> Dict[str, Any]: d["user_id_alt"] = self.user_id_alt if self.chat_id_alt: d["chat_id_alt"] = self.chat_id_alt + if self.guild_id: + d["guild_id"] = self.guild_id + if self.parent_chat_id: + d["parent_chat_id"] = self.parent_chat_id + if self.message_id: + d["message_id"] = self.message_id return d - + @classmethod def from_dict(cls, data: Dict[str, Any]) -> "SessionSource": return cls( @@ -135,6 +144,9 @@ def from_dict(cls, data: Dict[str, Any]) -> "SessionSource": chat_topic=data.get("chat_topic"), user_id_alt=data.get("user_id_alt"), chat_id_alt=data.get("chat_id_alt"), + guild_id=data.get("guild_id"), + parent_chat_id=data.get("parent_chat_id"), + message_id=data.get("message_id"), ) @@ -273,14 +285,34 @@ def build_session_context_prompt( "that you can only read messages sent directly to you and respond." ) elif context.source.platform == Platform.DISCORD: - lines.append("") - lines.append( - "**Platform notes:** You are running inside Discord. " - "You do NOT have access to Discord-specific APIs โ€” you cannot search " - "channel history, pin messages, manage roles, or list server members. " - "Do not promise to perform these actions. If the user asks, explain " - "that you can only read messages sent directly to you and respond." - ) + # The discord tool self-gates on DISCORD_BOT_TOKEN at registry + # check time. Match that condition so the prompt stays honest: + # with a token the agent has fetch_messages/search_members/ + # create_thread (and optionally discord_admin) and should know + # the IDs it can call them with; without one it really is + # limited to reading/replying via the gateway. + if (os.environ.get("DISCORD_BOT_TOKEN") or "").strip(): + src = context.source + id_lines = ["", "**Discord IDs (for the `discord` / `discord_admin` tools):**"] + if src.guild_id: + id_lines.append(f" - Guild: `{src.guild_id}`") + if src.thread_id and src.parent_chat_id: + id_lines.append(f" - Parent channel: `{src.parent_chat_id}`") + id_lines.append(f" - Thread: `{src.thread_id}` (use as `channel_id` for fetch_messages etc.)") + else: + id_lines.append(f" - Channel: `{src.chat_id}`") + if src.message_id: + id_lines.append(f" - Triggering message: `{src.message_id}`") + lines.extend(id_lines) + else: + lines.append("") + lines.append( + "**Platform notes:** You are running inside Discord. " + "You do NOT have access to Discord-specific APIs โ€” you cannot search " + "channel history, pin messages, manage roles, or list server members. " + "Do not promise to perform these actions. If the user asks, explain " + "that you can only read messages sent directly to you and respond." + ) # Connected platforms platforms_list = ["local (files on this machine)"] diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 58f8745959363..648267302e007 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -833,7 +833,7 @@ def _ensure_hermes_home_managed(home: Path): "auto_thread": True, # Auto-create threads on @mention in channels (like Slack) "reactions": True, # Add ๐Ÿ‘€/โœ…/โŒ reactions to messages during processing "channel_prompts": {}, # Per-channel ephemeral system prompts (forum parents apply to child threads) - # discord_server tool: restrict which actions the agent may call. + # discord / discord_admin tools: restrict which actions the agent may call. # Default (empty) = all actions allowed (subject to bot privileged intents). # Accepts comma-separated string ("list_guilds,list_channels,fetch_messages") # or YAML list. Unknown names are dropped with a warning at load time. diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index e89f961781f3b..1bfb36df0b8ab 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -67,12 +67,13 @@ ("messaging", "๐Ÿ“จ Cross-Platform Messaging", "send_message"), ("rl", "๐Ÿงช RL Training", "Tinker-Atropos training tools"), ("homeassistant", "๐Ÿ  Home Assistant", "smart home device control"), + ("discord_admin", "๐Ÿ›ก๏ธ Discord Server Admin", "list channels/roles, pin, assign roles"), ] # Toolsets that are OFF by default for new installs. # They're still in _HERMES_CORE_TOOLS (available at runtime if enabled), # but the setup checklist won't pre-select them for first-time users. -_DEFAULT_OFF_TOOLSETS = {"moa", "homeassistant", "rl"} +_DEFAULT_OFF_TOOLSETS = {"moa", "homeassistant", "rl", "discord_admin"} def _get_effective_configurable_toolsets(): @@ -549,7 +550,7 @@ def _get_platform_tools( include_default_mcp_servers: bool = True, ) -> Set[str]: """Resolve which individual toolset names are enabled for a platform.""" - from toolsets import resolve_toolset + from toolsets import resolve_toolset, TOOLSETS platform_toolsets = config.get("platform_toolsets") or {} toolset_names = platform_toolsets.get(platform) @@ -563,6 +564,8 @@ def _get_platform_tools( toolset_names = [str(ts) for ts in toolset_names] configurable_keys = {ts_key for ts_key, _, _ in CONFIGURABLE_TOOLSETS} + plugin_ts_keys = _get_plugin_toolset_keys() + platform_default_keys = {p["default_toolset"] for p in PLATFORMS.values()} # If the saved list contains any configurable keys directly, the user # has explicitly configured this platform โ€” use direct membership. @@ -585,16 +588,46 @@ def _get_platform_tools( ts_tools = set(resolve_toolset(ts_key)) if ts_tools and ts_tools.issubset(all_tool_names): enabled_toolsets.add(ts_key) + default_off = set(_DEFAULT_OFF_TOOLSETS) if platform in default_off: default_off.remove(platform) enabled_toolsets -= default_off + # Recover non-configurable platform toolsets (e.g. discord, feishu_doc, + # feishu_drive). These are part of the platform's default composite but + # absent from CONFIGURABLE_TOOLSETS, so they can't appear in the TUI + # checklist or in a user-saved config. Must run in BOTH branches โ€” + # otherwise saving via `hermes tools` (which flips has_explicit_config + # to True) silently drops them. + platform_tool_universe = set(resolve_toolset(PLATFORMS[platform]["default_toolset"])) + configurable_tool_universe = set() + for ck in configurable_keys: + configurable_tool_universe.update(resolve_toolset(ck)) + claimed = set() + for ts_key in enabled_toolsets: + claimed.update(resolve_toolset(ts_key)) + skip = configurable_keys | plugin_ts_keys | platform_default_keys + skip |= {k for k in TOOLSETS if k.startswith("hermes-")} + skip |= set(_DEFAULT_OFF_TOOLSETS) - {platform} + for ts_key, ts_def in TOOLSETS.items(): + if ts_key in skip: + continue + if ts_def.get("includes"): + continue + ts_tools = set(resolve_toolset(ts_key)) + if not ts_tools or not ts_tools.issubset(platform_tool_universe): + continue + if ts_tools.issubset(configurable_tool_universe): + continue + if not ts_tools.issubset(claimed): + enabled_toolsets.add(ts_key) + claimed.update(ts_tools) + # Plugin toolsets: enabled by default unless explicitly disabled. # A plugin toolset is "known" for a platform once `hermes tools` # has been saved for that platform (tracked via known_plugin_toolsets). # Unknown plugins default to enabled; known-but-absent = disabled. - plugin_ts_keys = _get_plugin_toolset_keys() if plugin_ts_keys: known_map = config.get("known_plugin_toolsets", {}) known_for_platform = set(known_map.get(platform, [])) @@ -609,7 +642,6 @@ def _get_platform_tools( # Preserve any explicit non-configurable toolset entries (for example, # custom toolsets or MCP server names saved in platform_toolsets). - platform_default_keys = {p["default_toolset"] for p in PLATFORMS.values()} explicit_passthrough = { ts for ts in toolset_names @@ -669,6 +701,7 @@ def _save_platform_tools(config: dict, platform: str, enabled_toolset_keys: Set[ existing_toolsets = config.get("platform_toolsets", {}).get(platform, []) if not isinstance(existing_toolsets, list): existing_toolsets = [] + existing_toolsets = [str(ts) for ts in existing_toolsets] # Preserve any entries that are NOT configurable toolsets and NOT platform # defaults (i.e. only MCP server names should be preserved) @@ -676,6 +709,8 @@ def _save_platform_tools(config: dict, platform: str, enabled_toolset_keys: Set[ entry for entry in existing_toolsets if entry not in configurable_keys and entry not in platform_default_keys } + if "no_mcp" not in enabled_toolset_keys: + preserved_entries.discard("no_mcp") # Merge preserved entries with new enabled toolsets config["platform_toolsets"][platform] = sorted(enabled_toolset_keys | preserved_entries) diff --git a/model_tools.py b/model_tools.py index 2b7767fda3faa..9b6e135475478 100644 --- a/model_tools.py +++ b/model_tools.py @@ -288,30 +288,34 @@ def get_tool_definitions( filtered_tools[i] = {"type": "function", "function": dynamic_schema} break - # Rebuild discord_server schema based on the bot's privileged intents - # (detected from GET /applications/@me) and the user's action allowlist - # in config. Hides actions the bot's intents don't support so the - # model never attempts them, and annotates fetch_messages when the + # Rebuild discord / discord_admin schemas based on the bot's privileged + # intents (detected from GET /applications/@me) and the user's action + # allowlist in config. Hides actions the bot's intents don't support so + # the model never attempts them, and annotates fetch_messages when the # MESSAGE_CONTENT intent is missing. - if "discord_server" in available_tool_names: - try: - from tools.discord_tool import get_dynamic_schema - dynamic = get_dynamic_schema() - except Exception: # pragma: no cover โ€” defensive, fall back to static - dynamic = None - if dynamic is None: - # Tool filtered out entirely (empty allowlist or detection disabled - # the only remaining actions). Drop it from the schema list. - filtered_tools = [ - t for t in filtered_tools - if t.get("function", {}).get("name") != "discord_server" - ] - available_tool_names.discard("discord_server") - else: - for i, td in enumerate(filtered_tools): - if td.get("function", {}).get("name") == "discord_server": - filtered_tools[i] = {"type": "function", "function": dynamic} - break + _discord_schema_fns = { + "discord": "get_dynamic_schema_core", + "discord_admin": "get_dynamic_schema_admin", + } + for discord_tool_name in _discord_schema_fns: + if discord_tool_name in available_tool_names: + try: + from tools import discord_tool as _dt + schema_fn = getattr(_dt, _discord_schema_fns[discord_tool_name]) + dynamic = schema_fn() + except Exception: + dynamic = None + if dynamic is None: + filtered_tools = [ + t for t in filtered_tools + if t.get("function", {}).get("name") != discord_tool_name + ] + available_tool_names.discard(discord_tool_name) + else: + for i, td in enumerate(filtered_tools): + if td.get("function", {}).get("name") == discord_tool_name: + filtered_tools[i] = {"type": "function", "function": dynamic} + break # Strip web tool cross-references from browser_navigate description when # web_search / web_extract are not available. The static schema says diff --git a/tests/hermes_cli/test_tools_config.py b/tests/hermes_cli/test_tools_config.py index b134fc98b3181..91b38e3750864 100644 --- a/tests/hermes_cli/test_tools_config.py +++ b/tests/hermes_cli/test_tools_config.py @@ -601,3 +601,122 @@ def test_picker_repairs_corrupt_config_section(self): _configure_imagegen_model("fal", config) assert isinstance(config["image_gen"], dict) assert config["image_gen"]["model"] == "fal-ai/flux-2/klein/9b" + + +def test_get_platform_tools_recovers_non_configurable_toolsets_from_composite(): + """Non-configurable toolsets whose tools are in the composite but not in + CONFIGURABLE_TOOLSETS should still appear in the result. + """ + from toolsets import TOOLSETS + from hermes_cli.tools_config import PLATFORMS + from unittest.mock import patch as mock_patch + + fake_toolsets = dict(TOOLSETS) + fake_toolsets["_test_platform_tool"] = { + "description": "test", + "tools": ["_test_special_tool"], + "includes": [], + } + fake_toolsets["hermes-_test_platform"] = { + "description": "test composite", + "tools": ["web_search", "web_extract", "terminal", "process", "_test_special_tool"], + "includes": [], + } + + test_platforms = { + "_test_platform": {"label": "Test", "default_toolset": "hermes-_test_platform"}, + } + + with mock_patch("hermes_cli.tools_config.PLATFORMS", {**PLATFORMS, **test_platforms}): + with mock_patch("toolsets.TOOLSETS", fake_toolsets): + enabled = _get_platform_tools({}, "_test_platform") + + assert "_test_platform_tool" in enabled + assert "web" in enabled + assert "terminal" in enabled + + +def test_get_platform_tools_second_pass_skips_fully_claimed_toolsets(): + """Toolsets whose tools are fully covered by configurable keys should NOT + be added by the second pass (prevents 'search', 'hermes-acp' noise). + """ + enabled = _get_platform_tools({}, "cli") + + assert "search" not in enabled + + +def test_get_platform_tools_discord_includes_discord_not_admin(): + enabled = _get_platform_tools({}, "discord") + assert "discord" in enabled + assert "discord_admin" not in enabled + + +def test_discord_admin_in_configurable_toolsets(): + assert any(ts_key == "discord_admin" for ts_key, _, _ in CONFIGURABLE_TOOLSETS) + + +def test_discord_admin_in_default_off(): + assert "discord_admin" in _DEFAULT_OFF_TOOLSETS + + +def test_get_platform_tools_feishu_includes_doc_and_drive(): + enabled = _get_platform_tools({}, "feishu") + assert "feishu_doc" in enabled + assert "feishu_drive" in enabled + + +def test_get_platform_tools_feishu_tools_not_on_other_platforms(): + for plat in ["cli", "telegram", "discord"]: + enabled = _get_platform_tools({}, plat) + assert "feishu_doc" not in enabled, f"feishu_doc leaked onto {plat}" + assert "feishu_drive" not in enabled, f"feishu_drive leaked onto {plat}" + + +def test_save_platform_tools_normalizes_numeric_entries(): + """YAML may parse bare numeric toolset names as int. They should be + normalized to str so they survive the save round-trip. + """ + config = { + "platform_toolsets": { + "cli": ["web", "terminal", 12306, "custom-mcp"] + } + } + + with patch("hermes_cli.tools_config.save_config"): + _save_platform_tools(config, "cli", {"web", "browser"}) + + saved = config["platform_toolsets"]["cli"] + assert "12306" in saved + assert 12306 not in saved + + +def test_save_platform_tools_clears_stale_no_mcp(): + """When the new selection doesn't include no_mcp, the sentinel should + be stripped from preserved entries so MCP servers are re-enabled. + """ + config = { + "platform_toolsets": { + "cli": ["web", "terminal", "no_mcp"] + } + } + + with patch("hermes_cli.tools_config.save_config"): + _save_platform_tools(config, "cli", {"web", "browser"}) + + saved = config["platform_toolsets"]["cli"] + assert "no_mcp" not in saved + + +def test_save_platform_tools_preserves_explicit_no_mcp(): + """When the new selection explicitly includes no_mcp, it should be kept.""" + config = { + "platform_toolsets": { + "cli": ["web", "no_mcp"] + } + } + + with patch("hermes_cli.tools_config.save_config"): + _save_platform_tools(config, "cli", {"web", "no_mcp"}) + + saved = config["platform_toolsets"]["cli"] + assert "no_mcp" in saved diff --git a/tests/test_toolsets.py b/tests/test_toolsets.py index 183f9e514f163..4e4289999c572 100644 --- a/tests/test_toolsets.py +++ b/tests/test_toolsets.py @@ -200,8 +200,8 @@ def test_all_includes_reference_existing_toolsets(self): def test_hermes_platforms_share_core_tools(self): """All hermes-* platform toolsets share the same core tools. - Platform-specific additions (e.g. ``discord_server`` on - hermes-discord, gated on DISCORD_BOT_TOKEN) are allowed on top โ€” + Platform-specific additions (e.g. ``discord`` / ``discord_admin`` + on hermes-discord, gated on DISCORD_BOT_TOKEN) are allowed on top โ€” the invariant is that the core set is identical across platforms. """ platforms = ["hermes-cli", "hermes-telegram", "hermes-discord", "hermes-whatsapp", "hermes-slack", "hermes-signal", "hermes-homeassistant"] diff --git a/tests/tools/test_discord_tool.py b/tests/tools/test_discord_tool.py index 34fe672132dd6..70b43903ecf8d 100644 --- a/tests/tools/test_discord_tool.py +++ b/tests/tools/test_discord_tool.py @@ -11,6 +11,8 @@ from tools.discord_tool import ( DiscordAPIError, _ACTIONS, + _ADMIN_ACTIONS, + _CORE_ACTIONS, _available_actions, _build_schema, _channel_type_name, @@ -21,8 +23,11 @@ _load_allowed_actions_config, _reset_capability_cache, check_discord_tool_requirements, - discord_server, + discord_admin_handler, + discord_core, get_dynamic_schema, + get_dynamic_schema_admin, + get_dynamic_schema_core, ) @@ -147,32 +152,32 @@ def test_http_error(self, mock_urlopen_fn): class TestDiscordServerValidation: def test_no_token(self, monkeypatch): monkeypatch.delenv("DISCORD_BOT_TOKEN", raising=False) - result = json.loads(discord_server(action="list_guilds")) + result = json.loads(discord_admin_handler(action="list_guilds")) assert "error" in result assert "DISCORD_BOT_TOKEN" in result["error"] def test_unknown_action(self, monkeypatch): monkeypatch.setenv("DISCORD_BOT_TOKEN", "test-token") - result = json.loads(discord_server(action="bad_action")) + result = json.loads(discord_core(action="bad_action")) assert "error" in result assert "Unknown action" in result["error"] assert "available_actions" in result def test_missing_required_guild_id(self, monkeypatch): monkeypatch.setenv("DISCORD_BOT_TOKEN", "test-token") - result = json.loads(discord_server(action="list_channels")) + result = json.loads(discord_admin_handler(action="list_channels")) assert "error" in result assert "guild_id" in result["error"] def test_missing_required_channel_id(self, monkeypatch): monkeypatch.setenv("DISCORD_BOT_TOKEN", "test-token") - result = json.loads(discord_server(action="fetch_messages")) + result = json.loads(discord_core(action="fetch_messages")) assert "error" in result assert "channel_id" in result["error"] def test_missing_multiple_params(self, monkeypatch): monkeypatch.setenv("DISCORD_BOT_TOKEN", "test-token") - result = json.loads(discord_server(action="add_role")) + result = json.loads(discord_admin_handler(action="add_role")) assert "error" in result assert "guild_id" in result["error"] assert "user_id" in result["error"] @@ -191,7 +196,7 @@ def test_list_guilds(self, mock_req, monkeypatch): {"id": "111", "name": "Test Server", "icon": "abc", "owner": True, "permissions": "123"}, {"id": "222", "name": "Other Server", "icon": None, "owner": False, "permissions": "456"}, ] - result = json.loads(discord_server(action="list_guilds")) + result = json.loads(discord_admin_handler(action="list_guilds")) assert result["count"] == 2 assert result["guilds"][0]["name"] == "Test Server" assert result["guilds"][1]["id"] == "222" @@ -219,7 +224,7 @@ def test_server_info(self, mock_req, monkeypatch): "premium_subscription_count": 5, "verification_level": 1, } - result = json.loads(discord_server(action="server_info", guild_id="111")) + result = json.loads(discord_admin_handler(action="server_info", guild_id="111")) assert result["name"] == "My Server" assert result["member_count"] == 42 assert result["online_count"] == 10 @@ -242,7 +247,7 @@ def test_list_channels_organized(self, mock_req, monkeypatch): {"id": "12", "name": "voice", "type": 2, "position": 1, "parent_id": "10", "topic": None, "nsfw": False}, {"id": "13", "name": "no-category", "type": 0, "position": 0, "parent_id": None, "topic": None, "nsfw": False}, ] - result = json.loads(discord_server(action="list_channels", guild_id="111")) + result = json.loads(discord_admin_handler(action="list_channels", guild_id="111")) assert result["total_channels"] == 3 # excludes the category itself groups = result["channel_groups"] # Uncategorized first @@ -257,7 +262,7 @@ def test_list_channels_organized(self, mock_req, monkeypatch): def test_empty_guild(self, mock_req, monkeypatch): monkeypatch.setenv("DISCORD_BOT_TOKEN", "test-token") mock_req.return_value = [] - result = json.loads(discord_server(action="list_channels", guild_id="111")) + result = json.loads(discord_admin_handler(action="list_channels", guild_id="111")) assert result["total_channels"] == 0 @@ -274,7 +279,7 @@ def test_channel_info(self, mock_req, monkeypatch): "topic": "Welcome!", "nsfw": False, "position": 0, "parent_id": "10", "rate_limit_per_user": 0, "last_message_id": "999", } - result = json.loads(discord_server(action="channel_info", channel_id="11")) + result = json.loads(discord_admin_handler(action="channel_info", channel_id="11")) assert result["name"] == "general" assert result["type"] == "text" assert result["guild_id"] == "111" @@ -293,7 +298,7 @@ def test_list_roles_sorted(self, mock_req, monkeypatch): {"id": "2", "name": "Admin", "position": 2, "color": 16711680, "mentionable": True, "managed": False, "hoist": True}, {"id": "3", "name": "Mod", "position": 1, "color": 255, "mentionable": True, "managed": False, "hoist": True}, ] - result = json.loads(discord_server(action="list_roles", guild_id="111")) + result = json.loads(discord_admin_handler(action="list_roles", guild_id="111")) assert result["count"] == 3 # Should be sorted by position descending assert result["roles"][0]["name"] == "Admin" @@ -317,7 +322,7 @@ def test_member_info(self, mock_req, monkeypatch): "joined_at": "2024-01-01T00:00:00Z", "premium_since": None, } - result = json.loads(discord_server(action="member_info", guild_id="111", user_id="42")) + result = json.loads(discord_admin_handler(action="member_info", guild_id="111", user_id="42")) assert result["username"] == "testuser" assert result["nickname"] == "Testy" assert result["roles"] == ["2", "3"] @@ -334,7 +339,7 @@ def test_search_members(self, mock_req, monkeypatch): mock_req.return_value = [ {"user": {"id": "42", "username": "testuser", "global_name": "Test", "bot": False}, "nick": None, "roles": []}, ] - result = json.loads(discord_server(action="search_members", guild_id="111", query="test")) + result = json.loads(discord_core(action="search_members", guild_id="111", query="test")) assert result["count"] == 1 assert result["members"][0]["username"] == "testuser" mock_req.assert_called_once_with( @@ -346,7 +351,7 @@ def test_search_members(self, mock_req, monkeypatch): def test_search_members_limit_capped(self, mock_req, monkeypatch): monkeypatch.setenv("DISCORD_BOT_TOKEN", "test-token") mock_req.return_value = [] - discord_server(action="search_members", guild_id="111", query="x", limit=200) + discord_core(action="search_members", guild_id="111", query="x", limit=200) call_params = mock_req.call_args[1]["params"] assert call_params["limit"] == "100" # Capped at 100 @@ -370,7 +375,7 @@ def test_fetch_messages(self, mock_req, monkeypatch): "pinned": False, }, ] - result = json.loads(discord_server(action="fetch_messages", channel_id="11")) + result = json.loads(discord_core(action="fetch_messages", channel_id="11")) assert result["count"] == 1 assert result["messages"][0]["content"] == "Hello world" assert result["messages"][0]["author"]["username"] == "user1" @@ -379,7 +384,7 @@ def test_fetch_messages(self, mock_req, monkeypatch): def test_fetch_messages_with_pagination(self, mock_req, monkeypatch): monkeypatch.setenv("DISCORD_BOT_TOKEN", "test-token") mock_req.return_value = [] - discord_server(action="fetch_messages", channel_id="11", before="999", limit=10) + discord_core(action="fetch_messages", channel_id="11", before="999", limit=10) call_params = mock_req.call_args[1]["params"] assert call_params["before"] == "999" assert call_params["limit"] == "10" @@ -396,7 +401,7 @@ def test_list_pins(self, mock_req, monkeypatch): mock_req.return_value = [ {"id": "500", "content": "Important announcement", "author": {"username": "admin"}, "timestamp": "2024-01-01T00:00:00Z"}, ] - result = json.loads(discord_server(action="list_pins", channel_id="11")) + result = json.loads(discord_admin_handler(action="list_pins", channel_id="11")) assert result["count"] == 1 assert result["pinned_messages"][0]["content"] == "Important announcement" @@ -410,7 +415,7 @@ class TestPinUnpin: def test_pin_message(self, mock_req, monkeypatch): monkeypatch.setenv("DISCORD_BOT_TOKEN", "test-token") mock_req.return_value = None # 204 - result = json.loads(discord_server(action="pin_message", channel_id="11", message_id="500")) + result = json.loads(discord_admin_handler(action="pin_message", channel_id="11", message_id="500")) assert result["success"] is True mock_req.assert_called_once_with("PUT", "/channels/11/pins/500", "test-token") @@ -418,7 +423,7 @@ def test_pin_message(self, mock_req, monkeypatch): def test_unpin_message(self, mock_req, monkeypatch): monkeypatch.setenv("DISCORD_BOT_TOKEN", "test-token") mock_req.return_value = None - result = json.loads(discord_server(action="unpin_message", channel_id="11", message_id="500")) + result = json.loads(discord_admin_handler(action="unpin_message", channel_id="11", message_id="500")) assert result["success"] is True @@ -431,7 +436,7 @@ class TestCreateThread: def test_create_standalone_thread(self, mock_req, monkeypatch): monkeypatch.setenv("DISCORD_BOT_TOKEN", "test-token") mock_req.return_value = {"id": "800", "name": "New Thread"} - result = json.loads(discord_server(action="create_thread", channel_id="11", name="New Thread")) + result = json.loads(discord_core(action="create_thread", channel_id="11", name="New Thread")) assert result["success"] is True assert result["thread_id"] == "800" # Verify the API call @@ -444,7 +449,7 @@ def test_create_standalone_thread(self, mock_req, monkeypatch): def test_create_thread_from_message(self, mock_req, monkeypatch): monkeypatch.setenv("DISCORD_BOT_TOKEN", "test-token") mock_req.return_value = {"id": "801", "name": "Discussion"} - result = json.loads(discord_server( + result = json.loads(discord_core( action="create_thread", channel_id="11", name="Discussion", message_id="1001", )) assert result["success"] is True @@ -463,7 +468,7 @@ class TestRoleManagement: def test_add_role(self, mock_req, monkeypatch): monkeypatch.setenv("DISCORD_BOT_TOKEN", "test-token") mock_req.return_value = None - result = json.loads(discord_server( + result = json.loads(discord_admin_handler( action="add_role", guild_id="111", user_id="42", role_id="2", )) assert result["success"] is True @@ -475,7 +480,7 @@ def test_add_role(self, mock_req, monkeypatch): def test_remove_role(self, mock_req, monkeypatch): monkeypatch.setenv("DISCORD_BOT_TOKEN", "test-token") mock_req.return_value = None - result = json.loads(discord_server( + result = json.loads(discord_admin_handler( action="remove_role", guild_id="111", user_id="42", role_id="2", )) assert result["success"] is True @@ -490,15 +495,23 @@ class TestErrorHandling: def test_api_error_handled(self, mock_req, monkeypatch): monkeypatch.setenv("DISCORD_BOT_TOKEN", "test-token") mock_req.side_effect = DiscordAPIError(403, '{"message": "Missing Access"}') - result = json.loads(discord_server(action="list_guilds")) + result = json.loads(discord_admin_handler(action="list_guilds")) assert "error" in result assert "403" in result["error"] @patch("tools.discord_tool._discord_request") - def test_unexpected_error_handled(self, mock_req, monkeypatch): + def test_unexpected_error_handled_admin(self, mock_req, monkeypatch): monkeypatch.setenv("DISCORD_BOT_TOKEN", "test-token") mock_req.side_effect = RuntimeError("something broke") - result = json.loads(discord_server(action="list_guilds")) + result = json.loads(discord_admin_handler(action="list_guilds")) + assert "error" in result + assert "something broke" in result["error"] + + @patch("tools.discord_tool._discord_request") + def test_unexpected_error_handled_core(self, mock_req, monkeypatch): + monkeypatch.setenv("DISCORD_BOT_TOKEN", "test-token") + mock_req.side_effect = RuntimeError("something broke") + result = json.loads(discord_core(action="fetch_messages", channel_id="11")) assert "error" in result assert "something broke" in result["error"] @@ -508,79 +521,109 @@ def test_unexpected_error_handled(self, mock_req, monkeypatch): # --------------------------------------------------------------------------- class TestRegistration: - def test_tool_registered(self): + def test_core_tool_registered(self): from tools.registry import registry - entry = registry._tools.get("discord_server") + entry = registry._tools.get("discord") assert entry is not None - assert entry.schema["name"] == "discord_server" + assert entry.schema["name"] == "discord" assert entry.toolset == "discord" assert entry.check_fn is not None assert entry.requires_env == ["DISCORD_BOT_TOKEN"] - def test_schema_actions(self): - """Static schema should list all actions (the model_tools post-processing - narrows this per-session; static registration is the superset).""" + def test_admin_tool_registered(self): from tools.registry import registry - entry = registry._tools["discord_server"] - actions = entry.schema["parameters"]["properties"]["action"]["enum"] - expected = [ - "list_guilds", "server_info", "list_channels", "channel_info", - "list_roles", "member_info", "search_members", "fetch_messages", - "list_pins", "pin_message", "unpin_message", "create_thread", - "add_role", "remove_role", - ] - assert set(actions) == set(expected) - assert set(_ACTIONS.keys()) == set(expected) + entry = registry._tools.get("discord_admin") + assert entry is not None + assert entry.schema["name"] == "discord_admin" + assert entry.toolset == "discord_admin" + assert entry.check_fn is not None + assert entry.requires_env == ["DISCORD_BOT_TOKEN"] + + def test_core_schema_actions(self): + """Core static schema should list only core actions.""" + from tools.registry import registry + entry = registry._tools["discord"] + actions = set(entry.schema["parameters"]["properties"]["action"]["enum"]) + assert actions == {"fetch_messages", "search_members", "create_thread"} + + def test_admin_schema_actions(self): + """Admin static schema should list only admin actions.""" + from tools.registry import registry + entry = registry._tools["discord_admin"] + actions = set(entry.schema["parameters"]["properties"]["action"]["enum"]) + expected_admin = set(_ACTIONS.keys()) - {"fetch_messages", "search_members", "create_thread"} + assert actions == expected_admin + + def test_all_actions_covered(self): + """Core + admin actions should cover all known actions.""" + assert set(_CORE_ACTIONS.keys()) | set(_ADMIN_ACTIONS.keys()) == set(_ACTIONS.keys()) + assert set(_CORE_ACTIONS.keys()) & set(_ADMIN_ACTIONS.keys()) == set() def test_schema_parameter_bounds(self): from tools.registry import registry - entry = registry._tools["discord_server"] + entry = registry._tools["discord"] props = entry.schema["parameters"]["properties"] assert props["limit"]["minimum"] == 1 assert props["limit"]["maximum"] == 100 assert props["auto_archive_duration"]["enum"] == [60, 1440, 4320, 10080] - def test_schema_description_is_action_manifest(self): - """The top-level description should include the action manifest - (one-line signatures per action) so the model can find required - params without re-reading every parameter description.""" + def test_core_schema_description(self): + """Core schema description should mention core actions.""" from tools.registry import registry - entry = registry._tools["discord_server"] + entry = registry._tools["discord"] desc = entry.schema["description"] - # Spot-check a few entries - assert "list_guilds()" in desc assert "fetch_messages(channel_id)" in desc + assert "search_members(guild_id, query)" in desc + assert "create_thread(channel_id, name)" in desc + # Admin actions should NOT be in core description + assert "list_guilds()" not in desc + assert "add_role(" not in desc + + def test_admin_schema_description(self): + """Admin schema description should mention admin actions.""" + from tools.registry import registry + entry = registry._tools["discord_admin"] + desc = entry.schema["description"] + assert "list_guilds()" in desc assert "add_role(guild_id, user_id, role_id)" in desc + # Core actions should NOT be in admin description + assert "fetch_messages(" not in desc + assert "create_thread(" not in desc def test_handler_callable(self): from tools.registry import registry - entry = registry._tools["discord_server"] + entry = registry._tools["discord"] assert callable(entry.handler) + entry_admin = registry._tools["discord_admin"] + assert callable(entry_admin.handler) # --------------------------------------------------------------------------- -# Toolset: discord_server only in hermes-discord +# Toolset: discord / discord_admin only in hermes-discord # --------------------------------------------------------------------------- class TestToolsetInclusion: - def test_discord_server_in_hermes_discord_toolset(self): + def test_discord_tools_in_hermes_discord_toolset(self): from toolsets import TOOLSETS - assert "discord_server" in TOOLSETS["hermes-discord"]["tools"] + assert "discord" in TOOLSETS["hermes-discord"]["tools"] + assert "discord_admin" in TOOLSETS["hermes-discord"]["tools"] - def test_discord_server_not_in_core_tools(self): + def test_discord_tools_not_in_core_tools(self): from toolsets import _HERMES_CORE_TOOLS - assert "discord_server" not in _HERMES_CORE_TOOLS + assert "discord" not in _HERMES_CORE_TOOLS + assert "discord_admin" not in _HERMES_CORE_TOOLS - def test_discord_server_not_in_other_toolsets(self): + def test_discord_tools_not_in_other_toolsets(self): from toolsets import TOOLSETS for name, ts in TOOLSETS.items(): - if name == "hermes-discord": - continue - # The gateway toolset might include it if it unions all platform tools - if name == "hermes-gateway": + if name in ("hermes-discord", "hermes-gateway", "discord", "discord_admin"): continue - assert "discord_server" not in ts.get("tools", []), ( - f"discord_server should not be in toolset '{name}'" + tools = ts.get("tools", []) + assert "discord" not in tools or name == "discord", ( + f"discord tool should not be in toolset '{name}'" + ) + assert "discord_admin" not in tools or name == "discord_admin", ( + f"discord_admin tool should not be in toolset '{name}'" ) @@ -798,41 +841,70 @@ def teardown_method(self): @patch("tools.discord_tool._discord_request") def test_no_token_returns_none(self, mock_req, monkeypatch): monkeypatch.delenv("DISCORD_BOT_TOKEN", raising=False) - assert get_dynamic_schema() is None + assert get_dynamic_schema_core() is None + assert get_dynamic_schema_admin() is None mock_req.assert_not_called() @patch("tools.discord_tool._discord_request") - def test_full_intents_full_schema(self, mock_req, monkeypatch): + def test_full_intents_core_schema(self, mock_req, monkeypatch): monkeypatch.setenv("DISCORD_BOT_TOKEN", "tok") monkeypatch.setattr( "hermes_cli.config.load_config", lambda: {"discord": {"server_actions": ""}}, ) mock_req.return_value = {"flags": (1 << 14) | (1 << 18)} - schema = get_dynamic_schema() - actions = schema["parameters"]["properties"]["action"]["enum"] - assert set(actions) == set(_ACTIONS.keys()) - # No content warning + schema = get_dynamic_schema_core() + actions = set(schema["parameters"]["properties"]["action"]["enum"]) + assert actions == set(_CORE_ACTIONS.keys()) + assert schema["name"] == "discord" + + @patch("tools.discord_tool._discord_request") + def test_full_intents_admin_schema(self, mock_req, monkeypatch): + monkeypatch.setenv("DISCORD_BOT_TOKEN", "tok") + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"discord": {"server_actions": ""}}, + ) + mock_req.return_value = {"flags": (1 << 14) | (1 << 18)} + schema = get_dynamic_schema_admin() + actions = set(schema["parameters"]["properties"]["action"]["enum"]) + assert actions == set(_ADMIN_ACTIONS.keys()) + assert schema["name"] == "discord_admin" + # No content warning when MESSAGE_CONTENT is enabled assert "MESSAGE_CONTENT" not in schema["description"] @patch("tools.discord_tool._discord_request") - def test_no_members_intent_removes_member_actions_from_schema( + def test_no_members_intent_removes_member_actions_from_admin_schema( self, mock_req, monkeypatch, ): + """member_info is an admin action; it should be hidden when + GUILD_MEMBERS intent is missing.""" monkeypatch.setenv("DISCORD_BOT_TOKEN", "tok") monkeypatch.setattr( "hermes_cli.config.load_config", lambda: {"discord": {"server_actions": ""}}, ) mock_req.return_value = {"flags": 1 << 18} # only MESSAGE_CONTENT - schema = get_dynamic_schema() + schema = get_dynamic_schema_admin() actions = schema["parameters"]["properties"]["action"]["enum"] - assert "search_members" not in actions assert "member_info" not in actions - # Manifest description should also not advertise them - assert "search_members" not in schema["description"] assert "member_info" not in schema["description"] + @patch("tools.discord_tool._discord_request") + def test_no_members_intent_hides_search_members_from_core( + self, mock_req, monkeypatch, + ): + """search_members is a core action gated by GUILD_MEMBERS intent.""" + monkeypatch.setenv("DISCORD_BOT_TOKEN", "tok") + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"discord": {"server_actions": ""}}, + ) + mock_req.return_value = {"flags": 1 << 18} # only MESSAGE_CONTENT + schema = get_dynamic_schema_core() + actions = schema["parameters"]["properties"]["action"]["enum"] + assert "search_members" not in actions + @patch("tools.discord_tool._discord_request") def test_no_message_content_adds_warning_note(self, mock_req, monkeypatch): monkeypatch.setenv("DISCORD_BOT_TOKEN", "tok") @@ -841,41 +913,53 @@ def test_no_message_content_adds_warning_note(self, mock_req, monkeypatch): lambda: {"discord": {"server_actions": ""}}, ) mock_req.return_value = {"flags": 1 << 14} # only GUILD_MEMBERS - schema = get_dynamic_schema() + schema = get_dynamic_schema_core() assert "MESSAGE_CONTENT" in schema["description"] # But fetch_messages is still available actions = schema["parameters"]["properties"]["action"]["enum"] assert "fetch_messages" in actions @patch("tools.discord_tool._discord_request") - def test_config_allowlist_narrows_schema(self, mock_req, monkeypatch): + def test_config_allowlist_narrows_admin_schema(self, mock_req, monkeypatch): monkeypatch.setenv("DISCORD_BOT_TOKEN", "tok") monkeypatch.setattr( "hermes_cli.config.load_config", lambda: {"discord": {"server_actions": "list_guilds,list_channels"}}, ) mock_req.return_value = {"flags": (1 << 14) | (1 << 18)} - schema = get_dynamic_schema() + schema = get_dynamic_schema_admin() actions = schema["parameters"]["properties"]["action"]["enum"] assert actions == ["list_guilds", "list_channels"] - # Manifest description should only show allowed ones (check for - # the signature marker, which is specific to manifest lines) assert "list_guilds()" in schema["description"] assert "add_role(" not in schema["description"] - assert "create_thread(" not in schema["description"] @patch("tools.discord_tool._discord_request") - def test_empty_allowlist_with_valid_values_hides_tool(self, mock_req, monkeypatch): + def test_empty_allowlist_with_valid_values_hides_tools(self, mock_req, monkeypatch): """If the allowlist resolves to zero valid actions (e.g. all names - were typos), get_dynamic_schema returns None so the tool is dropped - entirely rather than showing an empty enum.""" + were typos), get_dynamic_schema returns None so the tool is dropped.""" monkeypatch.setenv("DISCORD_BOT_TOKEN", "tok") monkeypatch.setattr( "hermes_cli.config.load_config", lambda: {"discord": {"server_actions": "typo_one,typo_two"}}, ) mock_req.return_value = {"flags": (1 << 14) | (1 << 18)} - assert get_dynamic_schema() is None + assert get_dynamic_schema_core() is None + assert get_dynamic_schema_admin() is None + + @patch("tools.discord_tool._discord_request") + def test_backward_compat_wrapper(self, mock_req, monkeypatch): + """get_dynamic_schema() should delegate to get_dynamic_schema_core().""" + monkeypatch.setenv("DISCORD_BOT_TOKEN", "tok") + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"discord": {"server_actions": ""}}, + ) + mock_req.return_value = {"flags": (1 << 14) | (1 << 18)} + schema = get_dynamic_schema() + assert schema is not None + assert schema["name"] == "discord" + actions = set(schema["parameters"]["properties"]["action"]["enum"]) + assert actions == set(_CORE_ACTIONS.keys()) # --------------------------------------------------------------------------- @@ -890,7 +974,7 @@ def test_denied_action_blocked_at_runtime(self, mock_req, monkeypatch): "hermes_cli.config.load_config", lambda: {"discord": {"server_actions": "list_guilds"}}, ) - result = json.loads(discord_server(action="add_role", guild_id="1", user_id="2", role_id="3")) + result = json.loads(discord_admin_handler(action="add_role", guild_id="1", user_id="2", role_id="3")) assert "error" in result assert "disabled by config" in result["error"] mock_req.assert_not_called() @@ -903,7 +987,7 @@ def test_allowed_action_proceeds(self, mock_req, monkeypatch): lambda: {"discord": {"server_actions": "list_guilds"}}, ) mock_req.return_value = [] - result = json.loads(discord_server(action="list_guilds")) + result = json.loads(discord_admin_handler(action="list_guilds")) assert "guilds" in result @@ -930,7 +1014,7 @@ def test_403_in_runtime_is_enriched(self, mock_req, monkeypatch): lambda: {"discord": {"server_actions": ""}}, ) mock_req.side_effect = DiscordAPIError(403, '{"message":"Missing Permissions"}') - result = json.loads(discord_server( + result = json.loads(discord_admin_handler( action="add_role", guild_id="1", user_id="2", role_id="3", )) assert "error" in result @@ -944,7 +1028,7 @@ def test_non_403_errors_are_not_enriched(self, mock_req, monkeypatch): lambda: {"discord": {"server_actions": ""}}, ) mock_req.side_effect = DiscordAPIError(500, "server error") - result = json.loads(discord_server(action="list_guilds")) + result = json.loads(discord_admin_handler(action="list_guilds")) assert "500" in result["error"] assert "MANAGE_ROLES" not in result["error"] @@ -961,10 +1045,10 @@ def teardown_method(self): _reset_capability_cache() @patch("tools.discord_tool._discord_request") - def test_discord_server_schema_rebuilt_by_get_tool_definitions( + def test_discord_admin_schema_rebuilt_by_get_tool_definitions( self, mock_req, monkeypatch, ): - """When model_tools.get_tool_definitions runs with discord_server + """When model_tools.get_tool_definitions runs with discord_admin available, it should replace the static schema with the dynamic one.""" monkeypatch.setenv("DISCORD_BOT_TOKEN", "tok") monkeypatch.setattr( @@ -976,16 +1060,16 @@ def test_discord_server_schema_rebuilt_by_get_tool_definitions( from model_tools import get_tool_definitions tools = get_tool_definitions(enabled_toolsets=["hermes-discord"], quiet_mode=True) - discord_tool = next( - (t for t in tools if t.get("function", {}).get("name") == "discord_server"), + discord_admin_tool = next( + (t for t in tools if t.get("function", {}).get("name") == "discord_admin"), None, ) - assert discord_tool is not None, "discord_server should be in the schema" - actions = discord_tool["function"]["parameters"]["properties"]["action"]["enum"] + assert discord_admin_tool is not None, "discord_admin should be in the schema" + actions = discord_admin_tool["function"]["parameters"]["properties"]["action"]["enum"] assert actions == ["list_guilds", "server_info"] @patch("tools.discord_tool._discord_request") - def test_discord_server_dropped_when_allowlist_empties_it( + def test_discord_tools_dropped_when_allowlist_empties_them( self, mock_req, monkeypatch, ): monkeypatch.setenv("DISCORD_BOT_TOKEN", "tok") @@ -998,4 +1082,6 @@ def test_discord_server_dropped_when_allowlist_empties_it( from model_tools import get_tool_definitions tools = get_tool_definitions(enabled_toolsets=["hermes-discord"], quiet_mode=True) names = [t.get("function", {}).get("name") for t in tools] + assert "discord" not in names + assert "discord_admin" not in names assert "discord_server" not in names diff --git a/tools/discord_tool.py b/tools/discord_tool.py index 1bdbbd43682f6..dff0c67669a9c 100644 --- a/tools/discord_tool.py +++ b/tools/discord_tool.py @@ -473,6 +473,12 @@ def _remove_role(token: str, guild_id: str, user_id: str, role_id: str, **_kwarg "remove_role": _remove_role, } +_CORE_ACTION_NAMES = frozenset({"fetch_messages", "search_members", "create_thread"}) +_ADMIN_ACTION_NAMES = frozenset(_ACTIONS.keys()) - _CORE_ACTION_NAMES + +_CORE_ACTIONS = {k: v for k, v in _ACTIONS.items() if k in _CORE_ACTION_NAMES} +_ADMIN_ACTIONS = {k: v for k, v in _ACTIONS.items() if k in _ADMIN_ACTION_NAMES} + # Single-source-of-truth manifest: action โ†’ (signature, one-line description). # Consumed by :func:`_build_schema` so the schema's top-level description # always matches the registered action set. @@ -531,7 +537,7 @@ def _load_allowed_actions_config() -> Optional[List[str]]: from hermes_cli.config import load_config cfg = load_config() except Exception as exc: - logger.debug("discord_server: could not load config (%s); allowing all actions.", exc) + logger.debug("discord: could not load config (%s); allowing all actions.", exc) return None raw = (cfg.get("discord") or {}).get("server_actions") @@ -586,12 +592,16 @@ def _available_actions( def _build_schema( actions: List[str], caps: Optional[Dict[str, Any]] = None, -) -> Dict[str, Any]: - """Build the tool schema for the given filtered action list.""" + tool_name: str = "discord", +) -> Optional[Dict[str, Any]]: + """Build the tool schema for the given filtered action list. + + Returns ``None`` when *actions* is empty โ€” callers should drop the + tool from registration in that case. + """ caps = caps or {} if not actions: - # Tool shouldn't be registered when empty, but guard anyway. - actions = list(_ACTIONS.keys()) + return None # Action manifest lines (action-first, parameter-scoped). manifest_lines = [ @@ -602,24 +612,36 @@ def _build_schema( manifest_block = "\n".join(manifest_lines) content_note = "" - if caps.get("detected") and caps.get("has_message_content") is False: + affected_actions = {"fetch_messages", "list_pins"} & set(actions) + if affected_actions and caps.get("detected") and caps.get("has_message_content") is False: + names = " and ".join(sorted(affected_actions)) content_note = ( - "\n\nNOTE: Bot does NOT have the MESSAGE_CONTENT privileged intent. " - "fetch_messages and list_pins will return message metadata (author, " + f"\n\nNOTE: Bot does NOT have the MESSAGE_CONTENT privileged intent. " + f"{names} will return message metadata (author, " "timestamps, attachments, reactions, pin state) but `content` will be " "empty for messages not sent as a direct mention to the bot or in DMs. " "Enable the intent in the Discord Developer Portal to see all content." ) - description = ( - "Query and manage a Discord server via the REST API.\n\n" - "Available actions:\n" - f"{manifest_block}\n\n" - "Call list_guilds first to discover guild_ids, then list_channels for " - "channel_ids. Runtime errors will tell you if the bot lacks a specific " - "per-guild permission (e.g. MANAGE_ROLES for add_role)." - f"{content_note}" - ) + if tool_name == "discord_admin": + description = ( + "Manage a Discord server via the REST API.\n\n" + "Available actions:\n" + f"{manifest_block}\n\n" + "Call list_guilds first to discover guild_ids, then list_channels for " + "channel_ids. Runtime errors will tell you if the bot lacks a specific " + "per-guild permission (e.g. MANAGE_ROLES for add_role)." + f"{content_note}" + ) + else: + description = ( + "Read and participate in a Discord server.\n\n" + "Available actions:\n" + f"{manifest_block}\n\n" + "Use the channel_id from the current conversation context. " + "Use search_members to look up user IDs by name prefix." + f"{content_note}" + ) properties: Dict[str, Any] = { "action": { @@ -676,7 +698,7 @@ def _build_schema( } return { - "name": "discord_server", + "name": tool_name, "description": description, "parameters": { "type": "object", @@ -686,28 +708,33 @@ def _build_schema( } -def get_dynamic_schema() -> Optional[Dict[str, Any]]: - """Return a schema filtered by current intents + config allowlist. - - Called by ``model_tools.get_tool_definitions`` as a post-processing - step so the schema the model sees always reflects reality. Returns - ``None`` when no actions are available (tool should be removed from - the schema list entirely). - """ +def _get_dynamic_schema( + action_subset: Dict[str, Any], + tool_name: str, +) -> Optional[Dict[str, Any]]: + """Build a dynamic schema for *action_subset* filtered by intents + config.""" token = _get_bot_token() if not token: return None - caps = _detect_capabilities(token) allowlist = _load_allowed_actions_config() - actions = _available_actions(caps, allowlist) + actions = [a for a in _available_actions(caps, allowlist) if a in action_subset] if not actions: - logger.warning( - "discord_server: config allowlist/intents left zero available actions; " - "hiding tool from this session." - ) return None - return _build_schema(actions, caps) + return _build_schema(actions, caps, tool_name=tool_name) + + +def get_dynamic_schema_core() -> Optional[Dict[str, Any]]: + return _get_dynamic_schema(_CORE_ACTIONS, "discord") + + +def get_dynamic_schema_admin() -> Optional[Dict[str, Any]]: + return _get_dynamic_schema(_ADMIN_ACTIONS, "discord_admin") + + +def get_dynamic_schema() -> Optional[Dict[str, Any]]: + """Backward-compat wrapper โ€” returns core schema.""" + return get_dynamic_schema_core() # --------------------------------------------------------------------------- @@ -774,11 +801,13 @@ def check_discord_tool_requirements() -> bool: # --------------------------------------------------------------------------- -# Main handler +# Handlers # --------------------------------------------------------------------------- -def discord_server( +def _run_discord_action( action: str, + valid_actions: Dict[str, Any], + tool_label: str, guild_id: str = "", channel_id: str = "", user_id: str = "", @@ -790,18 +819,17 @@ def discord_server( before: str = "", after: str = "", auto_archive_duration: int = 1440, - task_id: str = None, ) -> str: - """Execute a Discord server action.""" + """Shared handler logic for both discord tools.""" token = _get_bot_token() if not token: return json.dumps({"error": "DISCORD_BOT_TOKEN not configured."}) - action_fn = _ACTIONS.get(action) + action_fn = valid_actions.get(action) if not action_fn: return json.dumps({ "error": f"Unknown action: {action}", - "available_actions": list(_ACTIONS.keys()), + "available_actions": list(valid_actions.keys()), }) # Config-level allowlist gate (defense in depth โ€” schema already filtered, @@ -848,44 +876,64 @@ def discord_server( auto_archive_duration=auto_archive_duration, ) except DiscordAPIError as e: - logger.warning("Discord API error in action '%s': %s", action, e) + logger.warning("Discord API error in %s action '%s': %s", tool_label, action, e) if e.status == 403: return json.dumps({"error": _enrich_403(action, e.body)}) return json.dumps({"error": str(e)}) except Exception as e: - logger.exception("Unexpected error in discord_server action '%s'", action) + logger.exception("Unexpected error in %s action '%s'", tool_label, action) return json.dumps({"error": f"Unexpected error: {e}"}) +def discord_core(action: str, **kwargs) -> str: + """Execute a core Discord action (fetch_messages, search_members, create_thread).""" + return _run_discord_action(action, _CORE_ACTIONS, "discord", **kwargs) + + +def discord_admin_handler(action: str, **kwargs) -> str: + """Execute a Discord admin action (server management).""" + return _run_discord_action(action, _ADMIN_ACTIONS, "discord_admin", **kwargs) + + # --------------------------------------------------------------------------- # Tool registration # --------------------------------------------------------------------------- -# Register with the full unfiltered schema. ``model_tools.get_tool_definitions`` -# rebuilds this per-session via ``get_dynamic_schema`` so the model only ever -# sees intent-available, config-allowed actions. The static registration is a -# safe baseline for tools that inspect the registry directly. -_STATIC_SCHEMA = _build_schema(list(_ACTIONS.keys()), caps={"detected": False}) +_HANDLER_DEFAULTS = { + "action": "", "guild_id": "", "channel_id": "", "user_id": "", + "role_id": "", "message_id": "", "query": "", "name": "", + "limit": 50, "before": "", "after": "", "auto_archive_duration": 1440, +} + + +def _make_handler(handler_fn): + """Create a registry-compatible handler lambda for a discord handler.""" + return lambda args, **kw: handler_fn( + **{k: args.get(k, v) for k, v in _HANDLER_DEFAULTS.items()}, + ) + + +_STATIC_CORE_SCHEMA = _build_schema( + list(_CORE_ACTIONS.keys()), caps={"detected": False}, tool_name="discord", +) +_STATIC_ADMIN_SCHEMA = _build_schema( + list(_ADMIN_ACTIONS.keys()), caps={"detected": False}, tool_name="discord_admin", +) registry.register( - name="discord_server", + name="discord", toolset="discord", - schema=_STATIC_SCHEMA, - handler=lambda args, **kw: discord_server( - action=args.get("action", ""), - guild_id=args.get("guild_id", ""), - channel_id=args.get("channel_id", ""), - user_id=args.get("user_id", ""), - role_id=args.get("role_id", ""), - message_id=args.get("message_id", ""), - query=args.get("query", ""), - name=args.get("name", ""), - limit=args.get("limit", 50), - before=args.get("before", ""), - after=args.get("after", ""), - auto_archive_duration=args.get("auto_archive_duration", 1440), - task_id=kw.get("task_id"), - ), + schema=_STATIC_CORE_SCHEMA, + handler=_make_handler(discord_core), + check_fn=check_discord_tool_requirements, + requires_env=["DISCORD_BOT_TOKEN"], +) + +registry.register( + name="discord_admin", + toolset="discord_admin", + schema=_STATIC_ADMIN_SCHEMA, + handler=_make_handler(discord_admin_handler), check_fn=check_discord_tool_requirements, requires_env=["DISCORD_BOT_TOKEN"], ) diff --git a/toolsets.py b/toolsets.py index b3cdb2e7ae230..4543b06c01bba 100644 --- a/toolsets.py +++ b/toolsets.py @@ -202,6 +202,18 @@ "includes": [] }, + "discord": { + "description": "Discord read and participate tools (fetch messages, search members, create threads)", + "tools": ["discord"], + "includes": [], + }, + + "discord_admin": { + "description": "Discord server management (list channels/roles, pin messages, assign roles)", + "tools": ["discord_admin"], + "includes": [], + }, + "feishu_doc": { "description": "Read Feishu/Lark document content", "tools": ["feishu_doc_read"], @@ -317,8 +329,8 @@ "hermes-discord": { "description": "Discord bot toolset - full access (terminal has safety checks via dangerous command approval)", "tools": _HERMES_CORE_TOOLS + [ - # Discord server introspection & management (gated on DISCORD_BOT_TOKEN via check_fn) - "discord_server", + "discord", + "discord_admin", ], "includes": [] }, @@ -379,7 +391,13 @@ "hermes-feishu": { "description": "Feishu/Lark bot toolset - enterprise messaging via Feishu/Lark (full access)", - "tools": _HERMES_CORE_TOOLS, + "tools": _HERMES_CORE_TOOLS + [ + "feishu_doc_read", + "feishu_drive_list_comments", + "feishu_drive_list_comment_replies", + "feishu_drive_reply_comment", + "feishu_drive_add_comment", + ], "includes": [] },