fix(skills): rescan skill_commands cache when platform scope changes (#18739)
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 <leon@sgp43.com>
This commit is contained in:
parent
97acd66b4c
commit
c73594fe41
@ -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
|
||||
|
||||
|
||||
@ -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."""
|
||||
|
||||
Loading…
Reference in New Issue
Block a user