diff --git a/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py b/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py index 4376989c..be92f262 100644 --- a/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py +++ b/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py @@ -2469,8 +2469,8 @@ def validate_lowercase_f_field(path: Path, text: str) -> Iterable[Violation]: "apache.org/licenses/LICENSE-2.0", ) -# Files smaller than this threshold (bytes / characters) are treated as -# empty placeholder stubs and exempted from the license-header check. +# Files smaller than this threshold (bytes) are treated as empty +# placeholder stubs and exempted from the license-header check. _MIN_LICENSE_FILE_SIZE = 50 # Path components that mark generated or vendored subtrees that must not @@ -2485,7 +2485,7 @@ def collect_tool_python_files(root: Path | None = None) -> list[Path]: Excludes generated / vendored subtrees (``.venv``, ``site-packages``, ``node_modules``, ``__pycache__``) and empty placeholder files whose - content is shorter than ``_MIN_LICENSE_FILE_SIZE`` characters. + content is shorter than ``_MIN_LICENSE_FILE_SIZE`` bytes. """ base = (root or find_repo_root()) / TOOLS_DIR if not base.exists(): @@ -3297,8 +3297,18 @@ def validate_eval_coverage(root: Path | None = None) -> Iterable[Violation]: return eval_slugs: set[str] = set() if evals_base.exists(): - eval_slugs = {p.name for p in evals_base.iterdir() if p.is_dir()} - for skill_dir in sorted(skills_base.iterdir()): + try: + eval_slugs = {p.name for p in evals_base.iterdir() if p.is_dir()} + except OSError: + # Restrictive runners can deny listing; skip rather than crash + # (same posture as collect_tool_python_files on OSError). + return + try: + skill_dirs = sorted(skills_base.iterdir()) + except OSError: + # Same posture as collect_tool_python_files: unreadable → skip. + return + for skill_dir in skill_dirs: if not skill_dir.is_dir(): continue # A trusted-external-skill-source pointer dir carries its eval suite diff --git a/tools/skill-and-tool-validator/tests/test_validator.py b/tools/skill-and-tool-validator/tests/test_validator.py index 5e33d4c4..90456d08 100644 --- a/tools/skill-and-tool-validator/tests/test_validator.py +++ b/tools/skill-and-tool-validator/tests/test_validator.py @@ -20,6 +20,7 @@ from __future__ import annotations import subprocess +from collections.abc import Iterator from pathlib import Path import pytest @@ -1595,6 +1596,21 @@ def test_lowercase_f_field_in_soft_categories(self) -> None: _SPDX_PY_HEADER = "# SPDX-License-Identifier: Apache-2.0\n" +def _live_repo_root_for_header_check() -> Path | None: + """Return the magpie repo root when the live license-header check can run. + + ``find_repo_root`` never raises — out-of-tree it returns CWD — so the + live integration must additionally confirm this checkout actually + contains the validator package under ``tools/``. Returns ``None`` when + that marker is absent so callers can ``skipif`` instead of aborting. + """ + root = find_repo_root() + marker = root / "tools" / "skill-and-tool-validator" / "src" / "skill_and_tool_validator" + if marker.is_dir(): + return root + return None + + class TestValidateLicenseHeader: # ------------------------------------------------------------------ # # Python (.py) checks # @@ -1683,11 +1699,23 @@ def test_collect_tool_python_files_returns_empty_when_no_tools_dir(self, tmp_pat # Integration: real repo passes # # ------------------------------------------------------------------ # + def test_live_repo_header_check_skips_out_of_tree( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Out-of-tree cwd must make the live header-check skipif fire (not error).""" + monkeypatch.chdir(tmp_path) + assert _live_repo_root_for_header_check() is None + + @pytest.mark.skipif( + _live_repo_root_for_header_check() is None, + reason="not running inside a magpie repo checkout with tools/", + ) def test_real_repo_tool_python_files_all_have_headers(self) -> None: """Every non-trivial tool Python file in the real repo carries a license header.""" from skill_and_tool_validator import _LICENSE_PY_MARKERS - repo_root = find_repo_root() + repo_root = _live_repo_root_for_header_check() + assert repo_root is not None # skipif above guarantees this missing = [ p for p in collect_tool_python_files(repo_root) @@ -3635,6 +3663,42 @@ def test_non_directory_entries_in_skills_are_skipped(self, tmp_path: Path) -> No violations = list(validate_eval_coverage(tmp_path)) assert violations == [] + def test_unreadable_skills_dir_returns_no_violations( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """PermissionError on skills/.iterdir() must not abort the run.""" + skills_dir = tmp_path / "skills" + skills_dir.mkdir(parents=True) + self._make_skill(tmp_path, "alpha") + + real_iterdir = Path.iterdir + + def _boom(self: Path) -> Iterator[Path]: + if self.resolve() == skills_dir.resolve(): + raise PermissionError("denied") + return real_iterdir(self) + + monkeypatch.setattr(Path, "iterdir", _boom) + assert list(validate_eval_coverage(tmp_path)) == [] + + def test_unreadable_evals_dir_returns_no_violations( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """PermissionError on evals/.iterdir() must not abort or false-flag skills.""" + self._make_skill(tmp_path, "alpha") + evals_dir = tmp_path / "tools" / "skill-evals" / "evals" + evals_dir.mkdir(parents=True) + + real_iterdir = Path.iterdir + + def _boom(self: Path) -> Iterator[Path]: + if self.resolve() == evals_dir.resolve(): + raise PermissionError("denied") + return real_iterdir(self) + + monkeypatch.setattr(Path, "iterdir", _boom) + assert list(validate_eval_coverage(tmp_path)) == [] + # --------------------------------------------------------------------------- # docs/modes.md consistency check (check #11 — SOFT)