Fix nested skill references and prepare 1.0.2 (#111)

.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
 

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

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:

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" }

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:

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

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)