From 8f8a001284724b1fa7a7202f7f31a6d40f967872 Mon Sep 17 00:00:00 2001 From: jana-selva Date: Fri, 11 Sep 2026 11:13:08 +0530 Subject: [PATCH 01/14] 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 02/14] 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 03/14] 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 04/14] 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 05/14] 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 06/14] 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" + ) From d245bec053d589a43863d828d41d9cc0d914bb55 Mon Sep 17 00:00:00 2001 From: jana-selva Date: Tue, 15 Sep 2026 16:16:26 +0530 Subject: [PATCH 07/14] Add PTB skill installation session --- doc/changes/unreleased.md | 1 + .../features/agent_skills/index.rst | 13 +++++ exasol/toolbox/config.py | 6 +++ exasol/toolbox/nox/_skills.py | 10 ++++ exasol/toolbox/nox/tasks.py | 3 +- exasol/toolbox/util/skills.py | 38 ++++++++++++++ test/unit/skills_test.py | 9 ++++ test/unit/util/skill_utils_test.py | 49 +++++++++++++++++++ 8 files changed, 128 insertions(+), 1 deletion(-) diff --git a/doc/changes/unreleased.md b/doc/changes/unreleased.md index 1fd25163d..bb2e66c07 100644 --- a/doc/changes/unreleased.md +++ b/doc/changes/unreleased.md @@ -3,5 +3,6 @@ ## Features - #940: Added shared validation for packaged agent skills and the `skills:check` Nox session. +- #938: Added the `skills:install` Nox session for installing the packaged PTB agent skill. ## Summary diff --git a/doc/user_guide/features/agent_skills/index.rst b/doc/user_guide/features/agent_skills/index.rst index 3af270264..9e4f8141f 100644 --- a/doc/user_guide/features/agent_skills/index.rst +++ b/doc/user_guide/features/agent_skills/index.rst @@ -21,3 +21,16 @@ duplicated Markdown lines. Nox command examples are kept in the skill's 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. + +Installing the PTB skill +------------------------ + +Projects can install the PTB skill packaged by their current PTB dependency with: + +.. code-block:: shell + + poetry run -- nox -s skills:install + +The session copies the packaged skill into +``.agents/skills/exasol-python-toolbox``. Existing files in that skill directory +are replaced so the installed copy stays aligned with the PTB version. diff --git a/exasol/toolbox/config.py b/exasol/toolbox/config.py index 9e4b010ae..8f987ffad 100644 --- a/exasol/toolbox/config.py +++ b/exasol/toolbox/config.py @@ -310,6 +310,12 @@ def source_code_path(self) -> Path: """ return self.root_path / "exasol" / self.project_name + @computed_field # type: ignore[misc] + @property + def agent_skills_path(self) -> Path: + """Path where project-local agent skills are installed.""" + return self.root_path / ".agents" / "skills" + @computed_field # type: ignore[misc] @property def github_workflow_directory(self) -> Path: diff --git a/exasol/toolbox/nox/_skills.py b/exasol/toolbox/nox/_skills.py index 317ce468c..92158b84f 100644 --- a/exasol/toolbox/nox/_skills.py +++ b/exasol/toolbox/nox/_skills.py @@ -7,6 +7,7 @@ from exasol.toolbox.util.skills import ( get_packaged_skill_names, + install_skill, validate_skill, ) @@ -25,3 +26,12 @@ def check_skills(session: Session) -> None: for skill_name, errors in failures.items() ) session.error(f"Packaged skill validation failed:\n{details}") + + +@nox.session(name="skills:install", python=False) +def install_ptb_skill(session: Session) -> None: + """Install the PTB skill into the project's local agent skill directory.""" + from noxconfig import PROJECT_CONFIG + + target = install_skill(target_directory=PROJECT_CONFIG.agent_skills_path) + session.log(f"Installed {target.name} skill to {target}") diff --git a/exasol/toolbox/nox/tasks.py b/exasol/toolbox/nox/tasks.py index 8c910fb46..3793036ea 100644 --- a/exasol/toolbox/nox/tasks.py +++ b/exasol/toolbox/nox/tasks.py @@ -10,6 +10,7 @@ "integration_tests", "lint", "check_skills", + "install_ptb_skill", "open_docs", "prepare_release", "type_check", @@ -60,7 +61,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._skills import check_skills, install_ptb_skill 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 4f87af55f..bc384fc54 100644 --- a/exasol/toolbox/util/skills.py +++ b/exasol/toolbox/util/skills.py @@ -1,6 +1,8 @@ """Utilities for validating packaged agent skills.""" from collections.abc import Mapping +from pathlib import Path +import shutil from typing import Final import importlib_resources as resources @@ -61,6 +63,42 @@ def get_packaged_skill_names() -> tuple[str, ...]: ) +def _has_symlink_in_parents(path: Path) -> bool: + """Return whether a path or one of its existing parents is a symlink.""" + return any(candidate.is_symlink() for candidate in (path, *path.parents)) + + +def install_skill( + skill_name: str = PTB_SKILL_NAME, + target_directory: Path | None = None, +) -> Path: + """Install a packaged skill into a project-local agent skill directory.""" + if Path(skill_name).name != skill_name: + raise ValueError(f"invalid skill name: {skill_name}") + + source_files = get_skill_files(skill_name) + if not source_files: + raise ValueError(f"packaged skill does not exist: {skill_name}") + + target_directory = target_directory or Path.cwd() / ".agents" / "skills" + target_skill = target_directory / skill_name + if _has_symlink_in_parents(target_directory): + raise ValueError(f"refusing to use symlinked target directory: {target_directory}") + if target_skill.is_symlink(): + raise ValueError(f"refusing to replace symlink: {target_skill}") + if target_skill.exists() and not target_skill.is_dir(): + raise ValueError(f"skill target is not a directory: {target_skill}") + + if target_skill.exists(): + shutil.rmtree(target_skill) + target_skill.mkdir(parents=True, exist_ok=True) + for relative_path, source in source_files.items(): + destination = target_skill / relative_path + destination.parent.mkdir(parents=True, exist_ok=True) + destination.write_bytes(source.read_bytes()) + return target_skill + + 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) diff --git a/test/unit/skills_test.py b/test/unit/skills_test.py index 90076f8e6..373ecbcfd 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, + install_skill, validate_skill, ) @@ -42,6 +43,14 @@ def test_ptb_skill_resources_are_available(): assert skill_files[expected].is_file() +def test_ptb_skill_can_be_installed(tmp_path): + installed = install_skill(PTB_SKILL_NAME, tmp_path) + + assert installed == tmp_path / PTB_SKILL_NAME + for expected in SKILL_FILES: + assert (installed / expected).is_file() + + def test_ptb_skill_resources_are_packaged(tmp_path): build_output = tmp_path / "dist" result = run( diff --git a/test/unit/util/skill_utils_test.py b/test/unit/util/skill_utils_test.py index b1c75191f..ae3b0a44e 100644 --- a/test/unit/util/skill_utils_test.py +++ b/test/unit/util/skill_utils_test.py @@ -1,3 +1,5 @@ +import pytest + from exasol.toolbox.util import skills @@ -66,3 +68,50 @@ def test_validate_skill_requires_frontmatter(tmp_path, monkeypatch): assert "SKILL.md must start with YAML frontmatter" in skills.validate_skill( "example" ) + + +def test_install_skill_copies_all_files_and_replaces_previous_copy(tmp_path, monkeypatch): + source = tmp_path / "source" + source.mkdir() + skill_file = source / "SKILL.md" + reference = source / "references" / "guide.md" + reference.parent.mkdir() + skill_file.write_text("new", encoding="utf-8") + reference.write_text("guide", encoding="utf-8") + monkeypatch.setattr( + skills, + "get_skill_files", + lambda _: {"SKILL.md": skill_file, "references/guide.md": reference}, + ) + target_directory = tmp_path / ".agents" / "skills" + previous = target_directory / "example" + previous.mkdir(parents=True) + (previous / "stale.md").write_text("stale", encoding="utf-8") + + installed = skills.install_skill("example", target_directory) + + assert installed == previous + assert (installed / "SKILL.md").read_text(encoding="utf-8") == "new" + assert (installed / "references" / "guide.md").read_text(encoding="utf-8") == "guide" + assert not (installed / "stale.md").exists() + + +def test_install_skill_rejects_path_traversal(tmp_path): + with pytest.raises(ValueError, match="invalid skill name"): + skills.install_skill("../outside", tmp_path) + + +def test_install_skill_rejects_symlink_target(tmp_path, monkeypatch): + source = tmp_path / "source" + source.mkdir() + skill_file = source / "SKILL.md" + skill_file.write_text("skill", encoding="utf-8") + monkeypatch.setattr(skills, "get_skill_files", lambda _: {"SKILL.md": skill_file}) + target_directory = tmp_path / ".agents" / "skills" + target_directory.mkdir(parents=True) + target = tmp_path / "elsewhere" + target.mkdir() + (target_directory / "example").symlink_to(target, target_is_directory=True) + + with pytest.raises(ValueError, match="refusing to replace symlink"): + skills.install_skill("example", target_directory) From da92964fbba6cf867a466add07eb77eecf7a7d79 Mon Sep 17 00:00:00 2001 From: jana-selva Date: Fri, 18 Sep 2026 10:09:06 +0530 Subject: [PATCH 08/14] Complete PTB skill installation review fixes --- .../references/nox-sessions.md | 14 ++++++++++++++ .../references/source-routing.md | 2 ++ test/unit/config_test.py | 1 + test/unit/nox/_skills_test.py | 19 +++++++++++++++++++ 4 files changed, 36 insertions(+) diff --git a/exasol/toolbox/skills/exasol-python-toolbox/references/nox-sessions.md b/exasol/toolbox/skills/exasol-python-toolbox/references/nox-sessions.md index c22662e9a..82761c107 100644 --- a/exasol/toolbox/skills/exasol-python-toolbox/references/nox-sessions.md +++ b/exasol/toolbox/skills/exasol-python-toolbox/references/nox-sessions.md @@ -19,6 +19,13 @@ The sessions below match the PTB version that includes this skill. | `lint:typing` | Run type checks. | It runs Mypy on filtered project Python files. | | `lint:security` | Run security lint. | It runs Bandit and writes `.security.json`. | +## Agent skill sessions + +| Session | Use | Notes | +| --- | --- | --- | +| `skills:check` | Validate packaged PTB skills. | It checks common structure and content rules. | +| `skills:install` | Install the PTB agent skill. | It updates `.agents/skills/exasol-python-toolbox` from the installed PTB package. | + ## Test sessions | Session | Use | Notes | @@ -36,6 +43,13 @@ poetry run -- nox -s test:unit -- -k scenario poetry run -- nox -s test:integration -- --db-version 8.34.0 ``` +Agent skill command examples: + +```bash +poetry run -- nox -s skills:check +poetry run -- nox -s skills:install +``` + ## Documentation and changelog sessions | Session | Use | Notes | diff --git a/exasol/toolbox/skills/exasol-python-toolbox/references/source-routing.md b/exasol/toolbox/skills/exasol-python-toolbox/references/source-routing.md index 1c9559cf1..185c714d6 100644 --- a/exasol/toolbox/skills/exasol-python-toolbox/references/source-routing.md +++ b/exasol/toolbox/skills/exasol-python-toolbox/references/source-routing.md @@ -30,6 +30,8 @@ Read the source file before you explain a detailed rule. - `exasol/toolbox/nox/_format.py`: `format:fix` and `format:check`. - `exasol/toolbox/nox/_lint.py`: `lint:code`, `lint:typing`, and `lint:security`. +- `exasol/toolbox/nox/_skills.py`: packaged skill validation and installation + implementations. - `exasol/toolbox/nox/_matrix.py`: matrix output sessions for CI usage. - `exasol/toolbox/nox/_package.py`: package validation. - `exasol/toolbox/nox/_release.py`: release preparation, release update, and diff --git a/test/unit/config_test.py b/test/unit/config_test.py index cf86c057b..8b4f34fc7 100644 --- a/test/unit/config_test.py +++ b/test/unit/config_test.py @@ -45,6 +45,7 @@ def test_works_as_defined(tmp_path, test_project_config_factory): "dependency_manager": {"name": "poetry", "version": "2.3.0"}, "documentation_path": root_path / "doc", "has_documentation": True, + "agent_skills_path": root_path / ".agents" / "skills", "exasol_versions": ("8.29.13", "2025.1.8"), "excluded_python_paths": expand_paths(config, DEFAULT_EXCLUDED_PATHS), "github_workflow_directory": tmp_path / ".github" / "workflows", diff --git a/test/unit/nox/_skills_test.py b/test/unit/nox/_skills_test.py index 85d8dd878..d58732f0a 100644 --- a/test/unit/nox/_skills_test.py +++ b/test/unit/nox/_skills_test.py @@ -3,6 +3,7 @@ import pytest from nox.sessions import _SessionQuit +import noxconfig from exasol.toolbox.nox import _skills @@ -32,3 +33,21 @@ def test_check_skills_reports_all_failures(monkeypatch, nox_session): with pytest.raises(_SessionQuit, match="Packaged skill validation failed"): _skills.check_skills(nox_session) + + +def test_install_ptb_skill_uses_project_skill_directory( + monkeypatch, nox_session, tmp_path +): + target_directory = tmp_path / ".agents" / "skills" + target = target_directory / "exasol-python-toolbox" + monkeypatch.setattr( + noxconfig, + "PROJECT_CONFIG", + Mock(agent_skills_path=target_directory), + ) + install = Mock(return_value=target) + monkeypatch.setattr(_skills, "install_skill", install) + + _skills.install_ptb_skill(nox_session) + + install.assert_called_once_with(target_directory=target_directory) From 33cfa2a5d63af5783a332086c2486e465e38ece0 Mon Sep 17 00:00:00 2001 From: jana-selva Date: Fri, 18 Sep 2026 15:04:39 +0530 Subject: [PATCH 09/14] Improve skill validation error formatting --- exasol/toolbox/nox/_skills.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/exasol/toolbox/nox/_skills.py b/exasol/toolbox/nox/_skills.py index 92158b84f..909f34ae5 100644 --- a/exasol/toolbox/nox/_skills.py +++ b/exasol/toolbox/nox/_skills.py @@ -12,6 +12,12 @@ ) +def _format_skill_errors(skill_name: str, errors: tuple[str, ...]) -> str: + """Format validation errors for one skill.""" + error_list = "\n".join(f" - {error}" for error in errors) + return f"{skill_name}:\n{error_list}" + + @nox.session(name="skills:check", python=False) def check_skills(session: Session) -> None: """Validate the common structure and content rules for packaged skills.""" @@ -22,7 +28,7 @@ def check_skills(session: Session) -> None: 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) + _format_skill_errors(skill_name, errors) for skill_name, errors in failures.items() ) session.error(f"Packaged skill validation failed:\n{details}") From 70d1ff76dc0a4819c9145e81d18468c859041446 Mon Sep 17 00:00:00 2001 From: jana-selva Date: Fri, 18 Sep 2026 15:04:39 +0530 Subject: [PATCH 10/14] Improve skill validation error formatting --- exasol/toolbox/nox/_skills.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/exasol/toolbox/nox/_skills.py b/exasol/toolbox/nox/_skills.py index 317ce468c..7ca000755 100644 --- a/exasol/toolbox/nox/_skills.py +++ b/exasol/toolbox/nox/_skills.py @@ -11,6 +11,12 @@ ) +def _format_skill_errors(skill_name: str, errors: tuple[str, ...]) -> str: + """Format validation errors for one skill.""" + error_list = "\n".join(f" - {error}" for error in errors) + return f"{skill_name}:\n{error_list}" + + @nox.session(name="skills:check", python=False) def check_skills(session: Session) -> None: """Validate the common structure and content rules for packaged skills.""" @@ -21,7 +27,7 @@ def check_skills(session: Session) -> None: 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) + _format_skill_errors(skill_name, errors) for skill_name, errors in failures.items() ) session.error(f"Packaged skill validation failed:\n{details}") From 52a644a51d6b4a04c4638c44d2f68dd82f22d1e0 Mon Sep 17 00:00:00 2001 From: jana-selva Date: Tue, 22 Sep 2026 10:01:57 +0530 Subject: [PATCH 11/14] Address remaining skill validation review comments --- exasol/toolbox/nox/_skills.py | 10 +++++----- exasol/toolbox/util/skills.py | 15 +++++++++------ test/unit/nox/_skills_test.py | 6 +++++- test/unit/util/skill_utils_test.py | 10 ++++++++++ 4 files changed, 29 insertions(+), 12 deletions(-) diff --git a/exasol/toolbox/nox/_skills.py b/exasol/toolbox/nox/_skills.py index 7ca000755..a4a3dfd57 100644 --- a/exasol/toolbox/nox/_skills.py +++ b/exasol/toolbox/nox/_skills.py @@ -20,11 +20,11 @@ def _format_skill_errors(skill_name: str, errors: tuple[str, ...]) -> str: @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} + failures = {} + for skill_name in get_packaged_skill_names(): + errors = validate_skill(skill_name) + if errors: + failures[skill_name] = errors if failures: details = "\n".join( _format_skill_errors(skill_name, errors) diff --git a/exasol/toolbox/util/skills.py b/exasol/toolbox/util/skills.py index 4f87af55f..50d4286d5 100644 --- a/exasol/toolbox/util/skills.py +++ b/exasol/toolbox/util/skills.py @@ -52,13 +52,16 @@ def get_skill_files(skill_name: str = PTB_SKILL_NAME) -> Mapping[str, Traversabl def get_packaged_skill_names() -> tuple[str, ...]: """Return the names of all skills packaged with the toolbox.""" - return tuple( - sorted( - path.name - for path in resources.files(SKILLS_DIRECTORY).iterdir() - if path.is_dir() + try: + skills_path = resources.files(SKILLS_DIRECTORY) + return tuple( + sorted(path.name for path in skills_path.iterdir() if path.is_dir()) ) - ) + except (FileNotFoundError, ModuleNotFoundError) as error: + raise RuntimeError( + "Packaged PTB skills are unavailable. Reinstall exasol-toolbox " + "with its package resources." + ) from error def _validate_frontmatter(content: str, skill_name: str) -> list[str]: diff --git a/test/unit/nox/_skills_test.py b/test/unit/nox/_skills_test.py index 85d8dd878..860e501e8 100644 --- a/test/unit/nox/_skills_test.py +++ b/test/unit/nox/_skills_test.py @@ -30,5 +30,9 @@ def test_check_skills_reports_all_failures(monkeypatch, nox_session): Mock(side_effect=(("bad frontmatter",), ("missing SKILL.md",))), ) - with pytest.raises(_SessionQuit, match="Packaged skill validation failed"): + with pytest.raises(_SessionQuit) as error: _skills.check_skills(nox_session) + + message = str(error.value) + assert "one:\n - bad frontmatter" in message + assert "two:\n - missing SKILL.md" in message diff --git a/test/unit/util/skill_utils_test.py b/test/unit/util/skill_utils_test.py index b1c75191f..54fae9eb0 100644 --- a/test/unit/util/skill_utils_test.py +++ b/test/unit/util/skill_utils_test.py @@ -18,6 +18,16 @@ def test_get_packaged_skill_names_is_sorted(): ) +def test_get_packaged_skill_names_reports_missing_resources(monkeypatch): + def raise_file_not_found(_): + raise FileNotFoundError("skills") + + monkeypatch.setattr(skills.resources, "files", raise_file_not_found) + + with pytest.raises(RuntimeError, match="Packaged PTB skills are unavailable"): + 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() From df319d4ef3b82ba712fe10364b003cdbb7c9cded Mon Sep 17 00:00:00 2001 From: jana-selva Date: Tue, 22 Sep 2026 10:34:04 +0530 Subject: [PATCH 12/14] Add missing pytest import --- test/unit/util/skill_utils_test.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/unit/util/skill_utils_test.py b/test/unit/util/skill_utils_test.py index 54fae9eb0..f4d936d1b 100644 --- a/test/unit/util/skill_utils_test.py +++ b/test/unit/util/skill_utils_test.py @@ -1,3 +1,5 @@ +import pytest + from exasol.toolbox.util import skills From 9b5ed3ab8e417ddfbe396a6758efbb9640fc3bbc Mon Sep 17 00:00:00 2001 From: jana-selva Date: Tue, 22 Sep 2026 12:07:55 +0530 Subject: [PATCH 13/14] Format skill resource imports --- exasol/toolbox/util/skills.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exasol/toolbox/util/skills.py b/exasol/toolbox/util/skills.py index bc384fc54..212942472 100644 --- a/exasol/toolbox/util/skills.py +++ b/exasol/toolbox/util/skills.py @@ -1,8 +1,8 @@ """Utilities for validating packaged agent skills.""" +import shutil from collections.abc import Mapping from pathlib import Path -import shutil from typing import Final import importlib_resources as resources From 96fc33b6719ab8b2e746ecf3eb9dc84486e3833f Mon Sep 17 00:00:00 2001 From: jana-selva Date: Tue, 22 Sep 2026 12:15:49 +0530 Subject: [PATCH 14/14] Format skill validation files --- exasol/toolbox/util/skills.py | 4 +++- test/unit/nox/_skills_test.py | 1 + test/unit/util/skill_utils_test.py | 8 ++++++-- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/exasol/toolbox/util/skills.py b/exasol/toolbox/util/skills.py index 73a2d924f..fcf9e9fd0 100644 --- a/exasol/toolbox/util/skills.py +++ b/exasol/toolbox/util/skills.py @@ -86,7 +86,9 @@ def install_skill( target_directory = target_directory or Path.cwd() / ".agents" / "skills" target_skill = target_directory / skill_name if _has_symlink_in_parents(target_directory): - raise ValueError(f"refusing to use symlinked target directory: {target_directory}") + raise ValueError( + f"refusing to use symlinked target directory: {target_directory}" + ) if target_skill.is_symlink(): raise ValueError(f"refusing to replace symlink: {target_skill}") if target_skill.exists() and not target_skill.is_dir(): diff --git a/test/unit/nox/_skills_test.py b/test/unit/nox/_skills_test.py index c7a46519b..052ef5344 100644 --- a/test/unit/nox/_skills_test.py +++ b/test/unit/nox/_skills_test.py @@ -38,6 +38,7 @@ def test_check_skills_reports_all_failures(monkeypatch, nox_session): assert "one:\n - bad frontmatter" in message assert "two:\n - missing SKILL.md" in message + def test_install_ptb_skill_uses_project_skill_directory( monkeypatch, nox_session, tmp_path ): diff --git a/test/unit/util/skill_utils_test.py b/test/unit/util/skill_utils_test.py index bcc3c2445..7f36cd81a 100644 --- a/test/unit/util/skill_utils_test.py +++ b/test/unit/util/skill_utils_test.py @@ -80,7 +80,9 @@ def test_validate_skill_requires_frontmatter(tmp_path, monkeypatch): ) -def test_install_skill_copies_all_files_and_replaces_previous_copy(tmp_path, monkeypatch): +def test_install_skill_copies_all_files_and_replaces_previous_copy( + tmp_path, monkeypatch +): source = tmp_path / "source" source.mkdir() skill_file = source / "SKILL.md" @@ -102,7 +104,9 @@ def test_install_skill_copies_all_files_and_replaces_previous_copy(tmp_path, mon assert installed == previous assert (installed / "SKILL.md").read_text(encoding="utf-8") == "new" - assert (installed / "references" / "guide.md").read_text(encoding="utf-8") == "guide" + assert (installed / "references" / "guide.md").read_text( + encoding="utf-8" + ) == "guide" assert not (installed / "stale.md").exists()