diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index f2c6e56..d48a5f1 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.0) to validate and build from" + description: "Version tag (e.g. v1.0.1) to validate and build from" required: false type: string workflow_dispatch: inputs: version: - description: "Version tag (e.g. v1.0.0) to validate and build from" + description: "Version tag (e.g. v1.0.1) to validate and build from" required: false type: string diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d4e591..9995086 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,26 @@ 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.1] - 2026-10-01 + +### Fixed + +- Explicit skill directories now enable native SDK skills in empty mode without + enabling ambient discovery. Individual directories and parents are checked + before model execution; missing, unreadable, invalid, duplicate-name, or + undiscovered requested skills fail clearly without sending the task. +- Explicit directories combined with `enable_skills=False` are rejected. + Named `disabled_skills` remain supported for deliberate exclusions. + +### Added + +- Public `DiscoveredSkill` and `SkillDiscovery` evidence types and + `CopilotResult.skill_discovery`, preserving actual SDK availability and loading + diagnostics separately from skill reads. Native schema-4.0 evidence now requires + `eval_result.skill_discovery`; regenerate older evidence with the current producer. +- Unbilled real-SDK discovery and isolation checks, plus a request blocked before + network forwarding to verify ambient instructions do not enter model context. + ## [1.0.0] ### Breaking changes diff --git a/docs/contributing/evidence-structure.md b/docs/contributing/evidence-structure.md index 616cc25..dcfa011 100644 --- a/docs/contributing/evidence-structure.md +++ b/docs/contributing/evidence-structure.md @@ -19,6 +19,13 @@ execution configuration, turns, tool arguments and outputs, completion status, capture errors, request audits, stop reasons, usage, and missing-pricing models. Private SDK/session fields are not serialized. +`eval_result.skill_discovery` saves actual public SDK availability separately +from requested configuration and observed reads. It preserves metadata, loading +warnings/errors, and whether discovery completed; `null` means no check was +attempted. This field is required by the current schema-4.0 producer/reader. +Regenerate older evidence missing it with the current producer; do not infer +availability from supplied paths or patch saved JSON. + Preserve missing versus empty output and nullable usage. Do not replace unsupported property values with strings or synthesize uncaptured successful assertions. The loader rejects old schemas; there is no compatibility reader. diff --git a/docs/getting-started/skills.md b/docs/getting-started/skills.md index 0be9b58..ce72252 100644 --- a/docs/getting-started/skills.md +++ b/docs/getting-started/skills.md @@ -96,10 +96,42 @@ from pytest_skill_engineering.copilot import CopilotEval agent = CopilotEval( name="with-skill", + client_mode="empty", skill_directories=["skills/financial-advisor"], ) ``` +Supply either a directory containing `SKILL.md` or its parent containing multiple +immediate skill directories. Relative paths are resolved from the Python process's +current directory, not `working_directory`. + +Explicit `skill_directories` enable native SDK skill loading automatically, +including in `client_mode="empty"`. This does **not** enable ambient configuration +discovery. Empty mode still excludes personal/project skills and custom instructions, +file hooks, managed settings, and persona-provided instructions/tools. Supply the +tools needed by your task explicitly; empty mode's default tool list is empty. + +Before sending the task, the runner validates every requested skill using the +existing skill format checks, then checks availability through the public SDK +`session.rpc.skills.ensure_loaded()`, `reload()`, and `list()` APIs. Missing, +unreadable, invalid, duplicate-name, or undiscovered skills return +`success=False`, `stop_reason="execution_error"`, and a setup error without +sending a model message. A parent with no immediate `SKILL.md` files is an error; +unrelated child directories without `SKILL.md` are not skill packages. +SDK loading warnings are logged and saved; loading errors stop execution. +There is still only one execution attempt. + +`extra_config={"enable_skills": False}` conflicts with nonempty explicit +directories and raises `ValueError` from `build_session_config()`. For a +no-skill baseline, remove the directories instead. To deliberately disable +particular discovered skills, use `disabled_skills=["skill-name"]`; these skills +remain in discovery evidence with `enabled=False`. Passthrough directories follow +the same enable/check rules and cannot replace a nonempty `skill_directories` field. + +`result.skill_discovery` records actual SDK metadata and loading diagnostics, +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. + ## Testing Skill Effectiveness Compare agents with and without skills: diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 1c0cc8f..109a1a1 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -46,7 +46,8 @@ agent = CopilotEval( | `mcp_servers` | Copilot SDK server config mapping | | `allowed_tools` | Optional global tool allow-list | | `custom_agents` | Loaded `.agent.md` definitions | -| `skill_directories` | Skill packages to inject | +| `skill_directories` | Explicit individual skill directories or parents; enable loading and check SDK availability before sending | +| `disabled_skills` | Skill names deliberately disabled, retained in discovery evidence | | `max_turns` | Advisory top-level turn budget; used for subagent turn caps | | `timeout_s` | Hard wall-clock limit for the run | @@ -54,6 +55,25 @@ Each `copilot_eval` call makes one execution attempt. Startup, connection, and task failures are recorded without automatically starting another session. Rerun a selected pytest case explicitly when appropriate. +### Explicit skills and empty mode + +Nonempty `skill_directories` set the public SDK `enable_skills=True` by default. +`enable_config_discovery` remains false. In empty mode, personal/project discovery, +custom instructions, file hooks, managed settings, and persona injection stay +disabled. Explicit tools and system prompt content remain supported. + +Explicit directories combined with `extra_config={"enable_skills": False}` are +rejected, not silently ignored. `None` leaves the explicit-skills default enabled. +An empty directory list plus `enable_skills=False` is allowed. Use `disabled_skills` +to deliberately disable named skills within an otherwise loaded set. + +Every requested directory must contain a valid `SKILL.md` or immediate child +skill directories. Relative paths use the Python caller's current directory. +Validation and public SDK discovery run before the task is sent; setup failure +is recorded as an execution error with no model send. SDK metadata and diagnostics +are available as `result.skill_discovery` and in native JSON, separately from reads. +See [Skills](../getting-started/skills.md) for input and evidence details. + ## MCP server config shape Attach servers directly to `CopilotEval(mcp_servers={...})`. diff --git a/docs/reference/result.md b/docs/reference/result.md index 90889bd..cf7825c 100644 --- a/docs/reference/result.md +++ b/docs/reference/result.md @@ -24,6 +24,7 @@ description: "CopilotResult reference: tool calls, responses, token usage, files | `stop_reason` | `completed`, `tool_budget_exceeded`, `timeout`, `request_audit_error`, `execution_error`, or `cleanup_error`; `None` for a result not finalized by the runner | | `tool_calls_admitted` | Calls admitted by the framework after caller guards; not proof of execution or application success | | `request_audit` | Actual outgoing request records when `audit_requests=True` | +| `skill_discovery` | Actual pre-execution SDK skill metadata and diagnostics for explicit directories; `None` means no discovery check was attempted | `success` describes session execution, not whether the application did the right thing. A tool can fail and the session can recover. Check application state with @@ -34,6 +35,25 @@ Missing completion, uncorrelated events, conflicting duplicate events, and cancellation make session `success` false. A completed call whose output was not captured makes `evidence_complete` false without changing the reported tool outcome. +## Skill availability + +For explicit directories, `skill_discovery` contains `skills`, `warnings`, +`errors`, and `complete`. `complete=True` means discovery finished, not that it +succeeded: inspect `errors` and the result's `success` too. A failed local or SDK +check can retain an incomplete record. `skills=[]` means the completed SDK list +was empty; `skill_discovery=None` means availability was not checked. + +Each discovered skill retains SDK `name`, `description`, `source`, `enabled`, +`user_invocable`, `path`, `plugin_name`, `command_name`, and `argument_hint`. +Optional metadata remains `None` when absent. `disabled_skills` entries can +deliberately have `enabled=False`; unexpectedly disabled requested skills block +the task before a model send. + +Availability does not establish that a skill was read, followed, or useful. +Actual reads remain in the ordinary tool calls; application checks establish +task correctness. The runner neither forces a read nor changes the task prompt. +Native JSON saves discovery separately from configuration and tool calls. + ## Captured tool evidence Each item in `all_tool_calls` has these fields: diff --git a/examples/skill-dogfood/uv.lock b/examples/skill-dogfood/uv.lock index 583143b..0836b3b 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.0" +version = "1.0.1" source = { editable = "../../" } dependencies = [ { name = "github-copilot-sdk" }, @@ -1046,8 +1046,8 @@ name = "uvicorn" version = "0.54.0" source = { registry = "https://pypi.org/simple" } dependencies = [ - { name = "click" }, - { name = "h11" }, + { name = "click", marker = "python_full_version < '3.12' or python_full_version >= '3.14' or sys_platform != 'emscripten'" }, + { name = "h11", marker = "python_full_version < '3.12' or python_full_version >= '3.14' or sys_platform != 'emscripten'" }, ] sdist = { url = "https://files.pythonhosted.org/packages/da/34/30e9280707135d2cfc589dfff3cb796bd07a3aeb1a3e415ba09dd89d7bb4/uvicorn-0.54.0.tar.gz", hash = "sha256:a2e33cbfaa0306f8e6b0c13e0cb89d7d7a2da3e62b90c66e18c33d9807b28620", size = 112283, upload-time = "2026-09-25T06:52:37.601Z" } wheels = [ diff --git a/pyproject.toml b/pyproject.toml index ea829a1..57f130c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "pytest-skill-engineering" -version = "1.0.0" +version = "1.0.1" 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/copilot/__init__.py b/src/pytest_skill_engineering/copilot/__init__.py index 43b9d3d..fab9f4c 100644 --- a/src/pytest_skill_engineering/copilot/__init__.py +++ b/src/pytest_skill_engineering/copilot/__init__.py @@ -22,11 +22,14 @@ from pytest_skill_engineering.copilot.requests import RequestAudit from pytest_skill_engineering.copilot.result import CopilotResult, UsageInfo from pytest_skill_engineering.core.evals import load_custom_agent, load_custom_agents +from pytest_skill_engineering.core.result import DiscoveredSkill, SkillDiscovery __all__ = [ "CopilotEval", "CopilotResult", + "DiscoveredSkill", "RequestAudit", + "SkillDiscovery", "UsageInfo", "ClaudeCodePersona", "CopilotCLIPersona", diff --git a/src/pytest_skill_engineering/copilot/eval.py b/src/pytest_skill_engineering/copilot/eval.py index 83aa7a9..1be569c 100644 --- a/src/pytest_skill_engineering/copilot/eval.py +++ b/src/pytest_skill_engineering/copilot/eval.py @@ -139,7 +139,7 @@ class CopilotEval: # display_name, tools, mcp_servers, infer) custom_agents: list[CopilotCustomAgentConfig] = field(default_factory=list) - # Skill directories + # Explicit skills are enabled and checked before model execution. skill_directories: list[str] = field(default_factory=list) disabled_skills: list[str] = field(default_factory=list) @@ -235,8 +235,24 @@ def build_session_config(self) -> dict[str, Any]: config["hooks"] = self.hooks # Apply extra_config passthrough + if ( + self.skill_directories + and "skill_directories" in self.extra_config + and self.extra_config["skill_directories"] != self.skill_directories + ): + raise ValueError("extra_config must not override explicitly supplied skill_directories") config.update(self.extra_config) + if config.get("skill_directories"): + if config.get("enable_skills") is False: + raise ValueError( + "Explicit skill_directories conflict with enable_skills=False; " + "remove the directories for a no-skill baseline, or enable skills" + ) + if config.get("enable_skills") is not None and config["enable_skills"] is not True: + raise ValueError("enable_skills must be a boolean or None") + config["enable_skills"] = True + if self.client_mode == "empty": isolated = { "enable_config_discovery": False, diff --git a/src/pytest_skill_engineering/copilot/fixtures.py b/src/pytest_skill_engineering/copilot/fixtures.py index 8bd6a96..2fb594f 100644 --- a/src/pytest_skill_engineering/copilot/fixtures.py +++ b/src/pytest_skill_engineering/copilot/fixtures.py @@ -116,6 +116,7 @@ def _convert_to_aitest( stop_reason=result.stop_reason, usage=list(result.usage), tool_calls_admitted=result.tool_calls_admitted, + skill_discovery=deepcopy(result.skill_discovery), configuration=deepcopy( { "name": agent.name, diff --git a/src/pytest_skill_engineering/copilot/result.py b/src/pytest_skill_engineering/copilot/result.py index b2c1be6..9748dd4 100644 --- a/src/pytest_skill_engineering/copilot/result.py +++ b/src/pytest_skill_engineering/copilot/result.py @@ -12,7 +12,13 @@ from pytest_skill_engineering.copilot.contracts import CopilotResultAgent, SubagentInvocation from pytest_skill_engineering.copilot.requests import RequestAudit -from pytest_skill_engineering.core.result import StopReason, ToolCall, Turn, UsageInfo +from pytest_skill_engineering.core.result import ( + SkillDiscovery, + StopReason, + ToolCall, + Turn, + UsageInfo, +) __all__ = [ "CopilotResult", @@ -68,6 +74,7 @@ class CopilotResult: request_audit: list[RequestAudit] = field(default_factory=list) stop_reason: StopReason | None = None tool_calls_admitted: int = 0 + skill_discovery: SkillDiscovery | None = None @property def evidence_complete(self) -> bool: diff --git a/src/pytest_skill_engineering/copilot/runner.py b/src/pytest_skill_engineering/copilot/runner.py index 2b06403..c004f50 100644 --- a/src/pytest_skill_engineering/copilot/runner.py +++ b/src/pytest_skill_engineering/copilot/runner.py @@ -14,6 +14,7 @@ import asyncio import logging +from pathlib import Path from tempfile import TemporaryDirectory from typing import TYPE_CHECKING @@ -27,6 +28,8 @@ from pytest_skill_engineering.copilot.events import EventMapper from pytest_skill_engineering.copilot.requests import RequestAuditHandler from pytest_skill_engineering.copilot.result import CopilotResult, StopReason +from pytest_skill_engineering.copilot.skills import discover_requested_skills, requested_skill_files +from pytest_skill_engineering.core.result import SkillDiscovery if TYPE_CHECKING: from copilot.client import CopilotClient @@ -80,8 +83,20 @@ async def run_copilot(agent: CopilotEvalConfig, prompt: str) -> CopilotResult: reason: StopReason = "completed" error: str | None = None cleanup_errors: list[str] = [] + skill_discovery: SkillDiscovery | None = None + send_started = False try: session_config = agent.build_session_config() + skill_directories = session_config.get("skill_directories") or [] + requested_skills: dict[Path, str] = {} + if skill_directories: + skill_discovery = SkillDiscovery() + if not isinstance(skill_directories, list): + raise ValueError("skill_directories must be a list of directory paths") + requested_skills = requested_skill_files(skill_directories) + session_config["skill_directories"] = [ + str(Path(directory).resolve()) for directory in skill_directories + ] caller_hooks = dict(session_config.get("hooks") or {}) session_config["hooks"] = dict(caller_hooks) # Empty mode must not read persona instruction files or inject tools. @@ -123,12 +138,20 @@ def capture(event: SessionEvent) -> None: ) async def execute() -> None: - nonlocal session + nonlocal session, send_started assert client is not None async with asyncio.timeout(min(60, agent.timeout_s)): await client.start() async with asyncio.timeout(min(30, agent.timeout_s)): session = await client.create_session(**session_config) + if skill_discovery is not None: + await discover_requested_skills( + session, + requested_skills, + session_config.get("disabled_skills") or [], + skill_discovery, + ) + send_started = True await session.send_and_wait(prompt, timeout=agent.timeout_s) execution = asyncio.create_task(execute()) @@ -158,6 +181,9 @@ async def execute() -> None: except Exception as exc: logger.error("Copilot execution failed: %s", exc) reason, error = "execution_error", str(exc) + if skill_discovery is not None and not skill_discovery.complete: + error = f"Skill setup failed before model execution: {exc}" + skill_discovery.errors.append(error) finally: controls.closed = True for watcher in watchers: @@ -210,12 +236,13 @@ async def execute() -> None: result = mapper.build() result.agent = agent + result.skill_discovery = skill_discovery result.tool_calls_admitted = controls.admitted if result.capture_errors: result.capture_errors.extend(controls.incomplete_diagnostics(abort_phase=reason)) if audit is not None and capture_requests: result.request_audit = list(audit.records) - if audit.error or (session is not None and not audit.observed_requests): + if audit.error or (send_started and not audit.observed_requests): audit_error = audit.error or "No outbound model requests captured; audit unsupported" result.capture_errors.append(audit_error) if reason == "completed" or audit.error: diff --git a/src/pytest_skill_engineering/copilot/skills.py b/src/pytest_skill_engineering/copilot/skills.py new file mode 100644 index 0000000..a097127 --- /dev/null +++ b/src/pytest_skill_engineering/copilot/skills.py @@ -0,0 +1,104 @@ +"""Validate explicit skill inputs and verify native SDK availability before sending.""" + +from __future__ import annotations + +import logging +from pathlib import Path +from typing import TYPE_CHECKING + +from pytest_skill_engineering.core.result import DiscoveredSkill, SkillDiscovery +from pytest_skill_engineering.core.skill import Skill, SkillError + +if TYPE_CHECKING: + from copilot.session import CopilotSession + +logger = logging.getLogger(__name__) + + +def requested_skill_files(directories: list[str]) -> dict[Path, str]: + """Validate individual skill directories or parents of immediate skill directories. + + Relative directories are resolved from the Python caller's current directory, + then passed to the SDK as absolute paths. + """ + requested: dict[Path, str] = {} + names: dict[str, Path] = {} + for raw_directory in directories: + if not isinstance(raw_directory, str) or not raw_directory.strip(): + raise SkillError("skill_directories entries must be non-empty directory paths") + directory = Path(raw_directory).resolve() + if not directory.is_dir(): + raise SkillError( + f"Requested skill directory does not exist or is not a directory: {directory}" + ) + try: + candidates = ( + [directory] + if (directory / "SKILL.md").exists() + else [ + child + for child in sorted(directory.iterdir()) + if child.is_dir() and (child / "SKILL.md").exists() + ] + ) + if not candidates: + raise SkillError(f"No SKILL.md found in requested skill directory: {directory}") + for candidate in candidates: + skill = Skill.from_path(candidate) + path = (skill.path / "SKILL.md").resolve() + if skill.name in names and names[skill.name] != path: + raise SkillError( + f"Duplicate requested skill name '{skill.name}': " + f"{names[skill.name]} and {path}" + ) + names[skill.name] = path + requested[path] = skill.name + except OSError as exc: + raise SkillError(f"Cannot read requested skill directory {directory}: {exc}") from exc + return requested + + +async def discover_requested_skills( + session: CopilotSession, + requested: dict[Path, str], + disabled: list[str], + evidence: SkillDiscovery, +) -> None: + """Record SDK discovery and reject missing or unexpectedly disabled requested skills.""" + await session.rpc.skills.ensure_loaded() + diagnostics = await session.rpc.skills.reload() + evidence.warnings = list(diagnostics.warnings) + evidence.errors = list(diagnostics.errors) + for warning in evidence.warnings: + logger.warning("SDK skill discovery: %s", warning) + listed = await session.rpc.skills.list() + evidence.skills = [ + DiscoveredSkill( + name=skill.name, + description=skill.description, + source=skill.source.value, + enabled=skill.enabled, + user_invocable=skill.user_invocable, + path=skill.path, + plugin_name=skill.plugin_name, + command_name=skill.command_name, + argument_hint=skill.argument_hint, + ) + for skill in listed.skills + ] + evidence.complete = True + available = { + (Path(skill.path).resolve(), skill.name): skill + for skill in evidence.skills + if skill.path is not None + } + for path, name in requested.items(): + skill = available.get((path, name)) + if skill is None: + evidence.errors.append( + f"Requested skill '{name}' was not discovered by the SDK: {path}" + ) + elif not skill.enabled and name not in disabled: + evidence.errors.append(f"Requested skill '{name}' is unexpectedly disabled: {path}") + if evidence.errors: + raise SkillError("Skill setup failed before model execution: " + "; ".join(evidence.errors)) diff --git a/src/pytest_skill_engineering/core/__init__.py b/src/pytest_skill_engineering/core/__init__.py index 94a1113..d602f02 100644 --- a/src/pytest_skill_engineering/core/__init__.py +++ b/src/pytest_skill_engineering/core/__init__.py @@ -17,10 +17,12 @@ load_system_prompts, ) from pytest_skill_engineering.core.result import ( + DiscoveredSkill, EvalResult, ImageContent, MCPPrompt, MCPPromptArgument, + SkillDiscovery, SkillInfo, SubagentInvocation, ToolCall, @@ -38,6 +40,7 @@ __all__ = [ "AITestError", "EvalResult", + "DiscoveredSkill", "EngineTimeoutError", "HookDefinition", "ImageContent", @@ -48,6 +51,7 @@ "Prompt", "ServerStartError", "Skill", + "SkillDiscovery", "SkillError", "SkillInfo", "SkillMetadata", diff --git a/src/pytest_skill_engineering/core/result.py b/src/pytest_skill_engineering/core/result.py index c706a67..663b333 100644 --- a/src/pytest_skill_engineering/core/result.py +++ b/src/pytest_skill_engineering/core/result.py @@ -181,6 +181,31 @@ def __repr__(self) -> str: return f"SkillInfo({self.name}{refs})" +@dataclass(slots=True, frozen=True) +class DiscoveredSkill: + """Skill metadata actually returned by the public SDK, not observed use.""" + + name: str + description: str + source: str + enabled: bool + user_invocable: bool + path: str | None = None + plugin_name: str | None = None + command_name: str | None = None + argument_hint: str | None = None + + +@dataclass(slots=True) +class SkillDiscovery: + """Pre-execution discovery evidence, including incomplete or failed checks.""" + + skills: list[DiscoveredSkill] = field(default_factory=list) + warnings: list[str] = field(default_factory=list) + errors: list[str] = field(default_factory=list) + complete: bool = False + + @dataclass(slots=True) class ClarificationStats: """Statistics about clarification requests detected during execution. @@ -280,6 +305,7 @@ class EvalResult: # Recorded interface metadata. available_tools: list[ToolInfo] = field(default_factory=list) skill_info: SkillInfo | None = None + skill_discovery: SkillDiscovery | None = None effective_system_prompt: str = "" mcp_prompts: list[MCPPrompt] = field(default_factory=list) prompt_name: str | None = None diff --git a/src/pytest_skill_engineering/core/serialization.py b/src/pytest_skill_engineering/core/serialization.py index 81821f0..c062a02 100644 --- a/src/pytest_skill_engineering/core/serialization.py +++ b/src/pytest_skill_engineering/core/serialization.py @@ -56,7 +56,14 @@ def deserialize_suite_report(data: dict[str, Any]) -> SuiteReport: Reconstructs the full dataclass hierarchy from the serialized format. """ from pytest_skill_engineering.copilot.requests import RequestAudit - from pytest_skill_engineering.core.result import EvalResult, ToolCall, Turn, UsageInfo + from pytest_skill_engineering.core.result import ( + DiscoveredSkill, + EvalResult, + SkillDiscovery, + ToolCall, + Turn, + UsageInfo, + ) from pytest_skill_engineering.reporting.collector import SuiteReport, TestReport # Reconstruct tests @@ -214,6 +221,20 @@ def deserialize_suite_report(data: dict[str, Any]) -> SuiteReport: ) # Reconstruct agent result + discovery_data = _require_key(ar_data, "skill_discovery", context="EvalResult") + skill_discovery = None + if discovery_data is not None: + skill_discovery = SkillDiscovery( + skills=[ + DiscoveredSkill(**skill) + for skill in _require_key( + discovery_data, "skills", context="SkillDiscovery" + ) + ], + warnings=_require_key(discovery_data, "warnings", context="SkillDiscovery"), + errors=_require_key(discovery_data, "errors", context="SkillDiscovery"), + complete=_require_key(discovery_data, "complete", context="SkillDiscovery"), + ) eval_result = EvalResult( turns=turns, success=_require_key(ar_data, "success", context="EvalResult"), @@ -228,6 +249,7 @@ def deserialize_suite_report(data: dict[str, Any]) -> SuiteReport: assertions=assertions, available_tools=available_tools, skill_info=skill_info, + skill_discovery=skill_discovery, effective_system_prompt=_require_key( ar_data, "effective_system_prompt", context="EvalResult" ), diff --git a/tests/contracts/test_native_evidence.py b/tests/contracts/test_native_evidence.py index 42bd8de..47e07a3 100644 --- a/tests/contracts/test_native_evidence.py +++ b/tests/contracts/test_native_evidence.py @@ -136,6 +136,18 @@ def test_advisory_payload_is_an_error(tmp_path: Path) -> None: load_suite_report(path) +def test_missing_skill_discovery_is_not_inferred_from_configuration(tmp_path: Path) -> None: + path = tmp_path / "evidence.json" + data = serialize_dataclass(_suite()) + data["schema_version"] = "4.0" + result = data["tests"][0]["eval_result"] + result["configuration"]["skill_directories"] = ["requested-only"] + del result["skill_discovery"] + path.write_text(json.dumps(data), encoding="utf-8") + with pytest.raises(ValueError, match="missing required field 'skill_discovery'"): + load_suite_report(path) + + def test_empty_image_payload_is_not_changed_to_missing_evidence(tmp_path: Path) -> None: suite = _suite() result = suite.tests[0].eval_result diff --git a/tests/contracts/test_skill_availability.py b/tests/contracts/test_skill_availability.py new file mode 100644 index 0000000..e743da8 --- /dev/null +++ b/tests/contracts/test_skill_availability.py @@ -0,0 +1,253 @@ +"""Offline skill setup contracts; no model execution.""" + +from __future__ import annotations + +from pathlib import Path +from types import SimpleNamespace +from typing import Any, Literal + +import pytest +from copilot.generated.rpc import Skill as SDKSkill +from copilot.generated.rpc import SkillList, SkillsLoadDiagnostics +from copilot.generated.session_events import SkillSource + +from pytest_skill_engineering.copilot import CopilotEval, runner +from pytest_skill_engineering.copilot.fixtures import _convert_to_aitest +from pytest_skill_engineering.copilot.skills import requested_skill_files +from pytest_skill_engineering.core.serialization import serialize_dataclass +from pytest_skill_engineering.reporting import TestReport as CaseReport +from pytest_skill_engineering.reporting import build_suite_report, generate_json, load_suite_report + + +@pytest.mark.parametrize("mode", ["empty", "copilot-cli"]) +def test_explicit_skills_enable_loading_without_ambient_discovery( + mode: Literal["empty", "copilot-cli"], +) -> None: + agent = CopilotEval(client_mode=mode, skill_directories=["explicit-skills"]) + config = agent.build_session_config() + assert config["enable_skills"] is True + assert config["enable_config_discovery"] is False + + +def test_explicit_skills_cannot_be_globally_disabled() -> None: + agent = CopilotEval( + client_mode="empty", + skill_directories=["explicit-skills"], + extra_config={"enable_skills": False}, + ) + with pytest.raises(ValueError, match="enable_skills=False"): + agent.build_session_config() + + +@pytest.mark.parametrize("enabled", [None, True]) +def test_explicit_skills_with_passthrough_enable_setting(enabled: bool | None) -> None: + config = CopilotEval( + skill_directories=["explicit-skills"], extra_config={"enable_skills": enabled} + ).build_session_config() + assert config["enable_skills"] is True + + +def test_explicit_disable_without_directories_is_supported() -> None: + config = CopilotEval( + client_mode="empty", extra_config={"enable_skills": False} + ).build_session_config() + assert config["enable_skills"] is False + + +def test_passthrough_directories_cannot_evade_enable_or_replace_explicit_inputs() -> None: + config = CopilotEval( + client_mode="empty", extra_config={"skill_directories": ["explicit-skills"]} + ).build_session_config() + assert config["enable_skills"] is True + with pytest.raises(ValueError, match="override"): + CopilotEval( + skill_directories=["explicit-skills"], extra_config={"skill_directories": []} + ).build_session_config() + with pytest.raises(ValueError, match="enable_skills=False"): + CopilotEval( + extra_config={"skill_directories": ["explicit-skills"], "enable_skills": False} + ).build_session_config() + + +def _write_skill(parent: Path, name: str = "explicit-skill") -> Path: + directory = parent / name + directory.mkdir(parents=True) + (directory / "SKILL.md").write_text( + f"---\nname: {name}\ndescription: Explicit test skill\n---\n\n# Instructions\n", + encoding="utf-8", + ) + return directory + + +def test_relative_and_repeated_paths_resolve_to_the_same_explicit_skill( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + directory = _write_skill(tmp_path) + monkeypatch.chdir(tmp_path) + requested = requested_skill_files([directory.name, str(directory), str(tmp_path)]) + assert requested == {(directory / "SKILL.md").resolve(): directory.name} + + +@pytest.mark.parametrize("directories", [[""], [None], "not-a-list"]) +async def test_invalid_directory_argument_shapes_fail_before_start( + monkeypatch: pytest.MonkeyPatch, directories: Any +) -> None: + def forbid_client(*args: Any, **kwargs: Any) -> None: + pytest.fail("Invalid directory arguments must not start a client") + + monkeypatch.setattr(runner, "create_client", forbid_client) + result = await runner.run_copilot(CopilotEval(skill_directories=directories), "Do not send.") + assert not result.success and result.stop_reason == "execution_error" + assert result.error and "skill_directories" in result.error + + +@pytest.mark.parametrize( + "invalid", ["missing", "file", "empty", "invalid", "partial", "unreadable", "duplicate"] +) +async def test_bad_skill_inputs_fail_before_client_start( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, invalid: str +) -> None: + directory = tmp_path / "skills" + if invalid == "file": + directory.write_text("not a directory", encoding="utf-8") + elif invalid != "missing": + directory.mkdir() + if invalid in ("invalid", "partial"): + bad = _write_skill(directory, "invalid-skill") + (bad / "SKILL.md").write_text("No required frontmatter", encoding="utf-8") + if invalid == "partial": + _write_skill(directory, "valid-skill") + if invalid == "duplicate": + one = _write_skill(directory / "one") + two = _write_skill(directory / "two") + directories = [str(one), str(two)] + else: + directories = [str(directory)] + if invalid == "unreadable": + skill = _write_skill(directory) + original_read = Path.read_text + + def read(path: Path, *args: Any, **kwargs: Any) -> str: + if path == skill / "SKILL.md": + raise PermissionError("Unreadable requested skill") + return original_read(path, *args, **kwargs) + + monkeypatch.setattr(Path, "read_text", read) + + def forbid_client(*args: Any, **kwargs: Any) -> None: + pytest.fail("Invalid skill inputs must not start an SDK client") + + monkeypatch.setattr(runner, "create_client", forbid_client) + result = await runner.run_copilot( + CopilotEval(client_mode="empty", skill_directories=directories), "Do not execute." + ) + assert not result.success and result.stop_reason == "execution_error" + assert result.error and "Skill setup failed before model execution" in result.error + assert str(directory) in result.error + assert result.skill_discovery is not None + assert not result.skill_discovery.complete + assert result.skill_discovery.errors + assert result.usage == [] and result.turns == [] + + +@pytest.mark.parametrize( + "outcome", + [ + "loaded", + "missing", + "wrong-path", + "disabled", + "intentional-disable", + "sdk-error", + "rpc-error", + ], +) +async def test_runner_checks_sdk_availability_before_its_single_send_and_saves_evidence( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, outcome: str +) -> None: + directory = _write_skill(tmp_path) + path = str(directory / "SKILL.md") + calls: list[str] = [] + exposed = SDKSkill( + name=directory.name, + description="SDK description", + source=SkillSource.PROJECT, + enabled=outcome not in ("disabled", "intentional-disable"), + user_invocable=True, + path=path if outcome != "wrong-path" else str(tmp_path / "other" / "SKILL.md"), + ) + + class Skills: + async def ensure_loaded(self) -> None: + calls.append("ensure") + if outcome == "rpc-error": + raise RuntimeError("Skill RPC failed") + + async def reload(self) -> SkillsLoadDiagnostics: + calls.append("reload") + return SkillsLoadDiagnostics( + errors=["SDK skill parse failed"] if outcome == "sdk-error" else [], + warnings=["SDK skill warning"], + ) + + async def list(self) -> SkillList: + calls.append("list") + return SkillList([] if outcome == "missing" else [exposed]) + + class Session: + rpc = SimpleNamespace(skills=Skills()) + + async def send_and_wait(self, prompt: str, timeout: float) -> None: + calls.append("send") + + async def abort(self) -> None: + calls.append("abort") + + class Client: + async def start(self) -> None: + calls.append("start") + + async def create_session(self, **config: Any) -> Session: + calls.append("create") + assert config["enable_skills"] is True + assert config["enable_config_discovery"] is False + return Session() + + async def stop(self) -> None: + calls.append("stop") + + monkeypatch.setattr(runner, "create_client", lambda *args, **kwargs: Client()) + agent = CopilotEval( + client_mode="empty", + skill_directories=[str(directory)], + disabled_skills=[directory.name] if outcome == "intentional-disable" else [], + audit_requests=outcome not in ("loaded", "intentional-disable"), + ) + result = await runner.run_copilot(agent, "Do not force a skill read.") + succeeds = outcome in ("loaded", "intentional-disable") + assert result.success is succeeds + assert calls.count("start") == calls.count("create") == calls.count("stop") == 1 + assert calls.count("send") == int(succeeds) + assert calls.count("abort") == int(not succeeds) + discovery = result.skill_discovery + assert discovery is not None + assert discovery.complete is (outcome != "rpc-error") + assert discovery.warnings == ([] if outcome == "rpc-error" else ["SDK skill warning"]) + assert result.all_tool_calls == [] and result.usage == [] + if not succeeds: + assert result.stop_reason == "execution_error" and result.error + assert "No outbound model requests" not in result.error + assert discovery.errors + + converted = _convert_to_aitest(agent, result) + assert converted is not None + saved, _ = converted + assert saved.skill_discovery == discovery + assert saved.skill_discovery is not discovery + suite = build_suite_report( + [CaseReport(name="skill-setup", outcome="passed", duration_ms=0, eval_result=saved)], + name="Skill setup evidence", + ) + output = tmp_path / "evidence.json" + generate_json(suite, output) + assert serialize_dataclass(load_suite_report(output)) == serialize_dataclass(suite) diff --git a/tests/integration/copilot/test_skill_discovery.py b/tests/integration/copilot/test_skill_discovery.py new file mode 100644 index 0000000..74d7079 --- /dev/null +++ b/tests/integration/copilot/test_skill_discovery.py @@ -0,0 +1,154 @@ +"""Real SDK discovery and blocked-request checks. Never execute a model.""" + +from __future__ import annotations + +import asyncio +import json +from pathlib import Path + +import httpx +import pytest +from copilot import CopilotRequestContext, CopilotRequestHandler, CopilotWebSocketHandler +from copilot.session import CopilotSession + +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 +from pytest_skill_engineering.core.result import SkillDiscovery + +pytestmark = pytest.mark.copilot + + +def _write_skill(parent: Path, name: str) -> 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", + encoding="utf-8", + ) + return directory + + +@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 +) -> None: + async def forbid_send(*args: object, **kwargs: object) -> None: + pytest.fail("Discovery checks must not send model messages") + + monkeypatch.setattr(CopilotSession, "send", forbid_send) + monkeypatch.setattr(CopilotSession, "send_and_wait", forbid_send) + workspace = tmp_path / "workspace" + workspace.mkdir() + _write_skill(workspace / ".github" / "skills", "ambient-project") + (workspace / ".github" / "copilot-instructions.md").write_text( + "AMBIENT INSTRUCTIONS MUST NOT LOAD", encoding="utf-8" + ) + storage = tmp_path / "storage" + storage.mkdir() + _write_skill(storage / "skills", "ambient-personal") + parent = tmp_path / "explicit" + first = _write_skill(parent, "explicit-first") + second = _write_skill(parent, "explicit-second") + directories = { + "baseline": [], + "individual": [str(first)], + "parent": [str(parent)], + "disabled": [str(first)], + }[selection] + expected = { + "baseline": set(), + "individual": {str(first / "SKILL.md")}, + "parent": {str(first / "SKILL.md"), str(second / "SKILL.md")}, + "disabled": {str(first / "SKILL.md")}, + }[selection] + agent = CopilotEval( + client_mode="empty", + working_directory=str(workspace), + skill_directories=directories, + disabled_skills=["explicit-first"] if selection == "disabled" else [], + ) + client = create_client(str(workspace), mode="empty", base_directory=str(storage)) + try: + async with asyncio.timeout(60): + await client.start() + session = await client.create_session(**agent.build_session_config()) + await session.rpc.skills.ensure_loaded() + skills = (await session.rpc.skills.list()).skills + assert {skill.path for skill in skills} == expected + 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 + ) + assert discovery.complete and not discovery.errors and not discovery.warnings + assert {skill.path for skill in discovery.skills} == expected + finally: + assert await stop_client(client) == [] + + +async def test_empty_mode_omits_ambient_instructions_from_blocked_request(tmp_path: Path) -> None: + """Build a request, but block both transports before any network forwarding.""" + captured: list[bytes] = [] + received = asyncio.Event() + + class BlockRequests(CopilotRequestHandler): + async def send_request( + self, request: httpx.Request, ctx: CopilotRequestContext + ) -> httpx.Response: + captured.append(await request.aread()) + received.set() + raise RuntimeError("Model execution forbidden by discovery test") + + async def open_websocket(self, ctx: CopilotRequestContext) -> CopilotWebSocketHandler: + raise RuntimeError("WebSocket model execution forbidden by discovery test") + + workspace = tmp_path / "workspace" + workspace.mkdir() + _write_skill(workspace / ".github" / "skills", "ambient-project") + (workspace / ".github" / "copilot-instructions.md").write_text( + "AMBIENT_INSTRUCTION_SENTINEL", encoding="utf-8" + ) + (workspace / "AGENTS.md").write_text("AMBIENT_AGENT_SENTINEL", encoding="utf-8") + first = _write_skill(tmp_path / "explicit", "explicit-first") + agent = CopilotEval( + client_mode="empty", + model="gpt-4.1", + working_directory=str(workspace), + skill_directories=[str(first)], + instructions="EXPLICIT_INSTRUCTION_SENTINEL", + extra_config={ + "provider": { + "type": "openai", + "base_url": "http://127.0.0.1:1", + "api_key": "offline-not-a-credential", + "wire_api": "completions", + "transport": "http", + } + }, + ) + client = create_client( + str(workspace), + mode="empty", + base_directory=str(tmp_path / "storage"), + request_handler=BlockRequests(), + ) + try: + async with asyncio.timeout(60): + await client.start() + session = await client.create_session(**agent.build_session_config()) + await session.rpc.skills.ensure_loaded() + assert [skill.name for skill in (await session.rpc.skills.list()).skills] == [ + "explicit-first" + ] + await session.send("Check isolated configuration.") + await received.wait() + await session.abort() + payload = json.dumps(json.loads(captured[0])) + assert "EXPLICIT_INSTRUCTION_SENTINEL" in payload + assert "AMBIENT_INSTRUCTION_SENTINEL" not in payload + assert "AMBIENT_AGENT_SENTINEL" not in payload + assert "ambient-project" not in payload + finally: + assert await stop_client(client) == [] diff --git a/tests/unit/test_release_invariants.py b/tests/unit/test_release_invariants.py index 39ff982..cec2795 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_0() -> None: +def test_project_version_is_pinned_to_1_0_1() -> None: pyproject = tomllib.loads((ROOT / "pyproject.toml").read_text(encoding="utf-8")) - assert pyproject["project"]["version"] == "1.0.0" + assert pyproject["project"]["version"] == "1.0.1" 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.0" in workflow + assert "v1.0.1" in workflow def test_github_release_job_selects_repository_without_checkout() -> None: diff --git a/uv.lock b/uv.lock index d123719..c92f06e 100644 --- a/uv.lock +++ b/uv.lock @@ -1421,7 +1421,7 @@ wheels = [ [[package]] name = "pytest-skill-engineering" -version = "1.0.0" +version = "1.0.1" source = { editable = "." } dependencies = [ { name = "github-copilot-sdk" },