feat(discord): register skills under /skill command group with category subcommands (#9909)
Instead of consuming one top-level slash command slot per skill (hitting the 100-command limit with ~26 built-ins + 74 skills), skills are now organized under a single /skill group command with category-based subcommand groups: /skill creative ascii-art [args] /skill media gif-search [args] /skill mlops axolotl [args] Discord supports 25 subcommand groups × 25 subcommands = 625 max skills, well beyond the previous 74-slot ceiling. Categories are derived from the skill directory structure: - skills/creative/ascii-art/ → category 'creative' - skills/mlops/training/axolotl/ → category 'mlops' (top-level parent) - skills/dogfood/ → uncategorized (direct subcommand) Changes: - hermes_cli/commands.py: add discord_skill_commands_by_category() with category grouping, hub/disabled filtering, Discord limit enforcement - gateway/platforms/discord.py: replace top-level skill registration with _register_skill_group() using app_commands.Group hierarchy - tests: 7 new tests covering group creation, category grouping, uncategorized skills, hub exclusion, deep nesting, empty skills, and handler dispatch Inspired by Discord community suggestion from bottium.
This commit is contained in:
Teknium
@@ -19,10 +19,34 @@ def _ensure_discord_mock():
|
||||
discord_mod.Thread = type("Thread", (), {})
|
||||
discord_mod.ForumChannel = type("ForumChannel", (), {})
|
||||
discord_mod.Interaction = object
|
||||
|
||||
# Lightweight mock for app_commands.Group and Command used by
|
||||
# _register_skill_group.
|
||||
class _FakeGroup:
|
||||
def __init__(self, *, name, description, parent=None):
|
||||
self.name = name
|
||||
self.description = description
|
||||
self.parent = parent
|
||||
self._children: dict[str, object] = {}
|
||||
if parent is not None:
|
||||
parent.add_command(self)
|
||||
|
||||
def add_command(self, cmd):
|
||||
self._children[cmd.name] = cmd
|
||||
|
||||
class _FakeCommand:
|
||||
def __init__(self, *, name, description, callback, parent=None):
|
||||
self.name = name
|
||||
self.description = description
|
||||
self.callback = callback
|
||||
self.parent = parent
|
||||
|
||||
discord_mod.app_commands = SimpleNamespace(
|
||||
describe=lambda **kwargs: (lambda fn: fn),
|
||||
choices=lambda **kwargs: (lambda fn: fn),
|
||||
Choice=lambda **kwargs: SimpleNamespace(**kwargs),
|
||||
Group=_FakeGroup,
|
||||
Command=_FakeCommand,
|
||||
)
|
||||
|
||||
ext_mod = MagicMock()
|
||||
@@ -51,6 +75,12 @@ class FakeTree:
|
||||
|
||||
return decorator
|
||||
|
||||
def add_command(self, cmd):
|
||||
self.commands[cmd.name] = cmd
|
||||
|
||||
def get_commands(self):
|
||||
return [SimpleNamespace(name=n) for n in self.commands]
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def adapter():
|
||||
@@ -498,3 +528,79 @@ def test_discord_auto_thread_config_bridge(monkeypatch, tmp_path):
|
||||
|
||||
import os
|
||||
assert os.getenv("DISCORD_AUTO_THREAD") == "true"
|
||||
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# /skill group registration
|
||||
# ------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_register_skill_group_creates_group(adapter):
|
||||
"""_register_skill_group should register a '/skill' Group on the tree."""
|
||||
mock_categories = {
|
||||
"creative": [
|
||||
("ascii-art", "Generate ASCII art", "/ascii-art"),
|
||||
("excalidraw", "Hand-drawn diagrams", "/excalidraw"),
|
||||
],
|
||||
"media": [
|
||||
("gif-search", "Search for GIFs", "/gif-search"),
|
||||
],
|
||||
}
|
||||
mock_uncategorized = [
|
||||
("dogfood", "Exploratory QA testing", "/dogfood"),
|
||||
]
|
||||
|
||||
with patch(
|
||||
"hermes_cli.commands.discord_skill_commands_by_category",
|
||||
return_value=(mock_categories, mock_uncategorized, 0),
|
||||
):
|
||||
adapter._register_slash_commands()
|
||||
|
||||
tree = adapter._client.tree
|
||||
assert "skill" in tree.commands, "Expected /skill group to be registered"
|
||||
skill_group = tree.commands["skill"]
|
||||
assert skill_group.name == "skill"
|
||||
# Should have 2 category subgroups + 1 uncategorized subcommand
|
||||
children = skill_group._children
|
||||
assert "creative" in children
|
||||
assert "media" in children
|
||||
assert "dogfood" in children
|
||||
# Category groups should have their skills
|
||||
assert "ascii-art" in children["creative"]._children
|
||||
assert "excalidraw" in children["creative"]._children
|
||||
assert "gif-search" in children["media"]._children
|
||||
|
||||
|
||||
def test_register_skill_group_empty_skills_no_group(adapter):
|
||||
"""No /skill group should be added when there are zero skills."""
|
||||
with patch(
|
||||
"hermes_cli.commands.discord_skill_commands_by_category",
|
||||
return_value=({}, [], 0),
|
||||
):
|
||||
adapter._register_slash_commands()
|
||||
|
||||
tree = adapter._client.tree
|
||||
assert "skill" not in tree.commands
|
||||
|
||||
|
||||
def test_register_skill_group_handler_dispatches_command(adapter):
|
||||
"""Skill subcommand handlers should dispatch the correct /cmd-key text."""
|
||||
mock_categories = {
|
||||
"media": [
|
||||
("gif-search", "Search for GIFs", "/gif-search"),
|
||||
],
|
||||
}
|
||||
|
||||
with patch(
|
||||
"hermes_cli.commands.discord_skill_commands_by_category",
|
||||
return_value=(mock_categories, [], 0),
|
||||
):
|
||||
adapter._register_slash_commands()
|
||||
|
||||
skill_group = adapter._client.tree.commands["skill"]
|
||||
media_group = skill_group._children["media"]
|
||||
gif_cmd = media_group._children["gif-search"]
|
||||
assert gif_cmd.callback is not None
|
||||
# The callback name should reflect the skill
|
||||
assert "gif_search" in gif_cmd.callback.__name__
|
||||
|
||||
|
||||
@@ -1028,3 +1028,154 @@ class TestDiscordSkillCommands:
|
||||
assert len(name) <= _CMD_NAME_LIMIT, (
|
||||
f"Name '{name}' is {len(name)} chars (limit {_CMD_NAME_LIMIT})"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Discord skill commands grouped by category
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
from hermes_cli.commands import discord_skill_commands_by_category # noqa: E402
|
||||
|
||||
|
||||
class TestDiscordSkillCommandsByCategory:
|
||||
"""Tests for discord_skill_commands_by_category() — /skill group registration."""
|
||||
|
||||
def test_groups_skills_by_category(self, tmp_path, monkeypatch):
|
||||
"""Skills nested 2+ levels deep should be grouped by top-level category."""
|
||||
from unittest.mock import patch
|
||||
|
||||
fake_skills_dir = str(tmp_path / "skills")
|
||||
# Create the directory structure so resolve() works
|
||||
for p in [
|
||||
"skills/creative/ascii-art",
|
||||
"skills/creative/excalidraw",
|
||||
"skills/media/gif-search",
|
||||
]:
|
||||
(tmp_path / p).mkdir(parents=True, exist_ok=True)
|
||||
(tmp_path / p / "SKILL.md").write_text("---\nname: test\n---\n")
|
||||
|
||||
fake_cmds = {
|
||||
"/ascii-art": {
|
||||
"name": "ascii-art",
|
||||
"description": "Generate ASCII art",
|
||||
"skill_md_path": f"{fake_skills_dir}/creative/ascii-art/SKILL.md",
|
||||
},
|
||||
"/excalidraw": {
|
||||
"name": "excalidraw",
|
||||
"description": "Hand-drawn diagrams",
|
||||
"skill_md_path": f"{fake_skills_dir}/creative/excalidraw/SKILL.md",
|
||||
},
|
||||
"/gif-search": {
|
||||
"name": "gif-search",
|
||||
"description": "Search for GIFs",
|
||||
"skill_md_path": f"{fake_skills_dir}/media/gif-search/SKILL.md",
|
||||
},
|
||||
}
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
with (
|
||||
patch("agent.skill_commands.get_skill_commands", return_value=fake_cmds),
|
||||
patch("tools.skills_tool.SKILLS_DIR", tmp_path / "skills"),
|
||||
):
|
||||
categories, uncategorized, hidden = discord_skill_commands_by_category(
|
||||
reserved_names=set(),
|
||||
)
|
||||
|
||||
assert "creative" in categories
|
||||
assert "media" in categories
|
||||
assert len(categories["creative"]) == 2
|
||||
assert len(categories["media"]) == 1
|
||||
assert uncategorized == []
|
||||
assert hidden == 0
|
||||
|
||||
def test_root_level_skills_are_uncategorized(self, tmp_path, monkeypatch):
|
||||
"""Skills directly under SKILLS_DIR (only 1 path component) → uncategorized."""
|
||||
from unittest.mock import patch
|
||||
|
||||
fake_skills_dir = str(tmp_path / "skills")
|
||||
(tmp_path / "skills" / "dogfood").mkdir(parents=True, exist_ok=True)
|
||||
(tmp_path / "skills" / "dogfood" / "SKILL.md").write_text("")
|
||||
|
||||
fake_cmds = {
|
||||
"/dogfood": {
|
||||
"name": "dogfood",
|
||||
"description": "QA testing",
|
||||
"skill_md_path": f"{fake_skills_dir}/dogfood/SKILL.md",
|
||||
},
|
||||
}
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
with (
|
||||
patch("agent.skill_commands.get_skill_commands", return_value=fake_cmds),
|
||||
patch("tools.skills_tool.SKILLS_DIR", tmp_path / "skills"),
|
||||
):
|
||||
categories, uncategorized, hidden = discord_skill_commands_by_category(
|
||||
reserved_names=set(),
|
||||
)
|
||||
|
||||
assert categories == {}
|
||||
assert len(uncategorized) == 1
|
||||
assert uncategorized[0][0] == "dogfood"
|
||||
|
||||
def test_hub_skills_excluded(self, tmp_path, monkeypatch):
|
||||
"""Skills under .hub should be excluded."""
|
||||
from unittest.mock import patch
|
||||
|
||||
fake_skills_dir = str(tmp_path / "skills")
|
||||
(tmp_path / "skills" / ".hub" / "some-skill").mkdir(parents=True, exist_ok=True)
|
||||
(tmp_path / "skills" / ".hub" / "some-skill" / "SKILL.md").write_text("")
|
||||
|
||||
fake_cmds = {
|
||||
"/some-skill": {
|
||||
"name": "some-skill",
|
||||
"description": "Hub skill",
|
||||
"skill_md_path": f"{fake_skills_dir}/.hub/some-skill/SKILL.md",
|
||||
},
|
||||
}
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
with (
|
||||
patch("agent.skill_commands.get_skill_commands", return_value=fake_cmds),
|
||||
patch("tools.skills_tool.SKILLS_DIR", tmp_path / "skills"),
|
||||
):
|
||||
categories, uncategorized, hidden = discord_skill_commands_by_category(
|
||||
reserved_names=set(),
|
||||
)
|
||||
|
||||
assert categories == {}
|
||||
assert uncategorized == []
|
||||
|
||||
def test_deep_nested_skills_use_top_category(self, tmp_path, monkeypatch):
|
||||
"""Skills like mlops/training/axolotl should group under 'mlops'."""
|
||||
from unittest.mock import patch
|
||||
|
||||
fake_skills_dir = str(tmp_path / "skills")
|
||||
(tmp_path / "skills" / "mlops" / "training" / "axolotl").mkdir(parents=True, exist_ok=True)
|
||||
(tmp_path / "skills" / "mlops" / "training" / "axolotl" / "SKILL.md").write_text("")
|
||||
(tmp_path / "skills" / "mlops" / "inference" / "vllm").mkdir(parents=True, exist_ok=True)
|
||||
(tmp_path / "skills" / "mlops" / "inference" / "vllm" / "SKILL.md").write_text("")
|
||||
|
||||
fake_cmds = {
|
||||
"/axolotl": {
|
||||
"name": "axolotl",
|
||||
"description": "Fine-tuning with Axolotl",
|
||||
"skill_md_path": f"{fake_skills_dir}/mlops/training/axolotl/SKILL.md",
|
||||
},
|
||||
"/vllm": {
|
||||
"name": "vllm",
|
||||
"description": "vLLM inference",
|
||||
"skill_md_path": f"{fake_skills_dir}/mlops/inference/vllm/SKILL.md",
|
||||
},
|
||||
}
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
with (
|
||||
patch("agent.skill_commands.get_skill_commands", return_value=fake_cmds),
|
||||
patch("tools.skills_tool.SKILLS_DIR", tmp_path / "skills"),
|
||||
):
|
||||
categories, uncategorized, hidden = discord_skill_commands_by_category(
|
||||
reserved_names=set(),
|
||||
)
|
||||
|
||||
# Both should be under 'mlops' regardless of sub-category
|
||||
assert "mlops" in categories
|
||||
names = {n for n, _d, _k in categories["mlops"]}
|
||||
assert "axolotl" in names
|
||||
assert "vllm" in names
|
||||
assert len(uncategorized) == 0
|
||||
|
||||
Reference in New Issue
Block a user