diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 58f8745959..648267302e 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -833,7 +833,7 @@ DEFAULT_CONFIG = { "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 7546d3039e..45c6ba3dd9 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -67,12 +67,13 @@ CONFIGURABLE_TOOLSETS = [ ("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 2b7767fda3..9b6e135475 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 b4ea337d1d..4ac32d007c 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 183f9e514f..4e4289999c 100644 --- a/tests/test_toolsets.py +++ b/tests/test_toolsets.py @@ -200,8 +200,8 @@ class TestToolsetConsistency: 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 34fe672132..70b43903ec 100644 --- a/tests/tools/test_discord_tool.py +++ b/tests/tools/test_discord_tool.py @@ -11,6 +11,8 @@ import pytest from tools.discord_tool import ( DiscordAPIError, _ACTIONS, + _ADMIN_ACTIONS, + _CORE_ACTIONS, _available_actions, _build_schema, _channel_type_name, @@ -21,8 +23,11 @@ from tools.discord_tool import ( _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 @@ class TestDiscordRequest: 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 @@ class TestListGuilds: {"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 @@ class TestServerInfo: "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 @@ class TestListChannels: {"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 @@ class TestListChannels: 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 @@ class TestChannelInfo: "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 @@ class TestListRoles: {"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 @@ class TestMemberInfo: "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 @@ class TestSearchMembers: 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 @@ class TestSearchMembers: 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 @@ class TestFetchMessages: "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 @@ class TestFetchMessages: 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 @@ class TestListPins: 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 @@ class TestPinUnpin: 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 @@ class TestCreateThread: 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 @@ class TestRoleManagement: 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 @@ class TestErrorHandling: # --------------------------------------------------------------------------- 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": + if name in ("hermes-discord", "hermes-gateway", "discord", "discord_admin"): continue - # The gateway toolset might include it if it unions all platform tools - if name == "hermes-gateway": - 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,40 +841,69 @@ class TestDynamicSchema: @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 "member_info" not in actions + 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 - 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_message_content_adds_warning_note(self, mock_req, monkeypatch): @@ -841,41 +913,53 @@ class TestDynamicSchema: 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 @@ class TestRuntimeAllowlistEnforcement: "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 @@ class TestRuntimeAllowlistEnforcement: 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 @@ class Test403Enrichment: 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 @@ class Test403Enrichment: 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 @@ class TestModelToolsIntegration: _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 @@ class TestModelToolsIntegration: 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 @@ class TestModelToolsIntegration: 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 1bdbbd4368..dff0c67669 100644 --- a/tools/discord_tool.py +++ b/tools/discord_tool.py @@ -473,6 +473,12 @@ _ACTIONS = { "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 b3cdb2e7ae..a9e1193580 100644 --- a/toolsets.py +++ b/toolsets.py @@ -202,6 +202,18 @@ TOOLSETS = { "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 @@ TOOLSETS = { "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": [] },