From bd9afb027a3e1a9409f6173a9c11dd5e679832ba Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Fri, 24 Apr 2026 15:22:18 +0530 Subject: [PATCH 1/8] fix(tools): recover non-configurable toolsets from composite resolution The reverse-mapping loop in _get_platform_tools only checked CONFIGURABLE_TOOLSETS, silently dropping platform-specific toolsets like discord and feishu_doc whose tools were in the composite but had no configurable key. Add a second pass over TOOLSETS that picks up unclaimed toolsets whose tools are present in the resolved composite. --- hermes_cli/tools_config.py | 37 +++++++++++++++++++++-- tests/hermes_cli/test_tools_config.py | 42 +++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 3 deletions(-) diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index e89f961781f3..7546d3039ef5 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -549,7 +549,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 +563,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 +587,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 +641,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 diff --git a/tests/hermes_cli/test_tools_config.py b/tests/hermes_cli/test_tools_config.py index b134fc98b318..b4ea337d1d06 100644 --- a/tests/hermes_cli/test_tools_config.py +++ b/tests/hermes_cli/test_tools_config.py @@ -601,3 +601,45 @@ 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 From 57a2b97ae863662ad98e47d17e97c26973b53da6 Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Fri, 24 Apr 2026 15:47:29 +0530 Subject: [PATCH 2/8] feat(discord): split discord_server into discord + discord_admin tools Split the monolithic discord_server tool (14 actions) into two: - discord: core actions (fetch_messages, search_members, create_thread) that are useful for the agent's normal operation. Auto-enabled on the discord platform via the pipeline fix. - discord_admin: server management actions (list channels/roles, pins, role assignment) that require explicit opt-in via hermes tools. Added to CONFIGURABLE_TOOLSETS and _DEFAULT_OFF_TOOLSETS. --- hermes_cli/config.py | 2 +- hermes_cli/tools_config.py | 3 +- model_tools.py | 50 ++--- tests/hermes_cli/test_tools_config.py | 14 ++ tests/test_toolsets.py | 4 +- tests/tools/test_discord_tool.py | 278 +++++++++++++++++--------- tools/discord_tool.py | 174 ++++++++++------ toolsets.py | 16 +- 8 files changed, 353 insertions(+), 188 deletions(-) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 58f874595936..648267302e00 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 7546d3039ef5..45c6ba3dd9d0 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(): diff --git a/model_tools.py b/model_tools.py index 2b7767fda3fa..9b6e13547547 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 b4ea337d1d06..4ac32d007ce0 100644 --- a/tests/hermes_cli/test_tools_config.py +++ b/tests/hermes_cli/test_tools_config.py @@ -643,3 +643,17 @@ def test_get_platform_tools_second_pass_skips_fully_claimed_toolsets(): 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 diff --git a/tests/test_toolsets.py b/tests/test_toolsets.py index 183f9e514f16..4e4289999c57 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 34fe672132dd..70b43903ecf8 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 1bdbbd43682f..dff0c67669a9 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 b3cdb2e7ae23..a9e1193580ce 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": [] }, From 8feaa7cd1b0635da9c46d723137124c52609231c Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Fri, 24 Apr 2026 15:54:03 +0530 Subject: [PATCH 3/8] feat(feishu): wire feishu doc/drive tools into hermes-feishu composite The feishu_doc and feishu_drive tools were registered in the tool registry but never added to the hermes-feishu composite toolset. The pipeline fix from the prior commit now recovers them automatically once they are in the composite. --- tests/hermes_cli/test_tools_config.py | 13 +++++++++++++ toolsets.py | 8 +++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/tests/hermes_cli/test_tools_config.py b/tests/hermes_cli/test_tools_config.py index 4ac32d007ce0..d7a2f7c7c03c 100644 --- a/tests/hermes_cli/test_tools_config.py +++ b/tests/hermes_cli/test_tools_config.py @@ -657,3 +657,16 @@ def test_discord_admin_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}" diff --git a/toolsets.py b/toolsets.py index a9e1193580ce..4543b06c01bb 100644 --- a/toolsets.py +++ b/toolsets.py @@ -391,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": [] }, From ff9b0528a206927b9c3da09144c2af8bade39215 Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Fri, 24 Apr 2026 15:55:51 +0530 Subject: [PATCH 4/8] fix(tools): normalize numeric entries and clear stale no_mcp in _save_platform_tools YAML parses bare numeric toolset names (e.g. 12306:) as int, causing TypeError in sorted() since the read path normalizes to str but the save path did not. The no_mcp sentinel was preserved in existing entries even when the user re-enabled MCP servers, causing MCP to stay silently disabled. --- hermes_cli/tools_config.py | 3 ++ tests/hermes_cli/test_tools_config.py | 50 +++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index 45c6ba3dd9d0..1bfb36df0b8a 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -701,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) @@ -708,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/tests/hermes_cli/test_tools_config.py b/tests/hermes_cli/test_tools_config.py index d7a2f7c7c03c..91b38e375086 100644 --- a/tests/hermes_cli/test_tools_config.py +++ b/tests/hermes_cli/test_tools_config.py @@ -670,3 +670,53 @@ def test_get_platform_tools_feishu_tools_not_on_other_platforms(): 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 From 0eb85906b08dff9d1df5a12906a8b9128aa57493 Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Sat, 25 Apr 2026 00:09:12 +0530 Subject: [PATCH 5/8] feat(session): add guild_id/parent_chat_id/message_id to SessionSource MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Groundwork for injecting raw platform identifiers into the agent's system prompt. Currently only `thread_id` is exposed as a raw ID โ€” callers in a Discord thread had to guess `channel_id == thread_id` (which happens to work because threads are channels in Discord's REST API) and had no way to reference the parent channel, guild, or the triggering message. Adds three optional fields: - `guild_id` โ€” Discord guild / Slack workspace / Matrix server scope - `parent_chat_id` โ€” parent channel when chat_id refers to a thread - `message_id` โ€” ID of the triggering message (pin/reply/react) Extends `BasePlatformAdapter.build_source()` to accept + forward them and teaches `to_dict`/`from_dict` to serialize them. Behaviourally a no-op: nothing reads the fields yet and they default to None. --- gateway/platforms/base.py | 6 ++++++ gateway/session.py | 14 +++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index db76034989c5..cfbe9b1b2602 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/session.py b/gateway/session.py index db90d3121727..7353785272a9 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"), ) From 3c29834354b80affdc97a826a683211765bb33cf Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Sat, 25 Apr 2026 00:09:53 +0530 Subject: [PATCH 6/8] feat(discord): populate guild_id, parent_chat_id, message_id on SessionSource MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Discord knows all four identifiers for every inbound message โ€” guild, channel (or thread), parent channel when in a thread, and the triggering message. Pass them into ``SessionSource`` via the new ``build_source()`` kwargs so downstream code (context-prompt builder, delivery, logging) can use them without re-resolving from discord.py objects. For auto-threaded messages, remember the original channel as the parent before swapping ``chat_id`` to the freshly created thread. Behavioural: still a no-op โ€” nothing consumes these fields yet. --- gateway/platforms/discord.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/gateway/platforms/discord.py b/gateway/platforms/discord.py index f741d45b5f35..fe799339328f 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 From 0aa1269e568993c4b5b06787b8a693b9626dfa64 Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Sat, 25 Apr 2026 00:10:32 +0530 Subject: [PATCH 7/8] fix(session): gate stale "no Discord APIs" note on DISCORD_BOT_TOKEN MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Discord platform note in the session context prompt claimed the agent has no server-management APIs โ€” pre-dating the discord tool. With a bot token configured the agent actually has fetch_messages, search_members, create_thread, and optionally the discord_admin tool; telling the model otherwise causes it to refuse or apologise for calls it is fully able to make. Gate the disclaimer on DISCORD_BOT_TOKEN being unset, matching the tool's own ``check_fn``. Without a token the note still appears and remains accurate; with a token the model is no longer gaslit into refusing valid tool calls. --- gateway/session.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/gateway/session.py b/gateway/session.py index 7353785272a9..495c95d56420 100644 --- a/gateway/session.py +++ b/gateway/session.py @@ -285,14 +285,21 @@ 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." - ) + # Only emit the "no Discord APIs" disclaimer when the discord tool + # can't load. The tool self-gates on DISCORD_BOT_TOKEN at registry + # check time, so matching that condition keeps the prompt honest: + # with a token the agent has fetch_messages/search_members/ + # create_thread (and optionally discord_admin); without one it + # really is limited to reading/replying via the gateway. + if not (os.environ.get("DISCORD_BOT_TOKEN") or "").strip(): + 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)"] From 7efd91d4b465c26b151b3f5991727ec23e56b875 Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Sat, 25 Apr 2026 00:11:26 +0530 Subject: [PATCH 8/8] feat(session): inject Discord IDs block when discord tool is loaded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When DISCORD_BOT_TOKEN is set โ€” meaning the discord tool actually loads โ€” emit a dedicated IDs block in the session context prompt so the agent can call ``fetch_messages``, ``pin_message``, etc. with real identifiers instead of probing. Currently only ``thread_id`` was exposed as a raw ID (via the ``description`` string). The agent in a Discord thread had to guess that the thread ID doubles as a channel ID for the REST API (it does), and it had no way to reference the parent channel, the guild, or the triggering message at all. The block adapts to context: - Thread: guild / parent channel / thread / message - Channel: guild / channel / message - (DM has no guild/channel IDs worth listing; only message) Discord isn't in _PII_SAFE_PLATFORMS, so IDs ship unredacted. --- gateway/session.py | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/gateway/session.py b/gateway/session.py index 495c95d56420..b5d02edde042 100644 --- a/gateway/session.py +++ b/gateway/session.py @@ -285,13 +285,26 @@ def build_session_context_prompt( "that you can only read messages sent directly to you and respond." ) elif context.source.platform == Platform.DISCORD: - # Only emit the "no Discord APIs" disclaimer when the discord tool - # can't load. The tool self-gates on DISCORD_BOT_TOKEN at registry - # check time, so matching that condition keeps the prompt honest: + # 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); without one it - # really is limited to reading/replying via the gateway. - if not (os.environ.get("DISCORD_BOT_TOKEN") or "").strip(): + # 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. "