diff --git a/doc/changes/unreleased.md b/doc/changes/unreleased.md index fb4737052..e6c74ec6a 100644 --- a/doc/changes/unreleased.md +++ b/doc/changes/unreleased.md @@ -1,3 +1,7 @@ # Unreleased +## Features + +- Added shared validation for packaged agent skills and the `skills:check` Nox session. + ## Summary diff --git a/doc/user_guide/features/agent_skills/index.rst b/doc/user_guide/features/agent_skills/index.rst new file mode 100644 index 000000000..3af270264 --- /dev/null +++ b/doc/user_guide/features/agent_skills/index.rst @@ -0,0 +1,23 @@ +.. _agent_skills: + +Agent Skills +============ + +The PTB can package agent skills for use by projects and provides shared +validation for their common structure and content rules. + +Run the validation with: + +.. code-block:: shell + + poetry run -- nox -s skills:check + +The session validates every skill packaged in ``exasol.toolbox.skills``. It +checks that each skill has ``SKILL.md`` with complete frontmatter, contains no +unfinished TODO markers or forbidden repository-specific metadata, and has no +duplicated Markdown lines. Nox command examples are kept in the skill's +``references/nox-sessions.md`` file. + +These shared checks are intentionally separate from skill-specific tests. When +adding a skill, add its expected files and behavior assertions to that skill's +own test module, while ``skills:check`` covers the rules common to all skills. diff --git a/doc/user_guide/features/index.rst b/doc/user_guide/features/index.rst index 5c91b6c67..b31dfe9f7 100644 --- a/doc/user_guide/features/index.rst +++ b/doc/user_guide/features/index.rst @@ -12,6 +12,7 @@ Features creating_a_release managing_dependencies/index git_hooks/index + agent_skills/index metrics/collecting_metrics Uniform Project Layout diff --git a/exasol/toolbox/nox/_skills.py b/exasol/toolbox/nox/_skills.py new file mode 100644 index 000000000..78622d9e2 --- /dev/null +++ b/exasol/toolbox/nox/_skills.py @@ -0,0 +1,26 @@ +"""Nox sessions for validating packaged agent skills.""" + +from __future__ import annotations + +import nox +from nox import Session + +from exasol.toolbox.util.skills import get_packaged_skill_names, validate_skill + + +@nox.session(name="skills:check", python=False) +def check_skills(session: Session) -> None: + """Validate the common structure and content rules for packaged skills.""" + failures = { + skill_name: validate_skill(skill_name) + for skill_name in get_packaged_skill_names() + } + failures = { + skill_name: errors for skill_name, errors in failures.items() if errors + } + if failures: + details = "\n".join( + f"{skill_name}:\n" + "\n".join(f" - {error}" for error in errors) + for skill_name, errors in failures.items() + ) + session.error(f"Packaged skill validation failed:\n{details}") diff --git a/exasol/toolbox/nox/tasks.py b/exasol/toolbox/nox/tasks.py index 6be048000..618887c6e 100644 --- a/exasol/toolbox/nox/tasks.py +++ b/exasol/toolbox/nox/tasks.py @@ -9,6 +9,7 @@ "fix_format", "integration_tests", "lint", + "check_skills", "open_docs", "prepare_release", "type_check", @@ -59,6 +60,7 @@ def check(session: Session) -> None: updated, ) from exasol.toolbox.nox._release import prepare_release +from exasol.toolbox.nox._skills import check_skills from exasol.toolbox.nox._shared import ( Mode, _integration_test_context, diff --git a/exasol/toolbox/util/skills.py b/exasol/toolbox/util/skills.py index e18c83b87..8d811ed37 100644 --- a/exasol/toolbox/util/skills.py +++ b/exasol/toolbox/util/skills.py @@ -1,3 +1,5 @@ +"""Utilities for validating packaged agent skills.""" + from collections.abc import Mapping from pathlib import Path from typing import Final @@ -6,6 +8,16 @@ SKILLS_DIRECTORY: Final = "exasol.toolbox.skills" PTB_SKILL_NAME: Final = "exasol-python-toolbox" +SKILL_FRONTMATTER_SEPARATOR: Final = "---" +SKILL_FORBIDDEN_TERMS: Final = ( + "main-branch", + "main branch", + "master-branch", + "master branch", + "inventory", + "source-map", +) +SKILL_FILES: Final = ("SKILL.md",) def get_skill_path(skill_name: str = PTB_SKILL_NAME) -> Path: @@ -27,3 +39,82 @@ def get_skill_files(skill_name: str = PTB_SKILL_NAME) -> Mapping[str, Path]: for path in skill_path.rglob("*") if path.is_file() } + + +def get_packaged_skill_names() -> tuple[str, ...]: + """Return the names of all skills packaged with the toolbox.""" + skills_path = Path(str(resources.files(SKILLS_DIRECTORY))) + return tuple(sorted(path.name for path in skills_path.iterdir() if path.is_dir())) + + +def validate_skill(skill_name: str) -> tuple[str, ...]: + """Return deterministic validation errors for a packaged skill. + + The checks here are deliberately limited to properties shared by every PTB + skill. Assertions about a skill's specific content belong in that skill's + own tests. + """ + skill_files = get_skill_files(skill_name) + errors: list[str] = [] + + for expected_file in SKILL_FILES: + if expected_file not in skill_files: + errors.append(f"missing required file: {expected_file}") + + skill_file = skill_files.get("SKILL.md") + if skill_file is None: + return tuple(errors) + + content = skill_file.read_text(encoding="utf-8") + parts = content.split(SKILL_FRONTMATTER_SEPARATOR, maxsplit=2) + if len(parts) != 3 or parts[0].strip(): + errors.append("SKILL.md must start with YAML frontmatter") + else: + frontmatter = parts[1] + if f"name: {skill_name}" not in frontmatter: + errors.append(f"frontmatter name must be {skill_name}") + if "description:" not in frontmatter: + errors.append("frontmatter must contain a description") + + if "[TODO" in content: + errors.append("contains a TODO marker") + + all_content = "\n".join( + path.read_text(encoding="utf-8") + for path in skill_files.values() + ).lower() + for term in SKILL_FORBIDDEN_TERMS: + if term in all_content: + errors.append(f"contains forbidden term: {term}") + + nox_reference = "references/nox-sessions.md" + for relative_path, path in skill_files.items(): + if not relative_path.endswith(".md"): + continue + seen: dict[str, int] = {} + for line_number, line in enumerate( + path.read_text(encoding="utf-8").splitlines(), 1 + ): + normalized = line.strip().lower() + if ( + not normalized + or normalized in {"---", "```bash", "```"} + or normalized.startswith("|") + ): + continue + if normalized in seen: + errors.append( + f"{relative_path} duplicates line {seen[normalized]} " + f"at line {line_number}" + ) + seen[normalized] = line_number + + if relative_path != nox_reference: + text = path.read_text(encoding="utf-8") + if "poetry run -- nox -s" in text or "poetry run -- nox -l" in text: + errors.append( + f"{relative_path} contains Nox command syntax outside " + f"{nox_reference}" + ) + + return tuple(errors) diff --git a/test/unit/skills_test.py b/test/unit/skills_test.py index 56c4980b5..90076f8e6 100644 --- a/test/unit/skills_test.py +++ b/test/unit/skills_test.py @@ -8,6 +8,7 @@ PTB_SKILL_NAME, get_skill_files, get_skill_path, + validate_skill, ) PROJECT_ROOT = Path(__file__).parents[2] @@ -82,52 +83,8 @@ def test_ptb_skill_frontmatter_is_complete(): assert "[TODO" not in content -def test_ptb_skill_has_no_main_branch_metadata(): - forbidden = [ - "main-branch", - "main branch", - "master-branch", - "master branch", - "inventory", - "source-map", - ] - content = "\n".join( - path.read_text(encoding="utf-8") for path in SKILL.rglob("*") if path.is_file() - ).lower() - - for term in forbidden: - assert term not in content - - -def test_ptb_skill_has_no_duplicate_markdown_lines(): - for path in SKILL.rglob("*.md"): - seen = {} - for line_number, line in enumerate( - path.read_text(encoding="utf-8").splitlines(), 1 - ): - normalized = line.strip().lower() - if ( - not normalized - or normalized in {"---", "```bash", "```"} - or normalized.startswith("|") - ): - continue - assert normalized not in seen, ( - f"{path} duplicates line {seen[normalized]} at line {line_number}: " - f"{line}" - ) - seen[normalized] = line_number - - -def test_nox_command_syntax_is_only_in_nox_session_reference(): - nox_reference = SKILL / "references" / "nox-sessions.md" - for path in SKILL.rglob("*"): - if not path.is_file() or path == nox_reference: - continue - - content = path.read_text(encoding="utf-8") - assert "poetry run -- nox -s" not in content - assert "poetry run -- nox -l" not in content +def test_ptb_skill_passes_shared_validation(): + assert validate_skill(PTB_SKILL_NAME) == () def test_ptb_skill_eval_cases_are_valid(): diff --git a/test/unit/util/skills_test.py b/test/unit/util/skills_test.py new file mode 100644 index 000000000..7179cf481 --- /dev/null +++ b/test/unit/util/skills_test.py @@ -0,0 +1,18 @@ +from exasol.toolbox.util import skills + + +def test_validate_skill_accepts_packaged_ptb_skill(): + assert skills.validate_skill(skills.PTB_SKILL_NAME) == () + + +def test_validate_skill_reports_missing_skill_file(monkeypatch): + monkeypatch.setattr(skills, "get_skill_files", lambda _: {}) + + assert skills.validate_skill("example") == ("missing required file: SKILL.md",) + + +def test_get_packaged_skill_names_is_sorted(): + assert skills.PTB_SKILL_NAME in skills.get_packaged_skill_names() + assert skills.get_packaged_skill_names() == tuple( + sorted(skills.get_packaged_skill_names()) + )