From 4143b9fb102f3b3b3788a016ad7bcb86a2b13e18 Mon Sep 17 00:00:00 2001 From: Muhamad Galih Saputra <126875499+muhamadgalihsaputra@users.noreply.github.com> Date: Thu, 11 Jun 2026 10:32:38 +0700 Subject: [PATCH] Fix gateway channel auto-skill bundles --- gateway/config.py | 11 +- gateway/platforms/base.py | 72 ++++++---- gateway/run.py | 132 +++++++++++++----- plugins/platforms/discord/adapter.py | 1 + tests/gateway/test_discord_channel_config.py | 28 ++++ tests/gateway/test_discord_channel_skills.py | 118 +++++++++++----- .../test_gateway_channel_auto_bundles.py | 57 ++++++++ 7 files changed, 321 insertions(+), 98 deletions(-) create mode 100644 tests/gateway/test_discord_channel_config.py create mode 100644 tests/gateway/test_gateway_channel_auto_bundles.py diff --git a/gateway/config.py b/gateway/config.py index 33df3b1acffa9..a69a584b868be 100644 --- a/gateway/config.py +++ b/gateway/config.py @@ -969,8 +969,15 @@ def _merge_platform_map(source_platforms: Any) -> None: bridged["group_allow_admin_from"] = platform_cfg["group_allow_admin_from"] if "group_user_allowed_commands" in platform_cfg: bridged["group_user_allowed_commands"] = platform_cfg["group_user_allowed_commands"] - if plat in {Platform.DISCORD, Platform.SLACK} and "channel_skill_bindings" in platform_cfg: - bridged["channel_skill_bindings"] = platform_cfg["channel_skill_bindings"] + if plat in {Platform.DISCORD, Platform.SLACK}: + if "channel_skill_bindings" in platform_cfg: + bridged["channel_skill_bindings"] = platform_cfg["channel_skill_bindings"] + if "channel_skills" in platform_cfg: + channel_skills = platform_cfg["channel_skills"] + if isinstance(channel_skills, dict): + bridged["channel_skills"] = {str(k): v for k, v in channel_skills.items()} + else: + bridged["channel_skills"] = channel_skills if "channel_prompts" in platform_cfg: channel_prompts = platform_cfg["channel_prompts"] if isinstance(channel_prompts, dict): diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index b9273e7cca0c7..f33eab04d34fe 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -1726,9 +1726,7 @@ def resolve_channel_skills( ) -> list[str] | None: """Resolve auto-loaded skill(s) for a channel/thread from platform config. - Looks up ``channel_skill_bindings`` in the adapter's ``config.extra`` dict. - - Config format:: + Preferred config format:: channel_skill_bindings: - id: "C0123" # Slack channel ID or Discord channel/forum ID @@ -1736,6 +1734,12 @@ def resolve_channel_skills( - id: "D0ABCDE" skill: "solo-skill" # single string also accepted + Legacy/dict alias also accepted:: + + channel_skills: + "C0123": ["skill-a", "skill-b"] + "D0ABCDE": "solo-skill" + Prefers an exact match on *channel_id*; falls back to *parent_id* (useful for forum threads / Slack threads inheriting the parent channel's binding). @@ -1743,34 +1747,46 @@ def resolve_channel_skills( Returns a deduplicated list of skill names (order preserved), or None if no match is found. """ - bindings = config_extra.get("channel_skill_bindings") or [] - if not isinstance(bindings, list) or not bindings: + + def _normalize_skills(value: Any) -> list[str] | None: + if isinstance(value, str): + skill = value.strip() + return [skill] if skill else None + if isinstance(value, list): + seen: list[str] = [] + for name in value: + if not isinstance(name, str): + continue + normalized = name.strip() + if normalized and normalized not in seen: + seen.append(normalized) + return seen or None return None - ids_to_check: set[str] = set() - if channel_id: - ids_to_check.add(str(channel_id)) - if parent_id: - ids_to_check.add(str(parent_id)) + + ids_to_check: list[str] = [] + for key in (channel_id, parent_id): + if key: + normalized = str(key) + if normalized not in ids_to_check: + ids_to_check.append(normalized) if not ids_to_check: return None - for entry in bindings: - if not isinstance(entry, dict): - continue - entry_id = str(entry.get("id", "")) - if entry_id in ids_to_check: - skills = entry.get("skills") or entry.get("skill") - if isinstance(skills, str): - s = skills.strip() - return [s] if s else None - if isinstance(skills, list) and skills: - seen: list[str] = [] - for name in skills: - if not isinstance(name, str): - continue - nm = name.strip() - if nm and nm not in seen: - seen.append(nm) - return seen or None + + bindings = config_extra.get("channel_skill_bindings") or [] + if isinstance(bindings, list): + for entry in bindings: + if not isinstance(entry, dict): + continue + entry_id = str(entry.get("id", "")) + if entry_id in ids_to_check: + return _normalize_skills(entry.get("skills") or entry.get("skill")) + + channel_skills = config_extra.get("channel_skills") or {} + if isinstance(channel_skills, dict): + for key in ids_to_check: + if key in channel_skills: + return _normalize_skills(channel_skills.get(key)) + return None diff --git a/gateway/run.py b/gateway/run.py index 58c68a4b99c57..bcb0a3f5b14ae 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -7690,6 +7690,96 @@ async def _prepare_inbound_message_text( return message_text + def _inject_auto_skills_for_new_session( + self, + event, + session_entry, + task_id: str, + session_key: str, + *, + is_new_session: bool | None = None, + ) -> bool: + """Inject channel-bound skill or bundle payloads into a fresh session. + + Topic/channel bindings historically loaded individual skills only. Keep + that behavior, but also accept bundle slash names such as ``/ops-core`` + so config can reuse the same grouping mechanism as user-invoked bundle + commands. + """ + if is_new_session is None: + is_new_session = ( + getattr(session_entry, "created_at", None) + == getattr(session_entry, "updated_at", None) + or getattr(session_entry, "was_auto_reset", False) + or getattr(session_entry, "is_fresh_reset", False) + ) + auto = getattr(event, "auto_skill", None) + if not is_new_session or not auto: + return False + + skill_names = [auto] if isinstance(auto, str) else list(auto) + try: + from agent.skill_bundles import ( + build_bundle_invocation_message, + resolve_bundle_command_key, + ) + from agent.skill_commands import _build_skill_message, _load_skill_payload + + combined_parts: list[str] = [] + loaded_names: list[str] = [] + for raw_name in skill_names: + skill_name = str(raw_name or "").strip() + if not skill_name: + continue + + bundle_key = resolve_bundle_command_key(skill_name.lstrip("/")) + if bundle_key: + bundle = build_bundle_invocation_message( + bundle_key, + task_id=task_id, + ) + if bundle: + bundle_message, _bundle_loaded, bundle_missing = bundle + combined_parts.append(bundle_message) + loaded_names.append(bundle_key) + if bundle_missing: + logger.warning( + "[Gateway] Auto-bundle '%s' skipped missing skill(s): %s", + bundle_key, + bundle_missing, + ) + continue + + loaded = _load_skill_payload(skill_name, task_id=task_id) + if loaded: + loaded_skill, skill_dir, display_name = loaded + note = ( + f'[IMPORTANT: The "{display_name}" skill is auto-loaded. ' + "Follow its instructions for this session.]" + ) + part = _build_skill_message(loaded_skill, skill_dir, note) + if part: + combined_parts.append(part) + loaded_names.append(skill_name) + else: + logger.warning("[Gateway] Auto-skill/bundle '%s' not found", skill_name) + + if not combined_parts: + return False + + # Append the user's original text after all skill/bundle payloads. + combined_parts.append(event.text) + event.text = "\n\n".join(combined_parts) + logger.info( + "[Gateway] Auto-loaded skill(s)/bundle(s) %s for session %s", + loaded_names, + session_key, + ) + return True + except Exception as e: + logger.warning("[Gateway] Failed to auto-load skill(s)/bundle(s) %s: %s", skill_names, e) + return False + def _consume_pending_native_image_paths(self, session_key: str) -> List[str]: pending_native = getattr(self, "_pending_native_image_paths_by_session", None) if not pending_native: @@ -7926,41 +8016,13 @@ async def _handle_message_with_agent(self, event, source, _quick_key: str, run_g session_entry.was_auto_reset = False session_entry.auto_reset_reason = None - # Auto-load skill(s) for topic/channel bindings (Telegram DM Topics, - # Discord channel_skill_bindings). Supports a single name or ordered list. - # Only inject on NEW sessions — ongoing conversations already have the - # skill content in their conversation history from the first message. - _auto = getattr(event, "auto_skill", None) - if _is_new_session and _auto: - _skill_names = [_auto] if isinstance(_auto, str) else list(_auto) - try: - from agent.skill_commands import _load_skill_payload, _build_skill_message - _combined_parts: list[str] = [] - _loaded_names: list[str] = [] - for _sname in _skill_names: - _loaded = _load_skill_payload(_sname, task_id=_quick_key) - if _loaded: - _loaded_skill, _skill_dir, _display_name = _loaded - _note = ( - f'[IMPORTANT: The "{_display_name}" skill is auto-loaded. ' - f"Follow its instructions for this session.]" - ) - _part = _build_skill_message(_loaded_skill, _skill_dir, _note) - if _part: - _combined_parts.append(_part) - _loaded_names.append(_sname) - else: - logger.warning("[Gateway] Auto-skill '%s' not found", _sname) - if _combined_parts: - # Append the user's original text after all skill payloads - _combined_parts.append(event.text) - event.text = "\n\n".join(_combined_parts) - logger.info( - "[Gateway] Auto-loaded skill(s) %s for session %s", - _loaded_names, session_key, - ) - except Exception as e: - logger.warning("[Gateway] Failed to auto-load skill(s) %s: %s", _skill_names, e) + self._inject_auto_skills_for_new_session( + event, + session_entry, + _quick_key, + session_key, + is_new_session=_is_new_session, + ) # Load conversation history from transcript history = self.session_store.load_transcript(session_entry.session_id) diff --git a/plugins/platforms/discord/adapter.py b/plugins/platforms/discord/adapter.py index 46544cd1f44c5..26a635a30a9d0 100644 --- a/plugins/platforms/discord/adapter.py +++ b/plugins/platforms/discord/adapter.py @@ -3743,6 +3743,7 @@ def _build_slash_event(self, interaction: discord.Interaction, text: str) -> Mes message_type=msg_type, source=source, raw_message=interaction, + auto_skill=self._resolve_channel_skills(channel_id, parent_id or None), channel_prompt=self._resolve_channel_prompt(channel_id, parent_id or None), ) diff --git a/tests/gateway/test_discord_channel_config.py b/tests/gateway/test_discord_channel_config.py new file mode 100644 index 0000000000000..d0e044e26a895 --- /dev/null +++ b/tests/gateway/test_discord_channel_config.py @@ -0,0 +1,28 @@ +"""Tests for gateway config bridging of channel auto-skill settings.""" + + +def test_top_level_discord_channel_skills_are_bridged(monkeypatch, tmp_path): + from gateway.config import Platform, load_gateway_config + + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + (hermes_home / "config.yaml").write_text( + """ +discord: + enabled: true + token: token + channel_skills: + 123: + - skill-a + - skill-b +""", + encoding="utf-8", + ) + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.delenv("DISCORD_TOKEN", raising=False) + + config = load_gateway_config() + + assert config.platforms[Platform.DISCORD].extra["channel_skills"] == { + "123": ["skill-a", "skill-b"] + } diff --git a/tests/gateway/test_discord_channel_skills.py b/tests/gateway/test_discord_channel_skills.py index a1b958d06b062..87cfb7b8ec5ea 100644 --- a/tests/gateway/test_discord_channel_skills.py +++ b/tests/gateway/test_discord_channel_skills.py @@ -1,13 +1,15 @@ -"""Tests for Discord channel_skill_bindings auto-skill resolution.""" +"""Tests for Discord channel skill/bundle auto-loading.""" +from types import SimpleNamespace from unittest.mock import MagicMock -def _make_adapter(): +def _make_adapter(extra=None): """Create a minimal DiscordAdapter with mocked config.""" from plugins.platforms.discord.adapter import DiscordAdapter + adapter = object.__new__(DiscordAdapter) adapter.config = MagicMock() - adapter.config.extra = {} + adapter.config.extra = extra or {} return adapter @@ -17,47 +19,97 @@ def test_no_bindings_returns_none(self): assert adapter._resolve_channel_skills("123") is None def test_match_by_channel_id(self): - adapter = _make_adapter() - adapter.config.extra = { - "channel_skill_bindings": [ - {"id": "100", "skills": ["skill-a", "skill-b"]}, - ] - } + adapter = _make_adapter( + { + "channel_skill_bindings": [ + {"id": "100", "skills": ["skill-a", "skill-b"]}, + ] + } + ) assert adapter._resolve_channel_skills("100") == ["skill-a", "skill-b"] def test_match_by_parent_id(self): - adapter = _make_adapter() - adapter.config.extra = { - "channel_skill_bindings": [ - {"id": "200", "skills": ["forum-skill"]}, - ] - } + adapter = _make_adapter( + { + "channel_skill_bindings": [ + {"id": "200", "skills": ["forum-skill"]}, + ] + } + ) # channel_id doesn't match, but parent_id does (forum thread) assert adapter._resolve_channel_skills("999", parent_id="200") == ["forum-skill"] def test_no_match_returns_none(self): - adapter = _make_adapter() - adapter.config.extra = { - "channel_skill_bindings": [ - {"id": "100", "skills": ["skill-a"]}, - ] - } + adapter = _make_adapter( + { + "channel_skill_bindings": [ + {"id": "100", "skills": ["skill-a"]}, + ] + } + ) assert adapter._resolve_channel_skills("999") is None def test_single_skill_string(self): - adapter = _make_adapter() - adapter.config.extra = { - "channel_skill_bindings": [ - {"id": "100", "skill": "solo-skill"}, - ] - } + adapter = _make_adapter( + { + "channel_skill_bindings": [ + {"id": "100", "skill": "solo-skill"}, + ] + } + ) assert adapter._resolve_channel_skills("100") == ["solo-skill"] def test_dedup_preserves_order(self): - adapter = _make_adapter() - adapter.config.extra = { - "channel_skill_bindings": [ - {"id": "100", "skills": ["a", "b", "a", "c", "b"]}, - ] - } + adapter = _make_adapter( + { + "channel_skill_bindings": [ + {"id": "100", "skills": ["a", "b", "a", "c", "b"]}, + ] + } + ) assert adapter._resolve_channel_skills("100") == ["a", "b", "c"] + + def test_legacy_channel_skills_dict_match_by_channel_id(self): + adapter = _make_adapter( + { + "channel_skills": { + "100": ["skill-a", "skill-b", "skill-a"], + } + } + ) + assert adapter._resolve_channel_skills("100") == ["skill-a", "skill-b"] + + def test_legacy_channel_skills_dict_match_by_parent_id(self): + adapter = _make_adapter( + { + "channel_skills": { + "200": "forum-skill", + } + } + ) + assert adapter._resolve_channel_skills("999", parent_id="200") == ["forum-skill"] + + +class TestDiscordSlashEventAutoSkill: + def test_build_slash_event_sets_auto_skill(self): + adapter = _make_adapter( + { + "channel_skill_bindings": [ + {"id": "321", "skills": ["ops-skill"]}, + ], + "channel_prompts": {"321": "Command prompt"}, + } + ) + adapter.build_source = MagicMock(return_value=SimpleNamespace()) + adapter._get_effective_topic = MagicMock(return_value=None) + + interaction = SimpleNamespace( + channel_id=321, + channel=SimpleNamespace(name="general", guild=None, parent_id=None), + user=SimpleNamespace(id=1, display_name="Brenner"), + ) + + event = adapter._build_slash_event(interaction, "/retry") + + assert event.auto_skill == ["ops-skill"] + assert event.channel_prompt == "Command prompt" diff --git a/tests/gateway/test_gateway_channel_auto_bundles.py b/tests/gateway/test_gateway_channel_auto_bundles.py new file mode 100644 index 0000000000000..87f62bd907a1b --- /dev/null +++ b/tests/gateway/test_gateway_channel_auto_bundles.py @@ -0,0 +1,57 @@ +"""Regression tests for gateway channel auto-loading of skill bundles.""" +from types import SimpleNamespace + +import yaml + + +def _write_skill(base, name, body="Follow this skill."): + skill_dir = base / "skills" / name + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text( + f"""--- +name: {name} +description: Test skill {name} +--- +# {name} + +{body} +""", + encoding="utf-8", + ) + + +def test_auto_loader_accepts_skill_bundle_names(tmp_path, monkeypatch): + from gateway.run import GatewayRunner + + hermes_home = tmp_path / "hermes" + bundles_dir = hermes_home / "skill-bundles" + bundles_dir.mkdir(parents=True) + _write_skill(hermes_home, "skill-a") + _write_skill(hermes_home, "skill-b") + (bundles_dir / "ops-core.yaml").write_text( + yaml.safe_dump( + { + "name": "ops-core", + "description": "Ops baseline", + "skills": ["skill-a", "skill-b"], + } + ), + encoding="utf-8", + ) + + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.setenv("HERMES_BUNDLES_DIR", str(bundles_dir)) + # tools.skills_tool keeps SKILLS_DIR as a module-level compatibility + # constant, so point it at the temporary test home after import too. + monkeypatch.setattr("tools.skills_tool.SKILLS_DIR", hermes_home / "skills") + + runner = object.__new__(GatewayRunner) + event = SimpleNamespace(text="hello", auto_skill=["/ops-core"]) + session_entry = SimpleNamespace(created_at=1, updated_at=1, was_auto_reset=False) + + assert runner._inject_auto_skills_for_new_session(event, session_entry, "task-1", "session-1") + + assert "ops-core" in event.text + assert "skill-a" in event.text + assert "skill-b" in event.text + assert event.text.endswith("hello")