diff --git a/doc/api/index.rst b/doc/api/index.rst index 8ad43c961..3b9fc2209 100644 --- a/doc/api/index.rst +++ b/doc/api/index.rst @@ -7,5 +7,6 @@ :maxdepth: 2 base_config + skill_eval workflow_exceptions workflow_patcher_config diff --git a/doc/api/skill_eval.rst b/doc/api/skill_eval.rst new file mode 100644 index 000000000..454101aa8 --- /dev/null +++ b/doc/api/skill_eval.rst @@ -0,0 +1,10 @@ +.. _skill_eval: + +Skill Evaluation Cases +---------------------- + +These models define the structure of the ``eval_cases.yml`` file packaged with +an agent skill. + +.. automodule:: exasol.toolbox.util.skill_eval + :members: diff --git a/doc/changes/unreleased.md b/doc/changes/unreleased.md index f8092cb65..7e971f989 100644 --- a/doc/changes/unreleased.md +++ b/doc/changes/unreleased.md @@ -5,4 +5,5 @@ * #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. * #942: Added api-contract-audit skill for identifying mismatches between type annotations, docstrings, and runtime - behavior \ No newline at end of file + behavior +* #963: Extended packaged skill checks and installation to support multiple skills. diff --git a/doc/developer_guide/agent_skills.rst b/doc/developer_guide/agent_skills.rst new file mode 100644 index 000000000..afa65328a --- /dev/null +++ b/doc/developer_guide/agent_skills.rst @@ -0,0 +1,69 @@ +.. _developer_agent_skills: + +Testing Agent Skills +==================== + +Packaged Skills +--------------- + +The ``skills:check`` Nox session validates common structure and content rules +for every packaged skill. + +The PTB pytest suite verifies skill packaging and installation with: + +* ``test/unit/skills_test.py``: packaging, installation, required files, and + ``eval_cases.yml`` validation. +* ``test/unit/util/skill_test.py``: lower-level validation and + installation behavior. +* ``test/integration/project-template/nox_test.py``: verifies that + ``skills:check`` and ``skills:install`` operate in a newly created project. +* ``test/unit/nox/_skills_test.py``: tests Nox session behavior and failure + reporting. + +When adding a skill to the PTB: + +* Add its ``eval_cases.yml`` with representative prompts and expected response + characteristics. +* Add assertions for any files beyond ``SKILL.md`` that the skill must package + and install. +* Add deterministic, skill-specific assertions where the shared checks are + insufficient. + +These evaluation files and tests are PTB development resources; they are not +required by downstream projects using the packaged skills. + +Writing ``eval_cases.yml`` +-------------------------- + +An evaluation case should describe one distinct user goal. Keep the prompt +specific enough that a good response can be recognized from observable +evidence, and avoid requirements that merely repeat the skill's name or ask +for generic quality. + +Use ``must_include`` for concrete evidence that should appear in a correct +response, such as: + +* the names of the APIs or files that were inspected; +* the tools or checks that must be used, when they are part of the skill's + intended behavior; and +* the reported mismatch, affected API, severity, or other required result. + +Use ``must_not_include`` for concrete behavior the skill must avoid, such as +inventing findings, treating one source as authoritative without comparison, +or proposing implementation changes when the task asks only for an audit. +The prohibited text should describe an actual failure mode, not a broad word +that could legitimately occur in a response. + +For example, a signature and docstring audit can name specific existing +functions in its prompt and require evidence from +``inspect.signature()``, ``inspect.get_annotations()``, +``typing.get_type_hints()``, and ``inspect.getdoc()``. Its ``must_include`` +values can then require the inspected function names, the introspection tools, +and the concrete mismatch. This makes the case test the skill's intended +audit behavior instead of only testing whether the response is generally +relevant. + +Each case should have a unique ``id``, a meaningful ``category``, a focused +``prompt``, and non-empty ``must_include`` and ``must_not_include`` lists. +Cases should cover different behaviors rather than restating the same prompt +with minor wording changes. diff --git a/doc/developer_guide/developer_guide.rst b/doc/developer_guide/developer_guide.rst index 03037fd33..aced0a588 100644 --- a/doc/developer_guide/developer_guide.rst +++ b/doc/developer_guide/developer_guide.rst @@ -9,3 +9,4 @@ ../design plugins + agent_skills diff --git a/doc/user_guide/features/agent_skills/index.rst b/doc/user_guide/features/agent_skills/index.rst index 9e4f8141f..55870ff0d 100644 --- a/doc/user_guide/features/agent_skills/index.rst +++ b/doc/user_guide/features/agent_skills/index.rst @@ -6,6 +6,26 @@ Agent Skills The PTB can package agent skills for use by projects and provides shared validation for their common structure and content rules. +Packaged skills maintained by the PTB +------------------------------------- + +The following skill directories are provided by the PTB. Installing packaged +skills can replace a project-local skill with the same name, so choose local +skill names with this list in mind. + +.. list-table:: + :widths: 30 70 + :header-rows: 1 + + * - Skill directory + - Intended use + * - ``api-contract-audit`` + - Audits a Python library for mismatches between type annotations, + docstrings, user-facing documentation, and runtime behavior. + * - ``exasol-python-toolbox`` + - Guides agents in using PTB setup, Nox sessions, checks, workflows, + updates, releases, and configuration. + Run the validation with: .. code-block:: shell @@ -18,19 +38,20 @@ 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. +These shared checks are intentionally separate from skill-specific tests. The +test suite validates the structure of every packaged skill and validates the +structure of every available ``eval_cases.yml``. When adding a skill, add its +expected files and behavior assertions to the shared skill test patterns. -Installing the PTB skill ------------------------- +Installing packaged skills +--------------------------- -Projects can install the PTB skill packaged by their current PTB dependency with: +Projects can install all skills 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. +The session copies each packaged skill into its own directory below +``.agents/skills``. Existing files in those skill directories are replaced so +the installed copies stay aligned with the PTB version. diff --git a/exasol/toolbox/nox/_skills.py b/exasol/toolbox/nox/_skills.py index 2d968bf07..8367f5bbc 100644 --- a/exasol/toolbox/nox/_skills.py +++ b/exasol/toolbox/nox/_skills.py @@ -22,6 +22,7 @@ def _format_skill_errors(skill_name: str, errors: tuple[str, ...]) -> str: def check_skills(session: Session) -> None: """Validate the common structure and content rules for packaged skills.""" failures = {} + # Discover skills at runtime so newly packaged skills are installed too. for skill_name in get_packaged_skill_names(): errors = validate_skill(skill_name) if errors: @@ -35,9 +36,12 @@ def check_skills(session: Session) -> None: @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.""" +def install_skills(session: Session) -> None: + """Install all packaged skills into the project's local agent 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}") + for skill_name in get_packaged_skill_names(): + target = install_skill( + skill_name, 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 3793036ea..c298f85fd 100644 --- a/exasol/toolbox/nox/tasks.py +++ b/exasol/toolbox/nox/tasks.py @@ -10,7 +10,7 @@ "integration_tests", "lint", "check_skills", - "install_ptb_skill", + "install_skills", "open_docs", "prepare_release", "type_check", @@ -61,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, install_ptb_skill +from exasol.toolbox.nox._skills import check_skills, install_skills from exasol.toolbox.nox._shared import ( Mode, _integration_test_context, diff --git a/exasol/toolbox/skills/api-contract-audit/SKILL.md b/exasol/toolbox/skills/api-contract-audit/SKILL.md index de5e6f77e..3f76f6cd6 100644 --- a/exasol/toolbox/skills/api-contract-audit/SKILL.md +++ b/exasol/toolbox/skills/api-contract-audit/SKILL.md @@ -1,4 +1,5 @@ --- +# Generated and maintained by the exasol-toolbox. name: api-contract-audit description: Audit a Python library's public API for inconsistencies between type annotations, docstrings, user-facing documentation/examples, and actual runtime behavior. Use when reviewing API changes, checking whether public methods accept undocumented parameter shapes, or validating that docs and type hints match enforcement in code. --- @@ -87,4 +88,4 @@ If no findings are discovered, say that explicitly and mention any coverage limi - Do not rewrite the API contract on your own. If code, docs, and examples disagree, report the disagreement. - Do not stop at the first example. Check for the same pattern across sibling APIs. -- Do not treat private helper inconsistencies as findings unless they affect public behavior. \ No newline at end of file +- Do not treat private helper inconsistencies as findings unless they affect public behavior. diff --git a/exasol/toolbox/skills/exasol-python-toolbox/SKILL.md b/exasol/toolbox/skills/exasol-python-toolbox/SKILL.md index e9d6ee49a..523e54a50 100644 --- a/exasol/toolbox/skills/exasol-python-toolbox/SKILL.md +++ b/exasol/toolbox/skills/exasol-python-toolbox/SKILL.md @@ -1,4 +1,5 @@ --- +# Generated and maintained by the exasol-toolbox. name: exasol-python-toolbox description: Use this skill in Exasol Python projects that use exasol-toolbox/PTB. Use it for PTB setup, nox sessions, code checks, GitHub workflows, updates, releases, and PTB configuration. Use it when an agent must not replace PTB automation. --- 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 82761c107..063b45d39 100644 --- a/exasol/toolbox/skills/exasol-python-toolbox/references/nox-sessions.md +++ b/exasol/toolbox/skills/exasol-python-toolbox/references/nox-sessions.md @@ -23,8 +23,8 @@ The sessions below match the PTB version that includes this skill. | 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. | +| `skills:check` | Validate packaged skills. | It checks common structure and content rules for every packaged skill. | +| `skills:install` | Install packaged agent skills. | It updates each packaged skill below `.agents/skills` from the installed PTB package. | ## Test sessions diff --git a/exasol/toolbox/util/skill_eval.py b/exasol/toolbox/util/skill_eval.py new file mode 100644 index 000000000..06ac03ac4 --- /dev/null +++ b/exasol/toolbox/util/skill_eval.py @@ -0,0 +1,44 @@ +"""Models for validating packaged agent-skill evaluation cases.""" + +from typing import ( + Annotated, + Literal, +) + +from pydantic import ( + BaseModel, + ConfigDict, + Field, +) + +NonBlankString = Annotated[str, Field(pattern=r"\S")] + + +class ExpectedResponse(BaseModel): + """Required and forbidden content for one evaluation response.""" + + model_config = ConfigDict(extra="forbid") + + must_include: list[NonBlankString] = Field(min_length=1) + must_not_include: list[NonBlankString] = Field(min_length=1) + + +class EvalCase(BaseModel): + """One prompt and its expected response constraints.""" + + model_config = ConfigDict(extra="forbid") + + id: NonBlankString + category: NonBlankString + prompt: NonBlankString + expected: ExpectedResponse + + +class PackagedSkillEvalCases(BaseModel): + """Schema for a packaged skill's ``eval_cases.yml`` file.""" + + model_config = ConfigDict(extra="forbid") + + version: Literal[1] + skill: NonBlankString + cases: list[EvalCase] = Field(min_length=1) diff --git a/exasol/toolbox/util/skills.py b/exasol/toolbox/util/skills.py index fcf9e9fd0..6eae1ff0d 100644 --- a/exasol/toolbox/util/skills.py +++ b/exasol/toolbox/util/skills.py @@ -61,7 +61,7 @@ def get_packaged_skill_names() -> tuple[str, ...]: ) except (FileNotFoundError, ModuleNotFoundError) as error: raise RuntimeError( - "Packaged PTB skills are unavailable. Reinstall exasol-toolbox " + "Packaged skills are unavailable. Reinstall exasol-toolbox " "with its package resources." ) from error diff --git a/test/integration/project-template/nox_test.py b/test/integration/project-template/nox_test.py index 994e998fa..f42751e5a 100644 --- a/test/integration/project-template/nox_test.py +++ b/test/integration/project-template/nox_test.py @@ -1,5 +1,47 @@ from exasol.toolbox.util.version import Version +EXPECTED_PACKAGED_SKILL_FILE_COUNTS = { + "api-contract-audit": 1, + "exasol-python-toolbox": 5, +} +EXPECTED_NOX_SESSIONS = { + "format:fix", + "format:check", + "project:check", + "test:unit", + "test:integration", + "test:coverage", + "lint:code", + "lint:typing", + "lint:security", + "lint:dependencies", + "docs:multiversion", + "docs:build", + "docs:open", + "docs:clean", + "links:list", + "links:check", + "changelog:updated", + "release:prepare", + "release:update", + "release:trigger", + "skills:check", + "skills:install", + "matrix:generate", + "artifacts:validate", + "artifacts:copy", + "sonar:check", + "dependency:licenses", + "dependency:audit", + "vulnerabilities:update", + "vulnerabilities:resolved", + "dependency:sbom", + "package:check", + "workflow:check", + "workflow:generate", + "workflow:audit", +} + class TestSpecificNoxTasks: """ @@ -84,3 +126,37 @@ def test_install_github_workflows(self, poetry_path, run_command): file_list = run_command(["ls", ".github/workflows"]).stdout.splitlines() assert len(file_list) == 14 + + def test_skills_install_and_check(self, poetry_path, run_command, new_project): + skills_install = self._command(poetry_path, "skills:install") + run_command(skills_install) + + installed_skills = new_project / ".agents" / "skills" + installed_skill_names = { + path.name for path in installed_skills.iterdir() if path.is_dir() + } + assert set(EXPECTED_PACKAGED_SKILL_FILE_COUNTS) <= installed_skill_names + + for ( + skill_name, + expected_file_count, + ) in EXPECTED_PACKAGED_SKILL_FILE_COUNTS.items(): + installed_file_count = sum( + path.is_file() for path in (installed_skills / skill_name).rglob("*") + ) + assert installed_file_count == expected_file_count + + skills_check = self._command(poetry_path, "skills:check") + output = run_command(skills_check) + + assert output.returncode == 0 + + def test_exposed_nox_sessions(self, poetry_path, run_command): + output = run_command([poetry_path, "run", "--", "nox", "-l"]) + sessions = { + line[2:].split(" ->", maxsplit=1)[0] + for line in output.stdout.splitlines() + if line.startswith(("* ", "- ")) + } + + assert sessions == EXPECTED_NOX_SESSIONS diff --git a/test/unit/nox/_skills_test.py b/test/unit/nox/_skills_test.py index 052ef5344..f18149f3e 100644 --- a/test/unit/nox/_skills_test.py +++ b/test/unit/nox/_skills_test.py @@ -39,19 +39,25 @@ def test_check_skills_reports_all_failures(monkeypatch, nox_session): assert "two:\n - missing SKILL.md" in message -def test_install_ptb_skill_uses_project_skill_directory( +def test_install_skills_uses_project_skill_directory( monkeypatch, nox_session, tmp_path ): target_directory = tmp_path / ".agents" / "skills" - target = target_directory / "exasol-python-toolbox" + targets = {name: target_directory / name for name in ("one", "two")} monkeypatch.setattr( noxconfig, "PROJECT_CONFIG", Mock(agent_skills_path=target_directory), ) - install = Mock(return_value=target) + monkeypatch.setattr( + _skills, "get_packaged_skill_names", Mock(return_value=("one", "two")) + ) + install = Mock(side_effect=lambda name, target_directory: targets[name]) monkeypatch.setattr(_skills, "install_skill", install) - _skills.install_ptb_skill(nox_session) + _skills.install_skills(nox_session) - install.assert_called_once_with(target_directory=target_directory) + assert install.call_args_list == [ + (("one",), {"target_directory": target_directory}), + (("two",), {"target_directory": target_directory}), + ] diff --git a/test/unit/nox/tasks_test.py b/test/unit/nox/tasks_test.py new file mode 100644 index 000000000..800fe9c88 --- /dev/null +++ b/test/unit/nox/tasks_test.py @@ -0,0 +1,33 @@ +from unittest.mock import Mock + +from exasol.toolbox.nox import tasks +from exasol.toolbox.nox._shared import Mode + + +def test_project_check_runs_all_project_checks(monkeypatch, nox_session, tmp_path): + config = Mock(root_path=tmp_path) + context = {"coverage": True, "fwd-args": []} + python_files = ["src/example.py"] + integration_context = Mock(return_value=context) + get_python_files = Mock(return_value=python_files) + code_format = Mock() + pylint = Mock() + type_check = Mock() + coverage = Mock() + + monkeypatch.setattr(tasks, "PROJECT_CONFIG", config) + monkeypatch.setattr(tasks, "_integration_test_context", integration_context) + monkeypatch.setattr(tasks, "get_filtered_python_files", get_python_files) + monkeypatch.setattr(tasks, "_code_format", code_format) + monkeypatch.setattr(tasks, "_pylint", pylint) + monkeypatch.setattr(tasks, "_type_check", type_check) + monkeypatch.setattr(tasks, "_coverage", coverage) + + tasks.check(nox_session) + + integration_context.assert_called_once_with(nox_session, coverage=True) + get_python_files.assert_called_once_with(tmp_path) + code_format.assert_called_once_with(nox_session, Mode.Check, python_files) + pylint.assert_called_once_with(nox_session, python_files) + type_check.assert_called_once_with(nox_session, python_files) + coverage.assert_called_once_with(nox_session, config, context) diff --git a/test/unit/skills_test.py b/test/unit/skills_test.py index 373ecbcfd..925e62c95 100644 --- a/test/unit/skills_test.py +++ b/test/unit/skills_test.py @@ -2,14 +2,16 @@ from subprocess import run from zipfile import ZipFile +import pytest from ruamel.yaml import YAML +from exasol.toolbox.util.skill_eval import PackagedSkillEvalCases from exasol.toolbox.util.skills import ( PTB_SKILL_NAME, + get_packaged_skill_names, get_skill_files, get_skill_path, install_skill, - validate_skill, ) PROJECT_ROOT = Path(__file__).parents[2] @@ -21,18 +23,25 @@ "references/nox-sessions.md", "references/source-routing.md", ] -EVAL_CASES = ( - PROJECT_ROOT - / "test" - / "resources" - / "skills" - / "exasol-python-toolbox" - / "eval_cases.yml" -) -def _load_eval_cases() -> dict: - return YAML(typ="safe").load(EVAL_CASES) +def _eval_cases_path(skill_name: str) -> Path: + return ( + PROJECT_ROOT / "test" / "resources" / "skills" / skill_name / "eval_cases.yml" + ) + + +def _load_eval_cases(skill_name: str) -> dict: + return YAML(typ="safe").load(_eval_cases_path(skill_name)) + + +def _skills_with_eval_cases() -> list[str]: + # Keep this data-driven so adding a packaged skill requires no test edit. + return [ + skill_name + for skill_name in get_packaged_skill_names() + if _eval_cases_path(skill_name).is_file() + ] def test_ptb_skill_resources_are_available(): @@ -51,6 +60,14 @@ def test_ptb_skill_can_be_installed(tmp_path): assert (installed / expected).is_file() +@pytest.mark.parametrize("skill_name", get_packaged_skill_names()) +def test_packaged_skills_can_be_installed(skill_name, tmp_path): + installed = install_skill(skill_name, tmp_path) + + assert installed == tmp_path / skill_name + assert (installed / "SKILL.md").is_file() + + def test_ptb_skill_resources_are_packaged(tmp_path): build_output = tmp_path / "dist" result = run( @@ -92,32 +109,48 @@ def test_ptb_skill_frontmatter_is_complete(): assert "[TODO" not in content -def test_ptb_skill_passes_shared_validation(): - assert validate_skill(PTB_SKILL_NAME) == () +@pytest.mark.parametrize("skill_name", _skills_with_eval_cases()) +class TestPackagedSkillEvalCases: + @pytest.fixture(scope="module") + def eval_cases_by_skill(self): + # Parse each packaged artifact once so all checks use the same model. + return { + skill_name: PackagedSkillEvalCases.model_validate( + _load_eval_cases(skill_name) + ) + for skill_name in _skills_with_eval_cases() + } + + @pytest.fixture + def eval_cases(self, eval_cases_by_skill, skill_name): + return eval_cases_by_skill[skill_name] + def test_has_expected_metadata(self, eval_cases, skill_name): + assert eval_cases.version == 1 + assert eval_cases.skill == skill_name + assert eval_cases.cases -def test_ptb_skill_eval_cases_are_valid(): - eval_cases = _load_eval_cases() + def test_cases_have_required_fields(self, eval_cases): + for case in eval_cases.cases: + assert case.id + assert case.category + assert case.prompt - assert eval_cases["version"] == 1 - assert eval_cases["skill"] == "exasol-python-toolbox" - # Keep enough cases to cover the ticket scope, but not so many that the - # deterministic eval file becomes hard to review. - assert 6 <= len(eval_cases["cases"]) <= 8 + def test_cases_have_response_constraints(self, eval_cases): + for case in eval_cases.cases: + assert case.expected.must_include + assert case.expected.must_not_include + assert all(value.strip() for value in case.expected.must_include) + assert all(value.strip() for value in case.expected.must_not_include) - ids = [case["id"] for case in eval_cases["cases"]] - assert len(ids) == len(set(ids)) + def test_case_ids_are_unique(self, eval_cases): + ids = [case.id for case in eval_cases.cases] - for case in eval_cases["cases"]: - assert case["id"] - assert case["category"] - assert case["prompt"] - assert case["expected"]["must_include"] - assert case["expected"]["must_not_include"] + assert len(ids) == len(set(ids)) def test_ptb_skill_eval_cases_cover_ticket_scope(): - eval_cases = _load_eval_cases() + eval_cases = _load_eval_cases(PTB_SKILL_NAME) categories = {case["category"] for case in eval_cases["cases"]} assert { @@ -131,7 +164,7 @@ def test_ptb_skill_eval_cases_cover_ticket_scope(): def test_ptb_skill_eval_cases_do_not_define_llm_ci_execution(): - content = EVAL_CASES.read_text(encoding="utf-8").lower() + content = _eval_cases_path(PTB_SKILL_NAME).read_text(encoding="utf-8").lower() forbidden = [ "model:", diff --git a/test/unit/util/skill_utils_test.py b/test/unit/util/skill_test.py similarity index 73% rename from test/unit/util/skill_utils_test.py rename to test/unit/util/skill_test.py index 7f36cd81a..d96f254ba 100644 --- a/test/unit/util/skill_utils_test.py +++ b/test/unit/util/skill_test.py @@ -26,7 +26,7 @@ def raise_file_not_found(_): monkeypatch.setattr(skills.resources, "files", raise_file_not_found) - with pytest.raises(RuntimeError, match="Packaged PTB skills are unavailable"): + with pytest.raises(RuntimeError, match="Packaged skills are unavailable"): skills.get_packaged_skill_names() @@ -115,6 +115,42 @@ def test_install_skill_rejects_path_traversal(tmp_path): skills.install_skill("../outside", tmp_path) +def test_install_skill_rejects_missing_skill(tmp_path, monkeypatch): + monkeypatch.setattr(skills, "get_skill_files", lambda _: {}) + + with pytest.raises(ValueError, match="packaged skill does not exist"): + skills.install_skill("example", tmp_path) + + +def test_install_skill_rejects_symlinked_target_directory(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}) + real_target = tmp_path / "real-target" + real_target.mkdir() + target_directory = tmp_path / "linked-target" + target_directory.symlink_to(real_target, target_is_directory=True) + + with pytest.raises(ValueError, match="symlinked target directory"): + skills.install_skill("example", target_directory) + + +def test_install_skill_rejects_file_as_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_directory / "example").write_text("not a directory", encoding="utf-8") + + with pytest.raises(ValueError, match="not a directory"): + skills.install_skill("example", target_directory) + + def test_install_skill_rejects_symlink_target(tmp_path, monkeypatch): source = tmp_path / "source" source.mkdir()