From 8f8a001284724b1fa7a7202f7f31a6d40f967872 Mon Sep 17 00:00:00 2001 From: jana-selva Date: Fri, 11 Sep 2026 11:13:08 +0530 Subject: [PATCH 1/6] Add shared validation for packaged skills --- doc/changes/unreleased.md | 4 + .../features/agent_skills/index.rst | 23 +++++ doc/user_guide/features/index.rst | 1 + exasol/toolbox/nox/_skills.py | 26 ++++++ exasol/toolbox/nox/tasks.py | 2 + exasol/toolbox/util/skills.py | 91 +++++++++++++++++++ test/unit/skills_test.py | 49 +--------- test/unit/util/skills_test.py | 18 ++++ 8 files changed, 168 insertions(+), 46 deletions(-) create mode 100644 doc/user_guide/features/agent_skills/index.rst create mode 100644 exasol/toolbox/nox/_skills.py create mode 100644 test/unit/util/skills_test.py 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()) + ) From 70b89ce552b6b8cef22e58df8e7facb1598c7d78 Mon Sep 17 00:00:00 2001 From: jana-selva Date: Tue, 15 Sep 2026 13:23:47 +0530 Subject: [PATCH 2/6] Address PR check formatting --- doc/changes/unreleased.md | 2 +- exasol/toolbox/nox/_skills.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/doc/changes/unreleased.md b/doc/changes/unreleased.md index e6c74ec6a..1fd25163d 100644 --- a/doc/changes/unreleased.md +++ b/doc/changes/unreleased.md @@ -2,6 +2,6 @@ ## Features -- Added shared validation for packaged agent skills and the `skills:check` Nox session. +- #940: Added shared validation for packaged agent skills and the `skills:check` Nox session. ## Summary diff --git a/exasol/toolbox/nox/_skills.py b/exasol/toolbox/nox/_skills.py index 78622d9e2..de0a0ecfd 100644 --- a/exasol/toolbox/nox/_skills.py +++ b/exasol/toolbox/nox/_skills.py @@ -5,7 +5,10 @@ import nox from nox import Session -from exasol.toolbox.util.skills import get_packaged_skill_names, validate_skill +from exasol.toolbox.util.skills import ( + get_packaged_skill_names, + validate_skill, +) @nox.session(name="skills:check", python=False) From ac8a485772d47a8e0f49a96bb29b2aa6fe003161 Mon Sep 17 00:00:00 2001 From: jana-selva Date: Tue, 15 Sep 2026 13:28:06 +0530 Subject: [PATCH 3/6] Resolve failing pull request checks --- .github/workflows/build-and-publish.yml | 2 +- .github/workflows/check-release-tag.yml | 2 +- .github/workflows/checks.yml | 18 +++++++++--------- .github/workflows/dependency-update.yml | 2 +- .github/workflows/fast-tests.yml | 2 +- .github/workflows/gh-pages.yml | 2 +- .github/workflows/matrix.yml | 2 +- .github/workflows/report.yml | 2 +- .../{skills_test.py => skill_utils_test.py} | 0 9 files changed, 16 insertions(+), 16 deletions(-) rename test/unit/util/{skills_test.py => skill_utils_test.py} (100%) diff --git a/.github/workflows/build-and-publish.yml b/.github/workflows/build-and-publish.yml index 6f62f574f..b3fca0c7b 100644 --- a/.github/workflows/build-and-publish.yml +++ b/.github/workflows/build-and-publish.yml @@ -23,7 +23,7 @@ jobs: - name: Set up Python & Poetry Environment id: set-up-python-and-poetry-environment - uses: exasol/python-toolbox/.github/actions/python-environment@v10 + uses: exasol/python-toolbox/.github/actions/python-environment@v11 with: python-version: "3.10" poetry-version: "2.3.0" diff --git a/.github/workflows/check-release-tag.yml b/.github/workflows/check-release-tag.yml index 59c21632a..cf552af1d 100644 --- a/.github/workflows/check-release-tag.yml +++ b/.github/workflows/check-release-tag.yml @@ -21,7 +21,7 @@ jobs: - name: Set up Python & Poetry Environment id: set-up-python-and-poetry-environment - uses: exasol/python-toolbox/.github/actions/python-environment@v10 + uses: exasol/python-toolbox/.github/actions/python-environment@v11 with: python-version: "3.10" poetry-version: "2.3.0" diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index cac23c83e..143d9000e 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -20,7 +20,7 @@ jobs: - name: Set up Python & Poetry Environment id: set-up-python-and-poetry-environment - uses: exasol/python-toolbox/.github/actions/python-environment@v10 + uses: exasol/python-toolbox/.github/actions/python-environment@v11 with: python-version: "3.10" poetry-version: "2.3.0" @@ -48,7 +48,7 @@ jobs: - name: Set up Python & Poetry Environment id: set-up-python-and-poetry-environment - uses: exasol/python-toolbox/.github/actions/python-environment@v10 + uses: exasol/python-toolbox/.github/actions/python-environment@v11 with: python-version: "3.10" poetry-version: "2.3.0" @@ -75,7 +75,7 @@ jobs: - name: Set up Python & Poetry Environment id: set-up-python-and-poetry-environment - uses: exasol/python-toolbox/.github/actions/python-environment@v10 + uses: exasol/python-toolbox/.github/actions/python-environment@v11 with: python-version: ${{ matrix.python-versions }} poetry-version: "2.3.0" @@ -113,7 +113,7 @@ jobs: - name: Set up Python & Poetry Environment id: set-up-python-and-poetry-environment - uses: exasol/python-toolbox/.github/actions/python-environment@v10 + uses: exasol/python-toolbox/.github/actions/python-environment@v11 with: python-version: ${{ matrix.python-versions }} poetry-version: "2.3.0" @@ -141,7 +141,7 @@ jobs: - name: Set up Python & Poetry Environment id: set-up-python-and-poetry-environment - uses: exasol/python-toolbox/.github/actions/python-environment@v10 + uses: exasol/python-toolbox/.github/actions/python-environment@v11 with: python-version: ${{ matrix.python-versions }} poetry-version: "2.3.0" @@ -173,7 +173,7 @@ jobs: - name: Set up Python & Poetry Environment id: set-up-python-and-poetry-environment - uses: exasol/python-toolbox/.github/actions/python-environment@v10 + uses: exasol/python-toolbox/.github/actions/python-environment@v11 with: python-version: "3.10" poetry-version: "2.3.0" @@ -196,7 +196,7 @@ jobs: - name: Set up Python & Poetry Environment id: set-up-python-and-poetry-environment - uses: exasol/python-toolbox/.github/actions/python-environment@v10 + uses: exasol/python-toolbox/.github/actions/python-environment@v11 with: python-version: "3.10" poetry-version: "2.3.0" @@ -219,7 +219,7 @@ jobs: - name: Set up Python & Poetry Environment id: set-up-python-and-poetry-environment - uses: exasol/python-toolbox/.github/actions/python-environment@v10 + uses: exasol/python-toolbox/.github/actions/python-environment@v11 with: python-version: "3.10" poetry-version: "2.3.0" @@ -242,7 +242,7 @@ jobs: - name: Set up Python & Poetry Environment id: set-up-python-and-poetry-environment - uses: exasol/python-toolbox/.github/actions/python-environment@v10 + uses: exasol/python-toolbox/.github/actions/python-environment@v11 with: python-version: "3.10" poetry-version: "2.3.0" diff --git a/.github/workflows/dependency-update.yml b/.github/workflows/dependency-update.yml index 1abcdb14d..ebf821467 100644 --- a/.github/workflows/dependency-update.yml +++ b/.github/workflows/dependency-update.yml @@ -35,7 +35,7 @@ jobs: - name: Set up Python & Poetry Environment id: set-up-python-and-poetry-environment - uses: exasol/python-toolbox/.github/actions/python-environment@v10 + uses: exasol/python-toolbox/.github/actions/python-environment@v11 with: python-version: "3.10" poetry-version: "2.3.0" diff --git a/.github/workflows/fast-tests.yml b/.github/workflows/fast-tests.yml index 8c279573f..743dc1db6 100644 --- a/.github/workflows/fast-tests.yml +++ b/.github/workflows/fast-tests.yml @@ -25,7 +25,7 @@ jobs: fetch-depth: 0 - name: Set up Python & Poetry Environment id: set-up-python-and-poetry-environment - uses: exasol/python-toolbox/.github/actions/python-environment@v10 + uses: exasol/python-toolbox/.github/actions/python-environment@v11 with: python-version: ${{ matrix.python-versions }} poetry-version: "2.3.0" diff --git a/.github/workflows/gh-pages.yml b/.github/workflows/gh-pages.yml index f8071fcea..bd8ac0045 100644 --- a/.github/workflows/gh-pages.yml +++ b/.github/workflows/gh-pages.yml @@ -22,7 +22,7 @@ jobs: - name: Set up Python & Poetry Environment id: set-up-python-and-poetry-environment - uses: exasol/python-toolbox/.github/actions/python-environment@v10 + uses: exasol/python-toolbox/.github/actions/python-environment@v11 with: python-version: "3.10" poetry-version: "2.3.0" diff --git a/.github/workflows/matrix.yml b/.github/workflows/matrix.yml index f8ef31b94..100e49bcf 100644 --- a/.github/workflows/matrix.yml +++ b/.github/workflows/matrix.yml @@ -28,7 +28,7 @@ jobs: - name: Set up Python & Poetry Environment id: set-up-python-and-poetry-environment - uses: exasol/python-toolbox/.github/actions/python-environment@v10 + uses: exasol/python-toolbox/.github/actions/python-environment@v11 with: python-version: "3.10" poetry-version: "2.3.0" diff --git a/.github/workflows/report.yml b/.github/workflows/report.yml index 2520ddf12..87d6facc3 100644 --- a/.github/workflows/report.yml +++ b/.github/workflows/report.yml @@ -24,7 +24,7 @@ jobs: - name: Set up Python & Poetry Environment id: set-up-python-and-poetry-environment - uses: exasol/python-toolbox/.github/actions/python-environment@v10 + uses: exasol/python-toolbox/.github/actions/python-environment@v11 with: python-version: "3.10" poetry-version: "2.3.0" diff --git a/test/unit/util/skills_test.py b/test/unit/util/skill_utils_test.py similarity index 100% rename from test/unit/util/skills_test.py rename to test/unit/util/skill_utils_test.py From f411302b2d0de066ebd5b55376c264ca58a3312e Mon Sep 17 00:00:00 2001 From: jana-selva Date: Tue, 15 Sep 2026 14:59:24 +0530 Subject: [PATCH 4/6] Apply Black formatting to skill validation --- exasol/toolbox/nox/_skills.py | 4 +--- exasol/toolbox/util/skills.py | 3 +-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/exasol/toolbox/nox/_skills.py b/exasol/toolbox/nox/_skills.py index de0a0ecfd..317ce468c 100644 --- a/exasol/toolbox/nox/_skills.py +++ b/exasol/toolbox/nox/_skills.py @@ -18,9 +18,7 @@ def check_skills(session: Session) -> None: 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 - } + 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) diff --git a/exasol/toolbox/util/skills.py b/exasol/toolbox/util/skills.py index 8d811ed37..7e2c92540 100644 --- a/exasol/toolbox/util/skills.py +++ b/exasol/toolbox/util/skills.py @@ -80,8 +80,7 @@ def validate_skill(skill_name: str) -> tuple[str, ...]: errors.append("contains a TODO marker") all_content = "\n".join( - path.read_text(encoding="utf-8") - for path in skill_files.values() + path.read_text(encoding="utf-8") for path in skill_files.values() ).lower() for term in SKILL_FORBIDDEN_TERMS: if term in all_content: From 5932fee15d3f62f6c4b69d6727014b1bee7da8f3 Mon Sep 17 00:00:00 2001 From: jana-selva Date: Tue, 15 Sep 2026 15:04:14 +0530 Subject: [PATCH 5/6] Support packaged skill resource traversal --- exasol/toolbox/util/skills.py | 38 ++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/exasol/toolbox/util/skills.py b/exasol/toolbox/util/skills.py index 7e2c92540..8e57a9145 100644 --- a/exasol/toolbox/util/skills.py +++ b/exasol/toolbox/util/skills.py @@ -1,10 +1,10 @@ """Utilities for validating packaged agent skills.""" from collections.abc import Mapping -from pathlib import Path from typing import Final import importlib_resources as resources +from importlib_resources.abc import Traversable SKILLS_DIRECTORY: Final = "exasol.toolbox.skills" PTB_SKILL_NAME: Final = "exasol-python-toolbox" @@ -20,31 +20,45 @@ SKILL_FILES: Final = ("SKILL.md",) -def get_skill_path(skill_name: str = PTB_SKILL_NAME) -> Path: +def get_skill_path(skill_name: str = PTB_SKILL_NAME) -> Traversable: """ Return the path to a packaged skill. """ - return Path(str(resources.files(SKILLS_DIRECTORY) / skill_name)) + return resources.files(SKILLS_DIRECTORY) / skill_name -def get_skill_files(skill_name: str = PTB_SKILL_NAME) -> Mapping[str, Path]: +def _find_files( + root: Traversable, relative_directory: str = "" +) -> dict[str, Traversable]: + """Return all files below a package resource directory.""" + files: dict[str, Traversable] = {} + for child in root.iterdir(): + relative_path = f"{relative_directory}{child.name}" + if child.is_file(): + files[relative_path] = child + elif child.is_dir(): + files.update(_find_files(child, f"{relative_path}/")) + return files + + +def get_skill_files(skill_name: str = PTB_SKILL_NAME) -> Mapping[str, Traversable]: """ Return packaged skill files. The keys are paths relative to the skill root. """ - skill_path = get_skill_path(skill_name) - return { - str(path.relative_to(skill_path)): path - for path in skill_path.rglob("*") - if path.is_file() - } + return _find_files(get_skill_path(skill_name)) 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())) + return tuple( + sorted( + path.name + for path in resources.files(SKILLS_DIRECTORY).iterdir() + if path.is_dir() + ) + ) def validate_skill(skill_name: str) -> tuple[str, ...]: From 4f4697c63651bac8642822b048fae33df53f4cdb Mon Sep 17 00:00:00 2001 From: jana-selva Date: Tue, 15 Sep 2026 15:17:25 +0530 Subject: [PATCH 6/6] Improve skill validation coverage and maintainability --- exasol/toolbox/util/skills.py | 89 +++++++++++++++++------------- test/unit/nox/_skills_test.py | 34 ++++++++++++ test/unit/util/skill_utils_test.py | 50 +++++++++++++++++ 3 files changed, 135 insertions(+), 38 deletions(-) create mode 100644 test/unit/nox/_skills_test.py diff --git a/exasol/toolbox/util/skills.py b/exasol/toolbox/util/skills.py index 8e57a9145..4f87af55f 100644 --- a/exasol/toolbox/util/skills.py +++ b/exasol/toolbox/util/skills.py @@ -61,6 +61,50 @@ def get_packaged_skill_names() -> tuple[str, ...]: ) +def _validate_frontmatter(content: str, skill_name: str) -> list[str]: + """Validate the frontmatter of a skill description.""" + parts = content.split(SKILL_FRONTMATTER_SEPARATOR, maxsplit=2) + if len(parts) != 3 or parts[0].strip(): + return ["SKILL.md must start with YAML frontmatter"] + + errors = [] + 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") + return errors + + +def _validate_markdown_file(relative_path: str, content: str) -> list[str]: + """Validate shared rules for one Markdown file.""" + errors: list[str] = [] + seen: dict[str, int] = {} + ignored_lines = {"---", "```bash", "```"} + for line_number, line in enumerate(content.splitlines(), 1): + normalized = line.strip().lower() + if not normalized or normalized in ignored_lines 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 + return errors + + +def _validate_nox_syntax(relative_path: str, content: str) -> list[str]: + """Ensure Nox command syntax is kept in the dedicated reference.""" + nox_reference = "references/nox-sessions.md" + if relative_path != nox_reference and any( + command in content + for command in ("poetry run -- nox -s", "poetry run -- nox -l") + ): + return [f"{relative_path} contains Nox command syntax outside {nox_reference}"] + return [] + + def validate_skill(skill_name: str) -> tuple[str, ...]: """Return deterministic validation errors for a packaged skill. @@ -75,20 +119,12 @@ def validate_skill(skill_name: str) -> tuple[str, ...]: 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: + skill_content = skill_files.get("SKILL.md") + if skill_content 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") + content = skill_content.read_text(encoding="utf-8") + errors.extend(_validate_frontmatter(content, skill_name)) if "[TODO" in content: errors.append("contains a TODO marker") @@ -100,34 +136,11 @@ def validate_skill(skill_name: str) -> tuple[str, ...]: 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}" - ) + text = path.read_text(encoding="utf-8") + errors.extend(_validate_markdown_file(relative_path, text)) + errors.extend(_validate_nox_syntax(relative_path, text)) return tuple(errors) diff --git a/test/unit/nox/_skills_test.py b/test/unit/nox/_skills_test.py new file mode 100644 index 000000000..85d8dd878 --- /dev/null +++ b/test/unit/nox/_skills_test.py @@ -0,0 +1,34 @@ +from unittest.mock import Mock + +import pytest +from nox.sessions import _SessionQuit + +from exasol.toolbox.nox import _skills + + +def test_check_skills_passes_when_all_skills_are_valid(monkeypatch, nox_session): + monkeypatch.setattr( + _skills, "get_packaged_skill_names", Mock(return_value=("one",)) + ) + validate = Mock(return_value=()) + monkeypatch.setattr(_skills, "validate_skill", validate) + + _skills.check_skills(nox_session) + + validate.assert_called_once_with("one") + + +def test_check_skills_reports_all_failures(monkeypatch, nox_session): + monkeypatch.setattr( + _skills, + "get_packaged_skill_names", + Mock(return_value=("one", "two")), + ) + monkeypatch.setattr( + _skills, + "validate_skill", + Mock(side_effect=(("bad frontmatter",), ("missing SKILL.md",))), + ) + + with pytest.raises(_SessionQuit, match="Packaged skill validation failed"): + _skills.check_skills(nox_session) diff --git a/test/unit/util/skill_utils_test.py b/test/unit/util/skill_utils_test.py index 7179cf481..b1c75191f 100644 --- a/test/unit/util/skill_utils_test.py +++ b/test/unit/util/skill_utils_test.py @@ -16,3 +16,53 @@ def test_get_packaged_skill_names_is_sorted(): assert skills.get_packaged_skill_names() == tuple( sorted(skills.get_packaged_skill_names()) ) + + +def test_get_skill_files_recurses_into_resource_directories(tmp_path, monkeypatch): + reference = tmp_path / "references" / "guide.md" + reference.parent.mkdir() + reference.write_text("guide", encoding="utf-8") + monkeypatch.setattr(skills, "get_skill_path", lambda _: tmp_path) + + files = skills.get_skill_files("example") + + assert set(files) == {"references/guide.md"} + assert files["references/guide.md"].read_text(encoding="utf-8") == "guide" + + +def test_validate_skill_reports_shared_rule_violations(tmp_path, monkeypatch): + skill_file = tmp_path / "SKILL.md" + skill_file.write_text( + """--- +name: wrong +--- +[TODO: finish] +main branch +main branch +poetry run -- nox -s test:unit +""", + encoding="utf-8", + ) + (tmp_path / "notes.txt").write_text("notes", encoding="utf-8") + monkeypatch.setattr(skills, "get_skill_path", lambda _: tmp_path) + + errors = skills.validate_skill("example") + + assert "frontmatter name must be example" in errors + assert "frontmatter must contain a description" in errors + assert "contains a TODO marker" in errors + assert "contains forbidden term: main branch" in errors + assert "SKILL.md duplicates line 5 at line 6" in errors + assert ( + "SKILL.md contains Nox command syntax outside references/nox-sessions.md" + in errors + ) + + +def test_validate_skill_requires_frontmatter(tmp_path, monkeypatch): + (tmp_path / "SKILL.md").write_text("skill", encoding="utf-8") + monkeypatch.setattr(skills, "get_skill_path", lambda _: tmp_path) + + assert "SKILL.md must start with YAML frontmatter" in skills.validate_skill( + "example" + )