From c73594fe4196b5ee331d25f86774e66ad0f67a69 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sat, 2 May 2026 01:36:53 -0700 Subject: [PATCH] fix(skills): rescan skill_commands cache when platform scope changes (#18739) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The process-global `_skill_commands` dict in agent/skill_commands.py was seeded by whichever platform scanned first, and `get_skill_commands()` only rescanned when the cache was empty. In a long-lived gateway process serving multiple platforms (Telegram + Discord + Slack), the first platform's `skills.platform_disabled` view was silently inherited by the others — so a skill disabled for Telegram would also disappear from Discord's slash menu, and vice versa. Track the platform scope the cache was populated for (`_skill_commands_platform`) and rescan in `get_skill_commands()` when the currently-active platform no longer matches. Platform resolution uses the same precedence as `_is_skill_disabled`: `HERMES_PLATFORM` env var then `HERMES_SESSION_PLATFORM` from the gateway session context. Fixes #14536 Salvages #14570 by LeonSGP43. Co-authored-by: LeonSGP --- agent/skill_commands.py | 41 +++++++++++++++++++++-- tests/agent/test_skill_commands.py | 52 ++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 3 deletions(-) diff --git a/agent/skill_commands.py b/agent/skill_commands.py index ad1f0382..0276d5fc 100644 --- a/agent/skill_commands.py +++ b/agent/skill_commands.py @@ -6,6 +6,7 @@ can invoke skills via /skill-name commands. import json import logging +import os import re from pathlib import Path from typing import Any, Dict, Optional @@ -20,10 +21,35 @@ from agent.skill_preprocessing import ( logger = logging.getLogger(__name__) _skill_commands: Dict[str, Dict[str, Any]] = {} +_skill_commands_platform: Optional[str] = None # 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 _resolve_skill_commands_platform() -> Optional[str]: + """Return the current platform scope used for disabled-skill filtering. + + Used to detect when the active platform has shifted so + :func:`get_skill_commands` can drop a stale cache that was populated + for a different platform's ``skills.platform_disabled`` view (#14536). + + Resolves from (in order) ``HERMES_PLATFORM`` env var and + ``HERMES_SESSION_PLATFORM`` from the gateway session context. Returns + ``None`` when no platform scope is active (e.g. classic CLI, RL + rollouts, standalone scripts). + """ + try: + from gateway.session_context import get_session_env + + resolved_platform = ( + os.getenv("HERMES_PLATFORM") + or get_session_env("HERMES_SESSION_PLATFORM") + ) + except Exception: + resolved_platform = os.getenv("HERMES_PLATFORM") + return resolved_platform or None + def _load_skill_payload(skill_identifier: str, task_id: str | None = None) -> tuple[dict[str, Any], Path | None, str] | None: """Load a skill by name/path and return (loaded_payload, skill_dir, display_name).""" raw_identifier = (skill_identifier or "").strip() @@ -218,7 +244,8 @@ def scan_skill_commands() -> Dict[str, Dict[str, Any]]: Returns: Dict mapping "/skill-name" to {name, description, skill_md_path, skill_dir}. """ - global _skill_commands + global _skill_commands, _skill_commands_platform + _skill_commands_platform = _resolve_skill_commands_platform() _skill_commands = {} try: from tools.skills_tool import SKILLS_DIR, _parse_frontmatter, skill_matches_platform, _get_disabled_skill_names @@ -278,8 +305,16 @@ def scan_skill_commands() -> Dict[str, Dict[str, Any]]: def get_skill_commands() -> Dict[str, Dict[str, Any]]: - """Return the current skill commands mapping (scan first if empty).""" - if not _skill_commands: + """Return the current skill commands mapping (scan first if empty). + + Rescans when the active platform scope changes (e.g. a gateway + process serving Telegram and Discord concurrently) so each platform + sees its own ``skills.platform_disabled`` view (#14536). + """ + if ( + not _skill_commands + or _skill_commands_platform != _resolve_skill_commands_platform() + ): scan_skill_commands() return _skill_commands diff --git a/tests/agent/test_skill_commands.py b/tests/agent/test_skill_commands.py index 6879baed..bdea1738 100644 --- a/tests/agent/test_skill_commands.py +++ b/tests/agent/test_skill_commands.py @@ -125,6 +125,58 @@ class TestScanSkillCommands: assert "/knowledge-brain" in result assert result["/knowledge-brain"]["name"] == "knowledge-brain" + def test_get_skill_commands_rescans_when_platform_scope_changes(self, tmp_path): + """Platform-specific disabled-skill caches must not leak across platforms. + + Regression test for #14536: a gateway process serving Telegram + and Discord concurrently would seed the process-global cache + with whichever platform scanned first, and subsequent + ``get_skill_commands()`` calls from the other platform silently + inherited that filter. + """ + import agent.skill_commands as sc_mod + from agent.skill_commands import get_skill_commands + + def _disabled_skills(): + platform = os.getenv("HERMES_PLATFORM") + if platform == "telegram": + return {"telegram-only"} + if platform == "discord": + return {"discord-only"} + return set() + + with ( + patch("tools.skills_tool.SKILLS_DIR", tmp_path), + patch("tools.skills_tool._get_disabled_skill_names", side_effect=_disabled_skills), + patch.object(sc_mod, "_skill_commands", {}), + patch.object(sc_mod, "_skill_commands_platform", None), + ): + _make_skill(tmp_path, "shared") + _make_skill(tmp_path, "telegram-only") + _make_skill(tmp_path, "discord-only") + + with patch.dict(os.environ, {"HERMES_PLATFORM": "telegram"}): + telegram_commands = dict(get_skill_commands()) + + assert "/shared" in telegram_commands + assert "/discord-only" in telegram_commands + assert "/telegram-only" not in telegram_commands + + with patch.dict(os.environ, {"HERMES_PLATFORM": "discord"}): + discord_commands = dict(get_skill_commands()) + + assert "/shared" in discord_commands + assert "/telegram-only" in discord_commands + assert "/discord-only" not in discord_commands + + # Switching back to telegram must also rescan — not re-serve + # the discord view that was just cached. + with patch.dict(os.environ, {"HERMES_PLATFORM": "telegram"}): + telegram_again = dict(get_skill_commands()) + + assert "/telegram-only" not in telegram_again + assert "/discord-only" in telegram_again + def test_special_chars_stripped_from_cmd_key(self, tmp_path): """Skill names with +, /, or other special chars produce clean cmd keys."""