mirror of
https://github.com/NCBM/plyngent.git
synced 2026-11-07 23:47:54 +08:00
skills: discover roots and parse SKILL.md frontmatter
A skill is a directory with a SKILL.md; roots are ordered by priority (an explicit path, then our own user directory, then the directories other harnesses install into, with the project-scope one behind its own switch). The first copy of a name wins and the ones it shadows stay listed for reporting instead of vanishing. The frontmatter reader covers the subset those files actually use — scalars, booleans, ints, inline and block lists, | and > blocks, folded plain scalars — so no YAML dependency is needed; a nested mapping is skipped and a structurally broken block degrades to "no metadata" with a note, keeping a third-party skill listable rather than dropping it. Metadata is cached against the directory list and its SKILL.md timestamps, so a listing is cheap yet notices an edit.
This commit is contained in:
@@ -0,0 +1,195 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
from plyngent.config.models import SkillsConfig
|
||||
from plyngent.skills import SKILL_FILE, Skill, SkillStore, build_roots, validate_skill_name
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
def _write_skill(root: Path, name: str, *, description: str | None = "A test skill", body: str = "Body.") -> Path:
|
||||
directory = root / name
|
||||
directory.mkdir(parents=True, exist_ok=True)
|
||||
front = "---\n"
|
||||
if description is not None:
|
||||
front += f"name: {name}\ndescription: {description}\n"
|
||||
front += "---\n\n"
|
||||
_ = (directory / SKILL_FILE).write_text(front + body + "\n", encoding="utf-8")
|
||||
return directory
|
||||
|
||||
|
||||
def test_build_roots_orders_explicit_paths_first(tmp_path: Path) -> None:
|
||||
config = SkillsConfig(paths=["~/team-skills", "/srv/skills"])
|
||||
roots = build_roots(
|
||||
config,
|
||||
workspace=tmp_path / "proj",
|
||||
user_config_dir=tmp_path / "config",
|
||||
home=tmp_path / "home",
|
||||
)
|
||||
assert [(r.origin, r.scope, str(r.path)) for r in roots] == [
|
||||
("config", "user", str(tmp_path / "home" / "team-skills")),
|
||||
("config", "user", "/srv/skills"),
|
||||
("plyngent", "user", str(tmp_path / "config" / "skills")),
|
||||
("claude", "user", str(tmp_path / "home" / ".claude" / "skills")),
|
||||
("claude", "project", str(tmp_path / "proj" / ".claude" / "skills")),
|
||||
]
|
||||
|
||||
|
||||
def test_build_roots_respects_discover_and_project_switch(tmp_path: Path) -> None:
|
||||
config = SkillsConfig(discover=["plyngent"], include_project_roots=False)
|
||||
roots = build_roots(config, workspace=tmp_path, user_config_dir=tmp_path / "cfg", home=tmp_path)
|
||||
assert [r.origin for r in roots] == ["plyngent"]
|
||||
|
||||
|
||||
def test_store_discovers_skills_and_labels_their_root(tmp_path: Path) -> None:
|
||||
user = tmp_path / "config" / "skills"
|
||||
_ = _write_skill(user, "pdf-processing", description="Extract text from PDFs")
|
||||
store = SkillStore(
|
||||
build_roots(SkillsConfig(discover=["plyngent"]), workspace=tmp_path, user_config_dir=tmp_path / "config")
|
||||
)
|
||||
skills = store.skills
|
||||
assert [(s.name, s.description, s.root.label) for s in skills] == [
|
||||
("pdf-processing", "Extract text from PDFs", "plyngent-user"),
|
||||
]
|
||||
assert skills[0].skill_file.name == SKILL_FILE
|
||||
|
||||
|
||||
def test_store_lists_skills_from_several_roots_sorted(tmp_path: Path) -> None:
|
||||
config_dir = tmp_path / "config"
|
||||
_ = _write_skill(config_dir / "skills", "zeta")
|
||||
_ = _write_skill(tmp_path / "home" / ".claude" / "skills", "alpha")
|
||||
store = SkillStore(
|
||||
build_roots(SkillsConfig(), workspace=tmp_path, user_config_dir=config_dir, home=tmp_path / "home")
|
||||
)
|
||||
assert [s.name for s in store.skills] == ["alpha", "zeta"]
|
||||
assert [s.root.origin for s in store.skills] == ["claude", "plyngent"]
|
||||
|
||||
|
||||
def test_explicit_path_wins_and_shadowed_copy_stays_listed(tmp_path: Path) -> None:
|
||||
explicit = tmp_path / "team"
|
||||
config_dir = tmp_path / "config"
|
||||
_ = _write_skill(explicit, "shared", description="Team copy")
|
||||
_ = _write_skill(config_dir / "skills", "shared", description="User copy")
|
||||
config = SkillsConfig(paths=[str(explicit)], discover=["plyngent"])
|
||||
store = SkillStore(build_roots(config, workspace=tmp_path, user_config_dir=config_dir))
|
||||
|
||||
found = store.find("shared")
|
||||
assert found is not None
|
||||
assert found.description == "Team copy"
|
||||
assert [s.description for s in store.shadowed()["shared"]] == ["Team copy", "User copy"]
|
||||
assert len(store.all_skills) == 2
|
||||
|
||||
|
||||
def test_find_is_case_insensitive_fallback(tmp_path: Path) -> None:
|
||||
_ = _write_skill(tmp_path / "skills", "pdf-processing")
|
||||
store = SkillStore(build_roots(SkillsConfig(discover=["plyngent"]), workspace=tmp_path, user_config_dir=tmp_path))
|
||||
assert store.find("PDF-Processing") is not None
|
||||
assert store.find("nope") is None
|
||||
|
||||
|
||||
def test_root_itself_may_be_a_skill(tmp_path: Path) -> None:
|
||||
single = tmp_path / "my-skill"
|
||||
single.mkdir()
|
||||
_ = (single / SKILL_FILE).write_text("---\nname: my-skill\ndescription: One-off\n---\n\nBody.\n", encoding="utf-8")
|
||||
store = SkillStore(build_roots(SkillsConfig(paths=[str(single)]), workspace=tmp_path, user_config_dir=tmp_path))
|
||||
assert [s.name for s in store.skills] == ["my-skill"]
|
||||
|
||||
|
||||
def test_missing_roots_are_not_an_error(tmp_path: Path) -> None:
|
||||
store = SkillStore(
|
||||
build_roots(SkillsConfig(), workspace=tmp_path, user_config_dir=tmp_path / "nope", home=tmp_path)
|
||||
)
|
||||
assert store.skills == []
|
||||
assert store.shadowed() == {}
|
||||
|
||||
|
||||
def test_description_falls_back_to_the_first_paragraph(tmp_path: Path) -> None:
|
||||
directory = tmp_path / "skills" / "no-desc"
|
||||
directory.mkdir(parents=True)
|
||||
_ = (directory / SKILL_FILE).write_text(
|
||||
"# Heading\n\nFirst paragraph of the skill.\n\nSecond paragraph.\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
store = SkillStore(build_roots(SkillsConfig(discover=["plyngent"]), workspace=tmp_path, user_config_dir=tmp_path))
|
||||
skill = store.skills[0]
|
||||
assert skill.description == "First paragraph of the skill."
|
||||
assert skill.declared_name is None
|
||||
assert any("no frontmatter description" in issue for issue in skill.issues)
|
||||
|
||||
|
||||
def test_unusable_declared_name_falls_back_to_the_directory(tmp_path: Path) -> None:
|
||||
directory = tmp_path / "skills" / "dir-name"
|
||||
directory.mkdir(parents=True)
|
||||
_ = (directory / SKILL_FILE).write_text(
|
||||
'---\nname: "PDF Skills"\ndescription: Mixed case\n---\n\nBody.\n',
|
||||
encoding="utf-8",
|
||||
)
|
||||
store = SkillStore(build_roots(SkillsConfig(discover=["plyngent"]), workspace=tmp_path, user_config_dir=tmp_path))
|
||||
skill = store.skills[0]
|
||||
assert skill.name == "dir-name"
|
||||
assert skill.declared_name == "PDF Skills"
|
||||
assert any("not usable" in issue for issue in skill.issues)
|
||||
|
||||
|
||||
def test_broken_frontmatter_still_yields_a_listable_skill(tmp_path: Path) -> None:
|
||||
directory = tmp_path / "skills" / "broken"
|
||||
directory.mkdir(parents=True)
|
||||
_ = (directory / SKILL_FILE).write_text("---\nname: broken\nbroken line\n---\n\nBody text.\n", encoding="utf-8")
|
||||
store = SkillStore(build_roots(SkillsConfig(discover=["plyngent"]), workspace=tmp_path, user_config_dir=tmp_path))
|
||||
skill = store.skills[0]
|
||||
assert skill.name == "broken"
|
||||
assert skill.description # falls back to the body
|
||||
assert any("frontmatter not parsed" in issue for issue in skill.issues)
|
||||
|
||||
|
||||
def test_cache_picks_up_an_edit_and_reload_forces_a_rescan(tmp_path: Path) -> None:
|
||||
root = tmp_path / "skills"
|
||||
directory = _write_skill(root, "cached", description="Before")
|
||||
store = SkillStore(build_roots(SkillsConfig(discover=["plyngent"]), workspace=tmp_path, user_config_dir=tmp_path))
|
||||
assert store.skills[0].description == "Before"
|
||||
|
||||
_ = (directory / SKILL_FILE).write_text(
|
||||
"---\nname: cached\ndescription: After\n---\n\nBody.\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
assert store.skills[0].description == "After"
|
||||
|
||||
_ = _write_skill(root, "added")
|
||||
store.reload()
|
||||
assert [s.name for s in store.skills] == ["added", "cached"]
|
||||
|
||||
|
||||
def test_default_write_root_prefers_our_own_directory(tmp_path: Path) -> None:
|
||||
config_dir = tmp_path / "config"
|
||||
store = SkillStore(build_roots(SkillsConfig(), workspace=tmp_path, user_config_dir=config_dir, home=tmp_path))
|
||||
root = store.default_write_root()
|
||||
assert root is not None
|
||||
assert root.origin == "plyngent"
|
||||
assert root.path == config_dir / "skills"
|
||||
|
||||
explicit = SkillStore(
|
||||
build_roots(SkillsConfig(paths=["/srv/skills"], discover=[]), workspace=tmp_path, user_config_dir=config_dir)
|
||||
)
|
||||
fallback = explicit.default_write_root()
|
||||
assert fallback is not None and fallback.origin == "config"
|
||||
assert SkillStore([]).default_write_root() is None
|
||||
|
||||
|
||||
def test_validate_skill_name() -> None:
|
||||
assert validate_skill_name("pdf-processing") is None
|
||||
assert validate_skill_name("a1") is None
|
||||
assert validate_skill_name("") is not None
|
||||
assert validate_skill_name("PDF") is not None
|
||||
assert validate_skill_name("with_underscore") is not None
|
||||
assert validate_skill_name("x" * 65) is not None
|
||||
|
||||
|
||||
def test_skill_is_a_frozen_record(tmp_path: Path) -> None:
|
||||
directory = _write_skill(tmp_path / "skills", "frozen")
|
||||
store = SkillStore(build_roots(SkillsConfig(discover=["plyngent"]), workspace=tmp_path, user_config_dir=tmp_path))
|
||||
skill = store.skills[0]
|
||||
assert isinstance(skill, Skill)
|
||||
assert skill.path == directory
|
||||
assert skill.metadata["name"] == "frozen"
|
||||
@@ -0,0 +1,114 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from plyngent.skills.frontmatter import (
|
||||
FrontmatterError,
|
||||
parse_frontmatter,
|
||||
parse_yaml_subset,
|
||||
split_frontmatter,
|
||||
)
|
||||
|
||||
|
||||
def test_parse_frontmatter_basic() -> None:
|
||||
text = (
|
||||
"---\n"
|
||||
"name: pdf-processing\n"
|
||||
'description: "Extract text and tables from PDFs"\n'
|
||||
"version: 2\n"
|
||||
"license: MIT\n"
|
||||
"---\n"
|
||||
"\n"
|
||||
"# PDF processing\n"
|
||||
"\n"
|
||||
"Use pdftotext first.\n"
|
||||
)
|
||||
metadata, body = parse_frontmatter(text)
|
||||
assert metadata == {
|
||||
"name": "pdf-processing",
|
||||
"description": "Extract text and tables from PDFs",
|
||||
"version": 2,
|
||||
"license": "MIT",
|
||||
}
|
||||
assert body.startswith("# PDF processing")
|
||||
assert "pdftotext" in body
|
||||
|
||||
|
||||
def test_parse_frontmatter_without_block_keeps_the_whole_text() -> None:
|
||||
text = "# Just a skill\n\nNo frontmatter here.\n"
|
||||
metadata, body = parse_frontmatter(text)
|
||||
assert metadata == {}
|
||||
assert body == text
|
||||
|
||||
|
||||
def test_parse_frontmatter_unterminated_block_is_not_frontmatter() -> None:
|
||||
text = "---\nname: x\n\nbody without a closing fence\n"
|
||||
metadata, body = parse_frontmatter(text)
|
||||
assert metadata == {}
|
||||
assert body == text
|
||||
|
||||
|
||||
def test_frontmatter_closing_dots_are_accepted() -> None:
|
||||
block, body = split_frontmatter("---\nname: x\n...\nbody\n")
|
||||
assert block == "name: x"
|
||||
assert body == "body\n"
|
||||
|
||||
|
||||
def test_yaml_subset_types_lists_and_booleans() -> None:
|
||||
block = "\n".join(
|
||||
[
|
||||
"name: acme",
|
||||
"read_only: true",
|
||||
"confirm: no",
|
||||
"retries: 3",
|
||||
"allowed-tools: [read_file, grep_files]",
|
||||
"tags:",
|
||||
" - alpha",
|
||||
" - beta",
|
||||
]
|
||||
)
|
||||
assert parse_yaml_subset(block) == {
|
||||
"name": "acme",
|
||||
"read_only": True,
|
||||
"confirm": False,
|
||||
"retries": 3,
|
||||
"allowed-tools": ["read_file", "grep_files"],
|
||||
"tags": ["alpha", "beta"],
|
||||
}
|
||||
|
||||
|
||||
def test_yaml_subset_plain_scalar_folds_continuation_lines() -> None:
|
||||
block = "description: A long description\n that continues on the next line\n"
|
||||
assert parse_yaml_subset(block) == {
|
||||
"description": "A long description that continues on the next line",
|
||||
}
|
||||
|
||||
|
||||
def test_yaml_subset_block_scalars() -> None:
|
||||
literal = "description: |\n first\n second\n"
|
||||
folded = "description: >\n first\n second\n"
|
||||
assert parse_yaml_subset(literal) == {"description": "first\nsecond"}
|
||||
assert parse_yaml_subset(folded) == {"description": "first second"}
|
||||
|
||||
|
||||
def test_yaml_subset_nested_mapping_is_skipped() -> None:
|
||||
"""Unsupported nesting must not lose the flat fields around it."""
|
||||
block = "name: acme\nmetadata:\n author: someone\n homepage: https://example.com\nlicense: MIT\n"
|
||||
assert parse_yaml_subset(block) == {"name": "acme", "license": "MIT"}
|
||||
|
||||
|
||||
def test_yaml_subset_comments_and_blank_lines() -> None:
|
||||
block = "# a comment\n\nname: acme\n"
|
||||
assert parse_yaml_subset(block) == {"name": "acme"}
|
||||
|
||||
|
||||
def test_yaml_subset_rejects_broken_lines() -> None:
|
||||
with pytest.raises(FrontmatterError):
|
||||
_ = parse_yaml_subset("name: acme\nnot a pair\n")
|
||||
with pytest.raises(FrontmatterError):
|
||||
_ = parse_yaml_subset("- orphan item\n")
|
||||
|
||||
|
||||
def test_yaml_subset_rejects_indented_top_level() -> None:
|
||||
with pytest.raises(FrontmatterError):
|
||||
_ = parse_yaml_subset(" name: acme\n")
|
||||
Reference in New Issue
Block a user