diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index d48a5f1..801d6dd 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -20,13 +20,13 @@ on: workflow_call: inputs: version: - description: "Version tag (e.g. v1.0.1) to validate and build from" + description: "Version tag (e.g. v1.0.2) to validate and build from" required: false type: string workflow_dispatch: inputs: version: - description: "Version tag (e.g. v1.0.1) to validate and build from" + description: "Version tag (e.g. v1.0.2) to validate and build from" required: false type: string diff --git a/CHANGELOG.md b/CHANGELOG.md index 9995086..30559dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,18 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). frozen guidance exists only as a historical experiment fixture. Evaluating consumer-provided skills remains supported. +## [1.0.2] - 2026-10-01 + +### Fixed + +- Skill preflight now accepts nested Markdown references such as + `references/commands/read.md`, preserving distinct relative paths instead of + rejecting subdirectories. Recursive validation retains UTF-8, nonempty-file, + and readability checks, rejects escaping links and directory cycles (including + Windows junctions), and still runs before SDK startup or model execution. +- Documented the existing public `load_skill()` / `Skill.from_path()` preflight + so unbilled SDK discovery probes also check the package's local validation. + ## [1.0.1] - 2026-10-01 ### Fixed diff --git a/docs/getting-started/skills.md b/docs/getting-started/skills.md index ce72252..0f6c146 100644 --- a/docs/getting-started/skills.md +++ b/docs/getting-started/skills.md @@ -50,6 +50,20 @@ them always in context. Copilot loads skills natively and can read their `references/` files using its file tools. The plugin does not inject synthetic reference tools. +Reference documents may be organized in subdirectories, for example +`references/commands/read.md`. Link to them directly from `SKILL.md` using paths +relative to the skill root. The specification's advice to keep reference chains +one level deep is not a ban on nested folders. + +The package validates all reference files recursively as nonempty UTF-8 Markdown. +`Skill.references` uses paths relative to `references/`, with `/` separators on +every platform: `commands/read.md`. Flat names such as `budgeting-guide.md` are +unchanged, and equal basenames in different folders remain distinct. +Links are accepted only when their resolved targets stay within `references/`; +the `references/` directory itself must resolve within the skill directory. +Broken links, directory cycles, unreadable entries, and non-file entries are +explicit errors. These checks also apply to Windows directory junctions. + ### Example Reference Document ```markdown title="references/budgeting-guide.md" @@ -132,6 +146,33 @@ the same enable/check rules and cannot replace a nonempty `skill_directories` fi not a claim that the agent read or followed a skill. Observed reads remain ordinary tool-call evidence. No setup check tells the agent to read a skill. +### Preflight Without Model Execution + +Use the existing public `load_skill()` function (or `Skill.from_path()`) before +an SDK discovery probe: + +```python +from pytest_skill_engineering import load_skill + +# Each path must identify an individual skill, not a parent collection. +skill = load_skill(r"skills\financial-advisor") +assert skill.name == "financial-advisor" +print(sorted(skill.references)) +``` + +This uses the same local format, reference, and readability validation as the +evaluation runner. It starts no client and sends no model message; invalid input +raises `SkillError`. For a parent collection, call it for each immediate child +containing `SKILL.md`; a collection with no such children is not a valid input. + +Then use `CopilotEval.build_session_config()` and the public SDK +`session.rpc.skills.ensure_loaded()`, `reload()`, and `list()` APIs to check +discovered paths, names, enabled state, and loading errors, without calling +`send()` or `send_and_wait()`. Local validation and SDK discovery are separate +checks: SDK discovery alone does not exercise the package's local validation, +and `load_skill()` alone does not prove runtime availability or detect duplicate +names across multiple skill directories. + ## Testing Skill Effectiveness Compare agents with and without skills: diff --git a/examples/skill-dogfood/uv.lock b/examples/skill-dogfood/uv.lock index 0836b3b..c5ba0ee 100644 --- a/examples/skill-dogfood/uv.lock +++ b/examples/skill-dogfood/uv.lock @@ -626,7 +626,7 @@ wheels = [ [[package]] name = "pytest-skill-engineering" -version = "1.0.1" +version = "1.0.2" source = { editable = "../../" } dependencies = [ { name = "github-copilot-sdk" }, diff --git a/pyproject.toml b/pyproject.toml index 57f130c..c15851e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "pytest-skill-engineering" -version = "1.0.1" +version = "1.0.2" description = "Reliable Copilot execution and evidence for coding-agent-led testing of MCP servers, CLI tools, skills, and custom agents." readme = "README.md" license = { text = "MIT" } diff --git a/src/pytest_skill_engineering/core/plugin.py b/src/pytest_skill_engineering/core/plugin.py index 0f7b457..1a0e331 100644 --- a/src/pytest_skill_engineering/core/plugin.py +++ b/src/pytest_skill_engineering/core/plugin.py @@ -341,7 +341,7 @@ def _discover_skills(plugin_dir: Path) -> list[Skill]: def _validate_skill_reference_names(skills: list[Skill]) -> None: - """Reject duplicate reference basenames across loaded skills.""" + """Reject duplicate relative reference paths across loaded skills.""" owners: dict[str, str] = {} for skill in skills: for reference_name in skill.references: diff --git a/src/pytest_skill_engineering/core/skill.py b/src/pytest_skill_engineering/core/skill.py index e6041a0..e0af3a4 100644 --- a/src/pytest_skill_engineering/core/skill.py +++ b/src/pytest_skill_engineering/core/skill.py @@ -28,6 +28,7 @@ from __future__ import annotations import re +import stat from dataclasses import dataclass, field from pathlib import Path @@ -163,7 +164,8 @@ def from_path(cls, path: Path | str) -> Skill: path: Path to skill directory or SKILL.md file Returns: - Loaded Skill instance + Loaded Skill instance. Reference keys are slash-separated paths + relative to references/, including any nested subdirectories. Raises: SkillError: If skill cannot be loaded or is invalid @@ -197,7 +199,7 @@ def from_path(cls, path: Path | str) -> Skill: for name in ("references", "scripts", "assets"): component = skill_dir / name - if component.exists() and not component.is_dir(): + if (component.exists() or component.is_symlink()) and not component.is_dir(): raise SkillError(f"{component}: skill component must be a directory") # Load references if directory exists @@ -320,27 +322,59 @@ def _parse_skill_md(content: str) -> tuple[SkillMetadata, str]: def _load_references(refs_dir: Path) -> dict[str, str]: - """Load all files from references/ directory. + """Load Markdown files recursively, without escaping references/ or following cycles. Returns: - Dict mapping filename to content + Dict mapping slash-separated paths relative to references/ to content """ + root = _resolve_reference_path(refs_dir, refs_dir.parent.resolve()) references: dict[str, str] = {} - - for file_path in refs_dir.iterdir(): - if not file_path.is_file(): - raise SkillError(f"Invalid references entry (must be a file): {file_path}") - if file_path.suffix.lower() != ".md": - raise SkillError(f"Invalid reference file '{file_path}': only .md files are allowed") + pending = [(refs_dir, frozenset({root}))] + while pending: + directory, ancestors = pending.pop() try: - content = file_path.read_text(encoding="utf-8") - except UnicodeDecodeError as exc: - raise SkillError(f"Reference file must be valid UTF-8 text: {file_path}") from exc - if not content.strip(): - raise SkillError(f"Reference file must not be empty: {file_path}") - references[file_path.name] = content + entries = sorted(directory.iterdir()) + except OSError as exc: + raise SkillError(f"Cannot read references directory {directory}: {exc}") from exc + for file_path in entries: + resolved = _resolve_reference_path(file_path, root) + try: + mode = resolved.stat().st_mode + except OSError as exc: + raise SkillError(f"Cannot inspect reference entry {file_path}: {exc}") from exc + if stat.S_ISDIR(mode): + if resolved in ancestors: + raise SkillError(f"Reference directory cycle: {file_path}") + pending.append((file_path, ancestors | {resolved})) + continue + if not stat.S_ISREG(mode): + raise SkillError(f"Invalid references entry (must be a file): {file_path}") + if file_path.suffix.lower() != ".md": + raise SkillError( + f"Invalid reference file '{file_path}': only .md files are allowed" + ) + try: + content = resolved.read_text(encoding="utf-8") + except UnicodeDecodeError as exc: + raise SkillError(f"Reference file must be valid UTF-8 text: {file_path}") from exc + except OSError as exc: + raise SkillError(f"Cannot read reference file {file_path}: {exc}") from exc + if not content.strip(): + raise SkillError(f"Reference file must not be empty: {file_path}") + references[file_path.relative_to(refs_dir).as_posix()] = content + + return dict(sorted(references.items())) - return references + +def _resolve_reference_path(path: Path, root: Path) -> Path: + """Resolve links before traversal or reads, keeping their targets within the root.""" + try: + resolved = path.resolve(strict=True) + except (OSError, RuntimeError) as exc: + raise SkillError(f"Cannot resolve reference path {path}: {exc}") from exc + if not resolved.is_relative_to(root): + raise SkillError(f"Reference path escapes its root {root}: {path}") + return resolved _SCRIPT_EXTENSIONS = frozenset({".py", ".sh", ".js", ".bash"}) @@ -375,7 +409,8 @@ def _load_scripts(scripts_dir: Path) -> dict[str, str]: def load_skill(path: Path | str) -> Skill: """Load a skill from a path. - Convenience function wrapping Skill.from_path(). + Convenience function wrapping Skill.from_path(). This is the runner's local + skill validation, without SDK startup or model execution. Args: path: Path to skill directory or SKILL.md file diff --git a/tests/contracts/test_skill_availability.py b/tests/contracts/test_skill_availability.py index e743da8..966e7bf 100644 --- a/tests/contracts/test_skill_availability.py +++ b/tests/contracts/test_skill_availability.py @@ -79,6 +79,20 @@ def _write_skill(parent: Path, name: str = "explicit-skill") -> Path: return directory +@pytest.mark.parametrize("selection", ["individual", "parent"]) +def test_nested_reference_skill_passes_runner_input_validation( + tmp_path: Path, selection: str +) -> None: + directory = _write_skill(tmp_path) + commands = directory / "references" / "commands" + commands.mkdir(parents=True) + (commands / "read.md").write_text("# Read command", encoding="utf-8") + selected = directory if selection == "individual" else tmp_path + assert requested_skill_files([str(selected)]) == { + (directory / "SKILL.md").resolve(): directory.name + } + + def test_relative_and_repeated_paths_resolve_to_the_same_explicit_skill( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: @@ -102,7 +116,22 @@ def forbid_client(*args: Any, **kwargs: Any) -> None: @pytest.mark.parametrize( - "invalid", ["missing", "file", "empty", "invalid", "partial", "unreadable", "duplicate"] + "invalid", + [ + "missing", + "file", + "empty", + "invalid", + "partial", + "unreadable", + "duplicate", + "nested-extension", + "nested-utf8", + "nested-empty", + "nested-unreadable", + "nested-outside", + "nested-cycle", + ], ) async def test_bad_skill_inputs_fail_before_client_start( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, invalid: str @@ -133,6 +162,33 @@ def read(path: Path, *args: Any, **kwargs: Any) -> str: return original_read(path, *args, **kwargs) monkeypatch.setattr(Path, "read_text", read) + if invalid.startswith("nested-"): + skill = _write_skill(directory) + commands = skill / "references" / "commands" + commands.mkdir(parents=True) + reference = commands / "read.md" + if invalid == "nested-extension": + reference.with_suffix(".txt").write_text("# Wrong extension", encoding="utf-8") + elif invalid == "nested-utf8": + reference.write_bytes(b"\xff") + elif invalid == "nested-empty": + reference.write_text(" \n", encoding="utf-8") + elif invalid == "nested-outside": + target = tmp_path / "outside.md" + target.write_text("# Outside references", encoding="utf-8") + reference.symlink_to(target) + elif invalid == "nested-cycle": + reference.symlink_to(commands, target_is_directory=True) + else: + reference.write_text("# Unreadable reference", encoding="utf-8") + original_read = Path.read_text + + def read_reference(path: Path, *args: Any, **kwargs: Any) -> str: + if path == reference: + raise PermissionError("Unreadable nested reference") + return original_read(path, *args, **kwargs) + + monkeypatch.setattr(Path, "read_text", read_reference) def forbid_client(*args: Any, **kwargs: Any) -> None: pytest.fail("Invalid skill inputs must not start an SDK client") @@ -154,6 +210,7 @@ def forbid_client(*args: Any, **kwargs: Any) -> None: "outcome", [ "loaded", + "loaded-nested", "missing", "wrong-path", "disabled", @@ -166,6 +223,10 @@ async def test_runner_checks_sdk_availability_before_its_single_send_and_saves_e tmp_path: Path, monkeypatch: pytest.MonkeyPatch, outcome: str ) -> None: directory = _write_skill(tmp_path) + if outcome == "loaded-nested": + commands = directory / "references" / "commands" + commands.mkdir(parents=True) + (commands / "read.md").write_text("# Read command", encoding="utf-8") path = str(directory / "SKILL.md") calls: list[str] = [] exposed = SDKSkill( @@ -221,10 +282,10 @@ async def stop(self) -> None: client_mode="empty", skill_directories=[str(directory)], disabled_skills=[directory.name] if outcome == "intentional-disable" else [], - audit_requests=outcome not in ("loaded", "intentional-disable"), + audit_requests=outcome not in ("loaded", "loaded-nested", "intentional-disable"), ) result = await runner.run_copilot(agent, "Do not force a skill read.") - succeeds = outcome in ("loaded", "intentional-disable") + succeeds = outcome in ("loaded", "loaded-nested", "intentional-disable") assert result.success is succeeds assert calls.count("start") == calls.count("create") == calls.count("stop") == 1 assert calls.count("send") == int(succeeds) diff --git a/tests/integration/copilot/test_skill_discovery.py b/tests/integration/copilot/test_skill_discovery.py index 74d7079..815a3da 100644 --- a/tests/integration/copilot/test_skill_discovery.py +++ b/tests/integration/copilot/test_skill_discovery.py @@ -11,6 +11,7 @@ from copilot import CopilotRequestContext, CopilotRequestHandler, CopilotWebSocketHandler from copilot.session import CopilotSession +from pytest_skill_engineering import load_skill from pytest_skill_engineering.copilot import CopilotEval from pytest_skill_engineering.copilot.client import create_client, stop_client from pytest_skill_engineering.copilot.skills import discover_requested_skills, requested_skill_files @@ -19,19 +20,25 @@ pytestmark = pytest.mark.copilot -def _write_skill(parent: Path, name: str) -> Path: +def _write_skill(parent: Path, name: str, *, nested: bool = False) -> Path: directory = parent / name directory.mkdir(parents=True) (directory / "SKILL.md").write_text( - f"---\nname: {name}\ndescription: Explicit discovery test\n---\n\n# {name}\n", + f"---\nname: {name}\ndescription: Explicit discovery test\n---\n\n# {name}\n" + + ("See [read](references/commands/read.md).\n" if nested else ""), encoding="utf-8", ) + if nested: + references = directory / "references" / "commands" + references.mkdir(parents=True) + (references / "read.md").write_text("# Read command", encoding="utf-8") return directory +@pytest.mark.parametrize("nested", [False, True], ids=["flat", "nested"]) @pytest.mark.parametrize("selection", ["baseline", "individual", "parent", "disabled"]) async def test_empty_mode_discovers_only_explicit_skills_without_model_sends( - tmp_path: Path, monkeypatch: pytest.MonkeyPatch, selection: str + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, selection: str, nested: bool ) -> None: async def forbid_send(*args: object, **kwargs: object) -> None: pytest.fail("Discovery checks must not send model messages") @@ -48,8 +55,8 @@ async def forbid_send(*args: object, **kwargs: object) -> None: storage.mkdir() _write_skill(storage / "skills", "ambient-personal") parent = tmp_path / "explicit" - first = _write_skill(parent, "explicit-first") - second = _write_skill(parent, "explicit-second") + first = _write_skill(parent, "explicit-first", nested=nested) + second = _write_skill(parent, "explicit-second", nested=nested) directories = { "baseline": [], "individual": [str(first)], @@ -68,6 +75,11 @@ async def forbid_send(*args: object, **kwargs: object) -> None: skill_directories=directories, disabled_skills=["explicit-first"] if selection == "disabled" else [], ) + selected_skills = [first, second] if selection == "parent" else [first] if directories else [] + for directory in selected_skills: + skill = load_skill(directory) + assert skill.references == ({"commands/read.md": "# Read command"} if nested else {}) + requested = requested_skill_files(directories) client = create_client(str(workspace), mode="empty", base_directory=str(storage)) try: async with asyncio.timeout(60): @@ -79,9 +91,7 @@ async def forbid_send(*args: object, **kwargs: object) -> None: assert all(skill.enabled is (selection != "disabled") for skill in skills) assert (await session.rpc.skills.get_invoked()).skills == [] discovery = SkillDiscovery() - await discover_requested_skills( - session, requested_skill_files(directories), agent.disabled_skills, discovery - ) + await discover_requested_skills(session, requested, agent.disabled_skills, discovery) assert discovery.complete and not discovery.errors and not discovery.warnings assert {skill.path for skill in discovery.skills} == expected finally: diff --git a/tests/unit/test_plugin_skill_references.py b/tests/unit/test_plugin_skill_references.py index eee395b..95c3d7c 100644 --- a/tests/unit/test_plugin_skill_references.py +++ b/tests/unit/test_plugin_skill_references.py @@ -19,7 +19,9 @@ def _make_skill(root: Path, name: str, reference_name: str) -> Skill: ) refs = skill_dir / "references" refs.mkdir() - (refs / reference_name).write_text(f"# {reference_name}\n", encoding="utf-8") + reference = refs.joinpath(*reference_name.split("/")) + reference.parent.mkdir(parents=True, exist_ok=True) + reference.write_text(f"# {reference_name}\n", encoding="utf-8") return Skill.from_path(skill_dir) @@ -36,3 +38,16 @@ def test_unique_skill_reference_basenames_are_allowed(tmp_path: Path) -> None: skill_b = _make_skill(tmp_path, "beta", "lookup.md") _validate_skill_reference_names([skill_a, skill_b]) + + +def test_nested_reference_paths_do_not_collide_by_basename(tmp_path: Path) -> None: + skill_a = _make_skill(tmp_path, "alpha", "commands/guide.md") + skill_b = _make_skill(tmp_path, "beta", "topics/guide.md") + _validate_skill_reference_names([skill_a, skill_b]) + + +def test_duplicate_nested_reference_paths_are_rejected(tmp_path: Path) -> None: + skill_a = _make_skill(tmp_path, "alpha", "commands/guide.md") + skill_b = _make_skill(tmp_path, "beta", "commands/guide.md") + with pytest.raises(ValueError, match="commands/guide.md"): + _validate_skill_reference_names([skill_a, skill_b]) diff --git a/tests/unit/test_release_invariants.py b/tests/unit/test_release_invariants.py index cec2795..e51d346 100644 --- a/tests/unit/test_release_invariants.py +++ b/tests/unit/test_release_invariants.py @@ -8,9 +8,9 @@ ROOT = Path(__file__).parents[2] -def test_project_version_is_pinned_to_1_0_1() -> None: +def test_project_version_is_pinned_to_1_0_2() -> None: pyproject = tomllib.loads((ROOT / "pyproject.toml").read_text(encoding="utf-8")) - assert pyproject["project"]["version"] == "1.0.1" + assert pyproject["project"]["version"] == "1.0.2" def test_release_workflow_builds_and_validates_artifact_version() -> None: @@ -22,7 +22,7 @@ def test_release_workflow_builds_and_validates_artifact_version() -> None: def test_docs_workflow_examples_reference_current_release_tag() -> None: workflow = (ROOT / ".github" / "workflows" / "docs.yml").read_text(encoding="utf-8") - assert "v1.0.1" in workflow + assert "v1.0.2" in workflow def test_github_release_job_selects_repository_without_checkout() -> None: diff --git a/tests/unit/test_skill.py b/tests/unit/test_skill.py index af7e042..e9ab022 100644 --- a/tests/unit/test_skill.py +++ b/tests/unit/test_skill.py @@ -1,12 +1,219 @@ """Tests for core.skill module.""" +from __future__ import annotations + +import os +import stat +import subprocess +from collections.abc import Callable, Iterator from pathlib import Path +from typing import Any, Literal import pytest +from pytest_skill_engineering import load_skill from pytest_skill_engineering.core.skill import Skill, SkillError, SkillMetadata +@pytest.fixture +def nested_reference_skill(tmp_path: Path) -> Path: + directory = tmp_path / "nested-skill" + references = directory / "references" + (references / "commands" / "advanced").mkdir(parents=True) + (references / "topics").mkdir() + (directory / "SKILL.md").write_text( + "---\nname: nested-skill\ndescription: Nested reference test\n---\n\n" + "See [commands](references/commands/read.md).\n", + encoding="utf-8", + ) + for relative, content in { + "overview.md": "# Overview", + "commands/read.md": "# Read command", + "commands/advanced/read.md": "# Advanced read command", + "topics/read.md": "# Reading topics", + }.items(): + references.joinpath(*relative.split("/")).write_text(content, encoding="utf-8") + return directory + + +@pytest.mark.parametrize("loader", [Skill.from_path, load_skill]) +def test_nested_references_preserve_relative_paths( + nested_reference_skill: Path, loader: Callable[[Path | str], Skill] +) -> None: + skill = loader(nested_reference_skill) + assert skill.references == { + "commands/advanced/read.md": "# Advanced read command", + "commands/read.md": "# Read command", + "overview.md": "# Overview", + "topics/read.md": "# Reading topics", + } + assert skill.has_references + assert list(skill.references) == sorted(skill.references) + + +@pytest.mark.parametrize("invalid", ["extension", "utf8", "empty"]) +def test_nested_references_keep_file_validation(nested_reference_skill: Path, invalid: str) -> None: + reference = nested_reference_skill / "references" / "commands" / "read.md" + if invalid == "extension": + reference.rename(reference.with_suffix(".txt")) + message = r"only \.md files are allowed" + elif invalid == "utf8": + reference.write_bytes(b"\xff") + message = "valid UTF-8 text" + else: + reference.write_text(" \n", encoding="utf-8") + message = "must not be empty" + with pytest.raises(SkillError, match=message): + load_skill(nested_reference_skill) + + +@pytest.mark.parametrize("operation", ["list", "read", "stat"]) +def test_unreadable_nested_references_fail_with_the_path( + nested_reference_skill: Path, monkeypatch: pytest.MonkeyPatch, operation: str +) -> None: + reference = nested_reference_skill / "references" / "commands" / "read.md" + directory = reference.parent + if operation == "list": + original = Path.iterdir + + def list_directory(path: Path) -> Iterator[Path]: + if path == directory: + raise PermissionError("Blocked directory") + return original(path) + + monkeypatch.setattr(Path, "iterdir", list_directory) + expected_path = directory + elif operation == "read": + original_read = Path.read_text + + def read_file(path: Path, *args: Any, **kwargs: Any) -> str: + if path == reference: + raise PermissionError("Blocked reference") + return original_read(path, *args, **kwargs) + + monkeypatch.setattr(Path, "read_text", read_file) + expected_path = reference + else: + original_stat = Path.stat + + def inspect(path: Path, *args: Any, **kwargs: Any) -> os.stat_result: + if path == reference: + raise PermissionError("Blocked inspection") + return original_stat(path, *args, **kwargs) + + monkeypatch.setattr(Path, "stat", inspect) + expected_path = reference + with pytest.raises(SkillError) as error: + load_skill(nested_reference_skill) + assert str(expected_path) in str(error.value) + assert "Blocked" in str(error.value) + + +def test_nonregular_nested_reference_fails_before_reading( + nested_reference_skill: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + reference = nested_reference_skill / "references" / "commands" / "read.md" + original_stat = Path.stat + + def inspect(path: Path, *args: Any, **kwargs: Any) -> os.stat_result: + if path == reference: + return os.stat_result((stat.S_IFIFO, 0, 0, 1, 0, 0, 0, 0, 0, 0)) + return original_stat(path, *args, **kwargs) + + monkeypatch.setattr(Path, "stat", inspect) + with pytest.raises(SkillError, match="must be a file"): + load_skill(nested_reference_skill) + + +def _link_directory(link: Path, target: Path, kind: Literal["symlink", "junction"]) -> None: + if kind == "symlink": + link.symlink_to(target, target_is_directory=True) + else: + subprocess.run( + [os.environ["COMSPEC"], "/c", "mklink", "/J", str(link), str(target)], + check=True, + capture_output=True, + ) + + +@pytest.mark.parametrize( + "kind", + [ + "symlink", + pytest.param("junction", marks=pytest.mark.skipif(os.name != "nt", reason="Windows")), + ], +) +@pytest.mark.parametrize("target_kind", ["internal", "outside", "cycle", "root-outside"]) +def test_reference_directory_links_are_contained_and_cycle_checked( + nested_reference_skill: Path, + tmp_path: Path, + kind: Literal["symlink", "junction"], + target_kind: str, +) -> None: + references = nested_reference_skill / "references" + if target_kind == "internal": + target = references / "commands" + elif target_kind == "cycle": + target = references + else: + target = tmp_path / "outside" + target.mkdir() + (target / "secret.md").write_text("# Not a skill reference", encoding="utf-8") + if target_kind == "root-outside": + # Use a fresh skill so references/ itself can be a link. + skill = tmp_path / "linked-root" + skill.mkdir() + (skill / "SKILL.md").write_text( + "---\nname: linked-root\ndescription: Linked root test\n---\n\n# Instructions", + encoding="utf-8", + ) + link = skill / "references" + else: + skill = nested_reference_skill + link = references / "linked" + _link_directory(link, target, kind) + try: + if target_kind == "internal": + loaded = load_skill(skill) + assert loaded.references["linked/read.md"] == "# Read command" + assert loaded.references["linked/advanced/read.md"] == "# Advanced read command" + else: + message = "cycle" if target_kind == "cycle" else "escapes its root" + with pytest.raises(SkillError, match=message): + load_skill(skill) + finally: + if kind == "symlink": + link.unlink() + else: + link.rmdir() + + +@pytest.mark.parametrize("target_kind", ["internal", "outside", "broken", "cycle"]) +def test_reference_file_links_are_contained_and_resolvable( + nested_reference_skill: Path, tmp_path: Path, target_kind: str +) -> None: + references = nested_reference_skill / "references" + link = references / "linked.md" + if target_kind == "internal": + target = references / "commands" / "read.md" + elif target_kind == "cycle": + target = link + else: + target = tmp_path / "outside.md" + if target_kind == "outside": + target.write_text("# Not a skill reference", encoding="utf-8") + link.symlink_to(target) + try: + if target_kind == "internal": + assert load_skill(nested_reference_skill).references["linked.md"] == "# Read command" + else: + message = "escapes its root" if target_kind == "outside" else "Cannot resolve" + with pytest.raises(SkillError, match=message): + load_skill(nested_reference_skill) + finally: + link.unlink() + + def test_skill_name_rejects_trailing_hyphen() -> None: """Name must not end with a hyphen per Eval Skills naming rules.""" with pytest.raises(SkillError, match="Invalid skill name"): diff --git a/uv.lock b/uv.lock index c92f06e..92b4e94 100644 --- a/uv.lock +++ b/uv.lock @@ -1421,7 +1421,7 @@ wheels = [ [[package]] name = "pytest-skill-engineering" -version = "1.0.1" +version = "1.0.2" source = { editable = "." } dependencies = [ { name = "github-copilot-sdk" },