Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 115 additions & 0 deletions .github/scripts/check_repo.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
#!/usr/bin/env python3
"""Check that the marketplace, skills, Markdown and data files in this repository are valid."""

import json
import re
import sys
from pathlib import Path

import yaml

SKIP_DIRS = {".git", "node_modules", "__pycache__", ".venv"}
FENCE = re.compile(r"^ {0,3}(`{3,}|~{3,})(.*)$")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 (optional) Maintainers get no CI failure for an unclosed code block nested in a list item, and link-like text inside such a block can fail CI as a broken link. FENCE at .github/scripts/check_repo.py:12 only accepts 0-3 leading spaces, so a fence indented 4 or more spaces is treated as prose by split_fences. Fix: recognise fences at list-item indentation for both the opening and the closing line, for example ^\s* in place of ^ {0,3}, and add a test with an indented fence. The repo already has one such block at clinical-trial-forecast/SKILL.md:59-77.

Why this was flagged

Trigger: a Markdown file with a fenced block inside a list item, indented 4 or more spaces, reached through check_markdown (.github/scripts/check_repo.py:39) from the CI step python3 .github/scripts/check_repo.py. FENCE = re.compile(r"^ {0,3}({3,}|~{3,})(.*)$")at line 12 does not match those lines, sosplit_fences(lines 24-36) never setsmarkerand appends the fence lines and everything between them tooutside. Two results follow. An opening fence with no closing fence in a list item is never reported as "code block is never closed", which is the defect class this PR fixes by hand in 04-protocol-operations.md. And text such as xinside the block is passed toLINK.findall` at line 47 and reported as "broken link to gone.md", failing CI on valid content. clinical-trial-forecast/SKILL.md:59 and :77 are fences indented 5 spaces under list item 3, so lines 60-76 are already scanned as prose; they hold no links today, so nothing fails yet. The base branch has no check at all. No test in test_check_repo.py covers an indented fence.

Verification: Triggers when a Markdown file has a fenced block indented 4 or more spaces inside a list item and that block is either left unclosed or contains link-like text with a path target. In .github/scripts/check_repo.py, line 12 FENCE only matches fences with 0-3 leading spaces. The base has no checker at all. test_check_repo.py has no indented-fence case.

Finding ID: f-67d567f9

LINK = re.compile(r"\[[^\]]*\]\(([^)\s]+)(?:\s+\"[^\"]*\")?\)")
INLINE_CODE = re.compile(r"`[^`]*`")
SKILL_NAME = re.compile(r"[a-z0-9]+(-[a-z0-9]+)*")


def repo_files(root, *suffixes):
for path in sorted(root.rglob("*")):
if path.is_file() and path.suffix in suffixes and not SKIP_DIRS & set(path.relative_to(root).parts):
yield path


def split_fences(text):
"""Return the lines outside code blocks, and the line number of a block left open, if any."""
outside, opened, marker = [], None, None
for number, line in enumerate(text.splitlines(), 1):
match = FENCE.match(line)
if marker is None:
if match:
opened, marker = number, match.group(1)
else:
outside.append(line)
elif match and match.group(1)[0] == marker[0] and len(match.group(1)) >= len(marker) and not match.group(2).strip():
opened, marker = None, None
return outside, opened


def check_markdown(root):
problems = []
for path in repo_files(root, ".md"):
name = path.relative_to(root)
outside, opened = split_fences(path.read_text(encoding="utf-8"))
if opened:
problems.append(f"{name}:{opened}: code block is never closed")
for line in outside:
for target in LINK.findall(INLINE_CODE.sub("", line)):
target = target.split("#")[0]
is_path = "/" in target or "." in target # a bare word such as (url) is a format example
if is_path and not re.match(r"[a-z][a-z0-9+.-]*:", target) and not (path.parent / target).exists():
Comment on lines +48 to +50

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 (optional) Contributors get a false "broken link to …" CI failure for Markdown links that GitHub renders correctly. At .github/scripts/check_repo.py:50 the raw link text is joined to the file's folder and tested with exists(). A root-relative link such as /README.md is looked up at the filesystem root, and my%20file.md or file.md?plain=1 are looked up literally. Fix: normalise the target before the existence test so every valid GitHub link form resolves to the file it names: drop ?query, percent-decode, resolve a leading / against root, and match the scheme case-insensitively. No file in the repository uses these forms today.

Why this was flagged

Trigger: a pull request adds a Markdown link in one of these forms outside a code block: [x](/README.md), [x](my%20file.md), [x](file.md?plain=1) or [x](HTTPS://example.com). It arrives through check_markdown (.github/scripts/check_repo.py:39), called from main at line 107 and run by the CI step at .github/workflows/ci.yml:25. Line 48 strips only the #fragment. Line 50 evaluates (path.parent / target).exists(): joining an absolute target discards path.parent, so /README.md is tested at the filesystem root; %20 and ?plain=1 stay in the file name; the scheme test re.match(r"[a-z][a-z0-9+.-]*:", target) is case-sensitive, so an upper-case scheme is treated as a path. Line 51 appends broken link to ..., main returns 1 and the job fails although the link is valid on GitHub. The base branch has no link check. The existing tests (.github/scripts/test_check_repo.py:54-60) cover only plain relative paths, lower-case web links, anchors and bare words, so none of these forms is exercised.

Verification: Triggers when a Markdown file outside a code block contains a root-relative, percent-encoded, query-suffixed or upper-case-scheme link; no file in the tree at HEAD does today, so CI is not currently broken. In .github/scripts/check_repo.py, line 48, target = target.split("#")[0], is the only normalisation. The tests in .github/scripts/test_check_repo.py (lines 50-60) pin none of these forms.

Finding ID: f-f6b61711

problems.append(f"{name}: broken link to {target}")
return problems


def check_data_files(root):
problems = []
for path in repo_files(root, ".json"):
try:
json.loads(path.read_text(encoding="utf-8"))
except ValueError as error:
problems.append(f"{path.relative_to(root)}: invalid JSON: {error}")
for path in repo_files(root, ".yaml", ".yml"):
try:
list(yaml.safe_load_all(path.read_text(encoding="utf-8")))
except yaml.YAMLError as error:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 (optional) A contributor who adds a YAML file with an impossible date, or a .md/.yaml file that is not UTF-8, gets a bare traceback and loses every other reported problem. check_repo.py:65 catches only yaml.YAMLError. PyYAML raises a plain ValueError for a scalar such as 2024-02-30, and read_text at check_repo.py:63 and :43 raises UnicodeDecodeError. The JSON loop at :60 already catches ValueError, so the two loops disagree. Fix: every per-file read and parse must turn its failure into a problem line; catch (yaml.YAMLError, ValueError) at :65 and guard the read at :43.

Why this was flagged

The trigger is a data file such as released: 2024-02-30 in any .yaml/.yml, or a Latin-1 encoded .md or .yaml, added by a pull request and reached through main at .github/scripts/check_repo.py:107. yaml.safe_load_all at check_repo.py:64 builds a datetime.date for the timestamp tag and that constructor raises ValueError, which is not a yaml.YAMLError. read_text(encoding="utf-8") at check_repo.py:43 and :63 raises UnicodeDecodeError, also outside the handler. The exception escapes check_data_files or check_markdown, so the loop at check_repo.py:109-111 never prints the problems already collected by check_marketplace, and the traceback does not name the offending file. The JSON branch at check_repo.py:60 handles both cases with except ValueError. This is a different site and input from the filed check_skill item at :82. No such file exists at HEAD. Remedy: widen the handler and wrap the reads.

Verification: Triggers when a PR adds a non-UTF-8 .md/.yaml/.yml file, or a YAML file with an impossible date scalar. In .github/scripts/check_repo.py, line 63-65 is guarded only by except yaml.YAMLError, so UnicodeDecodeError escapes. An escaping exception discards every problem already collected and prints only a traceback. The test suite does not exercise these cases.

Finding ID: f-09e6e459

problems.append(f"{path.relative_to(root)}: invalid YAML: {error}")
return problems


def check_skill(root, folder):
skill = root / folder / "SKILL.md"
if not skill.is_file():
return [f"{folder}: listed in the marketplace but has no SKILL.md"]
match = re.match(r"---\n(.*?)\n---\n", skill.read_text(encoding="utf-8"), re.S)
if not match:
return [f"{folder}/SKILL.md: missing frontmatter"]
try:
fields = yaml.safe_load(match.group(1))
except yaml.YAMLError as error:
return [f"{folder}/SKILL.md: invalid frontmatter: {error}"]
problems = []
if not SKILL_NAME.fullmatch(str(fields.get("name", ""))):
problems.append(f"{folder}/SKILL.md: name must be lower-case letters, digits and hyphens")
if not fields.get("description"):
problems.append(f"{folder}/SKILL.md: description is missing")
if not (root / folder / "LICENSE.txt").is_file():
problems.append(f"{folder}: LICENSE.txt is missing")
return problems


def check_marketplace(root):
marketplace = json.loads((root / ".claude-plugin" / "marketplace.json").read_text(encoding="utf-8"))
listed = {
str(Path(skill)) for plugin in marketplace["plugins"] for skill in plugin.get("skills", [])
}
problems = []
for folder in sorted(listed):
problems += check_skill(root, folder)
for skill in sorted(root.glob("*/SKILL.md")):
if skill.parent.name not in listed:
problems.append(f"{skill.parent.name}: has a SKILL.md but is not in the marketplace")
return problems


def main(root="."):
root = Path(root).resolve()
problems = check_marketplace(root) + check_markdown(root) + check_data_files(root)
for problem in problems:
print(problem)
print(f"{len(problems)} problem(s) found")
return 1 if problems else 0


if __name__ == "__main__":
sys.exit(main(*sys.argv[1:]))
91 changes: 91 additions & 0 deletions .github/scripts/test_check_repo.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
"""Tests for check_repo.py. Run with: python3 -m unittest discover -s .github/scripts"""

import json
import tempfile
import unittest
from pathlib import Path

import check_repo

SKILL = "---\nname: good-skill\ndescription: Does a thing.\n---\n\n# Good skill\n"


class CheckRepoTest(unittest.TestCase):
def setUp(self):
folder = tempfile.TemporaryDirectory()
self.addCleanup(folder.cleanup)
self.root = Path(folder.name)
self.write("good-skill/SKILL.md", SKILL)
self.write("good-skill/LICENSE.txt", "Apache 2.0")
self.write("README.md", "# Title\n\n[Skill](good-skill/SKILL.md)\n\n```bash\nls\n```\n")
marketplace = {"plugins": [{"name": "good-skill", "skills": ["./good-skill"]}]}
self.write(".claude-plugin/marketplace.json", json.dumps(marketplace))

def write(self, name, text):
path = self.root / name
path.parent.mkdir(parents=True, exist_ok=True)
path.write_text(text, encoding="utf-8")

def problems(self):
return (
check_repo.check_marketplace(self.root)
+ check_repo.check_markdown(self.root)
+ check_repo.check_data_files(self.root)
)

def assert_finds(self, text):
self.assertTrue(any(text in problem for problem in self.problems()), self.problems())

def test_valid_repository_passes(self):
self.assertEqual(self.problems(), [])

def test_finds_unclosed_code_block(self):
self.write("README.md", "# Title\n\n```bash\nls\n")
self.assert_finds("README.md:3: code block is never closed")

def test_longer_fence_can_hold_a_shorter_one(self):
self.write("README.md", "````\n```\ninside\n```\n````\n")
self.assertEqual(self.problems(), [])

def test_finds_broken_link(self):
self.write("README.md", "[Gone](missing/file.md)\n")
self.assert_finds("broken link to missing/file.md")

def test_ignores_links_in_code_web_links_and_anchors(self):
self.write("README.md", "```\n[x](nope.md)\n```\n`[y](nope.md)` [web](https://a.b) [top](#title)\n")
self.assertEqual(self.problems(), [])

def test_ignores_format_example_links(self):
self.write("README.md", "Cite as [title](url).\n")
self.assertEqual(self.problems(), [])

def test_finds_invalid_json_and_yaml(self):
self.write("good-skill/data.json", "{")
self.write("good-skill/data.yaml", "a: [")
self.assert_finds("invalid JSON")
self.assert_finds("invalid YAML")

def test_reads_yaml_with_several_documents(self):
self.write("good-skill/two.yaml", "a: 1\n---\nb: 2\n")
self.assertEqual(self.problems(), [])

def test_finds_listed_skill_without_skill_file(self):
(self.root / "good-skill/SKILL.md").unlink()
self.assert_finds("has no SKILL.md")

def test_finds_skill_missing_from_marketplace(self):
self.write("stray/SKILL.md", SKILL)
self.assert_finds("stray: has a SKILL.md but is not in the marketplace")

def test_finds_bad_frontmatter(self):
self.write("good-skill/SKILL.md", "---\nname: Bad Name\n---\n")
self.assert_finds("name must be")
self.assert_finds("description is missing")

def test_finds_missing_license(self):
(self.root / "good-skill/LICENSE.txt").unlink()
self.assert_finds("LICENSE.txt is missing")


if __name__ == "__main__":
unittest.main()
36 changes: 36 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
name: CI

on:
pull_request:
push:
branches: [main]

permissions:
contents: read

jobs:
checks:
runs-on: ubuntu-latest
timeout-minutes: 10
steps:
- uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Every CI run may fail before any step runs, or run unreviewed code, if the pinned actions/checkout commit is not an upstream v4 release. .github/workflows/ci.yml:16 pins 11d5960a326750d5838078e36cf38b85af677262 with the comment # v4. That hash is not the v4.2.2 tag commit (11bd71901bbe5b1630ceea73d27597364c9af683), and it could not be confirmed against any v4 tag from this checkout (no network). Fix: pin the full commit SHA that an actions/checkout v4 release tag points to, and name the exact version in the comment (for example # v4.2.2) so reviewers and Dependabot can check it.

Why this was flagged

Condition: the SHA 11d5960a326750d5838078e36cf38b85af677262 at .github/workflows/ci.yml:16 is not a commit reachable in actions/checkout; this could not be checked from the checkout because remote access is refused. The workflow triggers on every pull_request and on push to main (.github/workflows/ci.yml:3-6). If the SHA does not exist, GitHub fails the job during set-up with "Unable to resolve action", so none of the new checks run and every PR shows a red CI. If the SHA exists only in a fork of the action's repository network, the job runs code that is not an upstream release while the comment says # v4. The base branch has no ci.yml; its other workflows use actions/checkout@ v4 (.github/workflows/claude.yml:29, .github/workflows/release.yml:16), which always resolves. No safeguard applies: the # v4 comment is not validated, and Dependabot (.github/dependabot.yml:3) only proposes later bumps. The neighbouring pin actions/setup-python@ a26af69be951a213d495a4c3e4e4022e16d87065 at line 17 does match the known v5.6.0 commit.

Verification: .github/workflows/ci.yml:16 is - uses: actions/checkout@ 11d5960a326750d5838078e36cf38b85af677262 # v4, the first step of the only job (checks, lines 12-15). The workflow runs on every pull_request and on push to main (lines 3-6). An unresolvable SHA fails the job there, so none of the steps on lines 17-36 would ever execute. No guard exists: there is no fallback ref.

Finding ID: f-556774cf

- uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5
with:
python-version: "3.12"
- name: Install tools
run: pip install ruff==0.14.10 pyyaml==6.0.2
- name: Test the repository checks
run: python3 -m unittest discover -s .github/scripts
- name: Check marketplace, skills, Markdown and data files
run: python3 .github/scripts/check_repo.py
- name: Validate the plugin marketplace
run: npx --yes @anthropic-ai/claude-code@2.1.310 plugin validate .
- name: Lint Python
run: ruff check .
- name: Lint shell scripts
run: find . -name '*.sh' -not -path './.git/*' -exec shellcheck --severity=warning {} +
- name: Run skill self-tests
run: |
grep -l -- '--self-test' */scripts/*.py | while read -r script; do
python3 "$script" --self-test
done
Comment on lines +34 to +36

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 (optional) Maintainers get a green "Run skill self-tests" step even when no self-test ran. At .github/workflows/ci.yml:34 grep -l feeds a while read loop, and the step sets no shell:, so it runs as bash -e without pipefail. If grep matches nothing or errors, the loop runs zero times and the step exits 0. Fix: make the step fail when grep fails or the discovered list is empty, for example shell: bash on the step (which adds -o pipefail) or collecting the matches first and testing they are non-empty. Today only binder-gauntlet/scripts/sequence_profile.py matches, so one rename or move silently drops all coverage.

Why this was flagged

Trigger: a later change moves or renames binder-gauntlet/scripts/sequence_profile.py out of the */scripts/*.py glob, nests it one level deeper, or renames its --self-test flag (binder-gauntlet/scripts/sequence_profile.py:249); it is the only file that matches today. The step at .github/workflows/ci.yml:33-36 has no shell: key and the workflow has no defaults.run.shell, so GitHub runs it as bash -e {0}, which does not set pipefail. grep -l then exits 1 (no match) or 2 (unmatched glob passed literally), but the pipeline status is that of the while loop, which ran zero iterations and returns 0. The step is reported as passed with no self-test executed. The base branch has no CI, so the new harm is a check that reports success without having run. bash -e does not help: errexit ignores a failure in a non-last pipeline element. A failing self-test inside the loop does still fail the step; only the zero-match case is open.

Verification: Triggers only when a later change leaves no */scripts/*.py file containing --self-test; nothing fails at HEAD. In .github/workflows/ci.yml lines 32-36 the step has no shell: key, so it runs as bash -e {0} with no pipefail. If grep exits 1 or 2, the loop reads nothing and returns 0, so the step is green with zero self-tests run. The base had no CI at all.

Finding ID: f-7e6f2144

Loading
Loading