refactor(skills): drop the redundant stacked-loader table override
The messaging gateway's stacked tokens come from the native (filesystem-only) split, and filesystem commands win every collision in the interactive map, so looking them up there loads the same skills. The table= override was a second guard on a boundary the split already holds; the gateway test still goes red when the native split resolves plugin tokens. Also pass encoding= to the new tests' write_text calls (Windows footgun lint).
This commit is contained in:
@@ -772,17 +772,12 @@ def split_stacked_skill_commands(rest: str, *, interactive: bool = False) -> tup
|
||||
|
||||
|
||||
def build_stacked_skill_invocation_message(
|
||||
cmd_keys: list[str], user_instruction: str = "", task_id: str | None = None, *, table: Mapping[str, Any] | None = None,
|
||||
cmd_keys: list[str], user_instruction: str = "", task_id: str | None = None,
|
||||
) -> Optional[tuple[str, list[str], list[str]]]:
|
||||
"""Build the user message for a stacked multi-skill slash invocation:
|
||||
``(message, loaded_skill_names, missing_skill_names)``, or ``None`` when no skill loaded.
|
||||
|
||||
*table* overrides the default interactive lookup table; the messaging gateway
|
||||
passes its filesystem-only map so a stacked token can never load a plugin
|
||||
skill (they are interactive-only) and so the per-platform disabled filter
|
||||
sees every loaded skill's real name.
|
||||
"""
|
||||
commands = table if table is not None else get_interactive_skill_commands()
|
||||
Keys come from ``split_stacked_skill_commands``; native callers only ever pass filesystem keys."""
|
||||
commands = get_interactive_skill_commands()
|
||||
keys = [k for k in cmd_keys if k]
|
||||
loaded_names, missing, _disabled, skill_blocks = _load_skill_blocks(
|
||||
keys,
|
||||
|
||||
@@ -1212,10 +1212,7 @@ class GatewayInboundMixin:
|
||||
_plat = source.platform.value if source.platform else None
|
||||
user_instruction = event.get_command_args().strip()
|
||||
# Stacked slash-skill invocations: `/skill-a /skill-b do XYZ` loads every leading skill
|
||||
# (up to 5), not just the first. Mirrors CLI. The native/messaging surface stays
|
||||
# filesystem-only: plugin skills are interactive-only (CLI/TUI/desktop), so both the
|
||||
# token scan and the loader run against this native map — a stacked `/plugin:skill`
|
||||
# token resolves as unknown here instead of silently loading a plugin skill body.
|
||||
# (up to 5), not just the first. Mirrors CLI. Native split: plugin skills are interactive-only.
|
||||
try:
|
||||
from agent.skill_commands import (
|
||||
build_stacked_skill_invocation_message as _build_stacked,
|
||||
@@ -1230,8 +1227,6 @@ class GatewayInboundMixin:
|
||||
# Per-platform disabled check: get_skill_commands() only applies the *global*
|
||||
# disabled list at scan time (process-global cache across platforms), and
|
||||
# split_stacked_skill_commands() only checks each extra token is a KNOWN skill.
|
||||
# Keyed on the same native map the loader uses, so every stacked skill's real
|
||||
# name is checked (a token that is not in the map is not a skill here).
|
||||
from agent.skill_utils import get_disabled_skill_names as _get_plat_disabled
|
||||
_plat_disabled = _get_plat_disabled(platform=_plat)
|
||||
if _skill_name and _skill_name in _plat_disabled:
|
||||
@@ -1245,7 +1240,7 @@ class GatewayInboundMixin:
|
||||
return t("gateway.skills.stacked_disabled", names=", ".join(_disabled_extra), platform=_plat)
|
||||
if extra_keys and _build_stacked is not None:
|
||||
stacked_result = _build_stacked(
|
||||
[cmd_key, *extra_keys], stacked_instruction, task_id=_quick_key, table=skill_cmds,
|
||||
[cmd_key, *extra_keys], stacked_instruction, task_id=_quick_key,
|
||||
)
|
||||
if not stacked_result:
|
||||
return t("gateway.skills.stacked_load_failed", command=command)
|
||||
|
||||
@@ -13,21 +13,21 @@ def _make_plugin_skill(home: Path, plugin_name: str, skill: str, body: str) -> N
|
||||
plugin = home / "plugins" / plugin_name
|
||||
md = plugin / "skills" / skill / "SKILL.md"
|
||||
md.parent.mkdir(parents=True, exist_ok=True)
|
||||
(plugin / "plugin.yaml").write_text(f"name: {plugin_name}\nversion: 0.1.0\n")
|
||||
(plugin / "plugin.yaml").write_text(f"name: {plugin_name}\nversion: 0.1.0\n", encoding="utf-8")
|
||||
(plugin / "__init__.py").write_text(
|
||||
"from pathlib import Path\n"
|
||||
f"def register(ctx):\n"
|
||||
f" ctx.register_skill({skill!r}, Path(__file__).parent / 'skills' / {skill!r} / 'SKILL.md')\n"
|
||||
f" ctx.register_skill({skill!r}, Path(__file__).parent / 'skills' / {skill!r} / 'SKILL.md')\n", encoding="utf-8"
|
||||
)
|
||||
md.write_text(f"---\nname: {skill}\ndescription: Guide\n---\n{body}\n")
|
||||
(home / "config.yaml").write_text(f"plugins:\n enabled: [{plugin_name}]\n")
|
||||
md.write_text(f"---\nname: {skill}\ndescription: Guide\n---\n{body}\n", encoding="utf-8")
|
||||
(home / "config.yaml").write_text(f"plugins:\n enabled: [{plugin_name}]\n", encoding="utf-8")
|
||||
|
||||
|
||||
def _make_local_skill(tmp_path: Path, name: str, body: str) -> None:
|
||||
skills = tmp_path / "skills"
|
||||
skills.mkdir(parents=True, exist_ok=True)
|
||||
(skills / name).mkdir(exist_ok=True)
|
||||
(skills / name / "SKILL.md").write_text(f"---\nname: {name}\ndescription: {name}\n---\n{body}\n")
|
||||
(skills / name / "SKILL.md").write_text(f"---\nname: {name}\ndescription: {name}\n---\n{body}\n", encoding="utf-8")
|
||||
|
||||
|
||||
def _rewrite(event_text: str, tmp_path, monkeypatch, plugin_home):
|
||||
|
||||
@@ -18,16 +18,16 @@ def test_plugin_skills_reach_tui_slash_menu_and_stack_per_profile(tmp_path, monk
|
||||
for label, home in homes.items():
|
||||
plugin = home / "plugins" / f"probe-{label}"
|
||||
plugin.mkdir(parents=True)
|
||||
(plugin / "plugin.yaml").write_text(f"name: probe-{label}\nversion: 0.1.0\n")
|
||||
(plugin / "plugin.yaml").write_text(f"name: probe-{label}\nversion: 0.1.0\n", encoding="utf-8")
|
||||
(plugin / "__init__.py").write_text(
|
||||
"from pathlib import Path\ndef register(ctx):\n"
|
||||
" for name in ('guide', 'review'):\n"
|
||||
" ctx.register_skill(name, Path(__file__).parent / 'skills' / name / 'SKILL.md')\n")
|
||||
" ctx.register_skill(name, Path(__file__).parent / 'skills' / name / 'SKILL.md')\n", encoding="utf-8")
|
||||
for name in ("guide", "review"):
|
||||
md = plugin / "skills" / name / "SKILL.md"
|
||||
md.parent.mkdir(parents=True)
|
||||
md.write_text(f"---\nname: {name}\ndescription: {label} {name}.\n---\n\n{label}-{name}-body\n")
|
||||
(home / "config.yaml").write_text(f"plugins:\n enabled: [probe-{label}]\n")
|
||||
md.write_text(f"---\nname: {name}\ndescription: {label} {name}.\n---\n\n{label}-{name}-body\n", encoding="utf-8")
|
||||
(home / "config.yaml").write_text(f"plugins:\n enabled: [probe-{label}]\n", encoding="utf-8")
|
||||
|
||||
monkeypatch.setenv("HERMES_HOME", str(homes["alpha"]))
|
||||
plugins._reset_plugin_managers_for_tests()
|
||||
|
||||
Reference in New Issue
Block a user