fix(skills): tolerate null requires/bins/env in skill metadata
This commit is contained in:
+15
-9
@@ -154,11 +154,21 @@ class SkillsLoader:
|
|||||||
sections.append("\n".join(lines))
|
sections.append("\n".join(lines))
|
||||||
return "\n\n".join(sections)
|
return "\n\n".join(sections)
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _requirement_lists(skill_meta: dict) -> tuple[list[str], list[str]]:
|
||||||
|
"""Return (bins, env) lists from skill metadata, tolerating null/wrong shapes."""
|
||||||
|
requires = skill_meta.get("requires") or {}
|
||||||
|
if not isinstance(requires, dict):
|
||||||
|
return [], []
|
||||||
|
bins_raw = requires.get("bins") or []
|
||||||
|
env_raw = requires.get("env") or []
|
||||||
|
bins = [str(v) for v in bins_raw if isinstance(v, str) and v.strip()] if isinstance(bins_raw, list) else []
|
||||||
|
env = [str(v) for v in env_raw if isinstance(v, str) and v.strip()] if isinstance(env_raw, list) else []
|
||||||
|
return bins, env
|
||||||
|
|
||||||
def _get_missing_requirements(self, skill_meta: dict) -> str:
|
def _get_missing_requirements(self, skill_meta: dict) -> str:
|
||||||
"""Get a description of missing requirements."""
|
"""Get a description of missing requirements."""
|
||||||
requires = skill_meta.get("requires", {})
|
required_bins, required_env_vars = self._requirement_lists(skill_meta)
|
||||||
required_bins = requires.get("bins", [])
|
|
||||||
required_env_vars = requires.get("env", [])
|
|
||||||
return ", ".join(
|
return ", ".join(
|
||||||
[f"CLI: {command_name}" for command_name in required_bins if not shutil.which(command_name)]
|
[f"CLI: {command_name}" for command_name in required_bins if not shutil.which(command_name)]
|
||||||
+ [f"ENV: {env_name}" for env_name in required_env_vars if not os.environ.get(env_name)]
|
+ [f"ENV: {env_name}" for env_name in required_env_vars if not os.environ.get(env_name)]
|
||||||
@@ -172,9 +182,7 @@ class SkillsLoader:
|
|||||||
|
|
||||||
def get_skill_requirements(self, name: str) -> dict[str, list[str]]:
|
def get_skill_requirements(self, name: str) -> dict[str, list[str]]:
|
||||||
"""Return explicit command/env requirements and currently missing entries."""
|
"""Return explicit command/env requirements and currently missing entries."""
|
||||||
requires = self._get_skill_meta(name).get("requires", {})
|
bins, env = self._requirement_lists(self._get_skill_meta(name))
|
||||||
bins = [str(value) for value in requires.get("bins", [])]
|
|
||||||
env = [str(value) for value in requires.get("env", [])]
|
|
||||||
return {
|
return {
|
||||||
"bins": bins,
|
"bins": bins,
|
||||||
"env": env,
|
"env": env,
|
||||||
@@ -219,9 +227,7 @@ class SkillsLoader:
|
|||||||
|
|
||||||
def _check_requirements(self, skill_meta: dict) -> bool:
|
def _check_requirements(self, skill_meta: dict) -> bool:
|
||||||
"""Check if skill requirements are met (bins, env vars)."""
|
"""Check if skill requirements are met (bins, env vars)."""
|
||||||
requires = skill_meta.get("requires", {})
|
required_bins, required_env_vars = self._requirement_lists(skill_meta)
|
||||||
required_bins = requires.get("bins", [])
|
|
||||||
required_env_vars = requires.get("env", [])
|
|
||||||
return all(shutil.which(cmd) for cmd in required_bins) and all(
|
return all(shutil.which(cmd) for cmd in required_bins) and all(
|
||||||
os.environ.get(var) for var in required_env_vars
|
os.environ.get(var) for var in required_env_vars
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -436,3 +436,45 @@ def test_get_skill_metadata_handles_yaml_types(tmp_path: Path) -> None:
|
|||||||
assert meta.get("always") is True
|
assert meta.get("always") is True
|
||||||
# metadata is a parsed dict, not a JSON string
|
# metadata is a parsed dict, not a JSON string
|
||||||
assert isinstance(meta.get("metadata"), dict)
|
assert isinstance(meta.get("metadata"), dict)
|
||||||
|
|
||||||
|
|
||||||
|
def test_check_requirements_tolerates_null_requires_and_lists(tmp_path: Path) -> None:
|
||||||
|
"""Null requires/bins/env must not crash skill listing (JSON/YAML nulls)."""
|
||||||
|
workspace = tmp_path / "ws"
|
||||||
|
ws_skills = workspace / "skills"
|
||||||
|
ws_skills.mkdir(parents=True)
|
||||||
|
_write_skill(
|
||||||
|
ws_skills,
|
||||||
|
"null-requires",
|
||||||
|
metadata_json={"always": True, "requires": None},
|
||||||
|
body="# Null requires",
|
||||||
|
)
|
||||||
|
_write_skill(
|
||||||
|
ws_skills,
|
||||||
|
"null-bins",
|
||||||
|
metadata_json={"always": True, "requires": {"bins": None, "env": None}},
|
||||||
|
body="# Null bins",
|
||||||
|
)
|
||||||
|
_write_skill(
|
||||||
|
ws_skills,
|
||||||
|
"null-elems",
|
||||||
|
metadata_json={"always": True, "requires": {"bins": [None, ""], "env": [None]}},
|
||||||
|
body="# Null elems",
|
||||||
|
)
|
||||||
|
builtin = tmp_path / "builtin"
|
||||||
|
builtin.mkdir()
|
||||||
|
loader = SkillsLoader(workspace, builtin_skills_dir=builtin)
|
||||||
|
|
||||||
|
assert loader._check_requirements(loader._get_skill_meta("null-requires")) is True
|
||||||
|
assert loader._check_requirements(loader._get_skill_meta("null-bins")) is True
|
||||||
|
assert loader._check_requirements(loader._get_skill_meta("null-elems")) is True
|
||||||
|
always = loader.get_always_skills()
|
||||||
|
assert set(always) >= {"null-requires", "null-bins", "null-elems"}
|
||||||
|
listed = {e["name"] for e in loader.list_skills(filter_unavailable=True)}
|
||||||
|
assert {"null-requires", "null-bins", "null-elems"} <= listed
|
||||||
|
assert loader.get_skill_requirements("null-requires") == {
|
||||||
|
"bins": [],
|
||||||
|
"env": [],
|
||||||
|
"missing_bins": [],
|
||||||
|
"missing_env": [],
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user