ouyangbo/EvoScientist-Multi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
a45563ea7f011461805f6bb66cfaf1dd90eabe54
EvoScientist-Multi/tests/test_skills_manager.py
T

1869 lines
68 KiB
Python
Raw Blame History

"""Tests for EvoScientist.tools.skills_manager module."""
from pathlib import Path
from unittest.mock import patch
import pytest
from EvoScientist.tools.skills_manager import (
SkillInfo,
_is_github_url,
_load_manifest,
_parse_github_url,
_parse_skill_md,
_record_install,
_reset_skills_changed_callbacks,
_validate_skill_dir,
fetch_remote_skill_index,
get_all_tags,
install_skill,
installed_provenance,
installed_sources,
list_expert_skills,
list_skills,
list_skills_by_tag,
register_skills_changed_callback,
resolve_remote_head,
uninstall_skill,
)
# =============================================================================
# Fixtures
# =============================================================================
@pytest.fixture
def temp_skills_dir(tmp_path):
"""Create a temporary skills directory, isolated from the real global tier."""
skills_dir = tmp_path / "skills"
skills_dir.mkdir()
empty_global = tmp_path / "global_skills"
empty_global.mkdir()
with patch("EvoScientist.paths.GLOBAL_SKILLS_DIR", empty_global):
yield skills_dir
@pytest.fixture
def sample_skill_dir(tmp_path):
"""Create a sample skill directory with SKILL.md."""
skill_dir = tmp_path / "sample-skill"
skill_dir.mkdir()
skill_md = skill_dir / "SKILL.md"
skill_md.write_text(
"""---
name: sample-skill
description: A sample skill for testing
---
# Sample Skill
This is a sample skill for testing purposes.
"""
)
return skill_dir
@pytest.fixture
def sample_skill_no_frontmatter(tmp_path):
"""Create a skill directory without YAML frontmatter."""
skill_dir = tmp_path / "no-frontmatter-skill"
skill_dir.mkdir()
skill_md = skill_dir / "SKILL.md"
skill_md.write_text(
"""# No Frontmatter Skill
This skill has no YAML frontmatter.
"""
)
return skill_dir
# =============================================================================
# Tests for _parse_skill_md
# =============================================================================
class TestParseSkillMd:
"""Tests for _parse_skill_md function."""
def test_parse_with_frontmatter(self, sample_skill_dir):
skill_md = sample_skill_dir / "SKILL.md"
result = _parse_skill_md(skill_md)
assert result.name == "sample-skill"
assert result.description == "A sample skill for testing"
def test_parse_without_frontmatter(self, sample_skill_no_frontmatter):
skill_md = sample_skill_no_frontmatter / "SKILL.md"
result = _parse_skill_md(skill_md)
# Should use directory name as fallback
assert result.name == "no-frontmatter-skill"
assert result.description == "(no description)"
def test_parse_with_empty_description_value(self, tmp_path):
"""A present-but-empty ``description:`` parses to YAML None; guard it."""
skill_dir = tmp_path / "empty-desc-skill"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text(
"""---
name: empty-desc-skill
description:
---
# Body
"""
)
result = _parse_skill_md(skill_dir / "SKILL.md")
assert result.description == "(no description)"
def test_parse_with_partial_frontmatter(self, tmp_path):
skill_dir = tmp_path / "partial-skill"
skill_dir.mkdir()
skill_md = skill_dir / "SKILL.md"
skill_md.write_text(
"""---
name: my-skill
---
# My Skill
"""
)
result = _parse_skill_md(skill_md)
assert result.name == "my-skill"
assert result.description == "(no description)"
# =============================================================================
# Tests for _parse_github_url
# =============================================================================
class TestParseGithubUrl:
"""Tests for _parse_github_url function."""
def test_parse_full_url_with_path(self):
url = "https://github.com/owner/repo/tree/main/my-skill"
repo, ref, path = _parse_github_url(url)
assert repo == "owner/repo"
assert ref == "main"
assert path == "my-skill"
def test_parse_full_url_without_path(self):
url = "https://github.com/owner/repo/tree/develop"
repo, ref, path = _parse_github_url(url)
assert repo == "owner/repo"
assert ref == "develop"
assert path is None
def test_parse_simple_repo_url(self):
url = "https://github.com/owner/repo"
repo, ref, path = _parse_github_url(url)
assert repo == "owner/repo"
assert ref is None
assert path is None
def test_parse_shorthand(self):
url = "owner/repo@my-skill"
repo, ref, path = _parse_github_url(url)
assert repo == "owner/repo"
assert ref is None
assert path == "my-skill"
def test_parse_url_without_protocol(self):
url = "github.com/owner/repo/tree/v1.0/path/to/skill"
repo, ref, path = _parse_github_url(url)
assert repo == "owner/repo"
assert ref == "v1.0"
assert path == "path/to/skill"
def test_invalid_url_raises(self):
with pytest.raises(ValueError, match="Cannot parse"):
_parse_github_url("not-a-valid-url")
# =============================================================================
# Tests for _is_github_url
# =============================================================================
class TestIsGithubUrl:
"""Tests for _is_github_url function."""
def test_github_com_url(self):
assert _is_github_url("https://github.com/owner/repo") is True
assert _is_github_url("http://github.com/owner/repo/tree/main/skill") is True
def test_shorthand(self):
assert _is_github_url("owner/repo@skill-name") is True
def test_local_path(self):
assert _is_github_url("./my-skill") is False
assert _is_github_url("/absolute/path/skill") is False
assert _is_github_url("../relative/path") is False
def test_other_urls(self):
assert _is_github_url("https://gitlab.com/owner/repo") is False
assert _is_github_url("file:///path/to/file") is False
# =============================================================================
# Tests for _validate_skill_dir
# =============================================================================
class TestValidateSkillDir:
"""Tests for _validate_skill_dir function."""
def test_valid_skill_dir(self, sample_skill_dir):
assert _validate_skill_dir(sample_skill_dir) is True
def test_invalid_skill_dir_no_skillmd(self, tmp_path):
empty_dir = tmp_path / "empty"
empty_dir.mkdir()
assert _validate_skill_dir(empty_dir) is False
def test_invalid_skill_dir_file_not_dir(self, tmp_path):
file_path = tmp_path / "file.txt"
file_path.write_text("not a directory")
assert _validate_skill_dir(file_path) is False
# =============================================================================
# Tests for install_skill
# =============================================================================
class TestInstallSkill:
"""Tests for install_skill function."""
def test_install_from_local_path(self, sample_skill_dir, temp_skills_dir):
result = install_skill(str(sample_skill_dir), str(temp_skills_dir))
assert result["success"] is True
assert result["name"] == "sample-skill"
assert "sample-skill" in result["path"]
# Verify the skill was copied
installed_path = Path(result["path"])
assert installed_path.exists()
assert (installed_path / "SKILL.md").exists()
def test_install_nonexistent_path(self, temp_skills_dir):
result = install_skill("/nonexistent/path", str(temp_skills_dir))
assert result["success"] is False
assert "does not exist" in result["error"]
def test_install_invalid_skill_no_skillmd(self, tmp_path, temp_skills_dir):
empty_dir = tmp_path / "empty-skill"
empty_dir.mkdir()
result = install_skill(str(empty_dir), str(temp_skills_dir))
assert result["success"] is False
assert "No SKILL.md" in result["error"]
def test_install_replaces_existing(self, sample_skill_dir, temp_skills_dir):
# Install first time
result1 = install_skill(str(sample_skill_dir), str(temp_skills_dir))
assert result1["success"] is True
# Modify the original skill
skill_md = sample_skill_dir / "SKILL.md"
skill_md.write_text(
"""---
name: sample-skill
description: Modified description
---
# Modified
"""
)
# Install again
result2 = install_skill(str(sample_skill_dir), str(temp_skills_dir))
assert result2["success"] is True
assert result2["description"] == "Modified description"
# =============================================================================
# Tests for list_skills
# =============================================================================
class TestListSkills:
"""Tests for list_skills function."""
def test_list_empty_dir(self, temp_skills_dir):
with patch("EvoScientist.paths.USER_SKILLS_DIR", temp_skills_dir):
skills = list_skills(include_system=False)
assert skills == []
def test_list_with_skills(self, sample_skill_dir, temp_skills_dir):
# Install a skill
install_skill(str(sample_skill_dir), str(temp_skills_dir))
with patch("EvoScientist.paths.USER_SKILLS_DIR", temp_skills_dir):
skills = list_skills(include_system=False)
assert len(skills) == 1
assert skills[0].name == "sample-skill"
assert skills[0].description == "A sample skill for testing"
assert skills[0].source == "workspace"
def test_list_multiple_skills(self, tmp_path, temp_skills_dir):
# Create and install multiple skills
for i in range(3):
skill_dir = tmp_path / f"skill-{i}"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text(
f"""---
name: skill-{i}
description: Skill number {i}
---
"""
)
install_skill(str(skill_dir), str(temp_skills_dir))
with patch("EvoScientist.paths.USER_SKILLS_DIR", temp_skills_dir):
skills = list_skills(include_system=False)
assert len(skills) == 3
names = [s.name for s in skills]
assert "skill-0" in names
assert "skill-1" in names
assert "skill-2" in names
# =============================================================================
# Tests for uninstall_skill
# =============================================================================
class TestUninstallSkill:
"""Tests for uninstall_skill function."""
def test_uninstall_existing_skill(self, sample_skill_dir, temp_skills_dir):
# Install first
install_skill(str(sample_skill_dir), str(temp_skills_dir))
with patch("EvoScientist.paths.USER_SKILLS_DIR", temp_skills_dir):
result = uninstall_skill("sample-skill")
assert result["success"] is True
# Verify the skill was removed
skill_path = temp_skills_dir / "sample-skill"
assert not skill_path.exists()
def test_uninstall_nonexistent_skill(self, temp_skills_dir):
with patch("EvoScientist.paths.USER_SKILLS_DIR", temp_skills_dir):
result = uninstall_skill("nonexistent-skill")
assert result["success"] is False
assert "not found" in result["error"]
# =============================================================================
# Tests for install manifest
# =============================================================================
class TestInstallManifest:
"""Tests for the per-tier .installed.yaml manifest."""
def _make_skill(self, parent: Path, name: str) -> Path:
d = parent / name
d.mkdir()
(d / "SKILL.md").write_text(
f"---\nname: {name}\ndescription: {name} description\n---\n\n# {name}\n"
)
return d
def test_local_install_records_source(self, sample_skill_dir, temp_skills_dir):
result = install_skill(str(sample_skill_dir), str(temp_skills_dir))
assert result["success"]
manifest = _load_manifest(temp_skills_dir)
# Local installs have no upstream commit — record source only.
assert manifest == {"sample-skill": {"source": str(sample_skill_dir)}}
def test_pack_install_records_one_source_for_all_children(
self, tmp_path, temp_skills_dir
):
"""A pack (root has no SKILL.md, multiple child skills) should record
the same user-facing source for every child skill, so detection by
source still works."""
repo = tmp_path / "evoskills-fake"
repo.mkdir()
self._make_skill(repo, "paper-writing")
self._make_skill(repo, "evo-memory")
self._make_skill(repo, "research-survey")
result = install_skill(str(repo), str(temp_skills_dir))
assert result["success"]
assert result["batch"]
manifest = _load_manifest(temp_skills_dir)
assert set(manifest) == {"paper-writing", "evo-memory", "research-survey"}
sources = {entry["source"] for entry in manifest.values()}
assert sources == {str(repo)}
def test_uninstall_clears_manifest_entry(self, sample_skill_dir, temp_skills_dir):
install_skill(str(sample_skill_dir), str(temp_skills_dir))
assert "sample-skill" in _load_manifest(temp_skills_dir)
with patch("EvoScientist.paths.USER_SKILLS_DIR", temp_skills_dir):
result = uninstall_skill("sample-skill")
assert result["success"]
assert "sample-skill" not in _load_manifest(temp_skills_dir)
def test_save_is_atomic_and_leaves_no_temp(self, sample_skill_dir, temp_skills_dir):
"""_save_manifest must rename a temp file into place, not overwrite,
so a crash mid-write can't leave a half-written manifest behind."""
install_skill(str(sample_skill_dir), str(temp_skills_dir))
manifest_files = sorted(p.name for p in temp_skills_dir.iterdir())
# Only the manifest itself should remain — no leftover .tmp siblings.
assert ".installed.yaml" in manifest_files
assert not any(name.endswith(".tmp") for name in manifest_files), (
f"unexpected temp file left behind: {manifest_files}"
)
def test_installed_sources_filters_missing_dirs(
self, sample_skill_dir, temp_skills_dir, tmp_path
):
"""If a skill dir was removed manually but the manifest entry lingers,
installed_sources() must not report it as installed."""
empty_global = tmp_path / "empty_global"
empty_global.mkdir()
with (
patch("EvoScientist.paths.USER_SKILLS_DIR", temp_skills_dir),
patch("EvoScientist.paths.GLOBAL_SKILLS_DIR", empty_global),
):
install_skill(str(sample_skill_dir), str(temp_skills_dir))
assert installed_sources() == {str(sample_skill_dir)}
# Manually wipe the dir, manifest still has the entry.
import shutil as _shutil
_shutil.rmtree(temp_skills_dir / "sample-skill")
assert "sample-skill" in _load_manifest(temp_skills_dir)
assert installed_sources() == set()
def test_record_install_persists_commit(self, temp_skills_dir, sample_skill_dir):
"""When _record_install gets a commit SHA it makes it through the
normalize-on-write pass and is readable as provenance."""
install_skill(str(sample_skill_dir), str(temp_skills_dir))
# Simulate a github install by manually recording a commit.
_record_install(
temp_skills_dir,
"sample-skill",
"owner/repo@skill",
commit="abc123def456",
)
with (
patch("EvoScientist.paths.USER_SKILLS_DIR", temp_skills_dir),
patch(
"EvoScientist.paths.GLOBAL_SKILLS_DIR",
temp_skills_dir.parent / "missing",
),
):
prov = installed_provenance()
assert prov == {"owner/repo@skill": {"commit": "abc123def456"}}
class TestResolveRemoteHead:
"""Tests for resolve_remote_head — the upstream-SHA helper used by onboard."""
def test_returns_none_for_local_path(self):
assert resolve_remote_head("/tmp/some/local/path") is None
def test_returns_none_when_git_unavailable(self):
with patch("EvoScientist.tools.skills_manager.subprocess.run") as run:
run.side_effect = FileNotFoundError("git not on PATH")
assert resolve_remote_head("owner/repo@skill") is None
def test_returns_none_on_timeout(self):
import subprocess as _sp
with patch("EvoScientist.tools.skills_manager.subprocess.run") as run:
run.side_effect = _sp.TimeoutExpired(cmd="git", timeout=5)
assert resolve_remote_head("owner/repo@skill") is None
def test_parses_first_sha_from_ls_remote_output(self):
proc = type(
"P",
(),
{
"returncode": 0,
"stdout": "deadbeef0000000000000000000000000000abcd\trefs/heads/main\n",
"stderr": "",
},
)()
with patch(
"EvoScientist.tools.skills_manager.subprocess.run", return_value=proc
):
sha = resolve_remote_head("owner/repo@skill")
assert sha == "deadbeef0000000000000000000000000000abcd"
# =============================================================================
# Tests for batch install
# =============================================================================
class TestBatchInstall:
"""Tests for batch installing multiple skills from one directory."""
def _make_skill(self, parent: Path, name: str, desc: str) -> Path:
"""Helper to create a minimal skill directory."""
d = parent / name
d.mkdir()
(d / "SKILL.md").write_text(
f"---\nname: {name}\ndescription: {desc}\n---\n\n# {name}\n"
)
return d
def test_batch_install_local_multiple_skills(self, tmp_path, temp_skills_dir):
"""Local path with no root SKILL.md but 3 sub-skills installs all."""
repo = tmp_path / "multi-repo"
repo.mkdir()
self._make_skill(repo, "skill-a", "Alpha")
self._make_skill(repo, "skill-b", "Beta")
self._make_skill(repo, "skill-c", "Gamma")
result = install_skill(str(repo), str(temp_skills_dir))
assert result["success"] is True
assert result.get("batch") is True
assert len(result["installed"]) == 3
assert result["failed"] == []
names = {r["name"] for r in result["installed"]}
assert names == {"skill-a", "skill-b", "skill-c"}
# Verify files copied
for name in names:
assert (temp_skills_dir / name / "SKILL.md").exists()
def test_batch_install_local_single_still_works(self, tmp_path, temp_skills_dir):
"""Local path with root SKILL.md still installs as single."""
self._make_skill(tmp_path, "single", "Just one")
result = install_skill(str(tmp_path / "single"), str(temp_skills_dir))
assert result["success"] is True
assert result.get("batch") is not True
assert result["name"] == "single"
def test_batch_install_local_empty_repo_fails(self, tmp_path, temp_skills_dir):
"""Local path with no skills at any level fails."""
empty = tmp_path / "empty-repo"
empty.mkdir()
result = install_skill(str(empty), str(temp_skills_dir))
assert result["success"] is False
assert "No SKILL.md" in result["error"]
def test_batch_install_local_mixed_dirs(self, tmp_path, temp_skills_dir):
"""Directories without SKILL.md are silently skipped."""
repo = tmp_path / "mixed"
repo.mkdir()
self._make_skill(repo, "real-skill", "Real")
(repo / "not-a-skill").mkdir() # no SKILL.md
(repo / "readme.md").write_text("# Readme") # file, not dir
result = install_skill(str(repo), str(temp_skills_dir))
assert result["success"] is True
assert result.get("batch") is not True # only 1 skill → single install
assert result["name"] == "real-skill"
# =============================================================================
# Tests for tag parsing
# =============================================================================
class TestParseSkillMdTags:
"""Tests for tag extraction in _parse_skill_md."""
def test_parse_with_metadata_tags(self, tmp_path):
"""Tags under metadata.tags are extracted."""
skill_dir = tmp_path / "tagged-skill"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text(
"""---
name: tagged-skill
description: A skill with tags
metadata:
tags: [core, research, ideation]
---
"""
)
result = _parse_skill_md(skill_dir / "SKILL.md")
assert result.tags == ["core", "research", "ideation"]
def test_parse_with_top_level_tags(self, tmp_path):
"""Top-level tags field takes precedence over metadata.tags."""
skill_dir = tmp_path / "top-tags"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text(
"""---
name: top-tags
description: Top-level tags
tags: [writing, review]
metadata:
tags: [should, not, appear]
---
"""
)
result = _parse_skill_md(skill_dir / "SKILL.md")
assert result.tags == ["writing", "review"]
def test_parse_no_tags(self, tmp_path):
"""Skills without tags return empty list."""
skill_dir = tmp_path / "no-tags"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text(
"""---
name: no-tags
description: No tags at all
---
"""
)
result = _parse_skill_md(skill_dir / "SKILL.md")
assert result.tags == []
def test_parse_comma_string_tags(self, tmp_path):
"""Tags given as comma-separated string are split into list."""
skill_dir = tmp_path / "string-tags"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text(
"""---
name: string-tags
description: Tags as string
tags: "core, research, writing"
---
"""
)
result = _parse_skill_md(skill_dir / "SKILL.md")
assert result.tags == ["core", "research", "writing"]
def test_parse_no_frontmatter_returns_empty_tags(self, sample_skill_no_frontmatter):
"""Skills without frontmatter return empty tags."""
result = _parse_skill_md(sample_skill_no_frontmatter / "SKILL.md")
assert result.tags == []
# =============================================================================
# Tests for list_skills_by_tag
# =============================================================================
class TestListSkillsByTag:
"""Tests for list_skills_by_tag function."""
def _make_tagged_skill(self, parent: Path, name: str, tags: list[str]) -> Path:
d = parent / name
d.mkdir()
tags_yaml = ", ".join(tags)
(d / "SKILL.md").write_text(
f"---\nname: {name}\ndescription: Skill {name}\n"
f"metadata:\n tags: [{tags_yaml}]\n---\n"
)
return d
def test_filter_by_tag(self, tmp_path, temp_skills_dir):
self._make_tagged_skill(temp_skills_dir, "skill-a", ["core", "writing"])
self._make_tagged_skill(temp_skills_dir, "skill-b", ["core", "research"])
self._make_tagged_skill(temp_skills_dir, "skill-c", ["research"])
with patch("EvoScientist.paths.USER_SKILLS_DIR", temp_skills_dir):
core = list_skills_by_tag("core")
assert len(core) == 2
assert {s.name for s in core} == {"skill-a", "skill-b"}
research = list_skills_by_tag("research")
assert len(research) == 2
assert {s.name for s in research} == {"skill-b", "skill-c"}
writing = list_skills_by_tag("writing")
assert len(writing) == 1
assert writing[0].name == "skill-a"
def test_filter_case_insensitive(self, tmp_path, temp_skills_dir):
self._make_tagged_skill(temp_skills_dir, "skill-x", ["Core", "Writing"])
with patch("EvoScientist.paths.USER_SKILLS_DIR", temp_skills_dir):
result = list_skills_by_tag("core")
assert len(result) == 1
assert result[0].name == "skill-x"
def test_filter_nonexistent_tag(self, tmp_path, temp_skills_dir):
self._make_tagged_skill(temp_skills_dir, "skill-y", ["core"])
with patch("EvoScientist.paths.USER_SKILLS_DIR", temp_skills_dir):
result = list_skills_by_tag("nonexistent")
assert result == []
# =============================================================================
# Tests for get_all_tags
# =============================================================================
class TestGetAllTags:
"""Tests for get_all_tags function."""
def _make_tagged_skill(self, parent: Path, name: str, tags: list[str]) -> Path:
d = parent / name
d.mkdir()
tags_yaml = ", ".join(tags)
(d / "SKILL.md").write_text(
f"---\nname: {name}\ndescription: Skill {name}\n"
f"metadata:\n tags: [{tags_yaml}]\n---\n"
)
return d
def test_returns_tags_with_counts(self, tmp_path, temp_skills_dir):
self._make_tagged_skill(temp_skills_dir, "skill-a", ["core", "writing"])
self._make_tagged_skill(temp_skills_dir, "skill-b", ["core", "research"])
self._make_tagged_skill(temp_skills_dir, "skill-c", ["research"])
with patch("EvoScientist.paths.USER_SKILLS_DIR", temp_skills_dir):
tags = get_all_tags()
tag_dict = dict(tags)
assert tag_dict["core"] == 2
assert tag_dict["research"] == 2
assert tag_dict["writing"] == 1
def test_empty_when_no_skills(self, temp_skills_dir):
with patch("EvoScientist.paths.USER_SKILLS_DIR", temp_skills_dir):
tags = get_all_tags()
assert tags == []
# =============================================================================
# Tests for fetch_remote_skill_index
# =============================================================================
class TestFetchRemoteSkillIndex:
"""Tests for fetch_remote_skill_index function."""
def test_fetch_from_local_clone(self, tmp_path):
"""Verify index is built correctly from cloned skills."""
# Create a fake repo structure
skills_root = tmp_path / "repo" / "skills"
skills_root.mkdir(parents=True)
for name, tags in [
("skill-a", ["core", "writing"]),
("skill-b", ["core", "research"]),
]:
d = skills_root / name
d.mkdir()
tags_yaml = ", ".join(tags)
(d / "SKILL.md").write_text(
f"---\nname: {name}\ndescription: Skill {name}\n"
f"metadata:\n tags: [{tags_yaml}]\n---\n"
)
# Mock _clone_repo to copy our fake repo to the temp dir
def fake_clone(repo, ref, dest):
import shutil
shutil.copytree(tmp_path / "repo", dest)
with patch(
"EvoScientist.tools.skills_manager._clone_repo", side_effect=fake_clone
):
# Clear cache to ensure fresh fetch
from EvoScientist.tools.skills_manager import _REMOTE_INDEX_CACHE
_REMOTE_INDEX_CACHE.clear()
index = fetch_remote_skill_index(repo="test/repo", path="skills")
assert len(index) == 2
names = {s["name"] for s in index}
assert names == {"skill-a", "skill-b"}
# Verify tags are populated
skill_a = next(s for s in index if s["name"] == "skill-a")
assert "core" in skill_a["tags"]
assert "writing" in skill_a["tags"]
# Verify install_source is set
assert "test/repo@" in skill_a["install_source"]
def test_fetch_caches_results(self, tmp_path):
"""Second call within TTL uses cache without cloning again."""
skills_root = tmp_path / "repo" / "skills"
skills_root.mkdir(parents=True)
d = skills_root / "cached-skill"
d.mkdir()
(d / "SKILL.md").write_text(
"---\nname: cached-skill\ndescription: Cached\n"
"metadata:\n tags: [core]\n---\n"
)
call_count = 0
def fake_clone(repo, ref, dest):
nonlocal call_count
call_count += 1
import shutil
shutil.copytree(tmp_path / "repo", dest)
with patch(
"EvoScientist.tools.skills_manager._clone_repo", side_effect=fake_clone
):
from EvoScientist.tools.skills_manager import _REMOTE_INDEX_CACHE
_REMOTE_INDEX_CACHE.clear()
index1 = fetch_remote_skill_index(repo="cache/test", path="skills")
index2 = fetch_remote_skill_index(repo="cache/test", path="skills")
assert call_count == 1 # Only cloned once
assert index1 == index2
# =============================================================================
# Tests for skill_manager tool — action="list" filtering
# =============================================================================
class TestSkillManagerList:
"""Tests for the skill_manager() tool's action='list' output.
These tests verify that the source-based filtering in skill_manager.py
correctly maps skills_manager.py's tier names ("workspace", "global",
"builtin") to the User Skills / System Skills display sections.
"""
def _make_skill(self, tmp_path, name, description="A skill"):
skill_dir = tmp_path / name
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text(
f"---\nname: {name}\ndescription: {description}\n---\n"
)
return skill_dir
def test_list_user_skills_workspace(self, tmp_path):
"""Workspace-tier skills appear under 'User Skills'."""
from EvoScientist.tools.skill_manager import skill_manager
workspace_dir = tmp_path / "workspace"
workspace_dir.mkdir()
global_dir = tmp_path / "global"
global_dir.mkdir()
self._make_skill(tmp_path, "ws-skill")
install_skill(str(tmp_path / "ws-skill"), str(workspace_dir))
with (
patch("EvoScientist.paths.USER_SKILLS_DIR", workspace_dir),
patch("EvoScientist.paths.GLOBAL_SKILLS_DIR", global_dir),
):
result = skill_manager.invoke({"action": "list", "include_system": False})
assert "User Skills (1)" in result
assert "ws-skill" in result
assert "System Skills" not in result
def test_list_user_skills_global(self, tmp_path):
"""Global-tier skills appear under 'User Skills'."""
from EvoScientist.tools.skill_manager import skill_manager
workspace_dir = tmp_path / "workspace"
workspace_dir.mkdir()
global_dir = tmp_path / "global"
global_dir.mkdir()
self._make_skill(tmp_path, "global-skill")
install_skill(str(tmp_path / "global-skill"), str(global_dir))
with (
patch("EvoScientist.paths.USER_SKILLS_DIR", workspace_dir),
patch("EvoScientist.paths.GLOBAL_SKILLS_DIR", global_dir),
):
result = skill_manager.invoke({"action": "list", "include_system": False})
assert "User Skills (1)" in result
assert "global-skill" in result
def test_list_include_system_shows_both_sections(self, tmp_path):
"""include_system=True shows both User Skills and System Skills sections."""
from EvoScientist.tools.skill_manager import skill_manager
from EvoScientist.tools.skills_manager import SkillInfo
workspace_dir = tmp_path / "workspace"
workspace_dir.mkdir()
global_dir = tmp_path / "global"
global_dir.mkdir()
self._make_skill(tmp_path, "user-skill")
install_skill(str(tmp_path / "user-skill"), str(workspace_dir))
builtin_skill = SkillInfo(
name="builtin-skill",
description="A built-in skill",
path=tmp_path / "builtin-skill",
source="builtin",
)
with (
patch("EvoScientist.paths.USER_SKILLS_DIR", workspace_dir),
patch("EvoScientist.paths.GLOBAL_SKILLS_DIR", global_dir),
patch(
"EvoScientist.tools.skills_manager.list_skills",
return_value=[
SkillInfo(
name="user-skill",
description="A user skill",
path=workspace_dir / "user-skill",
source="workspace",
),
builtin_skill,
],
),
):
result = skill_manager.invoke({"action": "list", "include_system": True})
assert "User Skills (1)" in result
assert "user-skill" in result
assert "System Skills (1)" in result
assert "builtin-skill" in result
def test_list_no_user_skills_returns_message(self, tmp_path):
"""Empty workspace and global dirs return the 'no user skills' message."""
from EvoScientist.tools.skill_manager import skill_manager
workspace_dir = tmp_path / "workspace"
workspace_dir.mkdir()
global_dir = tmp_path / "global"
global_dir.mkdir()
with (
patch("EvoScientist.paths.USER_SKILLS_DIR", workspace_dir),
patch("EvoScientist.paths.GLOBAL_SKILLS_DIR", global_dir),
):
result = skill_manager.invoke({"action": "list", "include_system": False})
assert "No user skills installed" in result
# =============================================================================
# Tests for expert-skill fields (agent-teams v1)
# =============================================================================
def _write_expert_skill(
parent: Path,
name: str,
*,
role: str = "One-line role",
byline: str = "Test persona",
capability_tags: list[str] | None = None,
avatar_hint: str = "star",
include_description: bool = True,
) -> Path:
"""Write an expert SKILL.md under parent/<name>/ and return the skill dir."""
skill_dir = parent / name
skill_dir.mkdir(parents=True, exist_ok=True)
tags_str = "[" + ", ".join(capability_tags or []) + "]" if capability_tags else "[]"
desc_line = f"description: A {name} expert skill\n" if include_description else ""
(skill_dir / "SKILL.md").write_text(
f"""---
name: {name}
{desc_line}type: expert
role: {role}
byline: {byline}
capability_tags: {tags_str}
avatar_hint: {avatar_hint}
---
# {name}
Expert-skill body.
"""
)
return skill_dir
class TestParseSkillMdExpertFields:
"""`_parse_skill_md` extracts expert-skill frontmatter fields onto SkillInfo."""
def test_utility_default_when_type_absent(self, sample_skill_dir):
"""Existing skills (no `type` field) default to utility with empty expert fields."""
result = _parse_skill_md(sample_skill_dir / "SKILL.md")
assert result.type == "utility"
assert result.role == ""
assert result.byline == ""
assert result.capability_tags == []
assert result.avatar_hint == ""
def test_expert_fields_extracted(self, tmp_path):
skill_dir = _write_expert_skill(
tmp_path,
"expert-a",
role="Expert in A",
byline="A Byline",
capability_tags=["tag-1", "tag-2"],
avatar_hint="atom",
)
result = _parse_skill_md(skill_dir / "SKILL.md")
assert result.type == "expert"
assert result.role == "Expert in A"
assert result.byline == "A Byline"
assert result.capability_tags == ["tag-1", "tag-2"]
assert result.avatar_hint == "atom"
def test_body_populated_from_skill_md(self, tmp_path):
"""`SkillInfo.body` carries the post-frontmatter content so the expert
container factory can build the system_prompt without re-reading disk."""
skill_dir = _write_expert_skill(tmp_path, "expert-body")
result = _parse_skill_md(skill_dir / "SKILL.md")
assert "# expert-body" in result.body
assert "Expert-skill body." in result.body
# Frontmatter fences must NOT leak into the body.
assert "---" not in result.body
def test_body_captured_when_no_frontmatter(self, tmp_path):
"""Skills without a frontmatter block still cache their full text as
the body — the expert-container factory can then use it without a
second disk read."""
skill_dir = tmp_path / "no-frontmatter"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text("Just body content, no fences.\n")
result = _parse_skill_md(skill_dir / "SKILL.md")
assert result.body.strip() == "Just body content, no fences."
def test_unknown_type_falls_back_to_utility(self, tmp_path, caplog):
"""A typo in `type` (e.g. `charcter`) must not silently register as an expert
and must log so authors can debug a missing-expert case."""
skill_dir = tmp_path / "typo-skill"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text(
"""---
name: typo-skill
description: Has a bad type value
type: charcter
role: This should be ignored
---
# Body
"""
)
result = _parse_skill_md(skill_dir / "SKILL.md")
assert result.type == "utility"
assert any(
"unrecognized type" in r.message and "charcter" in r.message
for r in caplog.records
)
def test_default_dispatch_frontmatter_is_not_read(self, tmp_path):
"""A skill cannot pin its own dispatch shape.
``default_dispatch`` used to partition experts into two disjoint
registries, which is how an installed expert could end up reachable
by nothing. Nothing consumes the field now — the orchestrator picks
a reach per task — so it must not reappear on ``SkillInfo``.
"""
skill_dir = tmp_path / "async-skill"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text(
"""---
name: async-skill
description: declares a dispatch shape the runtime ignores
type: expert
role: Some role
default_dispatch: async
---
# Body
"""
)
result = _parse_skill_md(skill_dir / "SKILL.md")
assert result.type == "expert"
assert not hasattr(result, "default_dispatch")
def test_capability_tags_accepts_comma_string(self, tmp_path):
"""capability_tags falls back to comma-separated string parsing (like `tags`)."""
skill_dir = tmp_path / "comma-tags"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text(
"""---
name: comma-tags
description: Comma-separated capability tags
type: expert
capability_tags: alpha, beta, gamma
---
# Body
"""
)
result = _parse_skill_md(skill_dir / "SKILL.md")
assert result.capability_tags == ["alpha", "beta", "gamma"]
def test_unreadable_skill_md_returns_placeholder(self, tmp_path, caplog):
"""A non-UTF-8 SKILL.md must degrade to an ``(unreadable)`` placeholder
rather than raise. ``list_skills`` sits on the agent-construction hot
path (via ``_fold_expert_subagents``); an uncaught ``UnicodeDecodeError``
would abort CLI startup for a single malformed skill in any tier."""
skill_dir = tmp_path / "bad-utf8"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_bytes(b"---\nname: bad\n---\n\xff\xfe")
result = _parse_skill_md(skill_dir / "SKILL.md")
assert result.name == "bad-utf8"
assert result.description == "(unreadable)"
assert result.type == "utility"
assert result.body == ""
assert any("could not read" in r.message for r in caplog.records)
def test_empty_name_value_coerces_to_parent_dir(self, tmp_path):
"""A ``name:`` line with no value parses to ``None`` in YAML. Without
the ``.get(...) or parent.name`` guard, ``SkillInfo.name`` becomes
``None`` and slips past the ``_fold_expert_subagents`` collision
check (a ``None``-named expert would land in the ``task`` schema)."""
skill_dir = tmp_path / "empty-name"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text(
"""---
name:
description: A skill with an empty name value
type: expert
role: some role
---
Body content.
"""
)
result = _parse_skill_md(skill_dir / "SKILL.md")
assert result.name == "empty-name"
def test_invalid_frontmatter_yaml_warns(self, tmp_path, caplog):
"""Malformed YAML frontmatter must return the ``(invalid frontmatter)``
placeholder AND log so the author sees why the skill didn't register."""
skill_dir = tmp_path / "bad-yaml"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text(
"""---
name: bad-yaml
description: broken YAML below
tags: [unterminated
---
Body.
"""
)
result = _parse_skill_md(skill_dir / "SKILL.md")
assert result.description == "(invalid frontmatter)"
assert any("invalid frontmatter YAML" in r.message for r in caplog.records)
def _write_actor_skill(
parent: Path,
name: str,
*,
agents_body: str = "## Persona\n\nYou are the test expert.\n\n## Envelope\n\n{}\n",
skill_frontmatter: str = "",
skill_body: str = "# Knowledge\n\nThe portable workflow.\n",
) -> Path:
"""Write a skill declaring itself an expert via a sibling AGENTS.md."""
skill_dir = parent / name
skill_dir.mkdir(parents=True, exist_ok=True)
(skill_dir / "SKILL.md").write_text(
f"---\nname: {name}\ndescription: A skill that can also act\n"
f"{skill_frontmatter}---\n\n{skill_body}"
)
(skill_dir / "AGENTS.md").write_text(agents_body)
return skill_dir
class TestAgentsMdExpertContract:
"""AGENTS.md presence is the expert declaration; its body is the prompt."""
def test_presence_classifies_as_expert(self, tmp_path):
skill_dir = _write_actor_skill(tmp_path, "actor-skill")
result = _parse_skill_md(skill_dir / "SKILL.md")
assert result.type == "expert"
assert result.expert_source == "agents_md"
assert result.agents_body.startswith("## Persona")
# SKILL.md stays pure knowledge and is still cached for in-turn use.
assert "The portable workflow." in result.body
def test_no_actor_frontmatter_needed(self, tmp_path):
"""The decoration fields the contract removed stay empty, not invented."""
skill_dir = _write_actor_skill(tmp_path, "actor-skill")
result = _parse_skill_md(skill_dir / "SKILL.md")
assert result.role == ""
assert result.byline == ""
assert result.capability_tags == []
assert result.avatar_hint == ""
def test_metadata_type_alone_does_not_classify(self, tmp_path):
"""``metadata.type: [skill, expert]`` is index-facing only.
It is a projection of the AGENTS.md declaration for consumers that
can't stat the directory. Reading it in the runtime would create a
second classifier free to drift from the file that actually holds the
persona — a skill would register as an expert with nothing to say.
"""
skill_dir = tmp_path / "index-only"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text(
"""---
name: index-only
description: Declares expert to the index but ships no actor definition
metadata:
type: [skill, expert]
tags: [core]
---
# Knowledge
"""
)
result = _parse_skill_md(skill_dir / "SKILL.md")
assert result.type == "utility"
assert result.expert_source == ""
# The tags path still reads metadata — only `type` is ignored there.
assert result.tags == ["core"]
def test_frontmatter_stripped_from_actor_definition(self, tmp_path):
"""AGENTS.md carries no frontmatter, but YAML must never reach the prompt."""
skill_dir = _write_actor_skill(
tmp_path,
"fm-actor",
agents_body="---\nname: ignored\n---\n\n## Persona\n\nBody only.\n",
)
result = _parse_skill_md(skill_dir / "SKILL.md")
assert result.agents_body == "## Persona\n\nBody only.\n"
def test_empty_actor_definition_stays_classified(self, tmp_path):
"""An empty AGENTS.md is still a declaration — a broken expert.
Downgrading it to a utility skill would hide the authoring bug; the
registration paths refuse it by name instead.
"""
skill_dir = _write_actor_skill(tmp_path, "blank-actor", agents_body=" \n")
result = _parse_skill_md(skill_dir / "SKILL.md")
assert result.type == "expert"
assert result.expert_source == "agents_md"
assert result.agents_body.strip() == ""
def test_unreadable_skill_md_keeps_expert_declaration(self, tmp_path):
"""SKILL.md and AGENTS.md are separate files with separate failures."""
skill_dir = tmp_path / "half-broken"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_bytes(b"---\nname: bad\n---\n\xff\xfe")
(skill_dir / "AGENTS.md").write_text("## Persona\n\nStill valid.\n")
result = _parse_skill_md(skill_dir / "SKILL.md")
assert result.description == "(unreadable)"
assert result.type == "expert"
assert result.expert_source == "agents_md"
assert result.agents_body == "## Persona\n\nStill valid.\n"
def test_agents_md_overrides_legacy_frontmatter(self, tmp_path, caplog):
"""A skill mid-migration resolves to one contract, not a blend."""
import logging
skill_dir = _write_actor_skill(
tmp_path,
"migrating",
skill_frontmatter=(
"type: expert\nrole: legacy role\nbyline: Legacy\n"
"capability_tags: [legacy]\navatar_hint: legacy-avatar\n"
"default_dispatch: sync\n"
),
)
with caplog.at_level(
logging.WARNING, logger="EvoScientist.tools.skills_manager"
):
result = _parse_skill_md(skill_dir / "SKILL.md")
assert result.expert_source == "agents_md"
# AGENTS.md wins on every axis: the legacy decoration fields must be
# cleared, not merely warned about — otherwise `role` is prepended to
# the AGENTS.md prompt and the gallery chips leak stale frontmatter.
assert result.role == ""
assert result.byline == ""
assert result.capability_tags == []
assert result.avatar_hint == ""
assert any(
"is ignored and should be removed" in r.message and "migrating" in r.message
for r in caplog.records
)
def test_agents_md_expert_name_is_directory_name(self, tmp_path):
"""Registry identity for an AGENTS.md expert is the directory name.
A frontmatter ``name:`` that disagrees with the directory would desync
the dispatch registry key from the skill the orchestrator names.
"""
skill_dir = tmp_path / "real-dir"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text(
"---\nname: mismatched-name\ndescription: d\n---\n\n# Knowledge\n"
)
(skill_dir / "AGENTS.md").write_text("## Persona\n\nBody.\n")
result = _parse_skill_md(skill_dir / "SKILL.md")
assert result.name == "real-dir"
def test_legacy_frontmatter_expert_warns_once(self, tmp_path, caplog):
"""The deprecated path keeps working, and says so — once per skill.
``_parse_skill_md`` runs on every ``list_skills`` call (agent
construction, /expert completion, GET /api/teams), so a per-parse
warning would bury real diagnostics under repeats.
"""
import logging
skill_dir = tmp_path / "legacy-expert"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text(
"""---
name: legacy-expert
description: Declares itself the old way
type: expert
role: legacy role
---
# Persona body
"""
)
with caplog.at_level(
logging.WARNING, logger="EvoScientist.tools.skills_manager"
):
first = _parse_skill_md(skill_dir / "SKILL.md")
_parse_skill_md(skill_dir / "SKILL.md")
assert first.type == "expert"
assert first.expert_source == "frontmatter"
assert first.role == "legacy role"
deprecations = [r for r in caplog.records if "deprecated" in r.message.lower()]
assert len(deprecations) == 1
def test_utility_skill_has_no_expert_source(self, tmp_path):
skill_dir = tmp_path / "plain"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text(
"---\nname: plain\ndescription: Plain skill\n---\n\n# Body\n"
)
result = _parse_skill_md(skill_dir / "SKILL.md")
assert result.type == "utility"
assert result.expert_source == ""
assert result.agents_body == ""
class TestListExpertSkills:
"""`list_expert_skills()` filters `list_skills()` to `type == 'expert'`."""
def test_returns_only_expert_skills(self, tmp_path):
workspace_dir = tmp_path / "workspace"
workspace_dir.mkdir()
global_dir = tmp_path / "global"
global_dir.mkdir()
# An expert skill and a utility skill, both in workspace tier.
_write_expert_skill(workspace_dir, "expert-a")
util = workspace_dir / "util-b"
util.mkdir()
(util / "SKILL.md").write_text(
"""---
name: util-b
description: Plain utility skill
---
# Body
"""
)
with (
patch("EvoScientist.paths.USER_SKILLS_DIR", workspace_dir),
patch("EvoScientist.paths.GLOBAL_SKILLS_DIR", global_dir),
):
all_skills = list_skills()
expert_skills = list_expert_skills(include_system=False)
assert {s.name for s in all_skills} == {"expert-a", "util-b"}
assert [s.name for s in expert_skills] == ["expert-a"]
def test_empty_when_no_expert_skills_installed(self, tmp_path):
workspace_dir = tmp_path / "workspace"
workspace_dir.mkdir()
global_dir = tmp_path / "global"
global_dir.mkdir()
util = workspace_dir / "util-only"
util.mkdir()
(util / "SKILL.md").write_text(
"""---
name: util-only
description: Utility
---
# Body
"""
)
with (
patch("EvoScientist.paths.USER_SKILLS_DIR", workspace_dir),
patch("EvoScientist.paths.GLOBAL_SKILLS_DIR", global_dir),
):
expert_skills = list_expert_skills(include_system=False)
assert expert_skills == []
class TestSkillManagerToolExpertSurface:
"""`skill_manager` @tool exposes the expert-skill fields and filter."""
def _mock_skills(self):
return [
SkillInfo(
name="expert-a",
description="A brainstorm expert",
path=Path("/skills/expert-a"),
source="builtin",
tags=["research"],
type="expert",
role="Research idea brainstormer",
byline="Ideation persona",
capability_tags=["Iteration", "ELO"],
avatar_hint="lightbulb",
),
SkillInfo(
name="util-b",
description="A utility skill",
path=Path("/skills/util-b"),
source="workspace",
tags=["core"],
),
]
def test_list_filters_to_expert_when_skill_type_set(self):
from EvoScientist.tools.skill_manager import skill_manager
with patch(
"EvoScientist.tools.skills_manager.list_skills",
return_value=self._mock_skills(),
):
out = skill_manager.invoke(
{"action": "list", "include_system": True, "skill_type": "expert"}
)
assert "expert-a" in out
assert "util-b" not in out
def test_skill_type_enum_contains_no_empty_string(self):
"""Gemini's function-declaration schema rejects empty enum values
(`GenerateContentRequest.tools[N].function_declarations[N].parameters.properties[skill_type].enum[0]: cannot be empty`).
The `skill_type` argument must use a non-empty sentinel (`"all"`)
as its no-filter default, never `""`.
This test guards against silently reintroducing the empty-string
default that broke the live agent-teams smoke on 2026-07-17.
"""
from EvoScientist.tools.skill_manager import skill_manager
schema = skill_manager.args_schema.model_json_schema()
skill_type_prop = schema.get("properties", {}).get("skill_type", {})
# Pydantic/JSON-schema serialization of a Literal[...] shows up as
# `enum` on the property directly OR nested under `anyOf`.
enum_values: list[str] = []
if "enum" in skill_type_prop:
enum_values = list(skill_type_prop["enum"])
else:
for branch in skill_type_prop.get("anyOf", []):
if "enum" in branch:
enum_values.extend(branch["enum"])
assert enum_values, "skill_type Literal should surface as enum in the schema"
assert "" not in enum_values, (
f"Empty string in skill_type enum will break Gemini: {enum_values}"
)
def test_list_all_sentinel_is_no_filter(self):
"""`skill_type='all'` (the default) must return every skill —
it's the no-filter case, not a bucket that only 'all' skills fall into."""
from EvoScientist.tools.skill_manager import skill_manager
with patch(
"EvoScientist.tools.skills_manager.list_skills",
return_value=self._mock_skills(),
):
out = skill_manager.invoke(
{"action": "list", "include_system": True, "skill_type": "all"}
)
# Both should appear — 'all' is not a filter to a bucket named "all".
assert "expert-a" in out
assert "util-b" in out
def test_list_all_when_skill_type_absent(self):
from EvoScientist.tools.skill_manager import skill_manager
with patch(
"EvoScientist.tools.skills_manager.list_skills",
return_value=self._mock_skills(),
):
out = skill_manager.invoke({"action": "list", "include_system": True})
assert "expert-a" in out
assert "util-b" in out
def test_list_returns_message_when_filter_matches_nothing(self):
from EvoScientist.tools.skill_manager import skill_manager
with patch(
"EvoScientist.tools.skills_manager.list_skills",
return_value=self._mock_skills()[1:], # only the utility
):
out = skill_manager.invoke(
{"action": "list", "include_system": True, "skill_type": "expert"}
)
assert "No expert skills found" in out
def test_info_surfaces_expert_fields(self):
from EvoScientist.tools.skill_manager import skill_manager
with patch(
"EvoScientist.tools.skills_manager.get_skill_info",
return_value=self._mock_skills()[0],
):
out = skill_manager.invoke({"action": "info", "name": "expert-a"})
assert "Type: expert" in out
assert "Role: Research idea brainstormer" in out
assert "Byline: Ideation persona" in out
assert "Capability tags: Iteration, ELO" in out
assert "Avatar hint: lightbulb" in out
# No dispatch line: an expert does not pin its own reach.
assert "Default dispatch" not in out
def test_info_omits_expert_block_for_utility_skills(self):
from EvoScientist.tools.skill_manager import skill_manager
with patch(
"EvoScientist.tools.skills_manager.get_skill_info",
return_value=self._mock_skills()[1],
):
out = skill_manager.invoke({"action": "info", "name": "util-b"})
assert "Type: expert" not in out
assert "Role:" not in out
assert "Byline:" not in out
assert "Capability tags:" not in out
assert "Default dispatch:" not in out
class TestSkillManagerInfo:
"""Tests for the skill_manager() tool's action='info' output.
Guards the sandbox-visible ``Path: /skills/<name>`` shape and the absence
of any host filesystem path in the agent-visible response. Agents burn
turns on ``cd <host-path> && …`` chains whenever the host path leaks.
"""
def _make_skill(self, parent, name, description="A skill"):
skill_dir = parent / name
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text(
f"---\nname: {name}\ndescription: {description}\n---\n"
)
return skill_dir
def test_info_reports_virtual_mount_path(self, tmp_path):
"""``Path:`` is the sandbox-visible ``/skills/<name>``, not the host path."""
from EvoScientist.tools.skill_manager import skill_manager
workspace_dir = tmp_path / "workspace"
workspace_dir.mkdir()
global_dir = tmp_path / "global-empty"
global_dir.mkdir()
self._make_skill(tmp_path, "info-skill")
install_skill(str(tmp_path / "info-skill"), str(workspace_dir))
with (
patch("EvoScientist.paths.USER_SKILLS_DIR", workspace_dir),
patch("EvoScientist.paths.GLOBAL_SKILLS_DIR", global_dir),
):
result = skill_manager.invoke({"action": "info", "name": "info-skill"})
assert "Path: /skills/info-skill" in result
def test_info_omits_host_path(self, tmp_path):
"""No host filesystem path leaks into the response.
Stronger than a label-only check: catches any future refactor that
keeps the path visible under a different label (``Local:``,
``Installed at:``, embedded in ``Source: …``).
"""
from EvoScientist.tools.skill_manager import skill_manager
from EvoScientist.tools.skills_manager import get_skill_info
workspace_dir = tmp_path / "workspace"
workspace_dir.mkdir()
global_dir = tmp_path / "global-empty"
global_dir.mkdir()
self._make_skill(tmp_path, "host-leak-guard")
install_skill(str(tmp_path / "host-leak-guard"), str(workspace_dir))
with (
patch("EvoScientist.paths.USER_SKILLS_DIR", workspace_dir),
patch("EvoScientist.paths.GLOBAL_SKILLS_DIR", global_dir),
):
info = get_skill_info("host-leak-guard")
result = skill_manager.invoke({"action": "info", "name": "host-leak-guard"})
assert str(info.path) not in result
class TestSkillManagerInstall:
"""Tests for the skill_manager() tool's action='install' output shape.
Covers both single-install and batch-install returns:
- Single: ``{"success": True, "name": ..., "path": ..., "description": ...}``.
- Batch: ``{"success": ..., "batch": True, "installed": [...], "failed": [...]}``
with no top-level ``name``, ``path``, ``description``, or ``error``.
"""
def _make_skill(self, parent, name, description="A skill"):
skill_dir = parent / name
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text(
f"---\nname: {name}\ndescription: {description}\n---\n"
)
return skill_dir
def test_install_single_reports_virtual_mount_path(self, tmp_path):
"""Single install: ``Path: /skills/<name>``, no host path."""
from EvoScientist.tools.skill_manager import skill_manager
workspace_dir = tmp_path / "workspace"
workspace_dir.mkdir()
global_dir = tmp_path / "global-empty"
global_dir.mkdir()
self._make_skill(tmp_path, "solo-skill")
with (
patch("EvoScientist.paths.USER_SKILLS_DIR", workspace_dir),
patch("EvoScientist.paths.GLOBAL_SKILLS_DIR", global_dir),
):
result = skill_manager.invoke(
{"action": "install", "source": str(tmp_path / "solo-skill")}
)
assert "Successfully installed skill: solo-skill" in result
assert "Path: /skills/solo-skill" in result
def test_install_single_omits_host_path(self, tmp_path):
"""Single install: no host filesystem path leaks into the response.
``install_skill(source)`` defaults to ``global_install=True``, so the
skill lands under ``GLOBAL_SKILLS_DIR`` rather than ``USER_SKILLS_DIR``.
Checking against a narrower directory (e.g. workspace_dir) would pass
even without the scrub - the check has to cover every path the tool
might resolve to. ``tmp_path`` covers both patched dirs and the source
path used by ``install_skill``.
"""
from EvoScientist.tools.skill_manager import skill_manager
workspace_dir = tmp_path / "workspace"
workspace_dir.mkdir()
global_dir = tmp_path / "global-empty"
global_dir.mkdir()
self._make_skill(tmp_path, "leak-guard-install")
with (
patch("EvoScientist.paths.USER_SKILLS_DIR", workspace_dir),
patch("EvoScientist.paths.GLOBAL_SKILLS_DIR", global_dir),
):
result = skill_manager.invoke(
{"action": "install", "source": str(tmp_path / "leak-guard-install")}
)
assert str(tmp_path) not in result
def test_install_batch_lists_each_skill_with_virtual_path(self, tmp_path):
"""Batch install: one block per installed skill, each with ``Path: /skills/<name>``."""
from EvoScientist.tools.skill_manager import skill_manager
workspace_dir = tmp_path / "workspace"
workspace_dir.mkdir()
global_dir = tmp_path / "global-empty"
global_dir.mkdir()
pack = tmp_path / "pack"
pack.mkdir()
self._make_skill(pack, "alpha", description="first")
self._make_skill(pack, "beta", description="second")
with (
patch("EvoScientist.paths.USER_SKILLS_DIR", workspace_dir),
patch("EvoScientist.paths.GLOBAL_SKILLS_DIR", global_dir),
):
result = skill_manager.invoke({"action": "install", "source": str(pack)})
assert "Successfully installed skill: alpha" in result
assert "Successfully installed skill: beta" in result
assert "Path: /skills/alpha" in result
assert "Path: /skills/beta" in result
# Same leak guard as ``test_install_single_omits_host_path``: batch
# returns must not surface any host path either. Cover every dir the
# install might resolve to.
assert str(tmp_path) not in result
def test_install_batch_all_fail_returns_error_list(self, tmp_path):
"""Batch install where every skill fails must not KeyError on the
missing top-level ``error`` field.
Pre-fix behavior: ``result['error']`` crashed because
``_batch_install_local`` returns ``{"success": False, "batch": True,
"installed": [], "failed": [{"name": ..., "error": ...}]}`` with no
top-level ``error`` key. This test pins the guard.
"""
from EvoScientist.tools.skill_manager import skill_manager
workspace_dir = tmp_path / "workspace"
workspace_dir.mkdir()
global_dir = tmp_path / "global-empty"
global_dir.mkdir()
batch_result = {
"success": False,
"batch": True,
"installed": [],
"failed": [
{"name": "broken-a", "error": "corrupt frontmatter"},
{"name": "broken-b", "error": "missing SKILL.md"},
],
}
with (
patch("EvoScientist.paths.USER_SKILLS_DIR", workspace_dir),
patch("EvoScientist.paths.GLOBAL_SKILLS_DIR", global_dir),
patch(
"EvoScientist.tools.skills_manager.install_skill",
return_value=batch_result,
),
):
result = skill_manager.invoke(
{"action": "install", "source": "some/source"}
)
# No KeyError, and every failure surfaced.
assert "broken-a" in result
assert "corrupt frontmatter" in result
assert "broken-b" in result
assert "missing SKILL.md" in result
def test_install_batch_partial_fail_surfaces_both(self, tmp_path):
"""Batch install with partial failure lists successes AND failures.
Pre-fix behavior: partial failures were silently dropped; only the
success blocks reached the agent.
"""
from EvoScientist.tools.skill_manager import skill_manager
workspace_dir = tmp_path / "workspace"
workspace_dir.mkdir()
global_dir = tmp_path / "global-empty"
global_dir.mkdir()
partial_result = {
"success": True,
"batch": True,
"installed": [
{
"name": "worked",
"path": str(workspace_dir / "worked"),
"description": "installed cleanly",
},
],
"failed": [
{"name": "broken", "error": "corrupt frontmatter"},
],
}
with (
patch("EvoScientist.paths.USER_SKILLS_DIR", workspace_dir),
patch("EvoScientist.paths.GLOBAL_SKILLS_DIR", global_dir),
patch(
"EvoScientist.tools.skills_manager.install_skill",
return_value=partial_result,
),
):
result = skill_manager.invoke(
{"action": "install", "source": "some/source"}
)
assert "Successfully installed skill: worked" in result
assert "Path: /skills/worked" in result
assert "broken" in result
assert "corrupt frontmatter" in result
# =============================================================================
# Tests for the skills-changed publish primitive
# =============================================================================
@pytest.fixture
def isolated_skills_changed_callbacks():
"""Clear the module-level callback list before and after each test so
subscribers registered elsewhere (e.g. by importing ``experts.py``) do
not leak in or out of these tests.
"""
_reset_skills_changed_callbacks()
yield
_reset_skills_changed_callbacks()
class TestSkillsChangedCallback:
"""Verifies install_skill / uninstall_skill fire subscribers on every
return path — success, early error return, and success-with-real-mutation.
"""
def test_install_skill_fires_callback_on_error_return(
self, isolated_skills_changed_callbacks, temp_skills_dir
):
fired: list[bool] = []
register_skills_changed_callback(lambda: fired.append(True))
result = install_skill("/nonexistent/path", str(temp_skills_dir))
assert result["success"] is False
assert fired == [True]
def test_install_skill_fires_callback_on_success(
self, isolated_skills_changed_callbacks, sample_skill_dir, temp_skills_dir
):
fired: list[bool] = []
register_skills_changed_callback(lambda: fired.append(True))
result = install_skill(str(sample_skill_dir), str(temp_skills_dir))
assert result["success"] is True
assert fired == [True]
def test_uninstall_skill_fires_callback_on_error_return(
self, isolated_skills_changed_callbacks
):
fired: list[bool] = []
register_skills_changed_callback(lambda: fired.append(True))
result = uninstall_skill("nonexistent-skill")
assert result["success"] is False
assert fired == [True]
def test_misbehaving_callback_does_not_break_return(
self, isolated_skills_changed_callbacks, temp_skills_dir
):
good: list[bool] = []
def bad_callback() -> None:
raise RuntimeError("subscriber intentionally raising")
register_skills_changed_callback(bad_callback)
register_skills_changed_callback(lambda: good.append(True))
# Bad callback runs first; the good one still fires; the install
# return value is unaffected.
result = install_skill("/nonexistent/path", str(temp_skills_dir))
assert result["success"] is False
assert good == [True]
Reference in New Issue View Git Blame Copy Permalink