diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 97a0c4b..b6f00d2 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -20,8 +20,18 @@ jobs: uses: ruby/setup-ruby@v1 with: ruby-version: '3.3' + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.12' + - name: Install Python validator dependencies + run: python3 -m pip install -r requirements-dev.txt - name: Validate skill format and links run: ruby scripts/validate-skills.rb + - name: Test eval manifest validation + run: python3 scripts/test-eval-validation.py + - name: Validate eval manifests + run: python3 scripts/validate-evals.py - name: Test changed-skill quality validation run: ruby scripts/test-validate-skill-quality.rb - name: Validate changed skill quality diff --git a/AGENTS.md b/AGENTS.md index 93ae4c9..6d9435c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -97,9 +97,31 @@ Every skill description must start with an imperative verb and define both when ## Eval Requirements -Every new skill must include `evals/evals.json` with at least five representative output-quality cases. Each case needs a realistic prompt, an expected outcome, and observable assertions. Trigger-only checks (should-trigger / should-not-trigger probes) are harness-specific and belong in a separate test set, not in `evals/evals.json`. +`evals/evals.json` is a versioned contract owned by this repository; it is not part of the normative Agent Skills specification. The declarative contract is [`schemas/evals-v1.schema.json`](schemas/evals-v1.schema.json), and `assertions` is the canonical case field. Do not substitute or alias `expectations`. -Existing skills are grandfathered via `scripts/grandfathered-skills.txt`. As overall eval coverage climbs past 25%, modified skills without evals receive a warning; past 50%, they fail CI. The coverage report is available via `python3 scripts/eval-coverage.py`. The ratchet is enforced in CI via `python3 scripts/eval-coverage.py --modified-from ` on every pull request. A skill is considered modified when any tracked file under its directory changes, not only `SKILL.md`. Coverage must not decrease between the base revision and the candidate; a decrease fails CI. +Every new skill must include a schema-versioned `evals/evals.json` with at least five representative output-quality cases. Each case needs a stable ID, realistic prompt, expected outcome, and observable assertions. Renaming an eval ID breaks durable evidence references; do not attempt heuristic rename matching. Trigger-only checks (should-trigger / should-not-trigger probes) are harness-specific and belong in a separate test set, not in `evals/evals.json`. + +Coverage reports these five states separately: + +| State | Evidence required | +|-------|-------------------| +| `manifest_present` | `evals/evals.json` exists. This alone does not prove behavioral quality. | +| `schema_valid` | The manifest passes the repository's v1 structural and semantic validation. | +| `executable_grader_bindings_present` | Not assessed in v1. Requires a separate versioned grader-binding contract. | +| `recent_run_evidence_present` | Not assessed in v1. Requires a separate versioned provenance/freshness contract. | +| `release_gated_evidence_present` | Not assessed in v1. Requires a separate versioned release-gate contract. | + +v1 only covers manifest structure and semantic validity. It does not establish runtime provenance or release-gate evidence. Validate the contract and run its focused tests with: + +```sh +python3 -m venv .venv +. .venv/bin/activate +python3 -m pip install -r requirements-dev.txt +python3 scripts/test-eval-validation.py +python3 scripts/validate-evals.py +``` + +Existing skills are grandfathered via `scripts/grandfathered-skills.txt`. As schema-valid manifest coverage climbs past 25%, modified skills without valid manifests receive a warning; past 50%, they fail CI. The coverage report is available via `python3 scripts/eval-coverage.py`. The ratchet is enforced in CI via `python3 scripts/eval-coverage.py --modified-from ` on every pull request. A skill is considered modified when any tracked file under its directory changes, not only `SKILL.md`. Schema-valid manifest coverage must not decrease between the base revision and the candidate; a decrease fails CI. ## Best Practices @@ -130,6 +152,7 @@ When creating or modifying a skill in this repo, validate against the format: - **`README.md` exists in the skill root** with all required sections (see [README Format](#readme-format) above) - **README is written for humans** — no agent instructions, JSON schemas, or progressive disclosure notes in the README. Those belong in `SKILL.md`. - **`evals/evals.json` exists** with at least five output-quality cases for new skills (see [Eval Requirements](#eval-requirements)) +- `python3 scripts/validate-evals.py` accepts every present eval manifest ### Generated Artifacts diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9db1d31..3ef45ff 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -20,7 +20,9 @@ Each skill must: - Use relative links that work from a fresh clone. - Use an imperative-verb description that defines both positive and negative trigger boundaries. - Describe when the skill should be loaded, and identify the nearest alternative when overlap matters. -- Include `evals/evals.json` with at least five representative output-quality cases for every new skill. Existing skills are grandfathered via `scripts/grandfathered-skills.txt`; the ratchet tightens as overall coverage climbs. The ratchet runs in CI on every pull request via `python3 scripts/eval-coverage.py --modified-from `. A skill counts as modified when any tracked file under its directory changes. Coverage must not decrease between the base revision and the candidate. +- Include `evals/evals.json` with explicit `schema_version: 1` and at least five representative output-quality cases for every new skill. This is a repository-owned contract, not part of the normative Agent Skills specification; see [`schemas/evals-v1.schema.json`](schemas/evals-v1.schema.json). Use canonical `assertions`, not `expectations`. Existing skills are grandfathered via `scripts/grandfathered-skills.txt`; the ratchet tightens as schema-valid manifest coverage climbs. The ratchet runs in CI on every pull request via `python3 scripts/eval-coverage.py --modified-from `. A skill counts as modified when any tracked file under its directory changes. Schema-valid manifest coverage must not decrease between the base revision and the candidate. + +The coverage report keeps claims separate. `manifest_present` means only that a file exists. Per skill, `schema_valid` is `not_applicable` when `evals/evals.json` is missing, `false` when a present manifest fails parsing/schema/semantic validation, and `true` only when the manifest passes repository v1 validation. Aggregate schema-valid coverage counts only skills where `schema_valid` is `true`. The remaining states are named but intentionally `not_assessed` in v1: `executable_grader_bindings_present`, `recent_run_evidence_present`, and `release_gated_evidence_present`. Those require separate versioned contracts for grader bindings, provenance/freshness, and release-gate evidence. ## Development @@ -29,8 +31,13 @@ Clone the repository and run the validators from its root: ```sh git clone https://github.com/magnus919/agent-skills.git cd agent-skills +python3 -m venv .venv +. .venv/bin/activate +python3 -m pip install -r requirements-dev.txt ruby scripts/validate-skills.rb ruby scripts/validate-skill-quality.rb --base origin/main +python3 scripts/test-eval-validation.py +python3 scripts/validate-evals.py python3 scripts/test-eval-coverage.py python3 scripts/eval-coverage.py ``` diff --git a/de-spin/evals/evals.json b/de-spin/evals/evals.json index 2a771ee..2f4fe21 100644 --- a/de-spin/evals/evals.json +++ b/de-spin/evals/evals.json @@ -1,4 +1,5 @@ { + "schema_version": 1, "skill_name": "de-spin", "evals": [ { diff --git a/esp32-development/evals/evals.json b/esp32-development/evals/evals.json index 1ad7049..7372cfc 100644 --- a/esp32-development/evals/evals.json +++ b/esp32-development/evals/evals.json @@ -1,4 +1,5 @@ { + "schema_version": 1, "skill_name": "esp32-development", "evals": [ { diff --git a/raleigh/evals/evals.json b/raleigh/evals/evals.json index f416ff7..71852c0 100644 --- a/raleigh/evals/evals.json +++ b/raleigh/evals/evals.json @@ -1,4 +1,5 @@ { + "schema_version": 1, "skill_name": "raleigh", "evals": [ { diff --git a/requirements-dev.txt b/requirements-dev.txt new file mode 100644 index 0000000..66cc01a --- /dev/null +++ b/requirements-dev.txt @@ -0,0 +1,2 @@ +jsonschema[format]==4.26.0 +requests==2.34.2 diff --git a/restic/evals/evals.json b/restic/evals/evals.json index d8bcbc9..f73f6b9 100644 --- a/restic/evals/evals.json +++ b/restic/evals/evals.json @@ -1,4 +1,5 @@ { + "schema_version": 1, "skill_name": "restic", "evals": [ { diff --git a/schemas/evals-v1.schema.json b/schemas/evals-v1.schema.json new file mode 100644 index 0000000..30cba2e --- /dev/null +++ b/schemas/evals-v1.schema.json @@ -0,0 +1,50 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://github.com/magnus919/agent-skills/schemas/evals-v1.schema.json", + "title": "agent-skills repository eval manifest v1", + "type": "object", + "additionalProperties": false, + "required": ["schema_version", "skill_name", "evals"], + "properties": { + "schema_version": {"type": "integer", "const": 1}, + "skill_name": {"type": "string", "minLength": 1, "pattern": "\\S"}, + "evals": { + "type": "array", + "minItems": 1, + "items": {"$ref": "#/$defs/eval_case"} + } + }, + "$defs": { + "relative_path": { + "type": "string", + "minLength": 1, + "pattern": "^(?=.*\\S)(?!/)(?!\\./)(?!.*//)(?!.*(?:^|/)\\.(?:/|$))(?!.*(?:^|/)\\.\\.(?:/|$))(?!.*\\\\)(?!.*\\/$).+$" + }, + "eval_case": { + "type": "object", + "additionalProperties": false, + "required": ["id", "prompt", "expected_output", "assertions"], + "properties": { + "id": { + "type": "string", + "minLength": 1, + "maxLength": 64, + "pattern": "^[a-z0-9]+(?:-[a-z0-9]+)*$" + }, + "prompt": {"type": "string", "minLength": 1, "pattern": "\\S"}, + "expected_output": {"type": "string", "minLength": 1, "pattern": "\\S"}, + "assertions": { + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": {"type": "string", "minLength": 1, "pattern": "\\S"} + }, + "files": { + "type": "array", + "uniqueItems": true, + "items": {"$ref": "#/$defs/relative_path"} + } + } + } + } +} diff --git a/scripts/eval-coverage.py b/scripts/eval-coverage.py index a960715..a965b0c 100644 --- a/scripts/eval-coverage.py +++ b/scripts/eval-coverage.py @@ -11,13 +11,17 @@ Usage: """ import argparse +import io import json import subprocess import sys +import tarfile +import tempfile from pathlib import Path +from eval_validation import NOT_ASSESSED, STATE_NAMES, ValidationResult, validate_manifest + ROOT = Path(__file__).resolve().parent.parent -GRANDFATHER_FILE = ROOT / "scripts" / "grandfathered-skills.txt" # Phase 3 ratchet thresholds (percent of skills with evals) WARN_THRESHOLD = 25 # modified skills without evals get a warning @@ -48,10 +52,28 @@ def find_skills() -> list[Path]: return sorted(skills) +def resolve_ref_to_commit(ref: str) -> str: + """Resolve *ref* to a commit SHA or raise ValueError. + + The caller must use only the returned SHA in subsequent git commands. + """ + result = subprocess.run( + ["git", "rev-parse", "--verify", "--end-of-options", f"{ref}^{{commit}}"], + cwd=ROOT, + capture_output=True, + text=True, + ) + if result.returncode != 0: + raise ValueError( + f"invalid --modified-from ref: {ref!r} is not an existing commit" + ) + return result.stdout.strip() + + def find_skills_at(ref: str) -> list[Path]: """Find canonical skill directories tracked at a git revision.""" result = subprocess.run( - ["git", "ls-tree", "-r", "--name-only", ref], + ["git", "ls-tree", "-r", "--name-only", ref, "--"], cwd=ROOT, check=True, capture_output=True, @@ -66,30 +88,16 @@ def find_skills_at(ref: str) -> list[Path]: skills.append(Path(name).parent) return sorted(skills) - -def load_grandfathered() -> set[str]: - if not GRANDFATHER_FILE.exists(): - return set() - return { - line.strip() - for line in GRANDFATHER_FILE.read_text().splitlines() - if line.strip() and not line.strip().startswith("#") - } +def check_eval_states(skill_dir: Path) -> ValidationResult: + """Return validation and evidence states for one skill.""" + evals_file = ROOT / skill_dir / "evals" / "evals.json" + return validate_manifest(evals_file, ROOT) def check_evals(skill_dir: Path) -> tuple[bool, int]: - """Return (has_valid_evals, case_count) for a skill directory.""" - evals_file = ROOT / skill_dir / "evals" / "evals.json" - if not evals_file.exists(): - return False, 0 - try: - data = json.loads(evals_file.read_text(encoding="utf-8")) - if isinstance(data, dict) and isinstance(data.get("evals"), list): - count = len(data["evals"]) - return count > 0, count - return False, 0 - except (json.JSONDecodeError, OSError): - return False, 0 + """Return (has_schema_valid_manifest, case_count) for compatibility.""" + result = check_eval_states(skill_dir) + return result.states["schema_valid"] is True, result.case_count def count_references(skill_name: str, all_skill_dirs: list[Path]) -> int: @@ -107,7 +115,7 @@ def count_references(skill_name: str, all_skill_dirs: list[Path]) -> int: return count -def modified_skills(base_ref: str) -> set[Path]: +def modified_skills(base_ref_commit: str) -> set[Path]: """Return skill directories with any tracked file changed since base_ref. A skill is considered modified when *any* file under its directory @@ -115,7 +123,7 @@ def modified_skills(base_ref: str) -> set[Path]: fixtures, README, and eval manifests. """ result = subprocess.run( - ["git", "diff", "--name-only", base_ref, "HEAD"], + ["git", "diff", "--name-only", base_ref_commit, "HEAD", "--"], cwd=ROOT, capture_output=True, text=True, @@ -129,7 +137,7 @@ def modified_skills(base_ref: str) -> set[Path]: # Include skills from both revisions so complete directory deletions # remain observable under their old name. known_skills = sorted( - set(find_skills()) | set(find_skills_at(base_ref)), + set(find_skills()) | set(find_skills_at(base_ref_commit)), key=lambda path: len(path.parts), reverse=True, ) @@ -159,62 +167,50 @@ def evaluate_ratchet( name = str(skill_dir) if coverage_pct >= FAIL_THRESHOLD: errors.append( - f"{name}: modified skill has no evals " + f"{name}: modified skill has no schema-valid eval manifest " f"(coverage {coverage_pct:.1f}% >= {FAIL_THRESHOLD}% — " "evals required on modification)" ) elif coverage_pct >= WARN_THRESHOLD: warnings.append( - f"{name}: modified skill has no evals " + f"{name}: modified skill has no schema-valid eval manifest " f"(coverage {coverage_pct:.1f}% >= {WARN_THRESHOLD}% — " "evals recommended)" ) return warnings, errors -def coverage_decreased(base_ref: str) -> tuple[bool, float, float]: +def coverage_decreased(base_ref_commit: str) -> tuple[bool, float, float]: """Compare eval coverage between base_ref and HEAD. Returns (decreased, base_pct, head_pct). Coverage is the percentage - of canonical skills that have a non-empty evals/evals.json. + of canonical skills that have a schema-valid v1 evals/evals.json. """ head_skills = find_skills() head_with = sum(1 for s in head_skills if check_evals(s)[0]) head_pct = (head_with / len(head_skills) * 100) if head_skills else 0.0 - # Count skills with evals at the base revision. - result = subprocess.run( - ["git", "ls-tree", "-r", "--name-only", base_ref], - cwd=ROOT, - capture_output=True, - text=True, - ) - base_skill_dirs: set[Path] = set() - for line in result.stdout.strip().splitlines(): - if ( - line - and line.endswith("/SKILL.md") - and "/agent-council/profiles/skills/" not in line - ): - base_skill_dirs.add(Path(line).parent) - + base_skill_dirs = set(find_skills_at(base_ref_commit)) base_with = 0 - for skill_dir in base_skill_dirs: - evals_path = f"{skill_dir}/evals/evals.json" - cat = subprocess.run( - ["git", "show", f"{base_ref}:{evals_path}"], - cwd=ROOT, + archive = subprocess.run( + ["git", "archive", "--format=tar", base_ref_commit, "--"], + cwd=ROOT, + check=True, + capture_output=True, + ).stdout + with tempfile.TemporaryDirectory() as tmp: + snapshot = Path(tmp) + with tarfile.open(fileobj=io.BytesIO(archive), mode="r:") as tar: + tar.extractall(snapshot, filter="data") + subprocess.run(["git", "init", "-q"], cwd=snapshot, check=True) + subprocess.run( + ["git", "add", "-f", "--all"], cwd=snapshot, check=True, capture_output=True, - text=True, ) - if cat.returncode != 0: - continue - try: - data = json.loads(cat.stdout) - if isinstance(data, dict) and isinstance(data.get("evals"), list) and len(data["evals"]) > 0: + for skill_dir in base_skill_dirs: + manifest = snapshot / skill_dir / "evals" / "evals.json" + if validate_manifest(manifest, snapshot).states["schema_valid"] is True: base_with += 1 - except (json.JSONDecodeError, ValueError): - pass base_pct = (base_with / len(base_skill_dirs) * 100) if base_skill_dirs else 0.0 return head_pct < base_pct, base_pct, head_pct @@ -230,22 +226,54 @@ def main() -> int: ) args = parser.parse_args() + resolved_base_ref = None + if args.modified_from: + try: + resolved_base_ref = resolve_ref_to_commit(args.modified_from) + except ValueError as exc: + print(f"ERROR: {exc}", file=sys.stderr) + return 2 + skills = find_skills() - grandfathered = load_grandfathered() total = len(skills) - with_evals: list[dict] = [] + skill_states: list[dict] = [] without_evals: list[str] = [] for skill_dir in skills: - has, count = check_evals(skill_dir) + validation = check_eval_states(skill_dir) + has = validation.states["schema_valid"] + count = validation.case_count name = str(skill_dir) - if has: - with_evals.append({"skill": name, "cases": count}) - else: + skill_states.append({"skill": name, "cases": count, **validation.states}) + if has is not True: without_evals.append(name) - coverage_pct = (len(with_evals) / total * 100) if total else 0.0 + supported_states = ("manifest_present", "schema_valid") + state_summary: dict[str, dict[str, object]] = {} + for state in supported_states: + count = sum(1 for entry in skill_states if entry[state] is True) + state_summary[state] = { + "assessment": "supported", + "count": count, + "percentage": round((count / total * 100) if total else 0.0, 1), + } + + not_assessed_reasons = { + "executable_grader_bindings_present": "No repository contract for executable grader bindings in schema v1.", + "recent_run_evidence_present": "No versioned provenance/freshness contract is defined for schema v1.", + "release_gated_evidence_present": "No versioned release-gate contract is defined for schema v1.", + } + for state in STATE_NAMES: + if state in state_summary: + continue + state_summary[state] = { + "assessment": NOT_ASSESSED, + "count": None, + "percentage": None, + "reason": not_assessed_reasons[state], + } + coverage_pct = state_summary["schema_valid"]["percentage"] # Sort skills without evals: most-referenced first, then alphabetical ref_counts = { @@ -256,8 +284,8 @@ def main() -> int: # Phase 3 ratchet check ratchet_warnings: list[str] = [] ratchet_errors: list[str] = [] - if args.modified_from: - modified = modified_skills(args.modified_from) + if resolved_base_ref: + modified = modified_skills(resolved_base_ref) ratchet_warnings, ratchet_errors = evaluate_ratchet( modified=modified, current=set(skills), @@ -266,7 +294,7 @@ def main() -> int: ) # Monotonic coverage floor: fail if coverage decreased. - decreased, base_pct, head_pct = coverage_decreased(args.modified_from) + decreased, base_pct, head_pct = coverage_decreased(resolved_base_ref) if decreased: ratchet_errors.append( f"eval coverage decreased from {base_pct:.1f}% to {head_pct:.1f}% " @@ -278,14 +306,8 @@ def main() -> int: json.dumps( { "total_skills": total, - "skills_with_evals": len(with_evals), - "skills_without_evals": len(without_evals), - "coverage_pct": round(coverage_pct, 1), - "with_evals": with_evals, - "without_evals": [ - {"skill": n, "references": ref_counts.get(n, 0)} - for n in without_evals - ], + "states": state_summary, + "skills": skill_states, "ratchet": { "warn_threshold": WARN_THRESHOLD, "fail_threshold": FAIL_THRESHOLD, @@ -297,14 +319,18 @@ def main() -> int: ) ) else: - print(f"Eval coverage: {len(with_evals)}/{total} skills ({coverage_pct:.1f}%)") + print("Eval evidence states (a manifest alone does not prove behavioral quality):") + for state in STATE_NAMES: + summary = state_summary[state] + if summary["assessment"] == "supported": + print( + f" {state}: {summary['count']}/{total} skills " + f"({summary['percentage']:.1f}%)" + ) + continue + print(f" {state}: not assessed ({summary['reason']})") print() - if with_evals: - print("Skills WITH evals:") - for entry in with_evals: - print(f" + {entry['skill']} ({entry['cases']} cases)") - print() - print(f"Skills WITHOUT evals ({len(without_evals)}), by reference count:") + print(f"Skills WITHOUT schema-valid eval manifests ({len(without_evals)}), by reference count:") for name in without_evals: refs = ref_counts.get(name, 0) print(f" - {name} (referenced by {refs} skills)") diff --git a/scripts/eval_validation.py b/scripts/eval_validation.py new file mode 100644 index 0000000..b4d0b1e --- /dev/null +++ b/scripts/eval_validation.py @@ -0,0 +1,364 @@ +#!/usr/bin/env python3 +"""Validation for this repository's eval manifest v1 contract.""" + +from __future__ import annotations + +import json +import re +import subprocess +from dataclasses import dataclass, field +from functools import lru_cache +from pathlib import Path, PurePosixPath +from typing import Any + +from jsonschema import Draft202012Validator + +SCHEMA_VERSION = 1 +STATE_NAMES = ( + "manifest_present", + "schema_valid", + "executable_grader_bindings_present", + "recent_run_evidence_present", + "release_gated_evidence_present", +) +NOT_ASSESSED = "not_assessed" +NOT_APPLICABLE = "not_applicable" +SCHEMA_PATH = Path(__file__).resolve().parent.parent / "schemas" / "evals-v1.schema.json" +CASE_ID_RE = re.compile(r"^[a-z0-9]+(?:-[a-z0-9]+)*$") + + +@dataclass +class ValidationResult: + manifest_path: Path + case_count: int = 0 + errors: list[str] = field(default_factory=list) + states: dict[str, bool | str] = field( + default_factory=lambda: { + "manifest_present": False, + "schema_valid": NOT_APPLICABLE, + "executable_grader_bindings_present": NOT_ASSESSED, + "recent_run_evidence_present": NOT_ASSESSED, + "release_gated_evidence_present": NOT_ASSESSED, + } + ) + + def error(self, location: str, message: str) -> None: + suffix = f" {location}" if location else "" + self.errors.append(f"{self.manifest_path}:{suffix}: {message}") + + +class DuplicateKeyError(ValueError): + def __init__(self, key: str): + super().__init__(f"duplicate key {key!r}") + self.key = key + + +def _reject_duplicate_keys(pairs: list[tuple[str, Any]]) -> dict[str, Any]: + out: dict[str, Any] = {} + for key, value in pairs: + if key in out: + raise DuplicateKeyError(key) + out[key] = value + return out + + +def _json_pointer(path: Any) -> str: + if not path: + return "$" + location = "$" + for part in path: + location += f"[{part}]" if isinstance(part, int) else f".{part}" + return location + + +def _nonempty_string(value: Any) -> bool: + return isinstance(value, str) and bool(value.strip()) + + +def _contains_control_characters(value: str) -> bool: + return any(ord(char) < 32 or ord(char) == 127 for char in value) + + +def _git(repo_root: Path, *args: str) -> subprocess.CompletedProcess[bytes]: + return subprocess.run(["git", *args], cwd=repo_root, capture_output=True, check=False) + + +def _tracked_files(repo_root: Path) -> set[str]: + result = _git(repo_root, "ls-files", "-z") + if result.returncode != 0: + return set() + return { + path + for path in result.stdout.decode("utf-8", errors="replace").split("\0") + if path + } + + +def _tracked_mode(repo_root: Path, relative_path: str) -> str | None: + result = _git(repo_root, "ls-files", "-s", "--", relative_path) + if result.returncode != 0 or not result.stdout: + return None + return result.stdout.decode("utf-8", errors="replace").split(None, 1)[0] + + +def _case_mismatch(tracked: set[str], relative_path: str) -> bool: + lower = relative_path.lower() + return any(path.lower() == lower and path != relative_path for path in tracked) + + +def _load_json_strict(path: Path) -> dict[str, Any]: + text = path.read_text(encoding="utf-8") + parsed = json.loads(text, object_pairs_hook=_reject_duplicate_keys) + if not isinstance(parsed, dict): + raise TypeError("top-level JSON value must be an object") + return parsed + + +@lru_cache(maxsize=1) +def _schema_validator() -> Draft202012Validator: + schema = json.loads(SCHEMA_PATH.read_text(encoding="utf-8")) + Draft202012Validator.check_schema(schema) + return Draft202012Validator(schema) + + +def _validate_schema(data: dict[str, Any], result: ValidationResult) -> None: + validator = _schema_validator() + for issue in sorted(validator.iter_errors(data), key=lambda e: list(e.path)): + result.error(_json_pointer(issue.absolute_path), issue.message) + + +def _targeted_schema_version_errors(data: dict[str, Any], result: ValidationResult) -> None: + if "schema_version" not in data: + result.error( + "$.schema_version", + "missing required schema_version; add \"schema_version\": 1", + ) + return + version = data["schema_version"] + if isinstance(version, str): + result.error( + "$.schema_version", + "schema_version must be integer 1, not a string", + ) + return + if isinstance(version, bool) or not isinstance(version, int): + result.error("$.schema_version", "schema_version must be integer 1") + return + if version != SCHEMA_VERSION: + result.error( + "$.schema_version", + f"unsupported schema_version {version!r}; supported version is 1", + ) + + +def _validate_case_aliases(case: dict[str, Any], index: int, result: ValidationResult) -> None: + has_assertions = "assertions" in case + has_expectations = "expectations" in case + if has_assertions and has_expectations: + result.error( + f"$.evals[{index}].expectations", + "assertions and expectations cannot both be present; keep assertions only", + ) + elif has_expectations: + result.error( + f"$.evals[{index}].expectations", + "expectations is not supported by repository schema v1; rename it to assertions", + ) + + +def _validate_fixture_path( + value: Any, + *, + skill_root: Path, + repo_root: Path, + tracked: set[str], + location: str, + result: ValidationResult, +) -> None: + if not _nonempty_string(value): + result.error(location, "must be a nonempty relative path string") + return + + path_text = value + if _contains_control_characters(path_text): + result.error(location, "must not contain control characters") + return + if "\\" in path_text: + result.error(location, "must not contain backslashes") + return + + lexical_parts = path_text.split("/") + if any(part in {"", ".", ".."} for part in lexical_parts): + result.error(location, "must not contain '.', '..', or empty path components") + return + + pure = PurePosixPath(path_text) + if pure.is_absolute() or not pure.parts: + result.error(location, "must be a relative path") + return + + candidate = skill_root.joinpath(*lexical_parts) + relative = candidate.relative_to(repo_root).as_posix() + + if _case_mismatch(tracked, relative): + result.error(location, f"path is case-mismatched with tracked Git path: {path_text}") + return + + probe = skill_root + for part in lexical_parts: + probe = probe / part + if probe.is_symlink(): + result.error(location, f"symlinks are not allowed in path components: {path_text}") + return + + if not candidate.exists(): + mode = _tracked_mode(repo_root, relative) + if mode is None: + result.error(location, f"file does not exist: {path_text}") + else: + result.error( + location, + f"file does not exist in working tree (tracked in Git index): {path_text}", + ) + return + + try: + resolved = candidate.resolve(strict=True) + resolved.relative_to(skill_root.resolve()) + except ValueError: + result.error(location, f"path escapes skill root: {path_text}") + return + except OSError: + result.error(location, f"file does not exist: {path_text}") + return + + if not resolved.is_file(): + result.error(location, f"must resolve to a regular file: {path_text}") + return + + if relative not in tracked: + result.error(location, f"file is not tracked by Git: {path_text}") + return + + mode = _tracked_mode(repo_root, relative) + if mode is not None and mode == "120000": + result.error(location, f"tracked symlink is not allowed: {path_text}") + + +def _semantic_checks(data: dict[str, Any], result: ValidationResult, repo_root: Path) -> None: + manifest_path = result.manifest_path + skill_root = manifest_path.parent.parent.resolve() + tracked = _tracked_files(repo_root) + + _targeted_schema_version_errors(data, result) + + skill_name = data.get("skill_name") + if skill_name != skill_root.name: + result.error( + "$.skill_name", + f"must equal containing skill directory name {skill_root.name!r}", + ) + + evals = data.get("evals") + if isinstance(evals, list): + result.case_count = len(evals) + else: + result.case_count = 0 + return + + seen_ids: dict[str, int] = {} + for index, case in enumerate(evals): + if not isinstance(case, dict): + continue + _validate_case_aliases(case, index, result) + + case_id = case.get("id") + if isinstance(case_id, str): + if len(case_id) > 64: + result.error(f"$.evals[{index}].id", "must be 1-64 characters") + if not CASE_ID_RE.fullmatch(case_id): + result.error( + f"$.evals[{index}].id", + "must match ^[a-z0-9]+(?:-[a-z0-9]+)*$", + ) + prior = seen_ids.get(case_id) + if prior is not None: + result.error( + f"$.evals[{index}].id", + f"duplicate case ID {case_id!r}; first seen at $.evals[{prior}].id", + ) + else: + seen_ids[case_id] = index + + assertions = case.get("assertions") + if isinstance(assertions, list): + seen_assertions: set[str] = set() + for assertion_index, assertion in enumerate(assertions): + if not _nonempty_string(assertion): + result.error( + f"$.evals[{index}].assertions[{assertion_index}]", + "must be a nonempty non-whitespace string", + ) + continue + if assertion in seen_assertions: + result.error( + f"$.evals[{index}].assertions[{assertion_index}]", + "duplicate assertion values are not allowed", + ) + continue + seen_assertions.add(assertion) + + files = case.get("files") + if isinstance(files, list): + for file_index, file_value in enumerate(files): + _validate_fixture_path( + file_value, + skill_root=skill_root, + repo_root=repo_root, + tracked=tracked, + location=f"$.evals[{index}].files[{file_index}]", + result=result, + ) + + +def validate_manifest(manifest_path: Path, repo_root: Path) -> ValidationResult: + """Validate one manifest against schema and repository semantics.""" + manifest_path = Path(manifest_path) + repo_root = Path(repo_root).resolve() + result = ValidationResult(manifest_path) + if not manifest_path.is_file(): + return result + + result.states["manifest_present"] = True + result.states["schema_valid"] = False + try: + data = _load_json_strict(manifest_path) + except DuplicateKeyError as exc: + result.error("$", f"duplicate JSON object key: {exc.key!r}") + return result + except json.JSONDecodeError as exc: + result.error("$", f"invalid JSON: {exc}") + return result + except OSError as exc: + result.error("$", f"unable to read manifest: {exc}") + return result + except TypeError as exc: + result.error("$", str(exc)) + return result + + _validate_schema(data, result) + _semantic_checks(data, result, repo_root) + + result.states["schema_valid"] = not result.errors + return result + + +def find_skill_manifests(repo_root: Path) -> list[Path]: + manifests = [] + for skill_file in Path(repo_root).glob("**/SKILL.md"): + if "agent-council/profiles/skills" in skill_file.as_posix(): + continue + manifest = skill_file.parent / "evals" / "evals.json" + if manifest.is_file(): + manifests.append(manifest) + return sorted(manifests) diff --git a/scripts/test-eval-coverage.py b/scripts/test-eval-coverage.py index e3e555f..e761bb0 100644 --- a/scripts/test-eval-coverage.py +++ b/scripts/test-eval-coverage.py @@ -6,16 +6,20 @@ coverage-decrease detection, and threshold behavior. """ import json +import io import os import subprocess import sys import tempfile import unittest +from contextlib import redirect_stderr, redirect_stdout from pathlib import Path +from unittest import mock # Import the module under test. The script is named eval-coverage.py # (hyphenated), so we load it via importlib rather than a normal import. import importlib.util # noqa: E402 +from eval_validation import NOT_APPLICABLE # noqa: E402 SCRIPT_DIR = Path(__file__).resolve().parent _spec = importlib.util.spec_from_file_location( @@ -49,7 +53,7 @@ def write_skill(repo: str, name: str, evals: list[dict] | None = None) -> None: evals_dir = skill_dir / "evals" evals_dir.mkdir(exist_ok=True) (evals_dir / "evals.json").write_text( - json.dumps({"skill_name": name, "evals": evals}, indent=2) + json.dumps({"schema_version": 1, "skill_name": name, "evals": evals}, indent=2) ) @@ -275,6 +279,24 @@ class TestCoverageDecreased(unittest.TestCase): self.assertAlmostEqual(base_pct, 100.0) self.assertAlmostEqual(head_pct, 50.0) + def test_replacing_valid_manifest_with_invalid_is_regression(self) -> None: + write_skill(self.repo, "s1", evals=[make_case("c1")]) + base = self._commit("valid manifest") + (Path(self.repo) / "s1" / "evals" / "evals.json").write_text( + json.dumps({"schema_version": 1, "skill_name": "s1", "evals": []}), + encoding="utf-8", + ) + self._commit("break manifest") + old_root = eval_coverage.ROOT + eval_coverage.ROOT = Path(self.repo) + try: + decreased, base_pct, head_pct = eval_coverage.coverage_decreased(base) + finally: + eval_coverage.ROOT = old_root + self.assertTrue(decreased) + self.assertAlmostEqual(base_pct, 100.0) + self.assertAlmostEqual(head_pct, 0.0) + def test_no_decrease_when_stable(self) -> None: write_skill(self.repo, "s1", evals=[make_case("c1")]) write_skill(self.repo, "s2") @@ -361,5 +383,162 @@ class TestRatchetThresholds(unittest.TestCase): self.assertEqual([], errors) +class TestBaseRefValidation(unittest.TestCase): + def setUp(self) -> None: + self.tmp = tempfile.TemporaryDirectory() + self.repo = self.tmp.name + git(self.repo, "init", "-b", "main") + git(self.repo, "config", "user.email", "test@test.invalid") + git(self.repo, "config", "user.name", "Test") + write_skill(self.repo, "alpha", evals=[make_case("c1")]) + git(self.repo, "add", "-A") + git(self.repo, "commit", "-m", "initial") + + def tearDown(self) -> None: + self.tmp.cleanup() + + def test_main_rejects_invalid_modified_from_ref(self) -> None: + old_root = eval_coverage.ROOT + eval_coverage.ROOT = Path(self.repo) + stderr = io.StringIO() + try: + with mock.patch.object( + sys, "argv", ["eval-coverage.py", "--modified-from", "does-not-exist"] + ), redirect_stderr(stderr): + exit_code = eval_coverage.main() + finally: + eval_coverage.ROOT = old_root + self.assertEqual(2, exit_code) + self.assertEqual( + "ERROR: invalid --modified-from ref: 'does-not-exist' is not an existing commit\n", + stderr.getvalue(), + ) + + def test_main_rejects_option_like_modified_from_ref(self) -> None: + old_root = eval_coverage.ROOT + eval_coverage.ROOT = Path(self.repo) + stderr = io.StringIO() + try: + with mock.patch.object( + sys, "argv", ["eval-coverage.py", "--modified-from=--name-only"] + ), redirect_stderr(stderr): + exit_code = eval_coverage.main() + finally: + eval_coverage.ROOT = old_root + self.assertEqual(2, exit_code) + self.assertEqual( + "ERROR: invalid --modified-from ref: '--name-only' is not an existing commit\n", + stderr.getvalue(), + ) + + def test_main_accepts_valid_branch_ref(self) -> None: + old_root = eval_coverage.ROOT + eval_coverage.ROOT = Path(self.repo) + stderr = io.StringIO() + try: + with mock.patch.object( + sys, "argv", ["eval-coverage.py", "--modified-from", "main", "--json"] + ), redirect_stderr(stderr), redirect_stdout(io.StringIO()): + exit_code = eval_coverage.main() + finally: + eval_coverage.ROOT = old_root + self.assertEqual(0, exit_code) + self.assertEqual("", stderr.getvalue()) + + +class TestCoverageOutput(unittest.TestCase): + def setUp(self) -> None: + self.tmp = tempfile.TemporaryDirectory() + self.repo = self.tmp.name + git(self.repo, "init", "-b", "main") + write_skill(self.repo, "covered", evals=[make_case("c1")]) + write_skill(self.repo, "uncovered") + git(self.repo, "add", "-A") + + def tearDown(self) -> None: + self.tmp.cleanup() + + def test_json_names_all_five_states_and_per_skill_values(self) -> None: + old_root = eval_coverage.ROOT + eval_coverage.ROOT = Path(self.repo) + output = io.StringIO() + try: + with mock.patch.object(sys, "argv", ["eval-coverage.py", "--json"]), redirect_stdout(output): + exit_code = eval_coverage.main() + finally: + eval_coverage.ROOT = old_root + self.assertEqual(0, exit_code) + report = json.loads(output.getvalue()) + expected = { + "manifest_present", + "schema_valid", + "executable_grader_bindings_present", + "recent_run_evidence_present", + "release_gated_evidence_present", + } + self.assertEqual(expected, set(report["states"])) + self.assertEqual("supported", report["states"]["manifest_present"]["assessment"]) + self.assertEqual("supported", report["states"]["schema_valid"]["assessment"]) + for key in ( + "executable_grader_bindings_present", + "recent_run_evidence_present", + "release_gated_evidence_present", + ): + self.assertEqual("not_assessed", report["states"][key]["assessment"]) + self.assertIsNone(report["states"][key]["count"]) + self.assertIsNone(report["states"][key]["percentage"]) + self.assertTrue(report["states"][key]["reason"]) + for skill in report["skills"]: + self.assertTrue(expected.issubset(skill)) + self.assertIsInstance(skill["manifest_present"], bool) + self.assertIn(skill["schema_valid"], (True, False, NOT_APPLICABLE)) + self.assertEqual("not_assessed", skill["executable_grader_bindings_present"]) + self.assertEqual("not_assessed", skill["recent_run_evidence_present"]) + self.assertEqual("not_assessed", skill["release_gated_evidence_present"]) + + per_skill = {entry["skill"]: entry for entry in report["skills"]} + self.assertTrue(per_skill["covered"]["schema_valid"]) + self.assertEqual(NOT_APPLICABLE, per_skill["uncovered"]["schema_valid"]) + + def test_human_output_uses_same_state_names_and_not_assessed_wording(self) -> None: + old_root = eval_coverage.ROOT + eval_coverage.ROOT = Path(self.repo) + output = io.StringIO() + try: + with mock.patch.object(sys, "argv", ["eval-coverage.py"]), redirect_stdout(output): + exit_code = eval_coverage.main() + finally: + eval_coverage.ROOT = old_root + self.assertEqual(0, exit_code) + text = output.getvalue() + self.assertIn("manifest_present", text) + self.assertIn("schema_valid", text) + self.assertIn("executable_grader_bindings_present: not assessed", text) + self.assertIn("recent_run_evidence_present: not assessed", text) + self.assertIn("release_gated_evidence_present: not assessed", text) + + def test_present_invalid_and_missing_manifest_reporting(self) -> None: + invalid_manifest = Path(self.repo) / "uncovered" / "evals" + invalid_manifest.mkdir(parents=True, exist_ok=True) + (invalid_manifest / "evals.json").write_text( + json.dumps({"schema_version": 1, "skill_name": "uncovered", "evals": []}), + encoding="utf-8", + ) + old_root = eval_coverage.ROOT + eval_coverage.ROOT = Path(self.repo) + try: + covered = eval_coverage.check_eval_states(Path("covered")) + uncovered = eval_coverage.check_eval_states(Path("uncovered")) + missing = eval_coverage.check_eval_states(Path("missing")) + finally: + eval_coverage.ROOT = old_root + self.assertTrue(covered.states["manifest_present"]) + self.assertTrue(covered.states["schema_valid"]) + self.assertTrue(uncovered.states["manifest_present"]) + self.assertFalse(uncovered.states["schema_valid"]) + self.assertFalse(missing.states["manifest_present"]) + self.assertEqual(NOT_APPLICABLE, missing.states["schema_valid"]) + + if __name__ == "__main__": unittest.main() diff --git a/scripts/test-eval-validation.py b/scripts/test-eval-validation.py new file mode 100644 index 0000000..c77e9fe --- /dev/null +++ b/scripts/test-eval-validation.py @@ -0,0 +1,388 @@ +#!/usr/bin/env python3 +"""Focused tests for the repository-owned eval manifest v1 contract.""" + +from __future__ import annotations + +import json +import os +import subprocess +import tempfile +import unittest +from pathlib import Path + +from eval_validation import NOT_APPLICABLE, SCHEMA_VERSION, find_skill_manifests, validate_manifest + + +SCRIPT_DIR = Path(__file__).resolve().parent +ROOT = SCRIPT_DIR.parent + + +def git(repo: Path, *args: str) -> None: + subprocess.run(["git", *args], cwd=repo, check=True, capture_output=True) + + +def git_output(repo: Path, *args: str) -> str: + completed = subprocess.run( + ["git", *args], + cwd=repo, + check=True, + capture_output=True, + text=True, + ) + return completed.stdout + + +def eval_case(**updates: object) -> dict[str, object]: + case: dict[str, object] = { + "id": "case-1", + "prompt": "Do the task.", + "expected_output": "A verified result.", + "assertions": ["Includes verification."], + } + case.update(updates) + return case + + +class EvalValidationTest(unittest.TestCase): + def setUp(self) -> None: + self.tmp = tempfile.TemporaryDirectory() + self.root = Path(self.tmp.name) + git(self.root, "init", "-q") + git(self.root, "config", "user.email", "test@example.invalid") + git(self.root, "config", "user.name", "Test") + self.skill = self.root / "example" + (self.skill / "evals").mkdir(parents=True) + (self.skill / "SKILL.md").write_text("---\nname: example\ndescription: test\n---\n", encoding="utf-8") + self.manifest = self.skill / "evals" / "evals.json" + + def tearDown(self) -> None: + self.tmp.cleanup() + + def valid(self, **updates: object) -> dict[str, object]: + payload: dict[str, object] = { + "schema_version": SCHEMA_VERSION, + "skill_name": "example", + "evals": [eval_case()], + } + payload.update(updates) + return payload + + def write(self, data: object, *, track_all: bool = True) -> None: + self.manifest.write_text(json.dumps(data, indent=2), encoding="utf-8") + if track_all: + git(self.root, "add", "-A") + + def errors(self, data: object) -> list[str]: + self.write(data) + return validate_manifest(self.manifest, self.root).errors + + def test_valid_manifest_passes(self) -> None: + self.write(self.valid()) + result = validate_manifest(self.manifest, self.root) + self.assertTrue(result.states["manifest_present"]) + self.assertTrue(result.states["schema_valid"]) + self.assertEqual("not_assessed", result.states["executable_grader_bindings_present"]) + self.assertEqual("not_assessed", result.states["recent_run_evidence_present"]) + self.assertEqual("not_assessed", result.states["release_gated_evidence_present"]) + self.assertEqual([], result.errors) + + def test_missing_manifest_reports_schema_valid_not_applicable(self) -> None: + missing_manifest = self.skill / "evals" / "missing.json" + result = validate_manifest(missing_manifest, self.root) + self.assertFalse(result.states["manifest_present"]) + self.assertEqual(NOT_APPLICABLE, result.states["schema_valid"]) + self.assertEqual([], result.errors) + + def test_targeted_schema_version_errors(self) -> None: + missing_errors = self.errors({"skill_name": "example", "evals": [eval_case()]}) + self.assertTrue(any("missing required schema_version" in error for error in missing_errors)) + + string_errors = self.errors(self.valid(schema_version="1")) + self.assertTrue(any("must be integer 1, not a string" in error for error in string_errors)) + + unknown_errors = self.errors(self.valid(schema_version=2)) + self.assertTrue(any("unsupported schema_version" in error for error in unknown_errors)) + + def test_boolean_and_non_integer_schema_version_errors(self) -> None: + bool_errors = self.errors(self.valid(schema_version=True)) + self.assertTrue(any("schema_version must be integer 1" in error for error in bool_errors)) + + float_errors = self.errors(self.valid(schema_version=1.5)) + self.assertTrue(any("schema_version must be integer 1" in error for error in float_errors)) + + def test_expectations_alias_errors(self) -> None: + legacy_only = eval_case() + legacy_only.pop("assertions") + legacy_only["expectations"] = ["legacy"] + errors = self.errors(self.valid(evals=[legacy_only])) + self.assertTrue(any("expectations is not supported" in error for error in errors)) + + both = eval_case(expectations=["legacy"]) + errors = self.errors(self.valid(evals=[both])) + self.assertTrue(any("cannot both be present" in error for error in errors)) + + def test_missing_required_fields_fail(self) -> None: + for field in ("id", "prompt", "expected_output", "assertions"): + with self.subTest(field=field): + case = eval_case() + case.pop(field) + errors = self.errors(self.valid(evals=[case])) + self.assertTrue(any(field in error for error in errors)) + + def test_whitespace_prompt_and_expected_output_fail(self) -> None: + errors = self.errors(self.valid(evals=[eval_case(prompt=" ")])) + self.assertTrue(any("prompt" in error for error in errors)) + errors = self.errors(self.valid(evals=[eval_case(expected_output="\t")])) + self.assertTrue(any("expected_output" in error for error in errors)) + + def test_invalid_ids_fail(self) -> None: + invalid_values: list[object] = [123, "", "UPPER", "has spaces", "path/slash", "a" * 65] + for bad in invalid_values: + with self.subTest(bad=bad): + errors = self.errors(self.valid(evals=[eval_case(id=bad)])) + self.assertTrue(any("id" in error for error in errors)) + + def test_duplicate_ids_fail(self) -> None: + errors = self.errors(self.valid(evals=[eval_case(id="dup"), eval_case(id="dup")])) + self.assertTrue(any("duplicate case ID" in error for error in errors)) + + def test_duplicate_assertions_fail(self) -> None: + errors = self.errors(self.valid(evals=[eval_case(assertions=["A", "A"])])) + self.assertTrue(any("duplicate assertion" in error for error in errors)) + + def test_empty_evals_fail(self) -> None: + errors = self.errors(self.valid(evals=[])) + self.assertTrue(any("evals" in error and "non-empty" in error.lower() for error in errors)) + + def test_empty_and_whitespace_only_assertions_fail(self) -> None: + for assertion in ("", " "): + with self.subTest(assertion=repr(assertion)): + errors = self.errors(self.valid(evals=[eval_case(assertions=[assertion])])) + self.assertTrue(any("nonempty non-whitespace" in error for error in errors)) + + def test_unknown_top_level_and_case_fields_fail(self) -> None: + top_level_errors = self.errors(self.valid(unknown_field=True)) + self.assertTrue( + any("Additional properties" in error and "unknown_field" in error for error in top_level_errors) + ) + + case_errors = self.errors(self.valid(evals=[eval_case(unknown_field=True)])) + self.assertTrue( + any("Additional properties" in error and "unknown_field" in error for error in case_errors) + ) + + def test_evidence_property_is_rejected_as_unknown_v1_property(self) -> None: + errors = self.errors(self.valid(evidence={"grader_bindings": ["scripts/grader.py"]})) + self.assertTrue(any("Additional properties" in error and "evidence" in error for error in errors)) + + def test_duplicate_json_object_keys_fail_before_semantic_validation(self) -> None: + self.manifest.write_text( + """ +{ + "schema_version": 1, + "schema_version": 2, + "skill_name": "example", + "evals": [{ + "id": "case-1", + "prompt": "P", + "expected_output": "E", + "assertions": ["A"] + }] +} +""".strip() + + "\n", + encoding="utf-8", + ) + git(self.root, "add", "-A") + result = validate_manifest(self.manifest, self.root) + self.assertFalse(result.states["schema_valid"]) + self.assertEqual(1, len(result.errors)) + self.assertIn("duplicate JSON object key", result.errors[0]) + + def test_malformed_json_manifest_is_present_but_schema_invalid(self) -> None: + self.manifest.write_text("{\n", encoding="utf-8") + git(self.root, "add", "-A") + result = validate_manifest(self.manifest, self.root) + self.assertTrue(result.states["manifest_present"]) + self.assertFalse(result.states["schema_valid"]) + self.assertTrue(any("invalid JSON" in error for error in result.errors)) + + def test_skill_name_must_match_containing_directory(self) -> None: + errors = self.errors(self.valid(skill_name="wrong")) + self.assertTrue(any("containing skill directory" in error for error in errors)) + + def test_files_accepts_tracked_regular_file(self) -> None: + fixture = self.skill / "fixtures" / "input.json" + fixture.parent.mkdir(parents=True, exist_ok=True) + fixture.write_text("{}\n", encoding="utf-8") + self.write(self.valid(evals=[eval_case(files=["fixtures/input.json"])])) + result = validate_manifest(self.manifest, self.root) + self.assertTrue(result.states["schema_valid"]) + + def test_data_dot_dot_filename_is_allowed(self) -> None: + fixture = self.skill / "fixtures" / "data..json" + fixture.parent.mkdir(parents=True, exist_ok=True) + fixture.write_text("{}\n", encoding="utf-8") + self.write(self.valid(evals=[eval_case(files=["fixtures/data..json"])])) + result = validate_manifest(self.manifest, self.root) + self.assertTrue(result.states["schema_valid"]) + + def test_duplicate_file_paths_fail(self) -> None: + fixture = self.skill / "fixtures" / "input.json" + fixture.parent.mkdir(parents=True, exist_ok=True) + fixture.write_text("{}\n", encoding="utf-8") + errors = self.errors( + self.valid(evals=[eval_case(files=["fixtures/input.json", "fixtures/input.json"])]), + ) + self.assertTrue(any("non-unique elements" in error for error in errors)) + + def test_lexical_dot_empty_and_trailing_slash_paths_fail(self) -> None: + fixture = self.skill / "fixtures" / "input.json" + fixture.parent.mkdir(parents=True, exist_ok=True) + fixture.write_text("{}\n", encoding="utf-8") + + for bad in ( + "fixtures/./input.json", + "fixtures//input.json", + "./input.json", + "fixtures/input.json/", + ): + with self.subTest(path=bad): + errors = self.errors(self.valid(evals=[eval_case(files=[bad])])) + self.assertTrue(any("files[0]" in error for error in errors)) + + def test_symlink_component_escaping_skill_is_rejected(self) -> None: + external_dir = self.root / "external" + external_dir.mkdir(parents=True, exist_ok=True) + (external_dir / "outside.json").write_text("{}\n", encoding="utf-8") + + fixtures_dir = self.skill / "fixtures" + fixtures_dir.mkdir(parents=True, exist_ok=True) + os.symlink(external_dir, fixtures_dir / "external") + git(self.root, "add", "-A") + + errors = self.errors(self.valid(evals=[eval_case(files=["fixtures/external/outside.json"])])) + self.assertTrue(any("symlink" in error for error in errors)) + + def test_tracked_but_missing_file_fails_when_index_still_lists_it(self) -> None: + fixture = self.skill / "fixtures" / "input.json" + fixture.parent.mkdir(parents=True, exist_ok=True) + fixture.write_text("{}\n", encoding="utf-8") + self.write(self.valid(evals=[eval_case(files=["fixtures/input.json"])])) + + fixture.unlink() + tracked_entry = git_output( + self.root, + "ls-files", + "-s", + "--", + str(fixture.relative_to(self.root)), + ) + self.assertNotEqual("", tracked_entry) + + errors = validate_manifest(self.manifest, self.root).errors + self.assertTrue(any("does not exist in working tree" in error for error in errors)) + + def test_nested_bundle_skill_name_matches_leaf_directory(self) -> None: + nested_skill = self.root / "bundles" / "demo-bundle" / "skills" / "child-skill" + (nested_skill / "evals").mkdir(parents=True) + (nested_skill / "SKILL.md").write_text( + "---\nname: child-skill\ndescription: test\n---\n", + encoding="utf-8", + ) + nested_manifest = nested_skill / "evals" / "evals.json" + nested_manifest.write_text( + json.dumps( + { + "schema_version": SCHEMA_VERSION, + "skill_name": "child-skill", + "evals": [eval_case()], + }, + indent=2, + ), + encoding="utf-8", + ) + git(self.root, "add", "-A") + + result = validate_manifest(nested_manifest, self.root) + self.assertTrue(result.states["schema_valid"]) + + def test_file_boundary_failures(self) -> None: + tracked = self.skill / "Fixtures" / "Input.json" + tracked.parent.mkdir(parents=True, exist_ok=True) + tracked.write_text("{}\n", encoding="utf-8") + outside = self.root / "outside.json" + outside.write_text("{}\n", encoding="utf-8") + directory = self.skill / "fixtures" + directory.mkdir(parents=True, exist_ok=True) + symlink_target = directory / "target.json" + symlink_target.write_text("{}\n", encoding="utf-8") + os.symlink(symlink_target, directory / "linked.json") + git(self.root, "add", "-A") + + invalid_paths = [ + str(tracked.resolve()), + "../outside.json", + "fixtures/missing.json", + "fixtures", + "fixtures\\bad.json", + "fixtures/linked.json", + "fixtures/\u0001bad.json", + "fixtures/./input.json", + "Fixtures/input.json", + ] + for bad in invalid_paths: + with self.subTest(path=bad): + errors = self.errors(self.valid(evals=[eval_case(files=[bad])])) + self.assertTrue(any("files[0]" in error for error in errors)) + + def test_missing_and_untracked_fixture_paths_report_distinct_reasons(self) -> None: + untracked = self.skill / "fixtures" / "untracked.json" + untracked.parent.mkdir(parents=True, exist_ok=True) + untracked.write_text("{}\n", encoding="utf-8") + + self.write(self.valid(evals=[eval_case(files=["fixtures/missing.json"])]), track_all=False) + git(self.root, "add", str(self.manifest.relative_to(self.root))) + missing_errors = validate_manifest(self.manifest, self.root).errors + self.assertTrue(any("does not exist" in error for error in missing_errors)) + self.assertFalse(any("not tracked by Git" in error for error in missing_errors)) + + self.write(self.valid(evals=[eval_case(files=["fixtures/untracked.json"])]), track_all=False) + git(self.root, "add", str(self.manifest.relative_to(self.root))) + tracked_untracked = git_output( + self.root, + "ls-files", + "--", + str(untracked.relative_to(self.root)), + ) + self.assertEqual("", tracked_untracked) + untracked_errors = validate_manifest(self.manifest, self.root).errors + self.assertTrue(any("not tracked by Git" in error for error in untracked_errors)) + self.assertFalse(any("does not exist" in error for error in untracked_errors)) + + +class RepositoryManifestTest(unittest.TestCase): + def test_known_migrated_set_is_present_and_all_discovered_manifests_validate(self) -> None: + known = { + "de-spin/evals/evals.json", + "esp32-development/evals/evals.json", + "raleigh/evals/evals.json", + "restic/evals/evals.json", + "supabase/evals/evals.json", + "vercel-eve/evals/evals.json", + "verification-methodology/evals/evals.json", + } + manifests = find_skill_manifests(ROOT) + discovered = {str(path.relative_to(ROOT)) for path in manifests} + self.assertTrue(known.issubset(discovered)) + + invalid = [] + for manifest in manifests: + result = validate_manifest(manifest, ROOT) + if result.states["schema_valid"] is not True: + invalid.append((manifest, result.errors)) + self.assertEqual([], invalid) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/validate-evals.py b/scripts/validate-evals.py new file mode 100644 index 0000000..f21053d --- /dev/null +++ b/scripts/validate-evals.py @@ -0,0 +1,27 @@ +#!/usr/bin/env python3 +"""Validate every eval manifest against the repository-owned v1 contract.""" + +import sys +from pathlib import Path + +from eval_validation import find_skill_manifests, validate_manifest + +ROOT = Path(__file__).resolve().parent.parent + + +def main() -> int: + manifests = find_skill_manifests(ROOT) + errors = [] + for manifest in manifests: + errors.extend(validate_manifest(manifest, ROOT).errors) + if errors: + print("\n".join(errors), file=sys.stderr) + return 1 + print( + f"Validated {len(manifests)} eval manifests against repository schema v1 and semantic repository checks." + ) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/validate-skills.rb b/scripts/validate-skills.rb index aab38f9..2a2c3e7 100755 --- a/scripts/validate-skills.rb +++ b/scripts/validate-skills.rb @@ -4,6 +4,7 @@ require "yaml" require "set" require "json" +require "open3" ROOT = File.expand_path("..", __dir__) ALLOWED_FIELDS = %w[name description license compatibility metadata allowed-tools].freeze @@ -12,6 +13,8 @@ GRANDFATHER_FILE = File.join(ROOT, "scripts", "grandfathered-skills.txt") MIN_EVAL_CASES = 5 errors = [] +eval_stdout, eval_stderr, eval_status = Open3.capture3("python3", File.join(ROOT, "scripts", "validate-evals.py"), chdir: ROOT) +errors.concat(eval_stderr.lines(chomp: true)) unless eval_status.success? skills = Dir.glob("#{ROOT}/**/SKILL.md").sort.reject do |skill| skill.include?("/agent-council/profiles/skills/") end diff --git a/supabase/evals/evals.json b/supabase/evals/evals.json index 031a0ca..a77f979 100644 --- a/supabase/evals/evals.json +++ b/supabase/evals/evals.json @@ -1,4 +1,5 @@ { + "schema_version": 1, "skill_name": "supabase", "evals": [ { diff --git a/vercel-eve/evals/evals.json b/vercel-eve/evals/evals.json index d878c3f..fa8b4f6 100644 --- a/vercel-eve/evals/evals.json +++ b/vercel-eve/evals/evals.json @@ -1,4 +1,5 @@ { + "schema_version": 1, "skill_name": "vercel-eve", "evals": [ { diff --git a/verification-methodology/evals/evals.json b/verification-methodology/evals/evals.json index 40775c5..a486225 100644 --- a/verification-methodology/evals/evals.json +++ b/verification-methodology/evals/evals.json @@ -1,4 +1,5 @@ { + "schema_version": 1, "skill_name": "verification-methodology", "evals": [ {