fix: respect per-platform disabled skills in Telegram menu and gateway dispatch (#4799)
Three interconnected bugs caused `hermes skills config` per-platform settings to be silently ignored: 1. telegram_menu_commands() never filtered disabled skills — all skills consumed menu slots regardless of platform config, hitting Telegram's 100 command cap. Now loads disabled skills for 'telegram' and excludes them from the menu. 2. Gateway skill dispatch executed disabled skills because get_skill_commands() (process-global cache) only filters by the global disabled list at scan time. Added per-platform check before execution, returning an actionable 'skill is disabled' message. 3. get_disabled_skill_names() only checked HERMES_PLATFORM env var, but the gateway sets HERMES_SESSION_PLATFORM instead. Added HERMES_SESSION_PLATFORM as fallback, plus an explicit platform= parameter for callers that know their platform (menu builder, gateway dispatch). Also added platform to prompt_builder's skills cache key so multi-platform gateways get correct per-platform skill prompts. Reported by SteveSkedasticity (CLAW community).
This commit is contained in:
@@ -488,11 +488,19 @@ def build_skills_system_prompt(
|
||||
return ""
|
||||
|
||||
# ── Layer 1: in-process LRU cache ─────────────────────────────────
|
||||
# Include the resolved platform so per-platform disabled-skill lists
|
||||
# produce distinct cache entries (gateway serves multiple platforms).
|
||||
_platform_hint = (
|
||||
os.environ.get("HERMES_PLATFORM")
|
||||
or os.environ.get("HERMES_SESSION_PLATFORM")
|
||||
or ""
|
||||
)
|
||||
cache_key = (
|
||||
str(skills_dir.resolve()),
|
||||
tuple(str(d) for d in external_dirs),
|
||||
tuple(sorted(str(t) for t in (available_tools or set()))),
|
||||
tuple(sorted(str(ts) for ts in (available_toolsets or set()))),
|
||||
_platform_hint,
|
||||
)
|
||||
with _SKILLS_PROMPT_CACHE_LOCK:
|
||||
cached = _SKILLS_PROMPT_CACHE.get(cache_key)
|
||||
|
||||
@@ -118,12 +118,17 @@ def skill_matches_platform(frontmatter: Dict[str, Any]) -> bool:
|
||||
# ── Disabled skills ───────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def get_disabled_skill_names() -> Set[str]:
|
||||
def get_disabled_skill_names(platform: str | None = None) -> Set[str]:
|
||||
"""Read disabled skill names from config.yaml.
|
||||
|
||||
Resolves platform from ``HERMES_PLATFORM`` env var, falls back to
|
||||
the global disabled list. Reads the config file directly (no CLI
|
||||
config imports) to stay lightweight.
|
||||
Args:
|
||||
platform: Explicit platform name (e.g. ``"telegram"``). When
|
||||
*None*, resolves from ``HERMES_PLATFORM`` or
|
||||
``HERMES_SESSION_PLATFORM`` env vars. Falls back to the
|
||||
global disabled list when no platform is determined.
|
||||
|
||||
Reads the config file directly (no CLI config imports) to stay
|
||||
lightweight.
|
||||
"""
|
||||
config_path = get_hermes_home() / "config.yaml"
|
||||
if not config_path.exists():
|
||||
@@ -140,7 +145,11 @@ def get_disabled_skill_names() -> Set[str]:
|
||||
if not isinstance(skills_cfg, dict):
|
||||
return set()
|
||||
|
||||
resolved_platform = os.getenv("HERMES_PLATFORM")
|
||||
resolved_platform = (
|
||||
platform
|
||||
or os.getenv("HERMES_PLATFORM")
|
||||
or os.getenv("HERMES_SESSION_PLATFORM")
|
||||
)
|
||||
if resolved_platform:
|
||||
platform_disabled = (skills_cfg.get("platform_disabled") or {}).get(
|
||||
resolved_platform
|
||||
|
||||
@@ -2060,6 +2060,19 @@ class GatewayRunner:
|
||||
skill_cmds = get_skill_commands()
|
||||
cmd_key = f"/{command}"
|
||||
if cmd_key in skill_cmds:
|
||||
# Check per-platform disabled status before executing.
|
||||
# get_skill_commands() only applies the *global* disabled
|
||||
# list at scan time; per-platform overrides need checking
|
||||
# here because the cache is process-global across platforms.
|
||||
_skill_name = skill_cmds[cmd_key].get("name", "")
|
||||
_plat = source.platform.value if source.platform else None
|
||||
if _plat and _skill_name:
|
||||
from agent.skill_utils import get_disabled_skill_names as _get_plat_disabled
|
||||
if _skill_name in _get_plat_disabled(platform=_plat):
|
||||
return (
|
||||
f"The **{_skill_name}** skill is disabled for {_plat}.\n"
|
||||
f"Enable it with: `hermes skills config`"
|
||||
)
|
||||
user_instruction = event.get_command_args().strip()
|
||||
msg = build_skill_invocation_message(
|
||||
cmd_key, user_instruction, task_id=_quick_key
|
||||
|
||||
@@ -414,6 +414,8 @@ def telegram_menu_commands(max_commands: int = 100) -> tuple[list[tuple[str, str
|
||||
|
||||
Skills are the only tier that gets trimmed when the cap is hit.
|
||||
User-installed hub skills are excluded — accessible via /skills.
|
||||
Skills disabled for the ``"telegram"`` platform (via ``hermes skills
|
||||
config``) are excluded from the menu entirely.
|
||||
|
||||
Returns:
|
||||
(menu_commands, hidden_count) where hidden_count is the number of
|
||||
@@ -444,6 +446,17 @@ def telegram_menu_commands(max_commands: int = 100) -> tuple[list[tuple[str, str
|
||||
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:
|
||||
@@ -459,6 +472,10 @@ def telegram_menu_commands(max_commands: int = 100) -> tuple[list[tuple[str, str
|
||||
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 = cmd_key.lstrip("/").replace("-", "_")
|
||||
desc = info.get("description", "")
|
||||
# Keep descriptions short — setMyCommands has an undocumented
|
||||
|
||||
@@ -587,3 +587,44 @@ class TestTelegramMenuCommands:
|
||||
assert 1 <= len(name) <= _TG_NAME_LIMIT, (
|
||||
f"Command '{name}' is {len(name)} chars (limit {_TG_NAME_LIMIT})"
|
||||
)
|
||||
|
||||
def test_excludes_telegram_disabled_skills(self, tmp_path, monkeypatch):
|
||||
"""Skills disabled for telegram should not appear in the menu."""
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
# Set up a config with a telegram-specific disabled list
|
||||
config_file = tmp_path / "config.yaml"
|
||||
config_file.write_text(
|
||||
"skills:\n"
|
||||
" platform_disabled:\n"
|
||||
" telegram:\n"
|
||||
" - my-disabled-skill\n"
|
||||
)
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
|
||||
# Mock get_skill_commands to return two skills
|
||||
fake_skills_dir = str(tmp_path / "skills")
|
||||
fake_cmds = {
|
||||
"/my-disabled-skill": {
|
||||
"name": "my-disabled-skill",
|
||||
"description": "Should be hidden",
|
||||
"skill_md_path": f"{fake_skills_dir}/my-disabled-skill/SKILL.md",
|
||||
"skill_dir": f"{fake_skills_dir}/my-disabled-skill",
|
||||
},
|
||||
"/my-enabled-skill": {
|
||||
"name": "my-enabled-skill",
|
||||
"description": "Should be visible",
|
||||
"skill_md_path": f"{fake_skills_dir}/my-enabled-skill/SKILL.md",
|
||||
"skill_dir": f"{fake_skills_dir}/my-enabled-skill",
|
||||
},
|
||||
}
|
||||
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, hidden = telegram_menu_commands(max_commands=100)
|
||||
|
||||
menu_names = {n for n, _ in menu}
|
||||
assert "my_enabled_skill" in menu_names
|
||||
assert "my_disabled_skill" not in menu_names
|
||||
|
||||
@@ -141,6 +141,109 @@ class TestIsSkillDisabled:
|
||||
assert _is_skill_disabled("discord-skill") is True
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# get_disabled_skill_names — explicit platform param & env var fallback
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestGetDisabledSkillNames:
|
||||
"""Tests for agent.skill_utils.get_disabled_skill_names."""
|
||||
|
||||
def test_explicit_platform_param(self, tmp_path, monkeypatch):
|
||||
"""Explicit platform= parameter should resolve per-platform list."""
|
||||
config = tmp_path / "config.yaml"
|
||||
config.write_text(
|
||||
"skills:\n"
|
||||
" disabled:\n"
|
||||
" - global-skill\n"
|
||||
" platform_disabled:\n"
|
||||
" telegram:\n"
|
||||
" - tg-only-skill\n"
|
||||
)
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
monkeypatch.delenv("HERMES_PLATFORM", raising=False)
|
||||
monkeypatch.delenv("HERMES_SESSION_PLATFORM", raising=False)
|
||||
|
||||
from agent.skill_utils import get_disabled_skill_names
|
||||
result = get_disabled_skill_names(platform="telegram")
|
||||
assert result == {"tg-only-skill"}
|
||||
|
||||
def test_session_platform_env_var(self, tmp_path, monkeypatch):
|
||||
"""HERMES_SESSION_PLATFORM should be used when HERMES_PLATFORM is unset."""
|
||||
config = tmp_path / "config.yaml"
|
||||
config.write_text(
|
||||
"skills:\n"
|
||||
" disabled:\n"
|
||||
" - global-skill\n"
|
||||
" platform_disabled:\n"
|
||||
" discord:\n"
|
||||
" - discord-skill\n"
|
||||
)
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
monkeypatch.delenv("HERMES_PLATFORM", raising=False)
|
||||
monkeypatch.setenv("HERMES_SESSION_PLATFORM", "discord")
|
||||
|
||||
from agent.skill_utils import get_disabled_skill_names
|
||||
result = get_disabled_skill_names()
|
||||
assert result == {"discord-skill"}
|
||||
|
||||
def test_hermes_platform_takes_precedence(self, tmp_path, monkeypatch):
|
||||
"""HERMES_PLATFORM should win over HERMES_SESSION_PLATFORM."""
|
||||
config = tmp_path / "config.yaml"
|
||||
config.write_text(
|
||||
"skills:\n"
|
||||
" platform_disabled:\n"
|
||||
" telegram:\n"
|
||||
" - tg-skill\n"
|
||||
" discord:\n"
|
||||
" - discord-skill\n"
|
||||
)
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
monkeypatch.setenv("HERMES_PLATFORM", "telegram")
|
||||
monkeypatch.setenv("HERMES_SESSION_PLATFORM", "discord")
|
||||
|
||||
from agent.skill_utils import get_disabled_skill_names
|
||||
result = get_disabled_skill_names()
|
||||
assert result == {"tg-skill"}
|
||||
|
||||
def test_explicit_param_overrides_env_vars(self, tmp_path, monkeypatch):
|
||||
"""Explicit platform= param should override all env vars."""
|
||||
config = tmp_path / "config.yaml"
|
||||
config.write_text(
|
||||
"skills:\n"
|
||||
" platform_disabled:\n"
|
||||
" telegram:\n"
|
||||
" - tg-skill\n"
|
||||
" slack:\n"
|
||||
" - slack-skill\n"
|
||||
)
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
monkeypatch.setenv("HERMES_PLATFORM", "telegram")
|
||||
monkeypatch.setenv("HERMES_SESSION_PLATFORM", "telegram")
|
||||
|
||||
from agent.skill_utils import get_disabled_skill_names
|
||||
result = get_disabled_skill_names(platform="slack")
|
||||
assert result == {"slack-skill"}
|
||||
|
||||
def test_no_platform_returns_global(self, tmp_path, monkeypatch):
|
||||
"""No platform env vars or param should return global list."""
|
||||
config = tmp_path / "config.yaml"
|
||||
config.write_text(
|
||||
"skills:\n"
|
||||
" disabled:\n"
|
||||
" - global-skill\n"
|
||||
" platform_disabled:\n"
|
||||
" telegram:\n"
|
||||
" - tg-skill\n"
|
||||
)
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
monkeypatch.delenv("HERMES_PLATFORM", raising=False)
|
||||
monkeypatch.delenv("HERMES_SESSION_PLATFORM", raising=False)
|
||||
|
||||
from agent.skill_utils import get_disabled_skill_names
|
||||
result = get_disabled_skill_names()
|
||||
assert result == {"global-skill"}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _find_all_skills — disabled filtering
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user