diff --git a/gateway/platforms/discord.py b/gateway/platforms/discord.py index 0ccac36b61..75ba3d1153 100644 --- a/gateway/platforms/discord.py +++ b/gateway/platforms/discord.py @@ -1695,6 +1695,47 @@ class DiscordAdapter(BasePlatformAdapter): async def slash_btw(interaction: discord.Interaction, question: str): await self._run_simple_slash(interaction, f"/btw {question}") + # Register installed skills as native slash commands (parity with + # Telegram, which uses telegram_menu_commands() in commands.py). + # Discord allows up to 100 application commands globally. + _DISCORD_CMD_LIMIT = 100 + try: + from hermes_cli.commands import discord_skill_commands + + existing_names = {cmd.name for cmd in tree.get_commands()} + remaining_slots = max(0, _DISCORD_CMD_LIMIT - len(existing_names)) + + skill_entries, skipped = discord_skill_commands( + max_slots=remaining_slots, + reserved_names=existing_names, + ) + + for discord_name, description, cmd_key in skill_entries: + # Closure factory to capture cmd_key per iteration + def _make_skill_handler(_key: str): + async def _skill_slash(interaction: discord.Interaction, args: str = ""): + await self._run_simple_slash(interaction, f"{_key} {args}".strip()) + return _skill_slash + + handler = _make_skill_handler(cmd_key) + handler.__name__ = f"skill_{discord_name.replace('-', '_')}" + + cmd = discord.app_commands.Command( + name=discord_name, + description=description, + callback=handler, + ) + discord.app_commands.describe(args="Optional arguments for the skill")(cmd) + tree.add_command(cmd) + + if skipped: + logger.warning( + "[%s] Discord slash command limit reached (%d): %d skill(s) not registered", + self.name, _DISCORD_CMD_LIMIT, skipped, + ) + except Exception as exc: + logger.warning("[%s] Failed to register skill slash commands: %s", self.name, exc) + def _build_slash_event(self, interaction: discord.Interaction, text: str) -> MessageEvent: """Build a MessageEvent from a Discord slash command interaction.""" is_dm = isinstance(interaction.channel, discord.DMChannel) diff --git a/hermes_cli/commands.py b/hermes_cli/commands.py index 07732b50f0..9bce834d04 100644 --- a/hermes_cli/commands.py +++ b/hermes_cli/commands.py @@ -372,7 +372,11 @@ def telegram_bot_commands() -> list[tuple[str, str]]: return result -_TG_NAME_LIMIT = 32 +_CMD_NAME_LIMIT = 32 +"""Max command name length shared by Telegram and Discord.""" + +# Backward-compat alias — tests and external code may reference the old name. +_TG_NAME_LIMIT = _CMD_NAME_LIMIT # Telegram Bot API allows only lowercase a-z, 0-9, and underscores in # command names. This regex strips everything else after initial conversion. @@ -394,13 +398,14 @@ def _sanitize_telegram_name(raw: str) -> str: return name.strip("_") -def _clamp_telegram_names( +def _clamp_command_names( entries: list[tuple[str, str]], reserved: set[str], ) -> list[tuple[str, str]]: - """Enforce Telegram's 32-char command name limit with collision avoidance. + """Enforce 32-char command name limit with collision avoidance. - Names exceeding 32 chars are truncated. If truncation creates a duplicate + Both Telegram and Discord cap slash command names at 32 characters. + Names exceeding the limit are truncated. If truncation creates a duplicate (against *reserved* names or earlier entries in the same batch), the name is shortened to 31 chars and a digit ``0``-``9`` is appended to differentiate. If all 10 digit slots are taken the entry is silently dropped. @@ -408,10 +413,10 @@ def _clamp_telegram_names( used: set[str] = set(reserved) result: list[tuple[str, str]] = [] for name, desc in entries: - if len(name) > _TG_NAME_LIMIT: - candidate = name[:_TG_NAME_LIMIT] + if len(name) > _CMD_NAME_LIMIT: + candidate = name[:_CMD_NAME_LIMIT] if candidate in used: - prefix = name[:_TG_NAME_LIMIT - 1] + prefix = name[:_CMD_NAME_LIMIT - 1] for digit in range(10): candidate = f"{prefix}{digit}" if candidate not in used: @@ -427,6 +432,129 @@ def _clamp_telegram_names( return result +# Backward-compat alias. +_clamp_telegram_names = _clamp_command_names + + +# --------------------------------------------------------------------------- +# Shared skill/plugin collection for gateway platforms +# --------------------------------------------------------------------------- + +def _collect_gateway_skill_entries( + platform: str, + max_slots: int, + reserved_names: set[str], + desc_limit: int = 100, + sanitize_name: "Callable[[str], str] | None" = None, +) -> tuple[list[tuple[str, str, str]], int]: + """Collect plugin + skill entries for a gateway platform. + + Priority order: + 1. Plugin slash commands (take precedence over skills) + 2. Built-in skill commands (fill remaining slots, alphabetical) + + Only skills are trimmed when the cap is reached. + Hub-installed skills are excluded. Per-platform disabled skills are + excluded. + + Args: + platform: Platform identifier for per-platform skill filtering + (``"telegram"``, ``"discord"``, etc.). + max_slots: Maximum number of entries to return (remaining slots after + built-in/core commands). + reserved_names: Names already taken by built-in commands. Mutated + in-place as new names are added. + desc_limit: Max description length (40 for Telegram, 100 for Discord). + sanitize_name: Optional name transform applied before clamping, e.g. + :func:`_sanitize_telegram_name` for Telegram. May return an + empty string to signal "skip this entry". + + Returns: + ``(entries, hidden_count)`` where *entries* is a list of + ``(name, description, cmd_key)`` triples and *hidden_count* is the + number of skill entries dropped due to the cap. ``cmd_key`` is the + original ``/skill-name`` key from :func:`get_skill_commands`. + """ + all_entries: list[tuple[str, str, str]] = [] + + # --- Tier 1: Plugin slash commands (never trimmed) --------------------- + plugin_pairs: list[tuple[str, str]] = [] + try: + from hermes_cli.plugins import get_plugin_manager + pm = get_plugin_manager() + plugin_cmds = getattr(pm, "_plugin_commands", {}) + for cmd_name in sorted(plugin_cmds): + name = sanitize_name(cmd_name) if sanitize_name else cmd_name + if not name: + continue + desc = "Plugin command" + if len(desc) > desc_limit: + desc = desc[:desc_limit - 3] + "..." + plugin_pairs.append((name, desc)) + except Exception: + pass + + plugin_pairs = _clamp_command_names(plugin_pairs, reserved_names) + reserved_names.update(n for n, _ in plugin_pairs) + # Plugins have no cmd_key — use empty string as placeholder + for n, d in plugin_pairs: + all_entries.append((n, d, "")) + + # --- Tier 2: Built-in skill commands (trimmed at cap) ----------------- + _platform_disabled: set[str] = set() + try: + from agent.skill_utils import get_disabled_skill_names + _platform_disabled = get_disabled_skill_names(platform=platform) + except Exception: + pass + + skill_triples: list[tuple[str, str, str]] = [] + try: + from agent.skill_commands import get_skill_commands + from tools.skills_tool import SKILLS_DIR + _skills_dir = str(SKILLS_DIR.resolve()) + _hub_dir = str((SKILLS_DIR / ".hub").resolve()) + skill_cmds = get_skill_commands() + for cmd_key in sorted(skill_cmds): + info = skill_cmds[cmd_key] + skill_path = info.get("skill_md_path", "") + if not skill_path.startswith(_skills_dir): + continue + if skill_path.startswith(_hub_dir): + continue + skill_name = info.get("name", "") + if skill_name in _platform_disabled: + continue + raw_name = cmd_key.lstrip("/") + name = sanitize_name(raw_name) if sanitize_name else raw_name + if not name: + continue + desc = info.get("description", "") + if len(desc) > desc_limit: + desc = desc[:desc_limit - 3] + "..." + skill_triples.append((name, desc, cmd_key)) + except Exception: + pass + + # Clamp names; _clamp_command_names works on (name, desc) pairs so we + # need to zip/unzip. + skill_pairs = [(n, d) for n, d, _ in skill_triples] + key_by_pair = {(n, d): k for n, d, k in skill_triples} + skill_pairs = _clamp_command_names(skill_pairs, reserved_names) + + # Skills fill remaining slots — only tier that gets trimmed + remaining = max(0, max_slots - len(all_entries)) + hidden_count = max(0, len(skill_pairs) - remaining) + for n, d in skill_pairs[:remaining]: + all_entries.append((n, d, key_by_pair.get((n, d), ""))) + + return all_entries[:max_slots], hidden_count + + +# --------------------------------------------------------------------------- +# Platform-specific wrappers +# --------------------------------------------------------------------------- + def telegram_menu_commands(max_commands: int = 100) -> tuple[list[tuple[str, str]], int]: """Return Telegram menu commands capped to the Bot API limit. @@ -445,84 +573,52 @@ def telegram_menu_commands(max_commands: int = 100) -> tuple[list[tuple[str, str skill commands omitted due to the cap. """ core_commands = list(telegram_bot_commands()) - # Reserve core names so plugin/skill truncation can't collide with them reserved_names = {n for n, _ in core_commands} all_commands = list(core_commands) - # Plugin slash commands get priority over skills - plugin_entries: list[tuple[str, str]] = [] - try: - from hermes_cli.plugins import get_plugin_manager - pm = get_plugin_manager() - plugin_cmds = getattr(pm, "_plugin_commands", {}) - for cmd_name in sorted(plugin_cmds): - tg_name = _sanitize_telegram_name(cmd_name) - if not tg_name: - continue - desc = "Plugin command" - if len(desc) > 40: - desc = desc[:37] + "..." - plugin_entries.append((tg_name, desc)) - except Exception: - pass - - # Clamp plugin names to 32 chars with collision avoidance - plugin_entries = _clamp_telegram_names(plugin_entries, reserved_names) - reserved_names.update(n for n, _ in plugin_entries) - all_commands.extend(plugin_entries) - - # Load per-platform disabled skills so they don't consume menu slots. - # get_skill_commands() already filters the *global* disabled list, but - # per-platform overrides (skills.platform_disabled.telegram) were never - # applied here — that's what this block fixes. - _platform_disabled: set[str] = set() - try: - from agent.skill_utils import get_disabled_skill_names - _platform_disabled = get_disabled_skill_names(platform="telegram") - except Exception: - pass - - # Remaining slots go to built-in skill commands (not hub-installed). - skill_entries: list[tuple[str, str]] = [] - try: - from agent.skill_commands import get_skill_commands - from tools.skills_tool import SKILLS_DIR - _skills_dir = str(SKILLS_DIR.resolve()) - _hub_dir = str((SKILLS_DIR / ".hub").resolve()) - skill_cmds = get_skill_commands() - for cmd_key in sorted(skill_cmds): - info = skill_cmds[cmd_key] - skill_path = info.get("skill_md_path", "") - if not skill_path.startswith(_skills_dir): - continue - if skill_path.startswith(_hub_dir): - continue - # Skip skills disabled for telegram - skill_name = info.get("name", "") - if skill_name in _platform_disabled: - continue - name = _sanitize_telegram_name(cmd_key.lstrip("/")) - if not name: - continue - desc = info.get("description", "") - # Keep descriptions short — setMyCommands has an undocumented - # total payload limit. 40 chars fits 100 commands safely. - if len(desc) > 40: - desc = desc[:37] + "..." - skill_entries.append((name, desc)) - except Exception: - pass - - # Clamp skill names to 32 chars with collision avoidance - skill_entries = _clamp_telegram_names(skill_entries, reserved_names) - - # Skills fill remaining slots — they're the only tier that gets trimmed remaining_slots = max(0, max_commands - len(all_commands)) - hidden_count = max(0, len(skill_entries) - remaining_slots) - all_commands.extend(skill_entries[:remaining_slots]) + entries, hidden_count = _collect_gateway_skill_entries( + platform="telegram", + max_slots=remaining_slots, + reserved_names=reserved_names, + desc_limit=40, + sanitize_name=_sanitize_telegram_name, + ) + # Drop the cmd_key — Telegram only needs (name, desc) pairs. + all_commands.extend((n, d) for n, d, _k in entries) return all_commands[:max_commands], hidden_count +def discord_skill_commands( + max_slots: int, + reserved_names: set[str], +) -> tuple[list[tuple[str, str, str]], int]: + """Return skill entries for Discord slash command registration. + + Same priority and filtering logic as :func:`telegram_menu_commands` + (plugins > skills, hub excluded, per-platform disabled excluded), but + adapted for Discord's constraints: + + - Hyphens are allowed in names (no ``-`` → ``_`` sanitization) + - Descriptions capped at 100 chars (Discord's per-field max) + + Args: + max_slots: Available command slots (100 minus existing built-in count). + reserved_names: Names of already-registered built-in commands. + + Returns: + ``(entries, hidden_count)`` where *entries* is a list of + ``(discord_name, description, cmd_key)`` triples. ``cmd_key`` is + the original ``/skill-name`` key needed for the slash handler callback. + """ + return _collect_gateway_skill_entries( + platform="discord", + max_slots=max_slots, + reserved_names=set(reserved_names), # copy — don't mutate caller's set + desc_limit=100, + ) + + def slack_subcommand_map() -> dict[str, str]: """Return subcommand -> /command mapping for Slack /hermes handler. diff --git a/tests/hermes_cli/test_commands.py b/tests/hermes_cli/test_commands.py index 1ff1a18aa3..81c262a840 100644 --- a/tests/hermes_cli/test_commands.py +++ b/tests/hermes_cli/test_commands.py @@ -12,9 +12,12 @@ from hermes_cli.commands import ( SUBCOMMANDS, SlashCommandAutoSuggest, SlashCommandCompleter, + _CMD_NAME_LIMIT, _TG_NAME_LIMIT, + _clamp_command_names, _clamp_telegram_names, _sanitize_telegram_name, + discord_skill_commands, gateway_help_lines, resolve_command, slack_subcommand_map, @@ -751,3 +754,238 @@ class TestTelegramMenuCommands: assert "valid_skill" in menu_names # No empty string in menu names assert "" not in menu_names + + +# --------------------------------------------------------------------------- +# Backward-compat aliases +# --------------------------------------------------------------------------- + +class TestBackwardCompatAliases: + """The renamed constants/functions still exist under the old names.""" + + def test_tg_name_limit_alias(self): + assert _TG_NAME_LIMIT == _CMD_NAME_LIMIT == 32 + + def test_clamp_telegram_names_is_clamp_command_names(self): + assert _clamp_telegram_names is _clamp_command_names + + +# --------------------------------------------------------------------------- +# Discord skill command registration +# --------------------------------------------------------------------------- + +class TestDiscordSkillCommands: + """Tests for discord_skill_commands() — centralized skill registration.""" + + def test_returns_skill_entries(self, tmp_path, monkeypatch): + """Skills under SKILLS_DIR (not .hub) should be returned.""" + from unittest.mock import patch + + fake_skills_dir = str(tmp_path / "skills") + fake_cmds = { + "/gif-search": { + "name": "gif-search", + "description": "Search for GIFs", + "skill_md_path": f"{fake_skills_dir}/gif-search/SKILL.md", + "skill_dir": f"{fake_skills_dir}/gif-search", + }, + "/code-review": { + "name": "code-review", + "description": "Review code changes", + "skill_md_path": f"{fake_skills_dir}/code-review/SKILL.md", + "skill_dir": f"{fake_skills_dir}/code-review", + }, + } + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + (tmp_path / "skills").mkdir(exist_ok=True) + with ( + patch("agent.skill_commands.get_skill_commands", return_value=fake_cmds), + patch("tools.skills_tool.SKILLS_DIR", tmp_path / "skills"), + ): + entries, hidden = discord_skill_commands( + max_slots=50, reserved_names=set(), + ) + + names = {n for n, _d, _k in entries} + assert "gif-search" in names + assert "code-review" in names + assert hidden == 0 + # Verify cmd_key is preserved for handler callbacks + keys = {k for _n, _d, k in entries} + assert "/gif-search" in keys + assert "/code-review" in keys + + def test_names_allow_hyphens(self, tmp_path, monkeypatch): + """Discord names should keep hyphens (unlike Telegram's _ sanitization).""" + from unittest.mock import patch + + fake_skills_dir = str(tmp_path / "skills") + fake_cmds = { + "/my-cool-skill": { + "name": "my-cool-skill", + "description": "A cool skill", + "skill_md_path": f"{fake_skills_dir}/my-cool-skill/SKILL.md", + "skill_dir": f"{fake_skills_dir}/my-cool-skill", + }, + } + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + (tmp_path / "skills").mkdir(exist_ok=True) + with ( + patch("agent.skill_commands.get_skill_commands", return_value=fake_cmds), + patch("tools.skills_tool.SKILLS_DIR", tmp_path / "skills"), + ): + entries, _ = discord_skill_commands( + max_slots=50, reserved_names=set(), + ) + + assert entries[0][0] == "my-cool-skill" # hyphens preserved + + def test_cap_enforcement(self, tmp_path, monkeypatch): + """Entries beyond max_slots should be hidden.""" + from unittest.mock import patch + + fake_skills_dir = str(tmp_path / "skills") + fake_cmds = { + f"/skill-{i:03d}": { + "name": f"skill-{i:03d}", + "description": f"Skill {i}", + "skill_md_path": f"{fake_skills_dir}/skill-{i:03d}/SKILL.md", + "skill_dir": f"{fake_skills_dir}/skill-{i:03d}", + } + for i in range(20) + } + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + (tmp_path / "skills").mkdir(exist_ok=True) + with ( + patch("agent.skill_commands.get_skill_commands", return_value=fake_cmds), + patch("tools.skills_tool.SKILLS_DIR", tmp_path / "skills"), + ): + entries, hidden = discord_skill_commands( + max_slots=5, reserved_names=set(), + ) + + assert len(entries) == 5 + assert hidden == 15 + + def test_excludes_discord_disabled_skills(self, tmp_path, monkeypatch): + """Skills disabled for discord should not appear.""" + from unittest.mock import patch + + config_file = tmp_path / "config.yaml" + config_file.write_text( + "skills:\n" + " platform_disabled:\n" + " discord:\n" + " - secret-skill\n" + ) + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + + fake_skills_dir = str(tmp_path / "skills") + fake_cmds = { + "/secret-skill": { + "name": "secret-skill", + "description": "Should not appear", + "skill_md_path": f"{fake_skills_dir}/secret-skill/SKILL.md", + "skill_dir": f"{fake_skills_dir}/secret-skill", + }, + "/public-skill": { + "name": "public-skill", + "description": "Should appear", + "skill_md_path": f"{fake_skills_dir}/public-skill/SKILL.md", + "skill_dir": f"{fake_skills_dir}/public-skill", + }, + } + (tmp_path / "skills").mkdir(exist_ok=True) + with ( + patch("agent.skill_commands.get_skill_commands", return_value=fake_cmds), + patch("tools.skills_tool.SKILLS_DIR", tmp_path / "skills"), + ): + entries, _ = discord_skill_commands( + max_slots=50, reserved_names=set(), + ) + + names = {n for n, _d, _k in entries} + assert "secret-skill" not in names + assert "public-skill" in names + + def test_reserved_names_not_overwritten(self, tmp_path, monkeypatch): + """Skills whose names collide with built-in commands should be skipped.""" + from unittest.mock import patch + + fake_skills_dir = str(tmp_path / "skills") + fake_cmds = { + "/status": { + "name": "status", + "description": "Skill that collides with built-in", + "skill_md_path": f"{fake_skills_dir}/status/SKILL.md", + "skill_dir": f"{fake_skills_dir}/status", + }, + } + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + (tmp_path / "skills").mkdir(exist_ok=True) + with ( + patch("agent.skill_commands.get_skill_commands", return_value=fake_cmds), + patch("tools.skills_tool.SKILLS_DIR", tmp_path / "skills"), + ): + entries, _ = discord_skill_commands( + max_slots=50, reserved_names={"status"}, + ) + + names = {n for n, _d, _k in entries} + assert "status" not in names + + def test_description_truncated_at_100_chars(self, tmp_path, monkeypatch): + """Descriptions exceeding 100 chars should be truncated.""" + from unittest.mock import patch + + fake_skills_dir = str(tmp_path / "skills") + long_desc = "x" * 150 + fake_cmds = { + "/verbose-skill": { + "name": "verbose-skill", + "description": long_desc, + "skill_md_path": f"{fake_skills_dir}/verbose-skill/SKILL.md", + "skill_dir": f"{fake_skills_dir}/verbose-skill", + }, + } + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + (tmp_path / "skills").mkdir(exist_ok=True) + with ( + patch("agent.skill_commands.get_skill_commands", return_value=fake_cmds), + patch("tools.skills_tool.SKILLS_DIR", tmp_path / "skills"), + ): + entries, _ = discord_skill_commands( + max_slots=50, reserved_names=set(), + ) + + assert len(entries[0][1]) == 100 + assert entries[0][1].endswith("...") + + def test_all_names_within_32_chars(self, tmp_path, monkeypatch): + """All returned names must respect the 32-char Discord limit.""" + from unittest.mock import patch + + fake_skills_dir = str(tmp_path / "skills") + long_name = "a" * 50 + fake_cmds = { + f"/{long_name}": { + "name": long_name, + "description": "Long name skill", + "skill_md_path": f"{fake_skills_dir}/{long_name}/SKILL.md", + "skill_dir": f"{fake_skills_dir}/{long_name}", + }, + } + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + (tmp_path / "skills").mkdir(exist_ok=True) + with ( + patch("agent.skill_commands.get_skill_commands", return_value=fake_cmds), + patch("tools.skills_tool.SKILLS_DIR", tmp_path / "skills"), + ): + entries, _ = discord_skill_commands( + max_slots=50, reserved_names=set(), + ) + + for name, _d, _k in entries: + assert len(name) <= _CMD_NAME_LIMIT, ( + f"Name '{name}' is {len(name)} chars (limit {_CMD_NAME_LIMIT})" + )