feat(skills): per-skill runtime compatibility (#119, hermes pattern)
SKILL.md frontmatter can now declare `runtime: [claude-code]` or `runtime: [hermes, claude-code]` to opt out of incompatible adapters instead of failing at first invocation. Default `["*"]` means universal — existing skill libraries need zero migration. Borrowed from hermes' declarative skill-compat pattern surfaced in the hermes architecture survey. The remaining two patterns (event-log layer, observability config block) stay open under #119. Wiring: - SkillMetadata.runtime: list[str] = ["*"] - _normalize_runtime_field accepts list, string-sugar, missing -> ["*"]; malformed warns and falls back to universal so a typo never silently drops a skill. - load_skills(..., current_runtime=...) filters out skills whose runtime list lacks "*" or current_runtime, with an INFO log line. - BaseAdapter.start passes type(self).name() so the live adapter drives the filter; SkillsWatcher takes the same kwarg so hot-reload honors it. 8 new tests cover default universal, no-field universal, explicit match/mismatch, string sugar, wildcard short-circuit, current_runtime=None (preserves old behavior), and malformed-warns-not-drops. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Hongming Wang
parent
e99f937630
commit
d0057912d2
@ -437,15 +437,19 @@ class BaseAdapter(ABC):
|
||||
if plugins.plugin_names:
|
||||
logger.info(f"Plugins: {', '.join(plugins.plugin_names)}")
|
||||
|
||||
# Load skills (workspace + plugin skills, deduped)
|
||||
loaded_skills = load_skills(config.config_path, config.tools)
|
||||
# Load skills (workspace + plugin skills, deduped). Pass the runtime
|
||||
# name so SKILL.md frontmatter `runtime: [...]` can opt skills out
|
||||
# of incompatible adapters (hermes won't load claude-code-only
|
||||
# skills, etc.).
|
||||
runtime_name = type(self).name()
|
||||
loaded_skills = load_skills(config.config_path, config.tools, current_runtime=runtime_name)
|
||||
seen_skill_ids = {s.metadata.id for s in loaded_skills}
|
||||
for plugin_skills_dir in plugins.skill_dirs:
|
||||
plugin_skill_names = [
|
||||
d for d in os.listdir(plugin_skills_dir)
|
||||
if os.path.isdir(os.path.join(plugin_skills_dir, d))
|
||||
]
|
||||
for skill in load_skills(plugin_skills_dir, plugin_skill_names):
|
||||
for skill in load_skills(plugin_skills_dir, plugin_skill_names, current_runtime=runtime_name):
|
||||
if skill.metadata.id not in seen_skill_ids:
|
||||
loaded_skills.append(skill)
|
||||
seen_skill_ids.add(skill.metadata.id)
|
||||
|
||||
@ -323,6 +323,7 @@ async def main(): # pragma: no cover
|
||||
config_path=config_path,
|
||||
skill_names=config.skills,
|
||||
on_reload=_on_skill_reload,
|
||||
current_runtime=runtime,
|
||||
)
|
||||
asyncio.create_task(skills_watcher.start())
|
||||
print(f"Skills hot-reload enabled for: {config.skills}")
|
||||
|
||||
@ -26,6 +26,12 @@ class SkillMetadata:
|
||||
description: str
|
||||
tags: list[str] = field(default_factory=list)
|
||||
examples: list[str] = field(default_factory=list)
|
||||
# Runtime compatibility — list of adapter `name()` values this skill
|
||||
# supports, or ["*"] for universal. Borrowed from hermes' declarative
|
||||
# skill-compat pattern: a skill that depends on claude-code-only tools
|
||||
# should declare `runtime: [claude-code]` so hermes (or any other
|
||||
# adapter) skips it at load time instead of failing at first invocation.
|
||||
runtime: list[str] = field(default_factory=lambda: ["*"])
|
||||
|
||||
|
||||
@dataclass
|
||||
@ -133,8 +139,39 @@ def load_skill_tools(scripts_dir: Path) -> list[Any]:
|
||||
return tools
|
||||
|
||||
|
||||
def load_skills(config_path: str, skill_names: list[str]) -> list[LoadedSkill]:
|
||||
"""Load all skills specified in the config."""
|
||||
def _normalize_runtime_field(raw: Any, skill_name: str) -> list[str]:
|
||||
"""Normalize the optional `runtime` frontmatter field to a list[str].
|
||||
|
||||
Accepts: ["*"] (default), ["claude-code"], "claude-code" (string sugar),
|
||||
or absent (-> ["*"]). Anything else logs a warning and falls back to
|
||||
universal so a malformed manifest doesn't silently filter the skill.
|
||||
"""
|
||||
if raw is None:
|
||||
return ["*"]
|
||||
if isinstance(raw, str):
|
||||
return [raw]
|
||||
if isinstance(raw, list) and all(isinstance(x, str) for x in raw):
|
||||
return raw or ["*"]
|
||||
logger.warning(
|
||||
"SKILL.md for '%s' has invalid `runtime` field %r; treating as universal",
|
||||
skill_name, raw,
|
||||
)
|
||||
return ["*"]
|
||||
|
||||
|
||||
def load_skills(
|
||||
config_path: str,
|
||||
skill_names: list[str],
|
||||
current_runtime: str | None = None,
|
||||
) -> list[LoadedSkill]:
|
||||
"""Load all skills specified in the config.
|
||||
|
||||
If ``current_runtime`` is provided, skills whose ``runtime`` frontmatter
|
||||
list does not include ``"*"`` or ``current_runtime`` are skipped (with a
|
||||
log line) instead of being loaded — matches hermes' declarative compat
|
||||
model so adapter-specific skills don't get force-loaded into runtimes
|
||||
that can't actually execute their tools.
|
||||
"""
|
||||
skills_dir = Path(config_path) / "skills"
|
||||
loaded = []
|
||||
|
||||
@ -171,12 +208,21 @@ def load_skills(config_path: str, skill_names: list[str]) -> list[LoadedSkill]:
|
||||
|
||||
frontmatter, instructions = parse_skill_frontmatter(skill_md)
|
||||
|
||||
runtime_compat = _normalize_runtime_field(frontmatter.get("runtime"), skill_name)
|
||||
if current_runtime is not None and "*" not in runtime_compat and current_runtime not in runtime_compat:
|
||||
logger.info(
|
||||
"Skipping skill '%s': runtime=%s not compatible with current=%s",
|
||||
skill_name, runtime_compat, current_runtime,
|
||||
)
|
||||
continue
|
||||
|
||||
metadata = SkillMetadata(
|
||||
id=skill_name,
|
||||
name=frontmatter.get("name", skill_name),
|
||||
description=frontmatter.get("description", ""),
|
||||
tags=frontmatter.get("tags", []),
|
||||
examples=frontmatter.get("examples", []),
|
||||
runtime=runtime_compat,
|
||||
)
|
||||
|
||||
# Executables live under scripts/ per the agentskills.io spec.
|
||||
|
||||
@ -67,10 +67,12 @@ class SkillsWatcher:
|
||||
config_path: str,
|
||||
skill_names: list[str],
|
||||
on_reload: Callable | None = None,
|
||||
current_runtime: str | None = None,
|
||||
) -> None:
|
||||
self.config_path = config_path
|
||||
self.skill_names = list(skill_names)
|
||||
self.on_reload = on_reload
|
||||
self.current_runtime = current_runtime
|
||||
self._hashes: dict[str, str] = {} # rel_path → sha256 hex
|
||||
self._running = False
|
||||
|
||||
@ -169,7 +171,7 @@ class SkillsWatcher:
|
||||
|
||||
try:
|
||||
from skill_loader.loader import load_skills
|
||||
loaded = load_skills(self.config_path, [skill_name])
|
||||
loaded = load_skills(self.config_path, [skill_name], current_runtime=self.current_runtime)
|
||||
|
||||
if loaded:
|
||||
skill = loaded[0]
|
||||
|
||||
@ -641,3 +641,91 @@ def test_load_skills_fail_open_if_no_scanner_wiring(tmp_path, monkeypatch):
|
||||
assert scan_kwargs[0]["fail_open"] is False, (
|
||||
"fail_open_if_no_scanner=False from config must be forwarded to scan_skill_dependencies"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Per-skill runtime compatibility (#119)
|
||||
# ---------------------------------------------------------------------------
|
||||
# A skill manifest can declare `runtime: [claude-code]` to opt out of being
|
||||
# loaded into incompatible adapters. Default is universal — this is the
|
||||
# important contract: existing skill libraries do NOT need to be migrated
|
||||
# and continue to load into every adapter.
|
||||
|
||||
|
||||
def _write_skill(tmp_path, name: str, runtime_block: str = "") -> None:
|
||||
skill_dir = tmp_path / "skills" / name
|
||||
skill_dir.mkdir(parents=True)
|
||||
(skill_dir / "SKILL.md").write_text(
|
||||
f"---\nname: {name.title()}\ndescription: x\n{runtime_block}---\n"
|
||||
f"Body for {name}."
|
||||
)
|
||||
|
||||
|
||||
def test_skill_metadata_runtime_default_universal():
|
||||
meta = SkillMetadata(id="t", name="T", description="d")
|
||||
assert meta.runtime == ["*"], "default runtime must be universal — no implicit filtering"
|
||||
|
||||
|
||||
def test_load_skills_no_runtime_field_is_universal(tmp_path):
|
||||
"""Skills without a `runtime` frontmatter field load into any adapter."""
|
||||
_write_skill(tmp_path, "legacy") # no runtime block
|
||||
from unittest.mock import patch
|
||||
with patch("skill_loader.loader.load_skill_tools", return_value=[]):
|
||||
loaded = load_skills(str(tmp_path), ["legacy"], current_runtime="hermes")
|
||||
assert len(loaded) == 1
|
||||
assert loaded[0].metadata.runtime == ["*"]
|
||||
|
||||
|
||||
def test_load_skills_explicit_match_loads(tmp_path):
|
||||
_write_skill(tmp_path, "claude-only", "runtime:\n - claude-code\n")
|
||||
from unittest.mock import patch
|
||||
with patch("skill_loader.loader.load_skill_tools", return_value=[]):
|
||||
loaded = load_skills(str(tmp_path), ["claude-only"], current_runtime="claude-code")
|
||||
assert len(loaded) == 1
|
||||
|
||||
|
||||
def test_load_skills_explicit_mismatch_skips(tmp_path):
|
||||
_write_skill(tmp_path, "claude-only", "runtime:\n - claude-code\n")
|
||||
from unittest.mock import patch
|
||||
with patch("skill_loader.loader.load_skill_tools", return_value=[]):
|
||||
loaded = load_skills(str(tmp_path), ["claude-only"], current_runtime="hermes")
|
||||
assert loaded == [], "skill must be filtered out of incompatible runtime"
|
||||
|
||||
|
||||
def test_load_skills_runtime_string_sugar(tmp_path):
|
||||
"""Bare string `runtime: claude-code` is normalized to ['claude-code']."""
|
||||
_write_skill(tmp_path, "sugary", "runtime: claude-code\n")
|
||||
from unittest.mock import patch
|
||||
with patch("skill_loader.loader.load_skill_tools", return_value=[]):
|
||||
loaded = load_skills(str(tmp_path), ["sugary"], current_runtime="claude-code")
|
||||
assert len(loaded) == 1
|
||||
assert loaded[0].metadata.runtime == ["claude-code"]
|
||||
|
||||
|
||||
def test_load_skills_runtime_wildcard_matches_anything(tmp_path):
|
||||
_write_skill(tmp_path, "wild", "runtime:\n - '*'\n - claude-code\n")
|
||||
from unittest.mock import patch
|
||||
with patch("skill_loader.loader.load_skill_tools", return_value=[]):
|
||||
loaded = load_skills(str(tmp_path), ["wild"], current_runtime="hermes")
|
||||
assert len(loaded) == 1, "wildcard must short-circuit the runtime check"
|
||||
|
||||
|
||||
def test_load_skills_no_current_runtime_loads_everything(tmp_path):
|
||||
"""When current_runtime is None (test/fallback), no filtering happens."""
|
||||
_write_skill(tmp_path, "claude-only", "runtime:\n - claude-code\n")
|
||||
from unittest.mock import patch
|
||||
with patch("skill_loader.loader.load_skill_tools", return_value=[]):
|
||||
loaded = load_skills(str(tmp_path), ["claude-only"])
|
||||
assert len(loaded) == 1, "absent current_runtime must preserve old behavior"
|
||||
|
||||
|
||||
def test_load_skills_malformed_runtime_treated_as_universal(tmp_path, caplog):
|
||||
"""A garbage runtime value warns + falls back to universal — never silently drops the skill."""
|
||||
_write_skill(tmp_path, "garbage", "runtime: 123\n")
|
||||
from unittest.mock import patch
|
||||
import logging
|
||||
with caplog.at_level(logging.WARNING, logger="skill_loader.loader"):
|
||||
with patch("skill_loader.loader.load_skill_tools", return_value=[]):
|
||||
loaded = load_skills(str(tmp_path), ["garbage"], current_runtime="hermes")
|
||||
assert len(loaded) == 1, "malformed runtime must not silently filter"
|
||||
assert any("invalid `runtime`" in r.message for r in caplog.records)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user