Revert "feat: skill prerequisites — hide skills with unmet runtime dependencies"
This commit is contained in:
parent
0df7df52f3
commit
b8120df860
17 changed files with 11 additions and 336 deletions
|
|
@ -8,7 +8,6 @@ from agent.prompt_builder import (
|
|||
_scan_context_content,
|
||||
_truncate_content,
|
||||
_read_skill_description,
|
||||
_skill_prerequisites_met,
|
||||
build_skills_system_prompt,
|
||||
build_context_files_prompt,
|
||||
CONTEXT_FILE_MAX_CHARS,
|
||||
|
|
@ -212,69 +211,6 @@ class TestBuildSkillsSystemPrompt:
|
|||
assert "imessage" in result
|
||||
assert "Send iMessages" in result
|
||||
|
||||
def test_excludes_skills_with_unmet_prerequisites(self, monkeypatch, tmp_path):
|
||||
"""Skills with missing env var prerequisites should not appear."""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
monkeypatch.delenv("MISSING_API_KEY_XYZ", raising=False)
|
||||
skills_dir = tmp_path / "skills" / "media"
|
||||
|
||||
gated = skills_dir / "gated-skill"
|
||||
gated.mkdir(parents=True)
|
||||
(gated / "SKILL.md").write_text(
|
||||
"---\nname: gated-skill\ndescription: Needs a key\n"
|
||||
"prerequisites:\n env_vars: [MISSING_API_KEY_XYZ]\n---\n"
|
||||
)
|
||||
|
||||
available = skills_dir / "free-skill"
|
||||
available.mkdir(parents=True)
|
||||
(available / "SKILL.md").write_text(
|
||||
"---\nname: free-skill\ndescription: No prereqs\n---\n"
|
||||
)
|
||||
|
||||
result = build_skills_system_prompt()
|
||||
assert "free-skill" in result
|
||||
assert "gated-skill" not in result
|
||||
|
||||
def test_includes_skills_with_met_prerequisites(self, monkeypatch, tmp_path):
|
||||
"""Skills with satisfied prerequisites should appear normally."""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
monkeypatch.setenv("MY_API_KEY", "test_value")
|
||||
skills_dir = tmp_path / "skills" / "media"
|
||||
|
||||
skill = skills_dir / "ready-skill"
|
||||
skill.mkdir(parents=True)
|
||||
(skill / "SKILL.md").write_text(
|
||||
"---\nname: ready-skill\ndescription: Has key\n"
|
||||
"prerequisites:\n env_vars: [MY_API_KEY]\n---\n"
|
||||
)
|
||||
|
||||
result = build_skills_system_prompt()
|
||||
assert "ready-skill" in result
|
||||
|
||||
|
||||
# =========================================================================
|
||||
# _skill_prerequisites_met
|
||||
# =========================================================================
|
||||
|
||||
|
||||
class TestSkillPrerequisitesMet:
|
||||
def test_met_or_absent(self, tmp_path, monkeypatch):
|
||||
"""No prereqs, met prereqs, and missing file all return True."""
|
||||
monkeypatch.setenv("PRESENT_KEY_123", "val")
|
||||
basic = tmp_path / "basic.md"
|
||||
basic.write_text("---\nname: basic\ndescription: basic\n---\n")
|
||||
ready = tmp_path / "ready.md"
|
||||
ready.write_text("---\nname: ready\ndescription: ready\nprerequisites:\n env_vars: [PRESENT_KEY_123]\n---\n")
|
||||
assert _skill_prerequisites_met(basic) is True
|
||||
assert _skill_prerequisites_met(ready) is True
|
||||
assert _skill_prerequisites_met(tmp_path / "nope.md") is True
|
||||
|
||||
def test_unmet_returns_false(self, tmp_path, monkeypatch):
|
||||
monkeypatch.delenv("NONEXISTENT_KEY_ABC", raising=False)
|
||||
skill = tmp_path / "SKILL.md"
|
||||
skill.write_text("---\nname: gated\ndescription: gated\nprerequisites:\n env_vars: [NONEXISTENT_KEY_ABC]\n---\n")
|
||||
assert _skill_prerequisites_met(skill) is False
|
||||
|
||||
|
||||
# =========================================================================
|
||||
# Context files prompt builder
|
||||
|
|
|
|||
|
|
@ -11,7 +11,6 @@ from tools.skills_tool import (
|
|||
_estimate_tokens,
|
||||
_find_all_skills,
|
||||
_load_category_description,
|
||||
check_skill_prerequisites,
|
||||
skill_matches_platform,
|
||||
skills_list,
|
||||
skills_categories,
|
||||
|
|
@ -465,124 +464,3 @@ class TestFindAllSkillsPlatformFiltering:
|
|||
assert len(skills_darwin) == 1
|
||||
assert len(skills_linux) == 1
|
||||
assert len(skills_win) == 0
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# check_skill_prerequisites
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestCheckSkillPrerequisites:
|
||||
def test_no_or_empty_prerequisites(self):
|
||||
"""No field, empty dict, or non-dict all pass."""
|
||||
assert check_skill_prerequisites({})[0] is True
|
||||
assert check_skill_prerequisites({"prerequisites": {}})[0] is True
|
||||
assert check_skill_prerequisites({"prerequisites": "curl"})[0] is True
|
||||
|
||||
def test_env_var_present_and_missing(self, monkeypatch):
|
||||
monkeypatch.setenv("MY_TEST_KEY", "val")
|
||||
monkeypatch.delenv("NONEXISTENT_TEST_VAR_XYZ", raising=False)
|
||||
assert check_skill_prerequisites({"prerequisites": {"env_vars": ["MY_TEST_KEY"]}})[0] is True
|
||||
met, missing = check_skill_prerequisites({"prerequisites": {"env_vars": ["NONEXISTENT_TEST_VAR_XYZ"]}})
|
||||
assert met is False
|
||||
assert "env $NONEXISTENT_TEST_VAR_XYZ" in missing
|
||||
|
||||
def test_command_present_and_missing(self):
|
||||
assert check_skill_prerequisites({"prerequisites": {"commands": ["python3"]}})[0] is True
|
||||
met, missing = check_skill_prerequisites({"prerequisites": {"commands": ["nonexistent_binary_xyz_123"]}})
|
||||
assert met is False
|
||||
assert "command `nonexistent_binary_xyz_123`" in missing
|
||||
|
||||
def test_mixed_env_and_commands(self, monkeypatch):
|
||||
monkeypatch.delenv("MISSING_A", raising=False)
|
||||
met, missing = check_skill_prerequisites({
|
||||
"prerequisites": {
|
||||
"env_vars": ["MISSING_A"],
|
||||
"commands": ["python3", "nonexistent_cmd_xyz"],
|
||||
}
|
||||
})
|
||||
assert met is False
|
||||
assert len(missing) == 2
|
||||
|
||||
def test_string_instead_of_list(self, monkeypatch):
|
||||
"""YAML scalar (string) should be coerced to a single-element list."""
|
||||
monkeypatch.delenv("SOLO_VAR", raising=False)
|
||||
assert check_skill_prerequisites({"prerequisites": {"env_vars": "SOLO_VAR"}})[0] is False
|
||||
assert check_skill_prerequisites({"prerequisites": {"commands": "nonexistent_cmd_xyz_solo"}})[0] is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _find_all_skills — prerequisites integration
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestFindAllSkillsPrerequisites:
|
||||
def test_skills_with_unmet_prereqs_flagged(self, tmp_path, monkeypatch):
|
||||
monkeypatch.delenv("NONEXISTENT_API_KEY_XYZ", raising=False)
|
||||
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||
_make_skill(
|
||||
tmp_path, "needs-key",
|
||||
frontmatter_extra="prerequisites:\n env_vars: [NONEXISTENT_API_KEY_XYZ]\n",
|
||||
)
|
||||
skills = _find_all_skills()
|
||||
assert len(skills) == 1
|
||||
assert skills[0]["prerequisites_met"] is False
|
||||
assert any("NONEXISTENT_API_KEY_XYZ" in m for m in skills[0]["prerequisites_missing"])
|
||||
|
||||
def test_skills_with_met_prereqs_no_flag(self, tmp_path, monkeypatch):
|
||||
monkeypatch.setenv("MY_PRESENT_KEY", "val")
|
||||
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||
_make_skill(
|
||||
tmp_path, "has-key",
|
||||
frontmatter_extra="prerequisites:\n env_vars: [MY_PRESENT_KEY]\n",
|
||||
)
|
||||
skills = _find_all_skills()
|
||||
assert len(skills) == 1
|
||||
assert "prerequisites_met" not in skills[0]
|
||||
|
||||
def test_skills_without_prereqs_no_flag(self, tmp_path):
|
||||
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||
_make_skill(tmp_path, "simple-skill")
|
||||
skills = _find_all_skills()
|
||||
assert len(skills) == 1
|
||||
assert "prerequisites_met" not in skills[0]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# skill_view — prerequisites warnings
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestSkillViewPrerequisites:
|
||||
def test_warns_on_unmet_prerequisites(self, tmp_path, monkeypatch):
|
||||
monkeypatch.delenv("MISSING_KEY_XYZ", raising=False)
|
||||
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||
_make_skill(
|
||||
tmp_path, "gated-skill",
|
||||
frontmatter_extra="prerequisites:\n env_vars: [MISSING_KEY_XYZ]\n",
|
||||
)
|
||||
raw = skill_view("gated-skill")
|
||||
result = json.loads(raw)
|
||||
assert result["success"] is True
|
||||
assert result["prerequisites_met"] is False
|
||||
assert "MISSING_KEY_XYZ" in result["prerequisites_warning"]
|
||||
|
||||
def test_no_warning_when_prereqs_met(self, tmp_path, monkeypatch):
|
||||
monkeypatch.setenv("PRESENT_KEY", "value")
|
||||
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||
_make_skill(
|
||||
tmp_path, "ready-skill",
|
||||
frontmatter_extra="prerequisites:\n env_vars: [PRESENT_KEY]\n",
|
||||
)
|
||||
raw = skill_view("ready-skill")
|
||||
result = json.loads(raw)
|
||||
assert result["success"] is True
|
||||
assert "prerequisites_warning" not in result
|
||||
|
||||
def test_no_warning_when_no_prereqs(self, tmp_path):
|
||||
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||
_make_skill(tmp_path, "plain-skill")
|
||||
raw = skill_view("plain-skill")
|
||||
result = json.loads(raw)
|
||||
assert result["success"] is True
|
||||
assert "prerequisites_warning" not in result
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue