From 0c1141e01144d22b11e7d0ae8c160a60f05e396d Mon Sep 17 00:00:00 2001 From: Tester Date: Fri, 3 Jul 2026 15:06:02 +0200 Subject: [PATCH] fix(validator): validate untracked-but-not-ignored tool directories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit collect_tool_dirs (added in #692) filters the tools/ scan through git so gitignored artifact directories are not treated as tools, but the tracked-only `git ls-files` filter silently drops a freshly-authored tool directory that has not been `git add`ed yet — exactly when a new tool most needs validating. Switch to `git ls-files --cached --others --exclude-standard` so untracked-but-not-ignored dirs are still checked; gitignored artifact directories remain excluded. Add a regression test. --- .../src/skill_and_tool_validator/__init__.py | 30 +++++++++++++++---- .../tests/test_validator.py | 25 ++++++++++++++++ 2 files changed, 49 insertions(+), 6 deletions(-) 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 a69caabf..372462fb 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 @@ -1550,17 +1550,35 @@ def collect_tool_dirs(root: Path | None = None) -> list[Path]: return [] dirs = sorted(d for d in base.iterdir() if d.is_dir() and not d.name.startswith(".")) - tracked_names = _git_tracked_tool_names(repo_root) - if tracked_names is None: + visible_names = _git_visible_tool_names(repo_root) + if visible_names is None: return dirs - return [d for d in dirs if d.name in tracked_names] + return [d for d in dirs if d.name in visible_names] -def _git_tracked_tool_names(root: Path) -> set[str] | None: - """Return top-level ``tools/`` entries tracked by git, if available.""" +def _git_visible_tool_names(root: Path) -> set[str] | None: + """Return top-level ``tools/`` entries git considers non-ignored + (tracked *or* untracked-but-not-``.gitignore``d), if git is available. + + Using ``--cached --others --exclude-standard`` — rather than tracked + files alone — means a freshly-authored tool directory that has not been + ``git add``ed yet is still validated, while gitignored artifact + directories (e.g. ``__pycache__``, build output) are excluded. + """ try: result = subprocess.run( - ["git", "-C", str(root), "ls-files", "-z", "--", str(TOOLS_DIR)], + [ + "git", + "-C", + str(root), + "ls-files", + "-z", + "--cached", + "--others", + "--exclude-standard", + "--", + str(TOOLS_DIR), + ], check=False, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, diff --git a/tools/skill-and-tool-validator/tests/test_validator.py b/tools/skill-and-tool-validator/tests/test_validator.py index 7d5130f1..b75d6680 100644 --- a/tools/skill-and-tool-validator/tests/test_validator.py +++ b/tools/skill-and-tool-validator/tests/test_validator.py @@ -2611,6 +2611,31 @@ def test_ignored_tool_artifact_directory_skipped(self, tmp_path: Path) -> None: violations = list(validate_tools(root)) assert violations == [] + def test_untracked_tool_directory_still_checked(self, tmp_path: Path) -> None: + # A freshly-authored tool directory that has not been ``git add``ed yet + # is not gitignored, so it must still be validated. Regression guard: an + # earlier tracked-only filter silently dropped such directories. + root = _make_tools_root(tmp_path) + subprocess.run(["git", "init"], cwd=root, check=True, capture_output=True) + (root / ".gitignore").write_text("tools/ignored-artifact/\n", encoding="utf-8") + + untracked_tool = root / "tools" / "brand-new" + untracked_tool.mkdir() + (untracked_tool / "README.md").write_text( + "# tools/brand-new\n\n**Capability:** substrate:framework-dev\n\nNew tool.\n\n" + _VALID_PREREQS, + encoding="utf-8", + ) + + ignored_tool = root / "tools" / "ignored-artifact" + ignored_tool.mkdir() + (ignored_tool / "junk.txt").write_text("", encoding="utf-8") + + # Only .gitignore is staged; the new tool is deliberately left untracked. + subprocess.run(["git", "add", ".gitignore"], cwd=root, check=True, capture_output=True) + + assert [d.name for d in collect_tool_dirs(root)] == ["brand-new"] + assert list(validate_tools(root)) == [] + def test_tool_missing_readme(self, tmp_path: Path) -> None: root = _make_tools_root(tmp_path) (root / "tools" / "no-readme").mkdir()