diff --git a/.github/workflows/check-plugin-content.yml b/.github/workflows/check-plugin-content.yml index 327429e..5ae869e 100644 --- a/.github/workflows/check-plugin-content.yml +++ b/.github/workflows/check-plugin-content.yml @@ -10,7 +10,12 @@ jobs: check: runs-on: ubuntu-latest steps: + # fetch-depth: 0 is required by check_version_bump_on_content_change, + # which diffs against origin/. A shallow clone (depth=1) does + # not create the remote tracking branch for origin/main on PR events. - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + fetch-depth: 0 - name: Set up Python uses: actions/setup-python@0b93645e9fea7318ecaed2b359559ac225c90a2b # v5.3.0 @@ -20,5 +25,11 @@ jobs: - name: Install dependencies run: pip install pyyaml==6.0.2 + - name: Run repo-validation unit tests + run: python3 scripts/test_check_plugin_repo.py + + - name: Run repo-validation checks + run: python3 scripts/check_plugin_repo.py + - name: Verify generated plugin content is in sync with sources/ run: python3 scripts/generate-plugin-content.py --check diff --git a/.github/workflows/installer-smoke-test.yml b/.github/workflows/installer-smoke-test.yml index 03c903f..81121c1 100644 --- a/.github/workflows/installer-smoke-test.yml +++ b/.github/workflows/installer-smoke-test.yml @@ -25,7 +25,7 @@ jobs: completion_file: .zsh/completions/_archastro steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - name: Generate fixture assets run: | @@ -84,7 +84,7 @@ jobs: runs-on: windows-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - name: Generate fixture assets shell: pwsh diff --git a/scripts/check_plugin_repo.py b/scripts/check_plugin_repo.py new file mode 100644 index 0000000..e850ef3 --- /dev/null +++ b/scripts/check_plugin_repo.py @@ -0,0 +1,612 @@ +#!/usr/bin/env python3 +""" +Repository-validation checks for the archastro-cli plugin repo. + +This module hosts all checks that validate the *state* of the repository +as opposed to the *content rendering* in `generate-plugin-content.py`. +The split is structural: + + generate-plugin-content.py → reads sources/, writes skill/command files + check_plugin_repo.py → reads repo state, reports correctness errors + +Keeping repo validation in its own module means: + +- New validation checks land here without bloating the generator file +- CI can invoke each script independently (failure modes don't interleave) +- Tests for each concern live in their own test module +- The generator stays narrowly focused on its rendering contract + +USAGE + + python3 scripts/check_plugin_repo.py + + Runs every check and exits non-zero if any reports errors. + +ERROR CONTRACT + + Each check function takes optional path / repo-root keyword arguments + (for testability) and returns `list[str]` of human-readable error + messages — empty list means the check passed. Checks do not raise; + the runner composes results across all checks so a single report can + surface multiple failures at once. + +CHECKS + Implemented: check_manifest_consistency, check_compat_key_refs, + check_slash_command_refs, check_hardcoded_versions, + check_version_bump_on_content_change +""" +from __future__ import annotations + +import json +import os +import re +import subprocess +import sys +from pathlib import Path +from typing import Callable, Iterable, Iterator + + +REPO_ROOT = Path(__file__).resolve().parent.parent + +# Plugin manifest files. These are hand-edited (not generated) but their +# `version` and `name` fields must agree across all three — the Claude +# Code plugin cache keys off the marketplace.json plugin version. +CLAUDE_MARKETPLACE_PATH = REPO_ROOT / ".claude-plugin" / "marketplace.json" +CLAUDE_PLUGIN_MANIFEST_PATH = ( + REPO_ROOT / ".claude-plugins" / "archastro" / ".claude-plugin" / "plugin.json" +) +CODEX_PLUGIN_MANIFEST_PATH = ( + REPO_ROOT / "plugins" / "archastro" / ".codex-plugin" / "plugin.json" +) + +PLUGIN_COMPATIBILITY_PATH = REPO_ROOT / "plugin-compatibility.json" + +# Content directories scanned by compat/slash-command/hardcoded-version checks. +CONTENT_ROOTS: list[Path] = [ + REPO_ROOT / "sources", + REPO_ROOT / ".claude-plugins" / "archastro" / "skills", + REPO_ROOT / ".claude-plugins" / "archastro" / "commands", + REPO_ROOT / "plugins" / "archastro" / "skills", +] + + +def _rel(p: Path) -> str: + """Return `p` relative to REPO_ROOT, or absolute (tests use tmpdirs).""" + try: + return str(p.relative_to(REPO_ROOT)) + except ValueError: + return str(p) + + +def _iter_content_files(roots: Iterable[Path] | None = None) -> Iterator[Path]: + """ + Yield markdown files under the plugin content directories, sorted for + deterministic output. Missing roots are skipped. + """ + if roots is None: + roots = CONTENT_ROOTS + for root in roots: + if not root.exists(): + continue + yield from sorted(root.rglob("*.md")) + + +# Repo-relative path prefixes that count as "plugin content" for the +# version-bump-on-change check. A change under any of these paths requires +# a corresponding manifest version bump so the Claude Code and Codex plugin +# caches refresh. Manifest files themselves are NOT content (they live under +# `.claude-plugin/` and `...path.../.claude-plugin/`, which don't match any +# of these prefixes). +_CONTENT_PATH_PREFIXES: tuple[str, ...] = ( + "sources/", + ".claude-plugins/archastro/skills/", + ".claude-plugins/archastro/commands/", + "plugins/archastro/skills/", +) + + +def _is_content_path(relpath: str) -> bool: + """True if `relpath` (repo-relative, forward-slash) is a content file.""" + return any(relpath.startswith(p) for p in _CONTENT_PATH_PREFIXES) + + +def _load_json_dict(path: Path) -> tuple[dict | None, str | None]: + """ + Load a JSON file and verify the top level is an object. Returns + `(data, None)` on success or `(None, error_message)` on failure. + Exactly one of the two elements is None. + """ + try: + data = json.loads(path.read_text()) + except OSError as e: + return None, f"{_rel(path)}: cannot read ({e})" + except json.JSONDecodeError as e: + return None, f"{_rel(path)}: invalid JSON ({e})" + if not isinstance(data, dict): + return None, f"{_rel(path)}: top-level JSON is not an object" + return data, None + + +def check_manifest_consistency( + marketplace_path: Path = CLAUDE_MARKETPLACE_PATH, + claude_plugin_path: Path = CLAUDE_PLUGIN_MANIFEST_PATH, + codex_plugin_path: Path = CODEX_PLUGIN_MANIFEST_PATH, +) -> list[str]: + """ + Verify the three plugin manifest files agree on `version` and `name`. + The Claude Code plugin cache keys off the marketplace version; drift + between the three silently ships broken releases. + + Returns a list of error messages (empty on success). + """ + errors: list[str] = [] + + # Load all three files defensively. A parse/read error on any single + # file is collected, but we stop after the loop since we can't compare + # against files we couldn't read. + manifests: dict[Path, dict] = {} + for path in (marketplace_path, claude_plugin_path, codex_plugin_path): + data, err = _load_json_dict(path) + if err is not None: + errors.append(err) + continue + assert data is not None # contract of _load_json_dict + manifests[path] = data + if errors: + return errors + + # marketplace.json is expected to contain exactly one plugin entry. + # Enforce that contract explicitly rather than silently indexing [0], + # so a future contributor adding a second entry cannot defeat the check. + plugins_list = manifests[marketplace_path].get("plugins") + if not isinstance(plugins_list, list): + errors.append( + f"{_rel(marketplace_path)}: expected `plugins` to be a list, " + f"found {type(plugins_list).__name__}" + ) + return errors + if len(plugins_list) != 1: + errors.append( + f"{_rel(marketplace_path)}: expected exactly one entry in `plugins`, " + f"found {len(plugins_list)}" + ) + return errors + marketplace_plugin = plugins_list[0] + if not isinstance(marketplace_plugin, dict): + errors.append(f"{_rel(marketplace_path)}: `plugins[0]` is not an object") + return errors + + claude_plugin = manifests[claude_plugin_path] + codex_plugin = manifests[codex_plugin_path] + + # For each checked field we enforce two separate invariants: + # (a) the field is present in every file (missing → per-file error) + # (b) the values across files agree (disagree → consolidated error) + # Splitting (a) from (b) prevents the `None == None == None` silent-pass + # failure mode where all three files omit the field and the set-equality + # check would otherwise collapse to `{None}` and report consistent. + def check_field(field: str, label: str) -> None: + values = { + _rel(marketplace_path): marketplace_plugin.get(field), + _rel(claude_plugin_path): claude_plugin.get(field), + _rel(codex_plugin_path): codex_plugin.get(field), + } + # Treat None (missing) and "" (explicit empty string) as equivalent + # failure modes. Without the empty-string branch, three files all + # set to `"version": ""` would produce `{""}` under set-equality + # and pass silently — same shape of bug as the None case. + missing = [path for path, v in values.items() if v is None or v == ""] + if missing: + errors.append( + f"plugin manifest `{field}` field missing or empty in:\n" + + "\n".join(f" {path}" for path in missing) + ) + return + if len(set(values.values())) != 1: + detail = "\n".join(f" {path}: {v!r}" for path, v in values.items()) + errors.append(f"plugin manifest {label} disagree:\n{detail}") + + # Hard invariant: plugin version (cache refresh depends on it). + check_field("version", "versions") + # Soft invariant: plugin name (not cache-critical but catches drift cheaply). + check_field("name", "names") + + return errors + + +# Matches `plugins..minimumCliVersion` references in skill and command +# markdown. Word boundaries at both ends prevent substring matches inside +# larger identifiers (e.g. `xplugins.X.minimumCliVersion` or +# `plugins.X.minimumCliVersionZ`). +_COMPAT_KEY_RE = re.compile(r"\bplugins\.([a-zA-Z0-9_-]+)\.minimumCliVersion\b") + + +def check_compat_key_refs( + compat_path: Path = PLUGIN_COMPATIBILITY_PATH, + content_files: Iterable[Path] | None = None, +) -> list[str]: + """ + Verify every `plugins..minimumCliVersion` reference in skill and + command markdown resolves to a plugin declared in plugin-compatibility.json. + Stale references silently fall through to the top-level minimumCliVersion, + hollowing out per-plugin version gating. + + Returns a list of error messages (empty on success). + """ + errors: list[str] = [] + + compat, err = _load_json_dict(compat_path) + if err is not None: + return [err] + assert compat is not None + + plugins_dict = compat.get("plugins") + if not isinstance(plugins_dict, dict): + return [ + f"{_rel(compat_path)}: expected `plugins` to be an object, " + f"found {type(plugins_dict).__name__}" + ] + + valid_names = set(plugins_dict.keys()) + + files = _iter_content_files() if content_files is None else content_files + for path in files: + try: + text = path.read_text() + except OSError: + continue + for lineno, line in enumerate(text.splitlines(), start=1): + for match in _COMPAT_KEY_RE.finditer(line): + name = match.group(1) + if name not in valid_names: + errors.append( + f"{_rel(path)}:{lineno}: references " + f"`plugins.{name}.minimumCliVersion` but `{name}` is " + f"not declared in plugin-compatibility.json " + f"(valid: {sorted(valid_names)})" + ) + + return errors + + +# Matches `/plugin:command` slash command references in skill and command +# markdown. Both segments use the same kebab/snake/alnum charset as plugin +# names elsewhere. The greedy `+` captures the full command identifier, +# so `/archastro:installer` captures `installer`, not a `install` prefix. +_SLASH_COMMAND_RE = re.compile(r"/([a-zA-Z0-9_-]+):([a-zA-Z0-9_-]+)") + + +def check_slash_command_refs( + marketplace_path: Path = CLAUDE_MARKETPLACE_PATH, + content_files: Iterable[Path] | None = None, + repo_root: Path | None = None, +) -> list[str]: + """ + Verify every `/plugin:command` reference in skill and command markdown + resolves to a plugin declared in marketplace.json and a command file + that actually exists under that plugin's commands/ directory. + + `repo_root` defaults to REPO_ROOT. `source` paths in marketplace.json + are resolved relative to it, and all resolved commands directories + must stay inside it. + + Returns a list of error messages (empty on success). + """ + errors: list[str] = [] + + if repo_root is None: + repo_root = REPO_ROOT + + marketplace, err = _load_json_dict(marketplace_path) + if err is not None: + return [err] + assert marketplace is not None + + plugins = marketplace.get("plugins") + if not isinstance(plugins, list): + return [ + f"{_rel(marketplace_path)}: expected `plugins` to be a list, " + f"found {type(plugins).__name__}" + ] + if not plugins: + return [f"{_rel(marketplace_path)}: `plugins` list is empty"] + + # Build {plugin_name: {command_name, ...}} from marketplace entries. + # Each plugin's `source` field points at its plugin tree root; commands + # live at `/commands/*.md`. A plugin with no commands/ directory + # contributes an empty set, which cleanly makes every command ref fail. + # + # `source` paths are resolved relative to `repo_root`. Resolved commands + # directories must stay inside it — a `source` that escapes the repo + # (via `../` or absolute path) would otherwise let the check enumerate + # arbitrary filesystem locations as "valid commands." + # + # Malformed entries (non-dict, or dict with missing/wrong-typed `name` + # or `source`) surface as explicit errors rather than being silently + # skipped — a contributor typing `"nmae"` should see the typo, not + # discover it later via a downstream "plugin X not declared" error. + repo_resolved = repo_root.resolve() + valid: dict[str, set[str]] = {} + for i, entry in enumerate(plugins): + if not isinstance(entry, dict): + errors.append( + f"{_rel(marketplace_path)}: plugins[{i}] is not an object " + f"(found {type(entry).__name__})" + ) + continue + name = entry.get("name") + source = entry.get("source") + if not isinstance(name, str) or not isinstance(source, str): + errors.append( + f"{_rel(marketplace_path)}: plugins[{i}] has missing or " + f"non-string `name`/`source` (name={name!r}, source={source!r})" + ) + continue + commands_dir = (repo_root / source / "commands").resolve() + if not commands_dir.is_relative_to(repo_resolved): + errors.append( + f"{_rel(marketplace_path)}: plugin {name!r} source " + f"{source!r} resolves outside the repo " + f"({commands_dir} not under {repo_resolved})" + ) + continue + if commands_dir.exists() and commands_dir.is_dir(): + valid[name] = {p.stem for p in commands_dir.glob("*.md")} + else: + valid[name] = set() + + if errors: + return errors + + files = _iter_content_files() if content_files is None else content_files + for path in files: + try: + text = path.read_text() + except OSError: + continue + for lineno, line in enumerate(text.splitlines(), start=1): + for match in _SLASH_COMMAND_RE.finditer(line): + plugin, command = match.group(1), match.group(2) + if plugin not in valid: + errors.append( + f"{_rel(path)}:{lineno}: references " + f"`/{plugin}:{command}` but plugin `{plugin}` is " + f"not declared in marketplace.json " + f"(valid: {sorted(valid)})" + ) + elif command not in valid[plugin]: + errors.append( + f"{_rel(path)}:{lineno}: references " + f"`/{plugin}:{command}` but command `{command}` does " + f"not exist in {plugin}'s commands/ directory " + f"(valid: {sorted(valid[plugin])})" + ) + + return errors + + +# Matches three-segment version literals like `0.3.1`. Each segment is +# 1-3 digits, which excludes 4-digit year components (e.g. `2026.04.10` +# would not match because `2026` exceeds the `\d{1,3}` bound). Word +# boundaries prevent substring matches inside longer identifiers. +_VERSION_RE = re.compile(r"\b\d{1,3}\.\d{1,3}\.\d{1,3}\b") + + +def _iter_scannable_lines(text: str) -> Iterator[tuple[int, str]]: + """ + Yield (1-based lineno, line) pairs from markdown text, skipping YAML + frontmatter at the top of the file and lines inside fenced code blocks. + Both fence delimiters and frontmatter delimiters are consumed but not + yielded. + """ + lines = text.splitlines() + i = 0 + n = len(lines) + + # Frontmatter: if the first line is `---` alone, consume until the + # next `---`. A file without frontmatter starts scanning at line 1. + if n > 0 and lines[0].strip() == "---": + i = 1 + while i < n and lines[i].strip() != "---": + i += 1 + i += 1 # consume the closing --- + + in_fence = False + while i < n: + stripped = lines[i].lstrip() + # Markdown fences may use either ``` or ~~~ as delimiters. + if stripped.startswith("```") or stripped.startswith("~~~"): + in_fence = not in_fence + elif not in_fence: + yield (i + 1, lines[i]) + i += 1 + + +def check_hardcoded_versions( + content_files: Iterable[Path] | None = None, +) -> list[str]: + """ + Flag hardcoded semver literals (N.N.N) in skill and command markdown + prose. Contributors should read from plugin-compatibility.json + dynamically so a version bump in the manifest flows through to every + reference automatically. YAML frontmatter and fenced code blocks are + excluded — version literals there are usually legitimate examples. + + Returns a list of error messages (empty on success). + """ + errors: list[str] = [] + files = _iter_content_files() if content_files is None else content_files + for path in files: + try: + text = path.read_text() + except OSError: + continue + for lineno, line in _iter_scannable_lines(text): + for match in _VERSION_RE.finditer(line): + version = match.group(0) + errors.append( + f"{_rel(path)}:{lineno}: hardcoded version string " + f"`{version}` — prefer reading from " + f"plugin-compatibility.json dynamically" + ) + return errors + + +def check_version_bump_on_content_change( + marketplace_path: Path = CLAUDE_MARKETPLACE_PATH, + base_ref: str | None = None, + repo_root: Path | None = None, +) -> list[str]: + """ + If any content file changed between `base_ref` and HEAD, verify the + marketplace plugin version also changed. Without a bump, Claude Code + and Codex serve stale content from their version-keyed caches. + + Assumes check_manifest_consistency has already validated cross-file + version agreement, so comparing one manifest is sufficient. + + Skips cleanly when: + - the base ref is unreachable (fresh clone, shallow fetch, local-only) + - git is unavailable + - no content files changed + - marketplace.json didn't exist at the base ref (new-plugin bootstrap) + + Returns a list of error messages (empty on success or skip). + """ + if repo_root is None: + repo_root = REPO_ROOT + + # Auto-detect base ref from GITHUB_BASE_REF (PR context) or fall back + # to origin/main for local dev and push events. On push-to-main the + # diff will be empty and the check trivially passes. + if base_ref is None: + github_base = os.environ.get("GITHUB_BASE_REF") + base_ref = f"origin/{github_base}" if github_base else "origin/main" + + def _warn(msg: str) -> None: + # Stderr warning that does NOT fail the check — lets skip conditions + # surface visibly so a misconfigured CI (shallow clone, missing + # origin/main) doesn't silently no-op this check. + print(f"WARN [version bump on content change]: {msg}", file=sys.stderr) + + def _git(*args: str) -> subprocess.CompletedProcess[str] | None: + # 30-second timeout guards against hung git invocations (slow NFS + # mounts, unusual credential/signing prompts, etc.) that would + # otherwise block CI for the default GitHub Actions step timeout. + # Missing git or a timeout both warn-and-return-None; callers + # treat None the same as a non-zero returncode. + try: + return subprocess.run( + ["git", "-C", str(repo_root), *args], + capture_output=True, + text=True, + timeout=30, + ) + except FileNotFoundError: + _warn("git executable not found; skipping check") + return None + except subprocess.TimeoutExpired: + _warn( + f"git {args[0]} timed out after 30s; skipping check. " + f"If this is unexpected, check for slow filesystems, " + f"stale .git/index.lock, or unusual credential prompts." + ) + return None + + base_check = _git("rev-parse", "--verify", f"{base_ref}^{{commit}}") + if base_check is None: + return [] + if base_check.returncode != 0: + _warn( + f"base ref {base_ref!r} is unreachable; skipping check. " + f"If unexpected, verify `fetch-depth: 0` on actions/checkout " + f"or run `git fetch origin main` locally." + ) + return [] + + diff = _git("diff", "--name-only", f"{base_ref}...HEAD") + if diff is None: + return [] + if diff.returncode != 0: + _warn(f"git diff against {base_ref} failed; skipping check") + return [] + changed_files = [line for line in diff.stdout.splitlines() if line] + content_changed = [f for f in changed_files if _is_content_path(f)] + if not content_changed: + return [] + + # Read current version from the working-tree marketplace. Load errors + # are silently skipped — a malformed current manifest is the manifest + # consistency check's concern, not this one's. + current, _ = _load_json_dict(marketplace_path) + if current is None: + return [] + try: + current_version = current["plugins"][0]["version"] + except (KeyError, IndexError, TypeError): + return [] + + # Read base version from the marketplace at the base ref. + try: + marketplace_relpath = marketplace_path.relative_to(repo_root).as_posix() + except ValueError: + _warn( + f"marketplace_path {marketplace_path!r} is not under " + f"repo_root {repo_root!r}; skipping check" + ) + return [] + base_show = _git("show", f"{base_ref}:{marketplace_relpath}") + if base_show is None: + return [] + if base_show.returncode != 0: + # marketplace.json didn't exist at base → treat as implicit bump. + return [] + try: + base_data = json.loads(base_show.stdout) + base_version = base_data["plugins"][0]["version"] + except (json.JSONDecodeError, KeyError, IndexError, TypeError): + return [] + + if current_version != base_version: + return [] + + detail = "\n".join(f" {f}" for f in content_changed) + return [ + f"plugin content changed but marketplace plugin version is still " + f"{current_version!r} (same as {base_ref}). Bump the version in all " + f"three manifest files or CI will serve stale content on install. " + f"Changed files:\n{detail}" + ] + + +# Ordered list of (name, callable) pairs. Runner invokes each in order so +# earlier checks can establish preconditions that later checks rely on +# (e.g. check_version_bump_on_content_change assumes check_manifest_consistency +# has already validated cross-file agreement, so it can compare just one +# manifest's version against the PR base). +CHECKS: list[tuple[str, Callable[[], list[str]]]] = [ + ("manifest consistency", check_manifest_consistency), + ("compat key refs", check_compat_key_refs), + ("slash command refs", check_slash_command_refs), + ("hardcoded versions", check_hardcoded_versions), + ("version bump on content change", check_version_bump_on_content_change), +] + + +def main() -> int: + any_failed = False + for name, check in CHECKS: + errors = check() + if errors: + any_failed = True + print(f"FAIL [{name}]:", file=sys.stderr) + for err in errors: + for line in err.splitlines(): + print(f" {line}", file=sys.stderr) + else: + print(f"OK [{name}]") + return 1 if any_failed else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/test_check_plugin_repo.py b/scripts/test_check_plugin_repo.py new file mode 100644 index 0000000..5e3be3b --- /dev/null +++ b/scripts/test_check_plugin_repo.py @@ -0,0 +1,1449 @@ +#!/usr/bin/env python3 +""" +Unit tests for scripts/check_plugin_repo.py repo-validation checks. + +Run directly: + + python3 scripts/test_check_plugin_repo.py + +Covers check_manifest_consistency, check_compat_key_refs, +check_slash_command_refs, check_hardcoded_versions, and +check_version_bump_on_content_change. +""" +import json +import os +import subprocess +import tempfile +import unittest +from pathlib import Path + +# scripts/check_plugin_repo.py has no hyphen, so it imports cleanly +# unlike generate-plugin-content.py which needs importlib. +import check_plugin_repo + + +class ManifestConsistencyTest(unittest.TestCase): + """Tests for check_manifest_consistency().""" + + def setUp(self) -> None: + self._tmp = tempfile.TemporaryDirectory() + self.tmp = Path(self._tmp.name) + self.marketplace_path = self.tmp / "marketplace.json" + self.claude_path = self.tmp / "claude-plugin.json" + self.codex_path = self.tmp / "codex-plugin.json" + + def tearDown(self) -> None: + self._tmp.cleanup() + + def _write(self, marketplace: dict, claude: dict, codex: dict) -> None: + self.marketplace_path.write_text(json.dumps(marketplace)) + self.claude_path.write_text(json.dumps(claude)) + self.codex_path.write_text(json.dumps(codex)) + + def _check(self) -> list[str]: + return check_plugin_repo.check_manifest_consistency( + marketplace_path=self.marketplace_path, + claude_plugin_path=self.claude_path, + codex_plugin_path=self.codex_path, + ) + + def _valid_triplet(self) -> tuple[dict, dict, dict]: + marketplace = { + "name": "archastro", + "plugins": [{"name": "archastro", "version": "0.7.2"}], + } + claude = {"name": "archastro", "version": "0.7.2"} + codex = {"name": "archastro", "version": "0.7.2"} + return marketplace, claude, codex + + # Happy path ---------------------------------------------------------- + + def test_all_three_consistent_passes(self): + self._write(*self._valid_triplet()) + self.assertEqual(self._check(), []) + + # Missing-field handling (regression for None == None == None bypass) - + + def test_all_three_missing_version_fails(self): + mp, cp, xp = self._valid_triplet() + del mp["plugins"][0]["version"] + del cp["version"] + del xp["version"] + self._write(mp, cp, xp) + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("`version` field missing or empty", errors[0]) + self.assertIn(str(self.marketplace_path), errors[0]) + self.assertIn(str(self.claude_path), errors[0]) + self.assertIn(str(self.codex_path), errors[0]) + + def test_one_missing_version_fails(self): + mp, cp, xp = self._valid_triplet() + del cp["version"] + self._write(mp, cp, xp) + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("`version` field missing or empty", errors[0]) + self.assertIn(str(self.claude_path), errors[0]) + self.assertNotIn(str(self.marketplace_path), errors[0]) + + def test_all_three_missing_name_fails(self): + mp, cp, xp = self._valid_triplet() + del mp["plugins"][0]["name"] + del cp["name"] + del xp["name"] + self._write(mp, cp, xp) + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("`name` field missing or empty", errors[0]) + + def test_all_three_empty_string_version_fails(self): + # Empty strings must be rejected alongside missing fields, otherwise + # three files agreeing at "" would pass set-equality. + mp, cp, xp = self._valid_triplet() + mp["plugins"][0]["version"] = "" + cp["version"] = "" + xp["version"] = "" + self._write(mp, cp, xp) + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("`version` field missing or empty", errors[0]) + self.assertIn(str(self.marketplace_path), errors[0]) + self.assertIn(str(self.claude_path), errors[0]) + self.assertIn(str(self.codex_path), errors[0]) + + def test_one_empty_string_version_fails(self): + # Mixed case: one empty, two populated. The empty-string file + # must surface as missing-or-empty, not as a drift disagreement. + mp, cp, xp = self._valid_triplet() + xp["version"] = "" + self._write(mp, cp, xp) + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("`version` field missing or empty", errors[0]) + self.assertIn(str(self.codex_path), errors[0]) + + # plugins[] arity (regression for silent `plugins[0]` indexing) -------- + + def test_empty_plugins_list_fails(self): + mp, cp, xp = self._valid_triplet() + mp["plugins"] = [] + self._write(mp, cp, xp) + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("exactly one entry in `plugins`", errors[0]) + self.assertIn("found 0", errors[0]) + + def test_two_plugin_entries_fails(self): + # Extra entries must fail loudly rather than being silently ignored + # by [0] indexing. + mp, cp, xp = self._valid_triplet() + mp["plugins"].append({"name": "ghost", "version": "99.99.99"}) + self._write(mp, cp, xp) + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("exactly one entry in `plugins`", errors[0]) + self.assertIn("found 2", errors[0]) + + def test_plugins_not_a_list_fails(self): + mp, cp, xp = self._valid_triplet() + mp["plugins"] = {"name": "archastro", "version": "0.7.2"} + self._write(mp, cp, xp) + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("`plugins` to be a list", errors[0]) + + def test_plugins_entry_not_a_dict_fails(self): + mp, cp, xp = self._valid_triplet() + mp["plugins"] = ["a string, not a plugin entry"] + self._write(mp, cp, xp) + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("`plugins[0]` is not an object", errors[0]) + + # Disagreement detection ----------------------------------------------- + + def test_version_drift_fails(self): + mp, cp, xp = self._valid_triplet() + mp["plugins"][0]["version"] = "0.7.3" + self._write(mp, cp, xp) + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("versions disagree", errors[0]) + self.assertIn("'0.7.3'", errors[0]) + self.assertIn("'0.7.2'", errors[0]) + + def test_name_drift_fails(self): + mp, cp, xp = self._valid_triplet() + cp["name"] = "archastro-renamed" + self._write(mp, cp, xp) + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("names disagree", errors[0]) + self.assertIn("'archastro-renamed'", errors[0]) + + # File-level errors ---------------------------------------------------- + + def test_missing_file_fails(self): + self._write(*self._valid_triplet()) + self.codex_path.unlink() + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("cannot read", errors[0]) + self.assertIn("codex-plugin.json", errors[0]) + + def test_invalid_json_fails(self): + self.marketplace_path.write_text("{not valid json") + self.claude_path.write_text( + json.dumps({"name": "archastro", "version": "0.7.2"}) + ) + self.codex_path.write_text( + json.dumps({"name": "archastro", "version": "0.7.2"}) + ) + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("invalid JSON", errors[0]) + + def test_marketplace_top_level_not_object_fails(self): + self.marketplace_path.write_text(json.dumps([1, 2, 3])) + self.claude_path.write_text( + json.dumps({"name": "archastro", "version": "0.7.2"}) + ) + self.codex_path.write_text( + json.dumps({"name": "archastro", "version": "0.7.2"}) + ) + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("top-level JSON is not an object", errors[0]) + + # Multi-error reporting ------------------------------------------------ + + def test_multiple_file_errors_reported_together(self): + # Two broken files — both should surface in a single check call. + self.marketplace_path.write_text("{not valid json") + self.claude_path.write_text( + json.dumps({"name": "archastro", "version": "0.7.2"}) + ) + self.codex_path.write_text("{also not valid") + errors = self._check() + self.assertEqual(len(errors), 2) + self.assertTrue(any("marketplace.json" in e for e in errors)) + self.assertTrue(any("codex-plugin.json" in e for e in errors)) + + +class CompatKeyRefsTest(unittest.TestCase): + """Tests for check_compat_key_refs().""" + + def setUp(self) -> None: + self._tmp = tempfile.TemporaryDirectory() + self.tmp = Path(self._tmp.name) + self.compat_path = self.tmp / "plugin-compatibility.json" + + def tearDown(self) -> None: + self._tmp.cleanup() + + def _write_compat(self, data: dict) -> None: + self.compat_path.write_text(json.dumps(data)) + + def _write_content(self, name: str, text: str) -> Path: + """Write a content file and return its path.""" + path = self.tmp / name + path.write_text(text) + return path + + def _check(self, *content_files: Path) -> list[str]: + return check_plugin_repo.check_compat_key_refs( + compat_path=self.compat_path, + content_files=list(content_files), + ) + + def _valid_compat(self) -> dict: + return { + "minimumCliVersion": "0.3.1", + "plugins": {"archastro": {"minimumCliVersion": "0.3.1"}}, + } + + # Happy path ---------------------------------------------------------- + + def test_valid_reference_passes(self): + self._write_compat(self._valid_compat()) + f = self._write_content( + "valid.md", + "Look up `plugins.archastro.minimumCliVersion` for the floor.", + ) + self.assertEqual(self._check(f), []) + + def test_no_references_passes(self): + self._write_compat(self._valid_compat()) + f = self._write_content("plain.md", "This file has no compat keys.") + self.assertEqual(self._check(f), []) + + def test_empty_content_file_list_passes(self): + self._write_compat(self._valid_compat()) + self.assertEqual(self._check(), []) + + # Stale-reference detection -------------------------------------------- + + def test_stale_plugin_name_fails(self): + self._write_compat(self._valid_compat()) + f = self._write_content( + "stale.md", + "Check `plugins.cli.minimumCliVersion` before invoking.", + ) + errors = self._check(f) + self.assertEqual(len(errors), 1) + self.assertIn("plugins.cli.minimumCliVersion", errors[0]) + self.assertIn("`cli` is", errors[0]) + self.assertIn("not declared", errors[0]) + self.assertIn("'archastro'", errors[0]) + self.assertIn(":1:", errors[0]) + + def test_line_number_accurate(self): + # Error line number must match the actual ref location, not line 1. + self._write_compat(self._valid_compat()) + f = self._write_content( + "offset.md", + "header line\n" + "second line\n" + "third line with `plugins.ghost.minimumCliVersion` ref\n", + ) + errors = self._check(f) + self.assertEqual(len(errors), 1) + self.assertIn(":3:", errors[0]) + + def test_stale_ref_in_multiple_files(self): + self._write_compat(self._valid_compat()) + f1 = self._write_content("a.md", "`plugins.foo.minimumCliVersion`") + f2 = self._write_content("b.md", "`plugins.bar.minimumCliVersion`") + errors = self._check(f1, f2) + self.assertEqual(len(errors), 2) + self.assertTrue(any("a.md" in e and "foo" in e for e in errors)) + self.assertTrue(any("b.md" in e and "bar" in e for e in errors)) + + def test_multiple_refs_on_same_line(self): + self._write_compat(self._valid_compat()) + f = self._write_content( + "dense.md", + "ref `plugins.foo.minimumCliVersion` then `plugins.bar.minimumCliVersion`", + ) + errors = self._check(f) + self.assertEqual(len(errors), 2) + self.assertTrue(all(":1:" in e for e in errors)) + + def test_valid_and_stale_mixed_in_same_file(self): + self._write_compat(self._valid_compat()) + f = self._write_content( + "mixed.md", + "valid `plugins.archastro.minimumCliVersion`\n" + "stale `plugins.oldname.minimumCliVersion`\n", + ) + errors = self._check(f) + self.assertEqual(len(errors), 1) + self.assertIn("oldname", errors[0]) + self.assertIn(":2:", errors[0]) + self.assertNotIn("archastro.minimumCliVersion", errors[0]) + + def test_case_sensitivity(self): + self._write_compat(self._valid_compat()) + f = self._write_content( + "case.md", + "Wrong case: `plugins.Archagents.minimumCliVersion`", + ) + errors = self._check(f) + self.assertEqual(len(errors), 1) + self.assertIn("Archagents", errors[0]) + + # Compat file error modes -------------------------------------------- + + def test_missing_compat_file_fails(self): + # compat_path doesn't exist + self.assertFalse(self.compat_path.exists()) + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("cannot read", errors[0]) + + def test_invalid_json_compat_file_fails(self): + self.compat_path.write_text("{not valid json") + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("invalid JSON", errors[0]) + + def test_compat_top_level_not_object_fails(self): + self.compat_path.write_text(json.dumps([1, 2, 3])) + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("top-level JSON is not an object", errors[0]) + + def test_compat_missing_plugins_key_fails(self): + # {minimumCliVersion: ...} but no top-level plugins object. + self.compat_path.write_text( + json.dumps({"minimumCliVersion": "0.3.1"}) + ) + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("`plugins` to be an object", errors[0]) + + def test_compat_plugins_not_an_object_fails(self): + # plugins is a list, not an object. + self.compat_path.write_text( + json.dumps({"plugins": ["archastro"]}) + ) + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("`plugins` to be an object", errors[0]) + + # Multi-plugin support ----------------------------------------------- + + def test_multiple_valid_plugins_all_accepted(self): + # When compat declares multiple plugins, all declared names are valid. + self._write_compat( + { + "plugins": { + "archastro": {"minimumCliVersion": "0.3.1"}, + "other": {"minimumCliVersion": "0.5.0"}, + } + } + ) + f = self._write_content( + "multi.md", + "`plugins.archastro.minimumCliVersion`\n" + "`plugins.other.minimumCliVersion`\n", + ) + self.assertEqual(self._check(f), []) + + def test_compat_with_empty_plugins_dict_rejects_all_refs(self): + # An empty plugins object means no valid names — every ref must fail, + # not pass as "no enforcement". + self._write_compat({"plugins": {}}) + f = self._write_content( + "x.md", + "`plugins.archastro.minimumCliVersion`", + ) + errors = self._check(f) + self.assertEqual(len(errors), 1) + self.assertIn("`archastro` is", errors[0]) + self.assertIn("not declared", errors[0]) + + def test_regex_requires_word_boundaries(self): + # Substrings of larger identifiers must not match. + # Uses `ghost` (invalid name) so a regex that substring-matches + # will emit false-positive errors this assertion catches; a valid + # name here would be silent under regression. + self._write_compat(self._valid_compat()) + f = self._write_content( + "wordboundaries.md", + "xplugins.ghost.minimumCliVersion\n" + "my_plugins.ghost.minimumCliVersion\n" + "plugins.ghost.minimumCliVersionZ\n" + "plugins.ghost.minimumCliVersion_suffix\n", + ) + self.assertEqual(self._check(f), []) + + +class SlashCommandRefsTest(unittest.TestCase): + """Tests for check_slash_command_refs().""" + + def setUp(self) -> None: + self._tmp = tempfile.TemporaryDirectory() + self.tmp = Path(self._tmp.name) + # Mirror the real repo layout: marketplace.json lives in + # /.claude-plugin/, and plugin trees hang off via + # `source` paths like "./.claude-plugins/". + self.marketplace_dir = self.tmp / ".claude-plugin" + self.marketplace_dir.mkdir() + self.marketplace_path = self.marketplace_dir / "marketplace.json" + self.plugin_root = self.tmp / ".claude-plugins" / "archastro" + self.commands_dir = self.plugin_root / "commands" + self.commands_dir.mkdir(parents=True) + + def tearDown(self) -> None: + self._tmp.cleanup() + + def _write_marketplace(self, plugins: list | None = None) -> None: + if plugins is None: + plugins = [ + {"name": "archastro", "source": "./.claude-plugins/archastro"} + ] + self.marketplace_path.write_text(json.dumps({"plugins": plugins})) + + def _add_command(self, name: str) -> None: + (self.commands_dir / f"{name}.md").write_text("# command body\n") + + def _write_content(self, name: str, text: str) -> Path: + path = self.tmp / name + path.write_text(text) + return path + + def _check(self, *content_files: Path) -> list[str]: + return check_plugin_repo.check_slash_command_refs( + marketplace_path=self.marketplace_path, + content_files=list(content_files), + repo_root=self.tmp, + ) + + # Happy path ---------------------------------------------------------- + + def test_valid_plugin_and_command_passes(self): + self._write_marketplace() + self._add_command("install") + f = self._write_content("ref.md", "Run `/archastro:install` now.") + self.assertEqual(self._check(f), []) + + def test_no_refs_passes(self): + self._write_marketplace() + self._add_command("install") + f = self._write_content("plain.md", "no slash commands here") + self.assertEqual(self._check(f), []) + + def test_empty_content_file_list_passes(self): + self._write_marketplace() + self._add_command("install") + self.assertEqual(self._check(), []) + + # Stale references ---------------------------------------------------- + + def test_stale_plugin_name_fails(self): + self._write_marketplace() + self._add_command("install") + f = self._write_content("stale.md", "Run `/oldname:install`") + errors = self._check(f) + self.assertEqual(len(errors), 1) + self.assertIn("/oldname:install", errors[0]) + self.assertIn("plugin `oldname` is", errors[0]) + self.assertIn("not declared", errors[0]) + self.assertIn("'archastro'", errors[0]) + self.assertIn(":1:", errors[0]) + + def test_stale_command_name_fails(self): + # Plugin exists, command file doesn't. + self._write_marketplace() + self._add_command("install") + f = self._write_content("stale.md", "Run `/archastro:uninstall`") + errors = self._check(f) + self.assertEqual(len(errors), 1) + self.assertIn("/archastro:uninstall", errors[0]) + self.assertIn("command `uninstall`", errors[0]) + self.assertIn("does not exist", errors[0]) + self.assertIn("archastro's commands/ directory", errors[0]) + self.assertIn("'install'", errors[0]) + + def test_both_stale_reports_plugin_error_only(self): + # If plugin is unknown, command existence isn't checked (plugin + # error is terminal). + self._write_marketplace() + self._add_command("install") + f = self._write_content("both.md", "`/oldname:uninstall`") + errors = self._check(f) + self.assertEqual(len(errors), 1) + self.assertIn("plugin `oldname`", errors[0]) + self.assertNotIn("command `uninstall`", errors[0]) + + def test_line_number_accurate(self): + self._write_marketplace() + self._add_command("install") + f = self._write_content( + "offset.md", + "header\n" + "second\n" + "ref `/archastro:ghost` on line 3\n", + ) + errors = self._check(f) + self.assertEqual(len(errors), 1) + self.assertIn(":3:", errors[0]) + + def test_multiple_files(self): + self._write_marketplace() + self._add_command("install") + f1 = self._write_content("a.md", "`/archastro:foo`") + f2 = self._write_content("b.md", "`/archastro:bar`") + errors = self._check(f1, f2) + self.assertEqual(len(errors), 2) + self.assertTrue(any("a.md" in e and "foo" in e for e in errors)) + self.assertTrue(any("b.md" in e and "bar" in e for e in errors)) + + def test_multiple_refs_on_same_line(self): + self._write_marketplace() + self._add_command("install") + f = self._write_content( + "dense.md", "`/archastro:foo` and `/archastro:bar`" + ) + errors = self._check(f) + self.assertEqual(len(errors), 2) + self.assertTrue(all(":1:" in e for e in errors)) + + def test_valid_and_stale_mixed(self): + self._write_marketplace() + self._add_command("install") + f = self._write_content( + "mixed.md", + "valid `/archastro:install`\n" + "stale `/archastro:missing`\n", + ) + errors = self._check(f) + self.assertEqual(len(errors), 1) + self.assertIn("missing", errors[0]) + self.assertIn(":2:", errors[0]) + + # Multi-command and multi-plugin ------------------------------------- + + def test_multiple_commands_all_valid(self): + self._write_marketplace() + for cmd in ("install", "auth", "impersonate"): + self._add_command(cmd) + f = self._write_content( + "multi.md", + "`/archastro:install`\n`/archastro:auth`\n`/archastro:impersonate`\n", + ) + self.assertEqual(self._check(f), []) + + def test_multi_plugin_marketplace(self): + # Two plugins, each with their own commands directory. Each plugin + # only accepts its own commands. + other_dir = self.tmp / ".claude-plugins" / "other" / "commands" + other_dir.mkdir(parents=True) + (other_dir / "foo.md").write_text("# foo") + self._write_marketplace( + [ + {"name": "archastro", "source": "./.claude-plugins/archastro"}, + {"name": "other", "source": "./.claude-plugins/other"}, + ] + ) + self._add_command("install") + f = self._write_content( + "multi.md", + "`/archastro:install`\n" # valid + "`/other:foo`\n" # valid + "`/archastro:foo`\n" # invalid: archastro has no `foo` + "`/other:install`\n", # invalid: other has no `install` + ) + errors = self._check(f) + self.assertEqual(len(errors), 2) + self.assertTrue(any("/archastro:foo" in e for e in errors)) + self.assertTrue(any("/other:install" in e for e in errors)) + + # Marketplace error modes -------------------------------------------- + + def test_missing_marketplace_fails(self): + # Don't write marketplace.json at all + self._add_command("install") + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("cannot read", errors[0]) + + def test_invalid_json_marketplace_fails(self): + self.marketplace_path.write_text("{nope") + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("invalid JSON", errors[0]) + + def test_marketplace_top_level_not_object_fails(self): + self.marketplace_path.write_text(json.dumps([1, 2, 3])) + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("top-level JSON is not an object", errors[0]) + + def test_marketplace_plugins_not_list_fails(self): + self.marketplace_path.write_text(json.dumps({"plugins": {}})) + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("`plugins` to be a list", errors[0]) + + def test_plugin_with_no_commands_dir_rejects_refs(self): + # Plugin declared but its commands/ directory is missing entirely. + import shutil + shutil.rmtree(self.commands_dir) + self._write_marketplace() + f = self._write_content("ref.md", "`/archastro:install`") + errors = self._check(f) + self.assertEqual(len(errors), 1) + self.assertIn("command `install`", errors[0]) + self.assertIn("does not exist", errors[0]) + + def test_empty_commands_dir_rejects_refs(self): + # commands/ exists but contains no .md files. Distinct from the + # missing-directory case; same outcome (empty valid set). + self._write_marketplace() + # setUp already created self.commands_dir empty; no _add_command call + f = self._write_content("ref.md", "`/archastro:install`") + errors = self._check(f) + self.assertEqual(len(errors), 1) + self.assertIn("command `install`", errors[0]) + self.assertIn("does not exist", errors[0]) + + def test_empty_plugins_list_fails(self): + self._write_marketplace(plugins=[]) + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("`plugins` list is empty", errors[0]) + + def test_plugin_entry_not_a_dict_fails(self): + self._write_marketplace( + plugins=["not a dict", 42, {"name": "archastro", + "source": "./.claude-plugins/archastro"}] + ) + self._add_command("install") + errors = self._check() + # Two errors: one for index 0 (string), one for index 1 (int). + self.assertEqual(len(errors), 2) + self.assertTrue(any("plugins[0]" in e and "str" in e for e in errors)) + self.assertTrue(any("plugins[1]" in e and "int" in e for e in errors)) + + def test_plugin_entry_missing_name_surfaces_error(self): + self._write_marketplace( + plugins=[{"source": "./.claude-plugins/archastro"}] # no name + ) + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("plugins[0]", errors[0]) + self.assertIn("missing or non-string", errors[0]) + self.assertIn("name=None", errors[0]) + + def test_partial_malformed_entry_still_surfaces(self): + # One valid entry + one malformed entry. Both produce output: the + # malformed entry gets its per-entry error (early return means the + # valid entry's commands aren't used, but that's fine — fix the + # corrupt entry first, then re-run). + self._write_marketplace( + plugins=[ + {"name": "archastro", "source": "./.claude-plugins/archastro"}, + {"name": "broken"}, # missing source + ] + ) + self._add_command("install") + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("plugins[1]", errors[0]) + self.assertIn("source=None", errors[0]) + + # Path containment --------------------------------------------------- + + def test_source_with_absolute_path_outside_repo_fails(self): + # A `source` pointing at an absolute path outside the repo must + # surface as a boundary error, not be followed. Uses a separate + # TemporaryDirectory so the fixture doesn't pollute self.tmp.parent. + with tempfile.TemporaryDirectory() as outside_root: + outside_commands = Path(outside_root) / "commands" + outside_commands.mkdir() + (outside_commands / "secret.md").write_text("# evil") + + self._write_marketplace( + [{"name": "archastro", "source": outside_root}] + ) + f = self._write_content("ref.md", "`/archastro:secret`") + errors = self._check(f) + self.assertEqual(len(errors), 1) + self.assertIn("resolves outside the repo", errors[0]) + self.assertIn("archastro", errors[0]) + + def test_source_with_relative_traversal_fails(self): + # Same failure mode via `../` traversal instead of absolute path. + with tempfile.TemporaryDirectory() as outside_root: + outside_commands = Path(outside_root) / "commands" + outside_commands.mkdir() + (outside_commands / "secret.md").write_text("# evil") + + # `source` uses `..` from the marketplace's nominal repo base + # (self.tmp) to escape — we compute the relative path. + import os + relative_source = os.path.relpath(outside_root, start=self.tmp) + self._write_marketplace( + [{"name": "archastro", "source": relative_source}] + ) + f = self._write_content("ref.md", "`/archastro:secret`") + errors = self._check(f) + self.assertEqual(len(errors), 1) + self.assertIn("resolves outside the repo", errors[0]) + + # Regex capture discipline ------------------------------------------- + + def test_regex_captures_full_command_name(self): + # Regression guard: the regex must capture the full command + # identifier, not a prefix. `/archastro:installer` must report + # `installer` as the unknown command, not silently succeed by + # matching just the `install` prefix. + self._write_marketplace() + self._add_command("install") + f = self._write_content( + "suffix.md", + "`/archastro:installer`\n" + "`/archastro:install_extended`\n", + ) + errors = self._check(f) + self.assertEqual(len(errors), 2) + self.assertTrue(any("`installer`" in e for e in errors)) + self.assertTrue(any("`install_extended`" in e for e in errors)) + + +class HardcodedVersionsTest(unittest.TestCase): + """Tests for check_hardcoded_versions().""" + + def setUp(self) -> None: + self._tmp = tempfile.TemporaryDirectory() + self.tmp = Path(self._tmp.name) + + def tearDown(self) -> None: + self._tmp.cleanup() + + def _write(self, name: str, text: str) -> Path: + path = self.tmp / name + path.write_text(text) + return path + + def _check(self, *files: Path) -> list[str]: + return check_plugin_repo.check_hardcoded_versions( + content_files=list(files) + ) + + # Happy path ---------------------------------------------------------- + + def test_no_versions_passes(self): + f = self._write("clean.md", "No version numbers here.") + self.assertEqual(self._check(f), []) + + def test_empty_file_passes(self): + f = self._write("empty.md", "") + self.assertEqual(self._check(f), []) + + def test_two_segment_version_not_matched(self): + # `1.2` is not a semver triple and should not fire. + f = self._write("two.md", "Python 3.12 has new features.") + self.assertEqual(self._check(f), []) + + def test_four_digit_year_not_matched(self): + # Date-like strings with 4-digit year components must not match + # because the regex bounds each segment to 1-3 digits. + f = self._write("date.md", "Released on 2026.04.10.") + self.assertEqual(self._check(f), []) + + # Hardcoded version detection ---------------------------------------- + + def test_version_in_prose_fails(self): + f = self._write("bad.md", "Requires archastro 0.3.1 or later.") + errors = self._check(f) + self.assertEqual(len(errors), 1) + self.assertIn("hardcoded version string", errors[0]) + self.assertIn("`0.3.1`", errors[0]) + self.assertIn(":1:", errors[0]) + + def test_version_in_inline_code_fails(self): + # Inline code (single backticks) is still prose — a literal + # version there is exactly what we want to flag. + f = self._write("inline.md", "Use `0.7.2` for the plugin cache.") + errors = self._check(f) + self.assertEqual(len(errors), 1) + self.assertIn("`0.7.2`", errors[0]) + + def test_multiple_versions_all_reported(self): + f = self._write( + "multi.md", + "first 0.3.1\nsecond 1.2.3\nthird 10.20.30\n", + ) + errors = self._check(f) + self.assertEqual(len(errors), 3) + self.assertTrue(any("`0.3.1`" in e and ":1:" in e for e in errors)) + self.assertTrue(any("`1.2.3`" in e and ":2:" in e for e in errors)) + self.assertTrue(any("`10.20.30`" in e and ":3:" in e for e in errors)) + + def test_line_number_accurate(self): + f = self._write( + "offset.md", + "line 1\n" + "line 2\n" + "line 3 with 4.5.6 version\n" + "line 4\n", + ) + errors = self._check(f) + self.assertEqual(len(errors), 1) + self.assertIn(":3:", errors[0]) + + def test_multiple_files(self): + f1 = self._write("a.md", "`1.0.0`") + f2 = self._write("b.md", "`2.0.0`") + errors = self._check(f1, f2) + self.assertEqual(len(errors), 2) + self.assertTrue(any("a.md" in e and "1.0.0" in e for e in errors)) + self.assertTrue(any("b.md" in e and "2.0.0" in e for e in errors)) + + # Frontmatter exclusion ----------------------------------------------- + + def test_version_in_frontmatter_ignored(self): + f = self._write( + "fm.md", + "---\n" + "name: test\n" + "version: 1.2.3\n" + "---\n" + "body without versions\n", + ) + self.assertEqual(self._check(f), []) + + def test_version_after_frontmatter_still_matched(self): + # Versions in the body after frontmatter must still fire. + f = self._write( + "after-fm.md", + "---\n" + "name: test\n" + "---\n" + "\n" + "Requires 0.3.1 to run.\n", + ) + errors = self._check(f) + self.assertEqual(len(errors), 1) + self.assertIn("`0.3.1`", errors[0]) + self.assertIn(":5:", errors[0]) + + def test_triple_dash_in_body_not_treated_as_frontmatter(self): + # A `---` in the body (e.g. a horizontal rule) is not a + # frontmatter delimiter because the file didn't start with one. + f = self._write( + "hr.md", + "intro\n" + "---\n" + "1.2.3 after horizontal rule\n", + ) + errors = self._check(f) + self.assertEqual(len(errors), 1) + self.assertIn("`1.2.3`", errors[0]) + self.assertIn(":3:", errors[0]) + + # Fenced code block exclusion ---------------------------------------- + + def test_version_in_fenced_code_block_ignored(self): + f = self._write( + "fence.md", + "Setup:\n" + "```bash\n" + "brew install archastro@0.3.1\n" + "```\n", + ) + self.assertEqual(self._check(f), []) + + def test_version_in_fenced_code_ignored_with_language(self): + # Fence opener may include a language tag. + f = self._write( + "lang.md", + "```python\n" + "VERSION = '0.3.1'\n" + "```\n", + ) + self.assertEqual(self._check(f), []) + + def test_mixed_prose_and_fenced_code(self): + # Prose version before the fence: fires. + # Version inside the fence: ignored. + # Prose version after the fence: fires. + f = self._write( + "mixed.md", + "Before 1.0.0 fence\n" + "```\n" + "inside 2.0.0 fence\n" + "```\n" + "After 3.0.0 fence\n", + ) + errors = self._check(f) + self.assertEqual(len(errors), 2) + self.assertTrue(any(":1:" in e and "1.0.0" in e for e in errors)) + self.assertTrue(any(":5:" in e and "3.0.0" in e for e in errors)) + + def test_unclosed_fence_skips_to_eof(self): + # An unclosed fence silently eats everything after it. Defensive + # behavior: better than crashing, and contributors usually close + # their fences. + f = self._write( + "unclosed.md", + "Before 1.0.0\n" + "```\n" + "inside 2.0.0\n" + "more lines 3.0.0\n", + ) + errors = self._check(f) + self.assertEqual(len(errors), 1) + self.assertIn("1.0.0", errors[0]) + + def test_tilde_fence_ignored(self): + # Markdown also supports `~~~` as a fence delimiter. + f = self._write( + "tilde.md", + "Setup:\n" + "~~~bash\n" + "install archastro@0.3.1\n" + "~~~\n", + ) + self.assertEqual(self._check(f), []) + + def test_unclosed_frontmatter_skips_whole_file(self): + # A file starting with `---` but lacking a closing `---` is + # treated as all-frontmatter. Lenient: better than crashing, and + # malformed frontmatter is a different class of defect that the + # contributor should fix at the file-format level. + f = self._write( + "unclosed-fm.md", + "---\n" + "name: test\n" + "version: 1.2.3\n", # no closing --- + ) + self.assertEqual(self._check(f), []) + + def test_fence_and_frontmatter_combined(self): + f = self._write( + "combo.md", + "---\n" + "name: test\n" + "version: 0.1.0\n" + "---\n" + "\n" + "Body version 1.2.3 in prose\n" + "```\n" + "Fenced 4.5.6\n" + "```\n" + "Trailing 7.8.9 prose\n", + ) + errors = self._check(f) + self.assertEqual(len(errors), 2) + self.assertTrue(any("1.2.3" in e for e in errors)) + self.assertTrue(any("7.8.9" in e for e in errors)) + self.assertFalse(any("0.1.0" in e for e in errors)) # frontmatter + self.assertFalse(any("4.5.6" in e for e in errors)) # fenced + + +class MainRunnerTest(unittest.TestCase): + """ + Tests for main() and the CHECKS registry. Monkey-patches CHECKS with + fixture lists so each test exercises exactly the shape it cares about; + one sanity test at the end asserts the real CHECKS registry has the + expected entries in the expected order. + """ + + def setUp(self) -> None: + self._original_checks = check_plugin_repo.CHECKS + + def tearDown(self) -> None: + check_plugin_repo.CHECKS = self._original_checks + + def _run_main(self) -> tuple[int, str, str]: + """Call main(), return (exit_code, stdout, stderr).""" + import io + import contextlib + stdout = io.StringIO() + stderr = io.StringIO() + with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr): + exit_code = check_plugin_repo.main() + return exit_code, stdout.getvalue(), stderr.getvalue() + + # Exit codes ---------------------------------------------------------- + + def test_all_checks_pass_returns_zero(self): + check_plugin_repo.CHECKS = [ + ("fixture-a", lambda: []), + ("fixture-b", lambda: []), + ] + exit_code, stdout, stderr = self._run_main() + self.assertEqual(exit_code, 0) + self.assertIn("OK [fixture-a]", stdout) + self.assertIn("OK [fixture-b]", stdout) + self.assertEqual(stderr, "") + + def test_any_check_fails_returns_one(self): + check_plugin_repo.CHECKS = [ + ("pass", lambda: []), + ("fail", lambda: ["something broke"]), + ] + exit_code, stdout, stderr = self._run_main() + self.assertEqual(exit_code, 1) + self.assertIn("OK [pass]", stdout) + self.assertIn("FAIL [fail]", stderr) + self.assertIn("something broke", stderr) + + def test_all_checks_fail_returns_one(self): + check_plugin_repo.CHECKS = [ + ("a", lambda: ["a broke"]), + ("b", lambda: ["b broke"]), + ] + exit_code, _, stderr = self._run_main() + self.assertEqual(exit_code, 1) + self.assertIn("FAIL [a]", stderr) + self.assertIn("FAIL [b]", stderr) + + def test_empty_checks_list_returns_zero(self): + # Degenerate case: no checks registered. Trivially passes. + check_plugin_repo.CHECKS = [] + exit_code, stdout, stderr = self._run_main() + self.assertEqual(exit_code, 0) + self.assertEqual(stdout, "") + self.assertEqual(stderr, "") + + # Ordering ------------------------------------------------------------ + + def test_checks_run_in_registry_order(self): + order: list[str] = [] + + def make_check(name: str): + def check() -> list[str]: + order.append(name) + return [] + return check + + check_plugin_repo.CHECKS = [ + ("first", make_check("first")), + ("second", make_check("second")), + ("third", make_check("third")), + ] + self._run_main() + self.assertEqual(order, ["first", "second", "third"]) + + def test_all_checks_run_even_after_a_failure(self): + # A failing check must not short-circuit the runner; later checks + # still run so their errors surface in the same report. + order: list[str] = [] + + def make_check(name: str, errs: list[str]): + def check() -> list[str]: + order.append(name) + return errs + return check + + check_plugin_repo.CHECKS = [ + ("a", make_check("a", ["a-err"])), + ("b", make_check("b", [])), + ("c", make_check("c", ["c-err"])), + ] + _, stdout, stderr = self._run_main() + self.assertEqual(order, ["a", "b", "c"]) + self.assertIn("FAIL [a]", stderr) + self.assertIn("FAIL [c]", stderr) + self.assertIn("OK [b]", stdout) + + # Output formatting --------------------------------------------------- + + def test_failed_check_output_routes_to_stderr(self): + check_plugin_repo.CHECKS = [("x", lambda: ["boom"])] + _, stdout, stderr = self._run_main() + self.assertIn("FAIL [x]:", stderr) + self.assertIn("boom", stderr) + # Failures do not leak into stdout. + self.assertNotIn("FAIL", stdout) + self.assertNotIn("boom", stdout) + + def test_passed_check_output_routes_to_stdout(self): + check_plugin_repo.CHECKS = [("y", lambda: [])] + _, stdout, stderr = self._run_main() + self.assertIn("OK [y]", stdout) + # Passes do not leak into stderr. + self.assertEqual(stderr, "") + + def test_multiline_error_each_line_indented(self): + # Errors with embedded newlines (e.g. check_manifest_consistency's + # multi-file reports) must have each line indented by 2 spaces under + # the header so the output stays legible. + check_plugin_repo.CHECKS = [ + ("multi", lambda: ["first line\nsecond line\nthird line"]), + ] + _, _, stderr = self._run_main() + self.assertIn(" first line", stderr) + self.assertIn(" second line", stderr) + self.assertIn(" third line", stderr) + + def test_multiple_errors_from_single_check_all_printed(self): + check_plugin_repo.CHECKS = [ + ("x", lambda: ["err1", "err2", "err3"]), + ] + _, _, stderr = self._run_main() + self.assertIn("err1", stderr) + self.assertIn("err2", stderr) + self.assertIn("err3", stderr) + + # Real CHECKS registry sanity ---------------------------------------- + + def test_real_checks_registry_has_expected_entries(self): + # The real CHECKS list should contain all 5 checks in the expected + # order. This guards against accidental removal or reordering that + # would break invariants documented in the comment above CHECKS. + names = [name for name, _ in self._original_checks] + self.assertEqual( + names, + [ + "manifest consistency", + "compat key refs", + "slash command refs", + "hardcoded versions", + "version bump on content change", + ], + ) + + def test_manifest_consistency_runs_before_version_bump(self): + # version-bump-on-content-change assumes check_manifest_consistency + # has already validated cross-file version agreement, so it can + # compare just one manifest against the PR base. Enforce the ordering + # invariant so a future reorder can't silently break it. + names = [name for name, _ in self._original_checks] + mc_idx = names.index("manifest consistency") + vb_idx = names.index("version bump on content change") + self.assertLess( + mc_idx, + vb_idx, + "manifest consistency must run before version-bump check", + ) + + def test_every_registered_check_is_callable(self): + for name, check in self._original_checks: + self.assertTrue( + callable(check), + f"CHECKS entry {name!r} is not callable", + ) + + +class VersionBumpOnContentChangeTest(unittest.TestCase): + """Tests for check_version_bump_on_content_change(). + + Each test sets up a real git repo in a tmpdir with an initial commit + (base state), then the test body applies whatever changes it wants + and runs the check against `base_ref=HEAD~1`. This exercises the real + git shell-out path rather than mocking it. + """ + + def setUp(self) -> None: + self._tmp = tempfile.TemporaryDirectory() + self.tmp = Path(self._tmp.name) + + self.marketplace_path = self.tmp / ".claude-plugin" / "marketplace.json" + self.marketplace_path.parent.mkdir() + self.sources_dir = self.tmp / "sources" + self.sources_dir.mkdir() + self.skills_dir = ( + self.tmp / ".claude-plugins" / "archastro" / "skills" + ) + self.skills_dir.mkdir(parents=True) + + # Initialize a fresh git repo and make the base commit at 0.1.0. + self._git("init", "-q", "-b", "main") + self._git("config", "user.email", "test@example.com") + self._git("config", "user.name", "Test") + # Environment overrides are also set for commit authorship so the + # test doesn't depend on the developer's global git config. + self._write_marketplace("0.1.0") + self._write_source("baseline.md", "# baseline\n") + self._git("add", ".") + self._git("commit", "-q", "-m", "base state") + + def tearDown(self) -> None: + self._tmp.cleanup() + + def _git(self, *args: str) -> subprocess.CompletedProcess: + """Run `git -C `. Test envs force a clean git identity.""" + env = os.environ.copy() + env.update( + { + "GIT_AUTHOR_NAME": "Test", + "GIT_AUTHOR_EMAIL": "test@example.com", + "GIT_COMMITTER_NAME": "Test", + "GIT_COMMITTER_EMAIL": "test@example.com", + } + ) + return subprocess.run( + ["git", "-C", str(self.tmp), *args], + check=True, + capture_output=True, + text=True, + env=env, + ) + + def _write_marketplace(self, version: str) -> None: + self.marketplace_path.write_text( + json.dumps( + { + "plugins": [ + {"name": "archastro", "version": version} + ] + } + ) + ) + + def _write_source(self, name: str, text: str) -> None: + (self.sources_dir / name).write_text(text) + + def _write_skill(self, name: str, text: str) -> None: + skill_dir = self.skills_dir / name + skill_dir.mkdir(parents=True, exist_ok=True) + (skill_dir / "SKILL.md").write_text(text) + + def _commit(self, message: str) -> None: + self._git("add", "-A") + self._git("commit", "-q", "-m", message) + + def _check(self, base_ref: str = "HEAD~1") -> list[str]: + return check_plugin_repo.check_version_bump_on_content_change( + marketplace_path=self.marketplace_path, + base_ref=base_ref, + repo_root=self.tmp, + ) + + # Happy path ---------------------------------------------------------- + + def test_no_changes_passes(self): + # HEAD == HEAD~1 has no diff at the skin level but git's `diff + # base...HEAD` still returns empty when there's nothing to compare. + # We create an empty second commit to model "nothing changed". + self._git("commit", "-q", "--allow-empty", "-m", "empty") + self.assertEqual(self._check(), []) + + def test_content_change_with_version_bump_passes(self): + self._write_source("baseline.md", "# baseline\n\nUpdated content.\n") + self._write_marketplace("0.1.1") + self._commit("update content and bump version") + self.assertEqual(self._check(), []) + + def test_non_content_change_without_bump_passes(self): + # README changes don't require a bump — not plugin content. + (self.tmp / "README.md").write_text("# repo readme\n") + self._commit("add readme") + self.assertEqual(self._check(), []) + + def test_scripts_change_without_bump_passes(self): + # scripts/ is infra, not plugin content. + scripts = self.tmp / "scripts" + scripts.mkdir() + (scripts / "helper.sh").write_text("#!/bin/sh\n") + self._commit("add helper script") + self.assertEqual(self._check(), []) + + def test_manifest_only_change_passes(self): + # Bumping the manifest without touching content is fine — maybe a + # metadata update, maybe preparation for a release with no content + # diff yet. No content changed → no bump required → pass. + self._write_marketplace("0.2.0") + self._commit("bump version without touching content") + self.assertEqual(self._check(), []) + + # Failure cases -------------------------------------------------------- + + def test_source_change_without_bump_fails(self): + self._write_source("baseline.md", "# baseline\n\nEdited body.\n") + self._commit("edit source without bumping version") + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("plugin content changed", errors[0]) + self.assertIn("still '0.1.0'", errors[0]) + self.assertIn("sources/baseline.md", errors[0]) + + def test_new_source_file_without_bump_fails(self): + self._write_source("new-skill.md", "# new skill\n") + self._commit("add new source") + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("sources/new-skill.md", errors[0]) + + def test_skill_tree_change_without_bump_fails(self): + # Changes under .claude-plugins/archastro/skills/ are content. + self._write_skill("chat", "---\nname: chat\n---\n\nbody\n") + self._commit("add skill") + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn(".claude-plugins/archastro/skills/chat/SKILL.md", errors[0]) + + def test_multiple_content_files_listed_in_error(self): + self._write_source("a.md", "# a\n") + self._write_source("b.md", "# b\n") + self._commit("add two sources") + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("sources/a.md", errors[0]) + self.assertIn("sources/b.md", errors[0]) + + def test_mixed_content_and_docs_still_fails(self): + # One content file + one non-content file, no bump → still fails + # (one content change is enough to require a bump). + self._write_source("content.md", "# content\n") + (self.tmp / "README.md").write_text("# readme\n") + self._commit("mixed change") + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("sources/content.md", errors[0]) + self.assertNotIn("README.md", errors[0]) + + def test_deleted_content_file_without_bump_fails(self): + # Deleting a skill file is a content change and requires a bump. + (self.sources_dir / "baseline.md").unlink() + self._commit("delete baseline") + errors = self._check() + self.assertEqual(len(errors), 1) + self.assertIn("sources/baseline.md", errors[0]) + + # Skip conditions ------------------------------------------------------ + + def test_unreachable_base_ref_skips_with_warning(self): + # Skip returns empty (pass), but prints a warning to stderr so a + # misconfigured CI doesn't silently no-op the check. + import io + import contextlib + stderr = io.StringIO() + with contextlib.redirect_stderr(stderr): + errors = check_plugin_repo.check_version_bump_on_content_change( + marketplace_path=self.marketplace_path, + base_ref="nonexistent-branch", + repo_root=self.tmp, + ) + self.assertEqual(errors, []) + self.assertIn("WARN", stderr.getvalue()) + self.assertIn("unreachable", stderr.getvalue()) + self.assertIn("fetch-depth: 0", stderr.getvalue()) + + def test_git_subprocess_timeout_skips_with_warning(self): + # If a git call hangs (slow NFS, credential prompt, stale lock), + # the subprocess timeout fires, the check warns and skips rather + # than blocking CI for the default 6-hour step timeout. + import io + import contextlib + import subprocess as sp + from unittest.mock import patch + + def fake_run(*args, **kwargs): + raise sp.TimeoutExpired(cmd=args[0] if args else [], timeout=30) + + stderr = io.StringIO() + with ( + contextlib.redirect_stderr(stderr), + patch("check_plugin_repo.subprocess.run", side_effect=fake_run), + ): + errors = check_plugin_repo.check_version_bump_on_content_change( + marketplace_path=self.marketplace_path, + base_ref="HEAD~1", + repo_root=self.tmp, + ) + self.assertEqual(errors, []) + self.assertIn("WARN", stderr.getvalue()) + self.assertIn("timed out", stderr.getvalue()) + + def test_base_without_marketplace_treats_as_implicit_bump(self): + # If marketplace.json didn't exist at the base ref (e.g. the file + # was added in this PR), there's no base version to compare against + # and the "current version" is definitionally new. Skip with no + # error — the contributor is bootstrapping the plugin. + self.marketplace_path.unlink() + self._git("add", "-A") + self._git("commit", "-q", "-m", "remove marketplace") + # Re-create marketplace and touch content — the base (HEAD~1) has no + # marketplace, so the check should treat this as an implicit bump. + self._write_marketplace("0.1.0") + self._write_source("baseline.md", "# baseline\n\nchange\n") + self._commit("add marketplace back with content change") + # base=HEAD~1 is the "remove marketplace" commit; marketplace.json + # does not exist there, so the show fails and we skip cleanly. + self.assertEqual(self._check(), []) + + def test_push_to_main_scenario_trivially_passes(self): + # Simulates a push-to-main where HEAD has already advanced past the + # nominal base. The diff against HEAD itself is empty → pass. + self._write_source("baseline.md", "# baseline\n\nupdated\n") + self._write_marketplace("0.1.1") + self._commit("content update") + # Diff HEAD...HEAD is empty. + errors = check_plugin_repo.check_version_bump_on_content_change( + marketplace_path=self.marketplace_path, + base_ref="HEAD", + repo_root=self.tmp, + ) + self.assertEqual(errors, []) + + +if __name__ == "__main__": + unittest.main()