From c69c57116501239c92b3e36f5040271d2c01a45b Mon Sep 17 00:00:00 2001 From: mimran-khan Date: Thu, 27 Aug 2026 01:43:53 +0530 Subject: [PATCH] fix: treat tools/ as an executable directory Schema already allows tools/ as the agentskills.io name for helpers, but quality scoring, script lint, and create-eval-dataset only looked at scripts/. A spec-compliant skill with tools/run.py was typed as guide-only, skipped lint, and generated Scripts: none. Fixes #89 Signed-off-by: mimran-khan --- CHANGELOG.md | 2 ++ src/skillevaluator/constants.py | 5 +++ src/skillevaluator/tier3/generate_dataset.py | 15 +++++--- .../validators/quality_score.py | 36 ++++++++++++------- src/skillevaluator/validators/script_lint.py | 22 ++++++------ tests/tier3/test_generate_dataset_results.py | 14 ++++++++ tests/validators/test_quality_score.py | 8 +++++ tests/validators/test_script_lint.py | 12 +++++++ 8 files changed, 87 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a2ad6515..07022def 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ All notable changes to SkillEvaluator are documented in this file. ### Fixed +- Quality scoring, script lint, and `create-eval-dataset` now treat `tools/` + the same as `scripts/` for executable helpers. - Tier 3 paired pass@k evidence now respects Python's active integer-string conversion limit, preserves nonzero Wilson interval widths and paired-effect directions at large case counts, and documents exact-rational omission diff --git a/src/skillevaluator/constants.py b/src/skillevaluator/constants.py index 5926ba41..95eeb5c1 100644 --- a/src/skillevaluator/constants.py +++ b/src/skillevaluator/constants.py @@ -46,6 +46,10 @@ {"agents", "references", "scripts", "assets", "evals", "tests", "tools", "config"} ) +# Directories that hold skill executables. ``scripts/`` is the historical +# SkillEvaluator name; ``tools/`` is the agentskills.io name. +EXECUTABLE_SKILL_DIRS = ("scripts", "tools") + # Env var that lets consumers EXTEND the allowed skill-root directory set per # repo (comma- or whitespace-separated) without editing bundled config — e.g. # ``SKILLEVALUATOR_SCHEMA_ALLOWED_DIRS="data,fixtures"``. Names are added to, @@ -347,6 +351,7 @@ QUALITY_EXCLUDED_DIRS = frozenset( { "scripts", + "tools", "references", "assets", "eval", diff --git a/src/skillevaluator/tier3/generate_dataset.py b/src/skillevaluator/tier3/generate_dataset.py index ca21d2dd..24d0206a 100644 --- a/src/skillevaluator/tier3/generate_dataset.py +++ b/src/skillevaluator/tier3/generate_dataset.py @@ -53,6 +53,7 @@ import yaml +from skillevaluator.constants import EXECUTABLE_SKILL_DIRS from skillevaluator.evaluation.results import DatasetGenerationError, DatasetGenerationResult from skillevaluator.validators.frontmatter_parser import FRONTMATTER_PATTERN @@ -122,10 +123,16 @@ def _parse_skill(skill_path: Path, prompt_file: str | None = None) -> dict[str, if frontmatter.get("description"): description = str(frontmatter["description"]).strip() - # Find scripts - scripts_dir = skill_path / "scripts" - if scripts_dir.is_dir(): - scripts = [f.name for f in scripts_dir.glob("*.py")] + # Find scripts in scripts/ (historical) and tools/ (agentskills.io) + seen_scripts: set[str] = set() + for dirname in EXECUTABLE_SKILL_DIRS: + directory = skill_path / dirname + if not directory.is_dir(): + continue + for script_file in directory.glob("*.py"): + if script_file.name not in seen_scripts: + seen_scripts.add(script_file.name) + scripts.append(script_file.name) # Detect interactive scripts from SKILL.md content interactive_scripts: set[str] = set() diff --git a/src/skillevaluator/validators/quality_score.py b/src/skillevaluator/validators/quality_score.py index c43105f7..d452d42d 100644 --- a/src/skillevaluator/validators/quality_score.py +++ b/src/skillevaluator/validators/quality_score.py @@ -27,6 +27,7 @@ import yaml from skillevaluator.constants import ( + EXECUTABLE_SKILL_DIRS, QUALITY_EXCLUDED_DIRS, QUALITY_RECOMMENDED_MAX_TOKENS, QUALITY_RESERVED_NAMES, @@ -141,14 +142,27 @@ def description(self) -> str: # Skill type detection # ----------------------------------------------------------------- + @staticmethod + def _executable_files(skill_path: Path, patterns: tuple[str, ...] = ("*.py", "*.sh")) -> list[Path]: + files: list[Path] = [] + for dirname in EXECUTABLE_SKILL_DIRS: + directory = skill_path / dirname + if directory.is_dir(): + for pattern in patterns: + files.extend(sorted(directory.glob(pattern))) + return files + + @staticmethod + def _has_executable_directory(skill_path: Path) -> bool: + return any((skill_path / dirname).is_dir() for dirname in EXECUTABLE_SKILL_DIRS) + @staticmethod def detect_skill_type(skill_path: Path) -> str: """Auto-detect skill type from directory structure. Returns one of: script-based, lib-based, resource-based, guide-only, hybrid. """ - scripts_dir = skill_path / "scripts" - has_scripts = scripts_dir.is_dir() and bool(list(scripts_dir.glob("*.py")) + list(scripts_dir.glob("*.sh"))) + has_scripts = bool(QualityScoreValidator._executable_files(skill_path)) has_lib = False for d in skill_path.iterdir(): @@ -444,19 +458,18 @@ def _check_type_specific( skill_type = qs.skill_type if skill_type in ("script-based", "hybrid"): - scripts_dir = skill_path / "scripts" - if scripts_dir.exists(): + script_files = self._executable_files(skill_path) + if self._has_executable_directory(skill_path): qs.has_scripts = True - py_sh = list(scripts_dir.glob("*.py")) + list(scripts_dir.glob("*.sh")) - qs.script_count = len(py_sh) + qs.script_count = len(script_files) if qs.script_count == 0: - dim.deduct(10, "warning", "scripts/ directory exists but contains no .py or .sh files") + dim.deduct(10, "warning", "scripts/ or tools/ exists but contains no .py or .sh files") else: dim.deduct( 25, "error", - "No scripts/ directory found (detected as script-based skill)", - "Create scripts/ directory with at least one executable script", + "No scripts/ or tools/ directory found (detected as script-based skill)", + "Create scripts/ or tools/ with at least one executable script", ) if "## Available Scripts" not in content and "| Script |" not in content: @@ -672,11 +685,8 @@ def _check_reliability( ) def _check_script_reliability(self, dim, skill_path: Path) -> None: - scripts_dir = skill_path / "scripts" - if not scripts_dir.exists(): - return no_error_handling = [] - for script in scripts_dir.glob("*.py"): + for script in self._executable_files(skill_path, patterns=("*.py",)): try: sc = script.read_text(encoding="utf-8") except Exception: diff --git a/src/skillevaluator/validators/script_lint.py b/src/skillevaluator/validators/script_lint.py index c4eb04e3..faa844d6 100644 --- a/src/skillevaluator/validators/script_lint.py +++ b/src/skillevaluator/validators/script_lint.py @@ -7,7 +7,7 @@ for code style and maintainability — they produce warnings but never fail validation. -Checks performed per .py file in scripts/: +Checks performed per .py file in scripts/ or tools/: - Flat script (no function definitions) - Deep nesting (> 6 levels of control flow) - Magic numbers (raw numeric constants) @@ -20,7 +20,7 @@ import ast from pathlib import Path -from skillevaluator.constants import SCRIPT_LINT_MAX_NESTING, SCRIPT_LINT_SAFE_CONSTANTS +from skillevaluator.constants import EXECUTABLE_SKILL_DIRS, SCRIPT_LINT_MAX_NESTING, SCRIPT_LINT_SAFE_CONSTANTS from skillevaluator.logging_config import get_logger from skillevaluator.models.result import Finding, Severity, ValidationResult from skillevaluator.validators.base import ValidatorBase @@ -123,19 +123,21 @@ def _lint_folder(self, root: Path) -> ValidationResult: return result def _lint_skill(self, skill_path: Path) -> ValidationResult: - """Lint all Python scripts in a skill's scripts/ directory.""" + """Lint all Python scripts in a skill's scripts/ or tools/ directory.""" result = ValidationResult( validator_name="SCRIPT_LINT", validator_description=self.description, ) - scripts_dir = skill_path / "scripts" - if not scripts_dir.is_dir(): - result.add_success(check_name="lint", message="No scripts/ directory found") - return result - - py_files = sorted(scripts_dir.glob("*.py")) + py_files: list[Path] = [] + for dirname in EXECUTABLE_SKILL_DIRS: + directory = skill_path / dirname + if directory.is_dir(): + py_files.extend(sorted(directory.glob("*.py"))) if not py_files: - result.add_success(check_name="lint", message="No Python scripts found in scripts/") + if not any((skill_path / dirname).is_dir() for dirname in EXECUTABLE_SKILL_DIRS): + result.add_success(check_name="lint", message="No scripts/ or tools/ directory found") + else: + result.add_success(check_name="lint", message="No Python scripts found in scripts/ or tools/") return result for script in py_files: diff --git a/tests/tier3/test_generate_dataset_results.py b/tests/tier3/test_generate_dataset_results.py index 45d1175d..940c00cf 100644 --- a/tests/tier3/test_generate_dataset_results.py +++ b/tests/tier3/test_generate_dataset_results.py @@ -298,3 +298,17 @@ def test_parse_skill_falls_back_to_defaults_on_malformed_frontmatter(tmp_path): parsed = _parse(tmp_path, "name: [unclosed\ndescription: broken") assert parsed["name"] == "my-skill" assert parsed["description"] == "" + + +def test_parse_skill_includes_tools_dir_scripts(tmp_path): + skill = tmp_path / "tools-skill" + skill.mkdir() + (skill / "SKILL.md").write_text( + "---\nname: tools-skill\ndescription: Spec-compliant executables live in tools/.\n---\n# x\n", + encoding="utf-8", + ) + tools = skill / "tools" + tools.mkdir() + (tools / "run.py").write_text("print('hello')\n", encoding="utf-8") + parsed = generate_dataset._parse_skill(skill) + assert parsed["scripts"] == ["run.py"] diff --git a/tests/validators/test_quality_score.py b/tests/validators/test_quality_score.py index 89f62151..e67de36e 100644 --- a/tests/validators/test_quality_score.py +++ b/tests/validators/test_quality_score.py @@ -199,6 +199,14 @@ def test_script_based(self, tmp_path): (sd / "run.py").write_text("print('hi')") assert QualityScoreValidator.detect_skill_type(d) == "script-based" + def test_tools_dir_is_script_based(self, tmp_path): + d = tmp_path / "tooled" + d.mkdir() + tools = d / "tools" + tools.mkdir() + (tools / "run.py").write_text("print('hi')") + assert QualityScoreValidator.detect_skill_type(d) == "script-based" + def test_lib_based(self, tmp_path): d = tmp_path / "lib" d.mkdir() diff --git a/tests/validators/test_script_lint.py b/tests/validators/test_script_lint.py index 7a8216eb..9daa5869 100644 --- a/tests/validators/test_script_lint.py +++ b/tests/validators/test_script_lint.py @@ -39,6 +39,18 @@ def test_flat_script_finding(self, skill_with_scripts): checks = [f.check_name for f in result.findings] assert "flat_script" in checks + def test_flat_script_finding_in_tools_dir(self, tmp_path): + skill_dir = tmp_path / "tools-skill" + skill_dir.mkdir() + (skill_dir / "SKILL.md").write_text("---\nname: tools-skill\ndescription: test\n---\n\n# Test\n") + tools = skill_dir / "tools" + tools.mkdir() + (tools / "flat.py").write_text("#!/usr/bin/env python3\nimport sys\nprint(sys.argv)\n") + v = ScriptLintValidator() + result = v.validate(skill_dir) + checks = [f.check_name for f in result.findings] + assert "flat_script" in checks + def test_deep_nesting_finding(self, skill_with_scripts): scripts = skill_with_scripts / "scripts" nested_code = (