From 259d8a018c7635749bd849a82928f6f6725e9a3e Mon Sep 17 00:00:00 2001 From: santhreal <64453045+santhreal@users.noreply.github.com> Date: Sat, 25 Jul 2026 21:47:05 -0700 Subject: [PATCH] fix(skills): tolerate null requires/bins/env in skill metadata --- nanobot/agent/skills.py | 24 +++++++++++------- tests/agent/test_skills_loader.py | 42 +++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 9 deletions(-) diff --git a/nanobot/agent/skills.py b/nanobot/agent/skills.py index 5d561f58..1e748f04 100644 --- a/nanobot/agent/skills.py +++ b/nanobot/agent/skills.py @@ -154,11 +154,21 @@ class SkillsLoader: sections.append("\n".join(lines)) 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: """Get a description of missing requirements.""" - requires = skill_meta.get("requires", {}) - required_bins = requires.get("bins", []) - required_env_vars = requires.get("env", []) + required_bins, required_env_vars = self._requirement_lists(skill_meta) return ", ".join( [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)] @@ -172,9 +182,7 @@ class SkillsLoader: def get_skill_requirements(self, name: str) -> dict[str, list[str]]: """Return explicit command/env requirements and currently missing entries.""" - requires = self._get_skill_meta(name).get("requires", {}) - bins = [str(value) for value in requires.get("bins", [])] - env = [str(value) for value in requires.get("env", [])] + bins, env = self._requirement_lists(self._get_skill_meta(name)) return { "bins": bins, "env": env, @@ -219,9 +227,7 @@ class SkillsLoader: def _check_requirements(self, skill_meta: dict) -> bool: """Check if skill requirements are met (bins, env vars).""" - requires = skill_meta.get("requires", {}) - required_bins = requires.get("bins", []) - required_env_vars = requires.get("env", []) + required_bins, required_env_vars = self._requirement_lists(skill_meta) return all(shutil.which(cmd) for cmd in required_bins) and all( os.environ.get(var) for var in required_env_vars ) diff --git a/tests/agent/test_skills_loader.py b/tests/agent/test_skills_loader.py index 6e6869a7..5229efac 100644 --- a/tests/agent/test_skills_loader.py +++ b/tests/agent/test_skills_loader.py @@ -436,3 +436,45 @@ def test_get_skill_metadata_handles_yaml_types(tmp_path: Path) -> None: assert meta.get("always") is True # metadata is a parsed dict, not a JSON string 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": [], + }