diff --git a/agent/skill_commands.py b/agent/skill_commands.py index d40572d55b..e12945a9c5 100644 --- a/agent/skill_commands.py +++ b/agent/skill_commands.py @@ -16,6 +16,9 @@ logger = logging.getLogger(__name__) _skill_commands: Dict[str, Dict[str, Any]] = {} _PLAN_SLUG_RE = re.compile(r"[^a-z0-9]+") +# Patterns for sanitizing skill names into clean hyphen-separated slugs. +_SKILL_INVALID_CHARS = re.compile(r"[^a-z0-9-]") +_SKILL_MULTI_HYPHEN = re.compile(r"-{2,}") def build_plan_path( @@ -196,7 +199,14 @@ def scan_skill_commands() -> Dict[str, Dict[str, Any]]: description = line[:80] break seen_names.add(name) + # Normalize to hyphen-separated slug, stripping + # non-alnum chars (e.g. +, /) to avoid invalid + # Telegram command names downstream. cmd_name = name.lower().replace(' ', '-').replace('_', '-') + cmd_name = _SKILL_INVALID_CHARS.sub('', cmd_name) + cmd_name = _SKILL_MULTI_HYPHEN.sub('-', cmd_name).strip('-') + if not cmd_name: + continue _skill_commands[f"/{cmd_name}"] = { "name": name, "description": description or f"Invoke the {name} skill", diff --git a/hermes_cli/commands.py b/hermes_cli/commands.py index e0c769d198..07732b50f0 100644 --- a/hermes_cli/commands.py +++ b/hermes_cli/commands.py @@ -366,13 +366,33 @@ def telegram_bot_commands() -> list[tuple[str, str]]: for cmd in COMMAND_REGISTRY: if not _is_gateway_available(cmd, overrides): continue - tg_name = cmd.name.replace("-", "_") - result.append((tg_name, cmd.description)) + tg_name = _sanitize_telegram_name(cmd.name) + if tg_name: + result.append((tg_name, cmd.description)) return result _TG_NAME_LIMIT = 32 +# Telegram Bot API allows only lowercase a-z, 0-9, and underscores in +# command names. This regex strips everything else after initial conversion. +_TG_INVALID_CHARS = re.compile(r"[^a-z0-9_]") +_TG_MULTI_UNDERSCORE = re.compile(r"_{2,}") + + +def _sanitize_telegram_name(raw: str) -> str: + """Convert a command/skill/plugin name to a valid Telegram command name. + + Telegram requires: 1-32 chars, lowercase a-z, digits 0-9, underscores only. + Steps: lowercase → replace hyphens with underscores → strip all other + invalid characters → collapse consecutive underscores → strip leading/ + trailing underscores. + """ + name = raw.lower().replace("-", "_") + name = _TG_INVALID_CHARS.sub("", name) + name = _TG_MULTI_UNDERSCORE.sub("_", name) + return name.strip("_") + def _clamp_telegram_names( entries: list[tuple[str, str]], @@ -436,7 +456,9 @@ def telegram_menu_commands(max_commands: int = 100) -> tuple[list[tuple[str, str pm = get_plugin_manager() plugin_cmds = getattr(pm, "_plugin_commands", {}) for cmd_name in sorted(plugin_cmds): - tg_name = cmd_name.replace("-", "_") + tg_name = _sanitize_telegram_name(cmd_name) + if not tg_name: + continue desc = "Plugin command" if len(desc) > 40: desc = desc[:37] + "..." @@ -479,7 +501,9 @@ def telegram_menu_commands(max_commands: int = 100) -> tuple[list[tuple[str, str skill_name = info.get("name", "") if skill_name in _platform_disabled: continue - name = cmd_key.lstrip("/").replace("-", "_") + 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. diff --git a/tests/agent/test_skill_commands.py b/tests/agent/test_skill_commands.py index cda4d89eb6..57ac7d6b58 100644 --- a/tests/agent/test_skill_commands.py +++ b/tests/agent/test_skill_commands.py @@ -102,6 +102,49 @@ class TestScanSkillCommands: assert "/disabled-skill" not in result + def test_special_chars_stripped_from_cmd_key(self, tmp_path): + """Skill names with +, /, or other special chars produce clean cmd keys.""" + with patch("tools.skills_tool.SKILLS_DIR", tmp_path): + # Simulate a skill named "Jellyfin + Jellystat 24h Summary" + skill_dir = tmp_path / "jellyfin-plus" + skill_dir.mkdir() + (skill_dir / "SKILL.md").write_text( + "---\nname: Jellyfin + Jellystat 24h Summary\n" + "description: Test skill\n---\n\nBody.\n" + ) + result = scan_skill_commands() + # The + should be stripped, not left as a literal character + assert "/jellyfin-jellystat-24h-summary" in result + # The old buggy key should NOT exist + assert "/jellyfin-+-jellystat-24h-summary" not in result + + def test_allspecial_name_skipped(self, tmp_path): + """Skill with name consisting only of special chars is silently skipped.""" + with patch("tools.skills_tool.SKILLS_DIR", tmp_path): + skill_dir = tmp_path / "bad-name" + skill_dir.mkdir() + (skill_dir / "SKILL.md").write_text( + "---\nname: +++\ndescription: Bad skill\n---\n\nBody.\n" + ) + result = scan_skill_commands() + # Should not create a "/" key or any entry + assert "/" not in result + assert result == {} + + def test_slash_in_name_stripped_from_cmd_key(self, tmp_path): + """Skill names with / chars produce clean cmd keys.""" + with patch("tools.skills_tool.SKILLS_DIR", tmp_path): + skill_dir = tmp_path / "sonarr-api" + skill_dir.mkdir() + (skill_dir / "SKILL.md").write_text( + "---\nname: Sonarr v3/v4 API\n" + "description: Test skill\n---\n\nBody.\n" + ) + result = scan_skill_commands() + assert "/sonarr-v3v4-api" in result + assert any("/" in k[1:] for k in result) is False # no unescaped / + + class TestResolveSkillCommandKey: """Telegram bot-command names disallow hyphens, so the menu registers skills with hyphens swapped for underscores. When Telegram autocomplete diff --git a/tests/hermes_cli/test_commands.py b/tests/hermes_cli/test_commands.py index 7cda509c4d..1ff1a18aa3 100644 --- a/tests/hermes_cli/test_commands.py +++ b/tests/hermes_cli/test_commands.py @@ -14,6 +14,7 @@ from hermes_cli.commands import ( SlashCommandCompleter, _TG_NAME_LIMIT, _clamp_telegram_names, + _sanitize_telegram_name, gateway_help_lines, resolve_command, slack_subcommand_map, @@ -198,6 +199,13 @@ class TestTelegramBotCommands: for name, _ in telegram_bot_commands(): assert "-" not in name, f"Telegram command '{name}' contains a hyphen" + def test_all_names_valid_telegram_chars(self): + """Telegram requires: lowercase a-z, 0-9, underscores only.""" + import re + tg_valid = re.compile(r"^[a-z0-9_]+$") + for name, _ in telegram_bot_commands(): + assert tg_valid.match(name), f"Invalid Telegram command name: {name!r}" + def test_excludes_cli_only_without_config_gate(self): names = {name for name, _ in telegram_bot_commands()} for cmd in COMMAND_REGISTRY: @@ -509,6 +517,53 @@ class TestGhostText: assert _suggestion("hello") is None +# --------------------------------------------------------------------------- +# Telegram command name sanitization +# --------------------------------------------------------------------------- + + +class TestSanitizeTelegramName: + """Tests for _sanitize_telegram_name() — Telegram requires [a-z0-9_] only.""" + + def test_hyphens_replaced_with_underscores(self): + assert _sanitize_telegram_name("my-skill-name") == "my_skill_name" + + def test_plus_sign_stripped(self): + """Regression: skill name 'Jellyfin + Jellystat 24h Summary'.""" + assert _sanitize_telegram_name("jellyfin-+-jellystat-24h-summary") == "jellyfin_jellystat_24h_summary" + + def test_slash_stripped(self): + """Regression: skill name 'Sonarr v3/v4 API Integration'.""" + assert _sanitize_telegram_name("sonarr-v3/v4-api-integration") == "sonarr_v3v4_api_integration" + + def test_uppercase_lowercased(self): + assert _sanitize_telegram_name("MyCommand") == "mycommand" + + def test_dots_and_special_chars_stripped(self): + assert _sanitize_telegram_name("skill.v2@beta!") == "skillv2beta" + + def test_consecutive_underscores_collapsed(self): + assert _sanitize_telegram_name("a---b") == "a_b" + assert _sanitize_telegram_name("a-+-b") == "a_b" + + def test_leading_trailing_underscores_stripped(self): + assert _sanitize_telegram_name("-leading") == "leading" + assert _sanitize_telegram_name("trailing-") == "trailing" + assert _sanitize_telegram_name("-both-") == "both" + + def test_digits_preserved(self): + assert _sanitize_telegram_name("skill-24h") == "skill_24h" + + def test_empty_after_sanitization(self): + assert _sanitize_telegram_name("+++") == "" + + def test_spaces_only_becomes_empty(self): + assert _sanitize_telegram_name(" ") == "" + + def test_already_valid(self): + assert _sanitize_telegram_name("valid_name_123") == "valid_name_123" + + # --------------------------------------------------------------------------- # Telegram command name clamping (32-char limit) # --------------------------------------------------------------------------- @@ -628,3 +683,71 @@ class TestTelegramMenuCommands: menu_names = {n for n, _ in menu} assert "my_enabled_skill" in menu_names assert "my_disabled_skill" not in menu_names + + def test_special_chars_in_skill_names_sanitized(self, tmp_path, monkeypatch): + """Skills with +, /, or other special chars produce valid Telegram names.""" + from unittest.mock import patch + import re + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + + fake_skills_dir = str(tmp_path / "skills") + fake_cmds = { + "/jellyfin-+-jellystat-24h-summary": { + "name": "Jellyfin + Jellystat 24h Summary", + "description": "Test", + "skill_md_path": f"{fake_skills_dir}/jellyfin/SKILL.md", + "skill_dir": f"{fake_skills_dir}/jellyfin", + }, + "/sonarr-v3/v4-api": { + "name": "Sonarr v3/v4 API", + "description": "Test", + "skill_md_path": f"{fake_skills_dir}/sonarr/SKILL.md", + "skill_dir": f"{fake_skills_dir}/sonarr", + }, + } + with ( + patch("agent.skill_commands.get_skill_commands", return_value=fake_cmds), + patch("tools.skills_tool.SKILLS_DIR", tmp_path / "skills"), + ): + (tmp_path / "skills").mkdir(exist_ok=True) + menu, _ = telegram_menu_commands(max_commands=100) + + # Every name must match Telegram's [a-z0-9_] requirement + tg_valid = re.compile(r"^[a-z0-9_]+$") + for name, _ in menu: + assert tg_valid.match(name), f"Invalid Telegram command name: {name!r}" + + def test_empty_sanitized_names_excluded(self, tmp_path, monkeypatch): + """Skills whose names sanitize to empty string are silently dropped.""" + from unittest.mock import patch + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + + fake_skills_dir = str(tmp_path / "skills") + fake_cmds = { + "/+++": { + "name": "+++", + "description": "All special chars", + "skill_md_path": f"{fake_skills_dir}/bad/SKILL.md", + "skill_dir": f"{fake_skills_dir}/bad", + }, + "/valid-skill": { + "name": "valid-skill", + "description": "Normal skill", + "skill_md_path": f"{fake_skills_dir}/valid/SKILL.md", + "skill_dir": f"{fake_skills_dir}/valid", + }, + } + with ( + patch("agent.skill_commands.get_skill_commands", return_value=fake_cmds), + patch("tools.skills_tool.SKILLS_DIR", tmp_path / "skills"), + ): + (tmp_path / "skills").mkdir(exist_ok=True) + menu, _ = telegram_menu_commands(max_commands=100) + + menu_names = {n for n, _ in menu} + # The valid skill should be present, the empty one should not + assert "valid_skill" in menu_names + # No empty string in menu names + assert "" not in menu_names