From d245bec053d589a43863d828d41d9cc0d914bb55 Mon Sep 17 00:00:00 2001 From: jana-selva Date: Tue, 15 Sep 2026 16:16:26 +0530 Subject: [PATCH 1/3] 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 2/3] 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 3/3] 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}")