mirror of
https://github.com/NCBM/plyngent.git
synced 2026-11-07 23:47:54 +08:00
tools/skills: list, read, and search skills
skill_list shows what was discovered (name, source root, directory, description) together with the copies a name collision shadowed and the notes from repaired frontmatter, so a third-party skill never disappears quietly. skill_read addresses a skill by name and defaults to its SKILL.md, reusing read_file for ranges, line numbers, and the continuation token; a bundled file is resolved inside the skill directory only, symlinks included. skill_search greps every visible skill (or one) and rebases each hit on its skill, which makes a match attributable without absolute paths. Reads need no grant: the host registers the roots as static read-only roots on the workspace policy, so read_file/grep_files see skill directories too while write access stays denied — skill writes will go through their own confirm. Grep used to drop every file outside the primary workspace root, which also hid hits under a granted directory; those now come back as absolute paths.
This commit is contained in:
@@ -821,6 +821,53 @@ def test_every_builtin_tool_has_a_pretty_line() -> None:
|
||||
assert missing == []
|
||||
|
||||
|
||||
async def test_pretty_skill_tools(capsys: pytest.CaptureFixture[str]) -> None:
|
||||
await render_events(
|
||||
_aiter(
|
||||
[_pretty_call("skill_list", "{}"), _result("skills: 3\npdf-processing [plyngent-user] /x — Extract PDFs")]
|
||||
)
|
||||
)
|
||||
out = capsys.readouterr().out
|
||||
assert "* Skills (3 skills)" in out
|
||||
|
||||
await render_events(
|
||||
_aiter(
|
||||
[
|
||||
_pretty_call("skill_read", '{"name": "pdf-processing"}'),
|
||||
_result("L1-40\n# PDF processing\n"),
|
||||
]
|
||||
)
|
||||
)
|
||||
out = capsys.readouterr().out
|
||||
assert "* Skill 'pdf-processing' L1-40 (done)" in out
|
||||
|
||||
await render_events(
|
||||
_aiter(
|
||||
[
|
||||
_pretty_call("skill_read", '{"name": "pdf-processing", "file": "scripts/run.sh"}'),
|
||||
_result("L1-3\n#!/bin/sh\n"),
|
||||
]
|
||||
)
|
||||
)
|
||||
out = capsys.readouterr().out
|
||||
assert "* Skill 'pdf-processing' scripts/run.sh L1-3 (done)" in out
|
||||
|
||||
await render_events(
|
||||
_aiter(
|
||||
[
|
||||
_pretty_call("skill_search", '{"pattern": "pdftotext"}'),
|
||||
_result("alpha/SKILL.md:3: Use pdftotext.\nbeta/notes.md:1: pdftotext again"),
|
||||
]
|
||||
)
|
||||
)
|
||||
out = capsys.readouterr().out
|
||||
assert "* Skill Search 'pdftotext' (2 matches in 2 skills)" in out
|
||||
|
||||
await render_events(_aiter([_pretty_call("skill_search", '{"pattern": "nope"}'), _result("(no matches)")]))
|
||||
out = capsys.readouterr().out
|
||||
assert "* Skill Search 'nope' (no matches)" in out
|
||||
|
||||
|
||||
async def test_pretty_prefix_shown_while_call_runs(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
capsys: pytest.CaptureFixture[str],
|
||||
|
||||
@@ -12,6 +12,7 @@ from plyngent.tools.chat import CHAT_TOOLS
|
||||
from plyngent.tools.file import FILE_TOOLS
|
||||
from plyngent.tools.net import NET_TOOLS
|
||||
from plyngent.tools.process import PROCESS_TOOLS
|
||||
from plyngent.tools.skills import SKILL_TOOLS
|
||||
from plyngent.tools.todo import TODO_TOOLS
|
||||
from plyngent.tools.vcs import VCS_TOOLS
|
||||
|
||||
@@ -19,7 +20,16 @@ from plyngent.tools.vcs import VCS_TOOLS
|
||||
def test_default_tool_names_match_group_lists() -> None:
|
||||
register_builtin_tools()
|
||||
selected = default_tool_definitions(surface="local")
|
||||
groups = [*ACCESS_TOOLS, *FILE_TOOLS, *PROCESS_TOOLS, *VCS_TOOLS, *CHAT_TOOLS, *TODO_TOOLS, *NET_TOOLS]
|
||||
groups = [
|
||||
*ACCESS_TOOLS,
|
||||
*FILE_TOOLS,
|
||||
*PROCESS_TOOLS,
|
||||
*VCS_TOOLS,
|
||||
*CHAT_TOOLS,
|
||||
*TODO_TOOLS,
|
||||
*SKILL_TOOLS,
|
||||
*NET_TOOLS,
|
||||
]
|
||||
assert sorted(t.name for t in selected) == sorted(t.name for t in groups)
|
||||
assert len(selected) == len(groups)
|
||||
|
||||
@@ -65,6 +75,9 @@ def test_read_only_classification() -> None:
|
||||
"vcs_log",
|
||||
"vcs_branch",
|
||||
"fetch",
|
||||
"skill_list",
|
||||
"skill_read",
|
||||
"skill_search",
|
||||
}
|
||||
|
||||
|
||||
|
||||
@@ -0,0 +1,194 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from contextlib import contextmanager
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
from plyngent.config.models import SkillsConfig
|
||||
from plyngent.skills import SKILL_FILE, SkillStore, build_roots
|
||||
from plyngent.tools.context import InstanceState, bind_instance
|
||||
from plyngent.tools.skills import SKILL_TOOL_NAMES, format_skill_table, skill_list, skill_read, skill_search
|
||||
from plyngent.tools.workspace import set_static_read_roots, set_workspace_root
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from collections.abc import Generator
|
||||
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
|
||||
|
||||
|
||||
@contextmanager
|
||||
def _bound(tmp_path: Path, *roots: Path, skills: SkillStore | None = None) -> Generator[InstanceState]:
|
||||
"""Bind an instance whose skill store scans *roots*, like the CLI does."""
|
||||
workspace = tmp_path / "project"
|
||||
workspace.mkdir(exist_ok=True)
|
||||
store = (
|
||||
skills
|
||||
if skills is not None
|
||||
else SkillStore(
|
||||
build_roots(
|
||||
SkillsConfig(paths=[str(r) for r in roots], discover=[]), workspace=workspace, user_config_dir=tmp_path
|
||||
)
|
||||
)
|
||||
)
|
||||
instance = InstanceState(workspace_root=workspace, skills=store)
|
||||
with bind_instance(instance):
|
||||
_ = set_workspace_root(workspace)
|
||||
_ = set_static_read_roots(roots)
|
||||
yield instance
|
||||
|
||||
|
||||
async def test_skill_list_without_a_store_returns_an_error(tmp_path: Path) -> None:
|
||||
with _bound(tmp_path, skills=None) as instance:
|
||||
instance.skills = None
|
||||
assert await skill_list.handler() == "error: skills are not available in this session"
|
||||
|
||||
|
||||
async def test_skill_list_reports_skills_roots_and_absent_dirs(tmp_path: Path) -> None:
|
||||
root = tmp_path / "team"
|
||||
_ = _write_skill(root, "acme-deploy", description="Deploy the acme stack")
|
||||
missing = tmp_path / "missing"
|
||||
with _bound(tmp_path, root, missing):
|
||||
out = await skill_list.handler()
|
||||
assert "skills: 1" in out
|
||||
assert "acme-deploy [config-user]" in out
|
||||
assert "Deploy the acme stack" in out
|
||||
assert str(root / "acme-deploy") in out
|
||||
assert "roots without a directory" in out
|
||||
assert str(missing) in out
|
||||
|
||||
|
||||
async def test_skill_list_marks_shadowed_copies(tmp_path: Path) -> None:
|
||||
high = tmp_path / "team"
|
||||
low = tmp_path / "personal"
|
||||
_ = _write_skill(high, "shared", description="Team copy")
|
||||
_ = _write_skill(low, "shared", description="Personal copy")
|
||||
with _bound(tmp_path, high, low):
|
||||
out = await skill_list.handler()
|
||||
assert "Team copy" in out
|
||||
assert "Personal copy" not in out
|
||||
assert "shadowed: shared — config-user wins (also config-user)" in out
|
||||
|
||||
|
||||
async def test_skill_list_shows_notes(tmp_path: Path) -> None:
|
||||
root = tmp_path / "team"
|
||||
directory = root / "no-desc"
|
||||
directory.mkdir(parents=True)
|
||||
_ = (directory / SKILL_FILE).write_text("# Heading\n\nBody paragraph.\n", encoding="utf-8")
|
||||
with _bound(tmp_path, root):
|
||||
out = await skill_list.handler()
|
||||
assert "notes: no-desc: no frontmatter description; using the first paragraph" in out
|
||||
|
||||
|
||||
async def test_skill_read_defaults_to_skill_md(tmp_path: Path) -> None:
|
||||
root = tmp_path / "team"
|
||||
_ = _write_skill(root, "acme", body="Step one.")
|
||||
with _bound(tmp_path, root):
|
||||
out = await skill_read.handler("acme")
|
||||
assert out.startswith("L1-")
|
||||
assert "Step one." in out
|
||||
|
||||
|
||||
async def test_skill_read_bundled_file_with_line_numbers(tmp_path: Path) -> None:
|
||||
root = tmp_path / "team"
|
||||
directory = _write_skill(root, "acme")
|
||||
(directory / "scripts").mkdir()
|
||||
_ = (directory / "scripts" / "run.sh").write_text("#!/bin/sh\necho hi\n", encoding="utf-8")
|
||||
with _bound(tmp_path, root):
|
||||
out = await skill_read.handler("acme", "scripts/run.sh", with_lineno=True)
|
||||
assert " 1|#!/bin/sh" in out
|
||||
|
||||
|
||||
async def test_skill_read_refuses_to_escape_the_skill(tmp_path: Path) -> None:
|
||||
root = tmp_path / "team"
|
||||
_ = _write_skill(root, "acme")
|
||||
_ = (tmp_path / "outside.txt").write_text("not yours\n", encoding="utf-8")
|
||||
with _bound(tmp_path, root):
|
||||
assert await skill_read.handler("acme", "../outside.txt") == (
|
||||
"error: file '../outside.txt' is outside skill 'acme'"
|
||||
)
|
||||
assert await skill_read.handler("acme", "/etc/hostname") == (
|
||||
"error: file '/etc/hostname' is outside skill 'acme'"
|
||||
)
|
||||
|
||||
|
||||
async def test_skill_read_unknown_skill_and_missing_file(tmp_path: Path) -> None:
|
||||
root = tmp_path / "team"
|
||||
_ = _write_skill(root, "acme")
|
||||
with _bound(tmp_path, root):
|
||||
assert (
|
||||
await skill_read.handler("nope")
|
||||
== "error: unknown skill 'nope'; call skill_list to see the discovered skills"
|
||||
)
|
||||
assert await skill_read.handler("acme", "missing.md") == "error: no such file in skill 'acme': missing.md"
|
||||
|
||||
|
||||
async def test_skill_read_truncates_with_a_continuation_token(tmp_path: Path) -> None:
|
||||
root = tmp_path / "team"
|
||||
_ = _write_skill(root, "acme", body="x" * 500)
|
||||
with _bound(tmp_path, root):
|
||||
out = await skill_read.handler("acme", max_chars=120)
|
||||
assert "truncate_token=" in out
|
||||
|
||||
|
||||
async def test_skill_search_attributes_hits_to_their_skill(tmp_path: Path) -> None:
|
||||
root = tmp_path / "team"
|
||||
_ = _write_skill(root, "alpha", body="Use pdftotext for extraction.")
|
||||
beta = _write_skill(root, "beta", body="No match in the body.")
|
||||
_ = (beta / "notes.md").write_text("pdftotext again\n", encoding="utf-8")
|
||||
with _bound(tmp_path, root):
|
||||
out = await skill_search.handler("pdftotext")
|
||||
assert "alpha/SKILL.md:" in out
|
||||
assert "beta/notes.md:" in out
|
||||
|
||||
|
||||
async def test_skill_search_single_skill_and_errors(tmp_path: Path) -> None:
|
||||
root = tmp_path / "team"
|
||||
_ = _write_skill(root, "alpha", body="Use pdftotext.")
|
||||
_ = _write_skill(root, "beta", body="Nothing to see.")
|
||||
with _bound(tmp_path, root):
|
||||
assert await skill_search.handler("pdftotext", skill="beta") == "(no matches)"
|
||||
assert (await skill_search.handler("pdftotext", skill="nope")).startswith("error: unknown skill")
|
||||
assert (await skill_search.handler("[")).startswith("error: invalid regex")
|
||||
|
||||
|
||||
async def test_skill_search_without_skills(tmp_path: Path) -> None:
|
||||
empty = tmp_path / "empty"
|
||||
empty.mkdir()
|
||||
with _bound(tmp_path, empty):
|
||||
assert await skill_search.handler("anything") == "(no skills to search)"
|
||||
|
||||
|
||||
async def test_skill_search_respects_max_matches(tmp_path: Path) -> None:
|
||||
root = tmp_path / "team"
|
||||
_ = _write_skill(root, "alpha", body="hit\nhit\nhit\nhit\n")
|
||||
with _bound(tmp_path, root):
|
||||
out = await skill_search.handler("hit", max_matches=2)
|
||||
assert out.count("alpha/SKILL.md:") == 2
|
||||
assert out.endswith("...[truncated at 2 matches]")
|
||||
|
||||
|
||||
def test_format_skill_table_lists_roots_in_priority_order(tmp_path: Path) -> None:
|
||||
root = tmp_path / "team"
|
||||
_ = _write_skill(root, "acme")
|
||||
store = SkillStore(
|
||||
build_roots(SkillsConfig(paths=[str(root)], discover=[]), workspace=tmp_path, user_config_dir=tmp_path)
|
||||
)
|
||||
table = format_skill_table(store)
|
||||
assert "skills: 1" in table
|
||||
assert "acme [config-user]" in table
|
||||
|
||||
|
||||
def test_skill_tool_names_cover_the_group() -> None:
|
||||
expected = ("skill_list", "skill_read", "skill_search")
|
||||
for name in expected:
|
||||
assert name in SKILL_TOOL_NAMES
|
||||
assert len(SKILL_TOOL_NAMES) == len(expected)
|
||||
Reference in New Issue
Block a user