diff --git a/changes/pstack-t3%2Fd176.md b/changes/pstack-t3%2Fd176.md new file mode 100644 index 00000000..065dd371 --- /dev/null +++ b/changes/pstack-t3%2Fd176.md @@ -0,0 +1 @@ +- `scripts/sync_upstream.py` rewrites only the README upstream badge url and alt version spans when it writes `upstream.json` and preserves every other byte and line ending. It exits 1 before that write when the badge line is missing or not that one line. It exits 1 before that write when the new version is not a dotted number and names that version as not carriable by the badge. `scripts/check.py` names `README.md` and both versions when the badge url or alt differs from the `upstream.json` version. It names `README.md` and that version when the badge is absent. That check runs at the default CLI and in the build and not in `check_tree`. The build fails on that finding. diff --git a/scripts/build.py b/scripts/build.py index 59e50d7c..75e70231 100644 --- a/scripts/build.py +++ b/scripts/build.py @@ -124,10 +124,10 @@ def build(destination, update_lock=False, skip_lock=False): staged = Path(temporary) / "skills" render(staged) sys.path.insert(0, str(ROOT / "scripts")) - from check import check_tree + from check import badge_findings, check_tree from catalog import write - findings = check_tree(staged) + findings = check_tree(staged) + badge_findings(ROOT / "README.md", ROOT / "upstream.json") if findings: raise SystemExit("check failed:\n " + "\n ".join(findings)) replace_tree(staged, destination) diff --git a/scripts/check.py b/scripts/check.py index 6ce9b999..e57d7681 100644 --- a/scripts/check.py +++ b/scripts/check.py @@ -1,8 +1,12 @@ #!/usr/bin/env python3 -"""Lint a generated skills tree for Cursor-only leftovers and broken structure, and the test modules for the git environment fence.""" +"""Lint a generated skills tree for Cursor-only leftovers and broken structure, and the test modules for the git environment fence. + +check_tree judges only the tree it is given. badge_findings reports a README upstream badge that is missing or that disagrees with the version in upstream.json. The default CLI run and the build add that repository check. An explicit tree argument gets no badge finding. +""" import ast import importlib.util +import json import re import sys @@ -51,6 +55,8 @@ ROLES_PY = Path(__file__).resolve().parents[1] / "t3/scripts/roles.py" TESTS_DIR = Path(__file__).resolve().parents[1] / "tests" +README_PATH = Path(__file__).resolve().parents[1] / "README.md" +UPSTREAM_PATH = Path(__file__).resolve().parents[1] / "upstream.json" # Every tests/test_*.py must carry fence_git_environment as it is in the reference module, and a setUpModule that contains a call to it. FENCE_REFERENCE = "test_landing.py" FENCE_FUNCTION = "fence_git_environment" @@ -58,6 +64,104 @@ LIGHT_HEADER_CELLS = frozenset({"light", "light mode", "light behavior", "in light mode"}) LIGHT_MODE_NAME = re.compile(r"(?i:\blight mode\b)|Mode: light") TABLE_SEPARATOR = re.compile(r"\|?\s*:?-{3,}:?\s*(\|\s*:?-{3,}:?\s*)*\|?") +_VERSION_TOKEN = r"[0-9]+(?:\.[0-9]+)+" +_BADGE_LINE = re.compile( + "(?P[ \t]*)" + '" + _VERSION_TOKEN + ")" + '-555\\.svg" alt="upstream pstack ' + "(?P" + _VERSION_TOKEN + ")" + '">' + "(?P[ \t]*)" +) + + +def _split_lines(text): + parts = [] + for raw in text.splitlines(keepends=True): + if raw.endswith("\r\n"): + parts.append((raw[:-2], "\r\n")) + elif raw.endswith("\n") or raw.endswith("\r"): + parts.append((raw[:-1], raw[-1:])) + else: + parts.append((raw, "")) + return parts + + +def _hits(text): + hits = [] + for content, _ending in _split_lines(text): + match = _BADGE_LINE.fullmatch(content) + if match: + hits.append(match) + return hits + + +def _readme_text(path): + try: + if not path.is_file(): + return None + with path.open("r", encoding="utf-8", newline="") as handle: + return handle.read() + except (OSError, UnicodeDecodeError): + return None + + +def _pin_version(path): + try: + if not path.is_file(): + return None + payload = json.loads(path.read_text(encoding="utf-8")) + except (OSError, UnicodeDecodeError, json.JSONDecodeError): + return None + if not isinstance(payload, dict): + return None + version = payload.get("version") + if not isinstance(version, str) or version == "" or "\n" in version: + return None + return version + + +def rewritten_readme(path, version): + if not isinstance(version, str) or re.fullmatch(_VERSION_TOKEN, version) is None: + return None + text = _readme_text(path) + if text is None: + return None + parts = _split_lines(text) + hit_index = None + hit_match = None + for index, (content, _ending) in enumerate(parts): + match = _BADGE_LINE.fullmatch(content) + if match: + if hit_index is not None: + return None + hit_index = index + hit_match = match + if hit_index is None or hit_match is None: + return None + content, ending = parts[hit_index] + url_start, url_end = hit_match.span("url") + alt_start, alt_end = hit_match.span("alt") + content = content[:alt_start] + version + content[alt_end:] + content = content[:url_start] + version + content[url_end:] + parts[hit_index] = (content, ending) + return "".join(content + ending for content, ending in parts) + + +def badge_findings(readme, upstream): + pin = _pin_version(upstream) + if pin is None: + return ["upstream.json has no version"] + text = _readme_text(readme) + hits = _hits(text) if text is not None else [] + if len(hits) != 1: + return [f"README.md has no upstream badge and upstream.json version is {pin}"] + url = hits[0].group("url") + alt = hits[0].group("alt") + if url == pin and alt == pin: + return [] + return [f"README.md badge url {url} alt {alt} disagrees with upstream.json version {pin}"] _catalog_module = None @@ -238,6 +342,8 @@ def fence_findings(tests_dir): def main(argv): tree = Path(argv[0]) if argv else Path(__file__).resolve().parents[1] / "skills" problems = check_tree(tree) + fence_findings(TESTS_DIR) + if not argv: + problems += badge_findings(README_PATH, UPSTREAM_PATH) print("\n".join(problems) or "ok") return 1 if problems else 0 diff --git a/scripts/sync_upstream.py b/scripts/sync_upstream.py index 5c68535c..eb19e39a 100644 --- a/scripts/sync_upstream.py +++ b/scripts/sync_upstream.py @@ -19,6 +19,11 @@ are written before the sync writes anything in the checkout, and under --dry-run too. DIR is refused when it resolves to the checkout or a path under it, and when it exists. DIR is not created when no override conflicts. + +When the sync writes upstream.json it also rewrites the README upstream badge +so the url and the alt text carry the new version. When the badge line is +missing or not in the expected form, the sync prints one line naming README.md +and exits with status 1 before it writes upstream.json or replaces vendor/pstack. """ import argparse @@ -342,8 +347,7 @@ def main(): meta = json.loads(UPSTREAM.read_text()) if args.check: return check(args, meta) - if args.merge: - sys.dont_write_bytecode = True + sys.dont_write_bytecode = True with tempfile.TemporaryDirectory(prefix="pstack-upstream-") as temporary: try: checkout = Path(temporary) / "plugins" @@ -357,6 +361,17 @@ def main(): import build outcomes = () + if not args.dry_run: + import check as readme_check + + version = plugin.get("version", meta.get("version")) + if not isinstance(version, str) or re.fullmatch(r"[0-9]+(?:\.[0-9]+)+", version) is None: + print(f"upstream version {version} cannot be carried by the README upstream badge", file=sys.stderr) + return 1 + new_readme = readme_check.rewritten_readme(ROOT / "README.md", version) + if new_readme is None: + print("README.md has no upstream badge in the expected form", file=sys.stderr) + return 1 if args.merge: lock = json.loads(build.LOCK.read_text()) if build.LOCK.exists() else {} outcomes = plan_merges(build.T3 / "overrides", build.VENDOR / "skills", source / "skills", lock) @@ -376,7 +391,8 @@ def main(): shutil.rmtree(source / ".git", ignore_errors=True) build.replace_tree(source, ROOT / "vendor/pstack") previous = meta["commit"] - meta.update(commit=commit, version=plugin.get("version", meta.get("version"))) + meta.update(commit=commit, version=version) + (ROOT / "README.md").write_bytes(new_readme.encode("utf-8")) UPSTREAM.write_text(json.dumps(meta, indent=2) + "\n") print(f"vendor/pstack: {previous[:12]} -> {commit[:12]} ({meta['version']})") if args.merge: diff --git a/tests/test_build.py b/tests/test_build.py index 1a0aad70..e0d47433 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -6,8 +6,12 @@ import tempfile import unittest from pathlib import Path +from unittest import mock ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(ROOT / "scripts")) + +import check # noqa: E402 CASES = ( ("t3/added/brigade/scripts/brigade.py", "brigade/scripts/brigade.py", 0o644), @@ -83,6 +87,18 @@ def test_built_files_keep_source_modes(self): self.assertEqual(source_mode, expected, source_rel) self.assertEqual(built_mode, expected, output_rel) + def test_build_fails_when_the_repository_badge_is_stale(self): + import build + + with tempfile.TemporaryDirectory() as directory: + out = Path(directory) / "skills" + stale = ["README.md badge url 0.0.0 alt 0.0.0 disagrees with upstream.json version 1.1.0"] + with mock.patch.object(check, "badge_findings", return_value=stale): + with self.assertRaises(SystemExit) as raised: + build.build(out) + self.assertIn("check failed", str(raised.exception)) + self.assertIn(stale[0], str(raised.exception)) + class BuildReferenceTest(unittest.TestCase): def test_default_build_writes_the_committed_command_line_reference_pages(self): diff --git a/tests/test_check.py b/tests/test_check.py index d39cdc0e..8c8f1895 100644 --- a/tests/test_check.py +++ b/tests/test_check.py @@ -1,7 +1,9 @@ import contextlib import inspect import io +import json import os +import shutil import subprocess import sys import tempfile @@ -65,6 +67,41 @@ def tests_tree(directory, modules): COPY = "Copy fence_git_environment and setUpModule from tests/test_landing.py" +README_AT_1_0_0 = ( + "Preamble keeps 1.0.0 and upstream pstack.\n" + ' license MIT\n' + ' upstream pstack 1.0.0\n' + "Postscript keeps 1.0.0.\n" +) +DISAGREE_1_0_0 = "README.md badge url 1.0.0 alt 1.0.0 disagrees with upstream.json version 1.1.0" +ABSENT_1_1_0 = "README.md has no upstream badge and upstream.json version is 1.1.0" + + +def badge_line(url, alt): + return ( + f' \n' + ) + + +def checker_checkout(root, readme): + scripts = root / "scripts" + scripts.mkdir(parents=True) + shutil.copy(ROOT / "scripts/check.py", scripts / "check.py") + roles = root / "t3/scripts" + roles.mkdir(parents=True) + shutil.copy(ROOT / "t3/scripts/roles.py", roles / "roles.py") + skill = root / "skills/demo" + skill.mkdir(parents=True) + (skill / "SKILL.md").write_text("---\nname: demo\ndescription: d\n---\n\nbody\n") + tests_tree(root, {}) + (root / "upstream.json").write_text(json.dumps({"version": "1.1.0"}) + "\n") + if readme is not None: + (root / "README.md").write_text(readme) + + +def run_check(root): + return subprocess.run([sys.executable, "scripts/check.py"], cwd=root, capture_output=True, text=True) class FenceCheckTest(unittest.TestCase): @@ -314,5 +351,86 @@ def test_a_sampler_stopped_early_keeps_the_rows_it_printed(self): self.assertEqual(lines[1].split("\t")[0], "x") +class BadgeCheckTest(unittest.TestCase): + def test_check_py_names_an_older_badge_and_exits_nonzero(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + checker_checkout(root, README_AT_1_0_0) + result = run_check(root) + self.assertEqual((result.returncode, result.stdout, result.stderr), (1, DISAGREE_1_0_0 + "\n", "")) + self.assertEqual((root / "README.md").read_text(), README_AT_1_0_0) + + def test_check_py_names_a_missing_badge(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + checker_checkout(root, None) + result = run_check(root) + self.assertEqual((result.returncode, result.stdout, result.stderr), (1, ABSENT_1_1_0 + "\n", "")) + self.assertFalse((root / "README.md").exists()) + + def test_badge_findings_name_each_slot(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + upstream = root / "upstream.json" + upstream.write_text(json.dumps({"version": "1.1.0"}) + "\n") + readme = root / "README.md" + cases = ( + ("url", badge_line("1.0.0", "1.1.0"), ["README.md badge url 1.0.0 alt 1.1.0 disagrees with upstream.json version 1.1.0"]), + ("alt", badge_line("1.1.0", "1.0.0"), ["README.md badge url 1.1.0 alt 1.0.0 disagrees with upstream.json version 1.1.0"]), + ("both", badge_line("1.0.0", "1.2.0"), ["README.md badge url 1.0.0 alt 1.2.0 disagrees with upstream.json version 1.1.0"]), + ("agreement", badge_line("1.1.0", "1.1.0"), []), + ("two lines", badge_line("1.1.0", "1.1.0") + badge_line("1.0.0", "1.0.0"), [ABSENT_1_1_0]), + ("not a badge", "not a badge\n", [ABSENT_1_1_0]), + ) + for name, text, expected in cases: + with self.subTest(slot=name): + readme.write_text(text) + self.assertEqual(check.badge_findings(readme, upstream), expected) + + def test_badge_findings_read_a_crlf_readme(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + upstream = root / "upstream.json" + upstream.write_text(json.dumps({"version": "1.1.0"}) + "\n") + readme = root / "README.md" + readme.write_bytes(README_AT_1_0_0.replace("\n", "\r\n").encode()) + self.assertEqual(check.badge_findings(readme, upstream), [DISAGREE_1_0_0]) + agreed = README_AT_1_0_0.replace("upstream%20pstack-1.0.0", "upstream%20pstack-1.1.0").replace( + "upstream pstack 1.0.0", "upstream pstack 1.1.0" + ) + readme.write_bytes(agreed.replace("\n", "\r\n").encode()) + self.assertEqual(check.badge_findings(readme, upstream), []) + + def test_check_tree_judges_only_the_given_tree(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + skill = root / "skills/demo" + skill.mkdir(parents=True) + (skill / "SKILL.md").write_text("---\nname: demo\ndescription: d\n---\n\nbody\n") + readme = root / "README.md" + upstream = root / "upstream.json" + readme.write_text(README_AT_1_0_0) + upstream.write_text(json.dumps({"version": "1.1.0"}) + "\n") + with mock.patch.object(check, "README_PATH", readme), mock.patch.object(check, "UPSTREAM_PATH", upstream): + self.assertEqual(check.check_tree(root / "skills"), []) + self.assertEqual(check.badge_findings(readme, upstream), [DISAGREE_1_0_0]) + + def test_an_explicit_tree_gets_no_repository_badge_finding(self): + with tempfile.TemporaryDirectory() as directory: + skills = Path(directory) / "skills" + skills.mkdir() + tests = tests_tree(directory, {}) + readme = Path(directory) / "README.md" + upstream = Path(directory) / "upstream.json" + readme.write_text(README_AT_1_0_0) + upstream.write_text(json.dumps({"version": "1.1.0"}) + "\n") + output = io.StringIO() + with mock.patch.object(check, "TESTS_DIR", tests), mock.patch.object( + check, "README_PATH", readme + ), mock.patch.object(check, "UPSTREAM_PATH", upstream), contextlib.redirect_stdout(output): + status = check.main([str(skills)]) + self.assertEqual((status, output.getvalue()), (0, "ok\n")) + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_sync_upstream.py b/tests/test_sync_upstream.py index fb5b2127..56890bd7 100644 --- a/tests/test_sync_upstream.py +++ b/tests/test_sync_upstream.py @@ -109,6 +109,20 @@ "Once the merges are written, review each merged override and re-port each override whose row says so. " "Then run python3 scripts/build.py --update-lock, which runs the build's Cursor-leftover check.", ] +README_AT_1_0_0 = ( + "Preamble keeps 1.0.0 and upstream pstack.\n" + ' license MIT\n' + ' upstream pstack 1.0.0\n' + "Postscript keeps 1.0.0.\n" +) +README_AT_1_1_0 = ( + "Preamble keeps 1.0.0 and upstream pstack.\n" + ' license MIT\n' + ' upstream pstack 1.1.0\n' + "Postscript keeps 1.0.0.\n" +) +BADGE_LINE_1_0_0 = ' upstream pstack 1.0.0' +REFUSAL = "README.md has no upstream badge in the expected form\n" def fence_git_environment(): @@ -168,10 +182,12 @@ def new_checkout(directory, repository, pinned): (root / "scripts").mkdir(parents=True) shutil.copy(SCRIPT, root / "scripts/sync_upstream.py") shutil.copy(ROOT / "scripts/build.py", root / "scripts/build.py") + shutil.copy(ROOT / "scripts/check.py", root / "scripts/check.py") write_tree(root, T3_TREE) write_tree(root / "vendor", {name: text for name, text in PINNED_TREE.items() if name.startswith("pstack/")}) meta = {"repository": str(repository), "commit": pinned, "path": "pstack", "version": "1.0.0"} (root / "upstream.json").write_text(json.dumps(meta, indent=2) + "\n") + (root / "README.md").write_text(README_AT_1_0_0) return root @@ -490,6 +506,68 @@ def test_without_check_the_sync_still_replaces_vendor_and_moves_the_pin(self): ) +class BadgeSyncTest(CheckoutCase): + def test_a_newer_upstream_rewrites_the_badge_and_leaves_every_other_readme_byte(self): + head = commit(self.upstream, {PLUGIN: '{"name": "pstack", "version": "1.1.0"}\n'}) + endings = ( + ("lf", README_AT_1_0_0, README_AT_1_1_0), + ("crlf", README_AT_1_0_0.replace("\n", "\r\n"), README_AT_1_1_0.replace("\n", "\r\n")), + ("mixed", README_AT_1_0_0.replace("\n", "\r\n", 1), README_AT_1_1_0.replace("\n", "\r\n", 1)), + ("no final newline", README_AT_1_0_0[:-1], README_AT_1_1_0[:-1]), + ) + for flags in ((), ("--ref", head), ("--merge",), ("--merge", "--ref", head)): + for name, before, after in endings: + with self.subTest(flags=flags, endings=name): + (self.root / "README.md").write_bytes(before.encode()) + result = self.sync(*flags) + self.assertEqual(result.stderr, "") + self.assertEqual((self.root / "README.md").read_bytes(), after.encode()) + self.assertEqual( + json.loads((self.root / "upstream.json").read_text()), + {"repository": str(self.upstream), "commit": head, "path": "pstack", "version": "1.1.0"}, + ) + + def test_a_missing_badge_exits_before_the_pin_write(self): + commit(self.upstream, {PLUGIN: '{"name": "pstack", "version": "1.1.0"}\n'}) + duplicated = README_AT_1_0_0.replace(BADGE_LINE_1_0_0, BADGE_LINE_1_0_0 + "\n" + BADGE_LINE_1_0_0, 1) + split = README_AT_1_0_0.replace( + BADGE_LINE_1_0_0, BADGE_LINE_1_0_0.replace('">\n