diff --git a/CHANGELOG.md b/CHANGELOG.md index 02d0981..b499d83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ for distribution releases. and weekly full sweeps, with an opt-in live-network Task command. Pull-request checks include repeated URLs added on new lines; HTTPS requests connect only to vetted public addresses and warn rather than bypass configured proxies. + A new distribution version's unpublished compare and release-tag links in the + changelog are reported as notices only for unredirected 404s in PR checks. - Added offline inventory and opt-in live validation for actionable public container image tags in skill and eval examples, including implicit `latest` for untagged references. Docker Hub and gcr.io manifests are checked for diff --git a/README.md b/README.md index b3bd31f..710f66d 100644 --- a/README.md +++ b/README.md @@ -204,8 +204,11 @@ CI runs this network-dependent check separately from deterministic `task` valida pull requests check added-line URL occurrences, while the weekly schedule and manual dispatch check all URLs. This advisory workflow is **not required for merges during initial rollout**. Confirmed HTTP 404/410 and missing Docker Docs anchors are reported -as failures; access denial, rate limiting, server/network errors, and uncertain anchors -are warnings. When an HTTPS proxy is configured, the checker warns rather than bypassing +as failures, except that PR checks with a changed catalog distribution version report +an unredirected 404 for that version's exact `CHANGELOG.md` compare and release-tag +links as notices until publication. Full sweeps and all other broken links still fail. +Access denial, rate limiting, server/network errors, and uncertain anchors are warnings. +When an HTTPS proxy is configured, the checker warns rather than bypassing it; these warnings cannot confirm link availability. Redirects are reported so maintainers can update moved links. An intentional session-dependent Slack invitation (`https://dockr.ly/slack`) is excluded from automated checks. diff --git a/scripts/check_external_links.py b/scripts/check_external_links.py index 26a0e0d..47f6ec2 100644 --- a/scripts/check_external_links.py +++ b/scripts/check_external_links.py @@ -21,6 +21,9 @@ from check_links import DOCUMENTATION_GLOBS, ROOT_DOCUMENTS, FENCED_CODE_RE REPO_ROOT = Path(__file__).resolve().parent.parent +CATALOG_VERSION_RE = re.compile( + r"^version[ \t]*:[ \t]*(?P['\"]?)(?P(?:0|[1-9][0-9]*)\.(?:0|[1-9][0-9]*)\.(?:0|[1-9][0-9]*))(?P=quote)[ \t]*(?:#.*)?$" +) METADATA = ("catalog.yaml", "skills.sh.json", "gemini-extension.json", ".github/ISSUE_TEMPLATE/config.yml") DOCUMENTS = (*ROOT_DOCUMENTS, "CODE_OF_CONDUCT.md") MANIFESTS = (".claude-plugin/*.json", ".cursor-plugin/*.json", ".codex-plugin/*.json", ".agents/plugins/*.json", ".github/plugin/*.json") @@ -50,6 +53,7 @@ class Result: level: str # error, warning, notice, ok message: str final_url: str = "" + redirected: bool = False def eligible(path: str) -> bool: @@ -157,6 +161,31 @@ def occurrences(root: Path, base: str | None = None) -> list[Occurrence]: return found +def expected_release_urls(base_version: str, head_version: str) -> set[str]: + """Return links that cannot exist until a new distribution release is published.""" + if tuple(map(int, head_version.split("."))) <= tuple(map(int, base_version.split("."))): + return set() + origin = "https://github.com/docker/skills" + return { + f"{origin}/compare/v{head_version}...HEAD", + f"{origin}/releases/tag/v{head_version}", + } + + +def _catalog_version(content: str) -> str: + lines = [line for line in content.splitlines() if re.match(r"^version[ \t]*:", line)] + if len(lines) != 1 or not (match := CATALOG_VERSION_RE.fullmatch(lines[0])): + raise ValueError("catalog.yaml must contain exactly one valid top-level X.Y.Z version") + return match.group("version") + + +def _expected_pr_release_urls(root: Path, base: str) -> set[str]: + ancestor = _git(root, "merge-base", base, "HEAD").decode().strip() + base_version = _catalog_version(_git(root, "show", f"{ancestor}:catalog.yaml").decode("utf-8")) + head_version = _catalog_version(_git(root, "show", "HEAD:catalog.yaml").decode("utf-8")) + return expected_release_urls(base_version, head_version) + + class _Anchors(html.parser.HTMLParser): def __init__(self) -> None: super().__init__() @@ -251,6 +280,7 @@ def open_pinned(req: request.Request, addresses: tuple[tuple[int, int, int, tupl def fetch(url: str) -> Result: original = url current = url + redirected = False for _ in range(MAX_REDIRECTS + 1): addresses = public_https(current) if addresses is None: @@ -265,12 +295,13 @@ def fetch(url: str) -> Result: if not destination: return Result("warning", f"HTTP {exc.code} without Location", current) current = parse.urljoin(current, destination) + redirected = True if not parse.urlsplit(current).fragment and parse.urlsplit(original).fragment: current += "#" + parse.urlsplit(original).fragment continue exc.close() if exc.code in {404, 410}: - return Result("error", f"HTTP {exc.code}", current) + return Result("error", f"HTTP {exc.code}", current, redirected) return Result("warning", f"HTTP {exc.code}", current) except (OSError, ValueError, http.client.HTTPException) as exc: return Result("warning", f"network error: {exc}", current) @@ -314,10 +345,17 @@ def check(url: str) -> Result: return result -def emit(found: list[Occurrence], results: dict[str, Result], summary: Path | None = None) -> bool: +def emit(found: list[Occurrence], results: dict[str, Result], summary: Path | None = None, + expected: set[str] | None = None) -> bool: counts = {key: 0 for key in ("error", "warning", "notice", "ok")} + reported = [] for item in found: result = results[item.url] + if (expected and item.path == "CHANGELOG.md" and item.url in expected + and result.level == "error" and result.message == "HTTP 404" + and result.final_url == item.url and not result.redirected): + result = Result("notice", "HTTP 404 (release link not published yet)", result.final_url) + reported.append((item, result)) counts[result.level] += 1 if result.level != "ok": message = f"{item.url}: {result.message}" @@ -331,8 +369,7 @@ def emit(found: list[Occurrence], results: dict[str, Result], summary: Path | No if summary: with summary.open("a", encoding="utf-8") as output: output.write(f"### External HTTPS links\n\n{text}\n\n") - for item in found: - result = results[item.url] + for item, result in reported: if result.level != "ok": output.write(f"- **{result.level}** `{item.path}:{item.line}` `{item.url}`: {result.message}\n") return counts["error"] > 0 @@ -345,6 +382,7 @@ def main(argv: list[str] | None = None) -> int: args = parser.parse_args(argv) try: found = occurrences(args.root, args.base) + expected = _expected_pr_release_urls(args.root, args.base) if args.base is not None else None unique = list(dict.fromkeys(item.url for item in found)) pool = concurrent.futures.ThreadPoolExecutor(max_workers=WORKERS) futures = {pool.submit(check, url): url for url in unique} @@ -368,7 +406,7 @@ def main(argv: list[str] | None = None) -> int: results[futures[future]] = Result("warning", "overall check deadline exceeded") finally: pool.shutdown(wait=False, cancel_futures=True) - return int(emit(found, results, Path(os.environ["GITHUB_STEP_SUMMARY"]) if os.environ.get("GITHUB_STEP_SUMMARY") else None)) + return int(emit(found, results, Path(os.environ["GITHUB_STEP_SUMMARY"]) if os.environ.get("GITHUB_STEP_SUMMARY") else None, expected)) except (OSError, UnicodeError, ValueError) as exc: print(f"external link check: {exc}", file=sys.stderr) return 2 diff --git a/scripts/test_check_external_links.py b/scripts/test_check_external_links.py index c03612b..3abc680 100644 --- a/scripts/test_check_external_links.py +++ b/scripts/test_check_external_links.py @@ -12,6 +12,7 @@ from urllib import error, request import check_external_links as links +from prepare_release import rotate_changelog class ExtractionTests(unittest.TestCase): @@ -117,6 +118,172 @@ def git(*args): ]) +class ReleaseLinkTests(unittest.TestCase): + ORIGIN = "https://github.com/docker/skills" + + def test_expected_urls_are_exactly_the_head_release_links(self): + self.assertEqual(links.expected_release_urls("1.2.3", "1.3.0"), { + f"{self.ORIGIN}/compare/v1.3.0...HEAD", + f"{self.ORIGIN}/releases/tag/v1.3.0", + }) + self.assertEqual(links.expected_release_urls("1.2.3", "1.2.3"), set()) + + def test_downgrade_release_links_remain_errors(self): + release = f"{self.ORIGIN}/releases/tag/v0.2.0" + self.assertEqual(links.expected_release_urls("0.3.0", "0.2.0"), set()) + # Version components must be ordered numerically, not as strings. + self.assertEqual(links.expected_release_urls("0.10.0", "0.9.0"), set()) + with mock.patch("sys.stdout", new_callable=io.StringIO) as output: + self.assertTrue(links.emit( + [links.Occurrence("CHANGELOG.md", 1, release)], + {release: links.Result("error", "HTTP 404", release)}, + expected=links.expected_release_urls("0.3.0", "0.2.0"), + )) + self.assertIn("1 error, 0 warning, 0 notice", output.getvalue()) + + def test_pr_release_rotation_fetches_and_reports_notice_per_occurrence(self): + old = ("## [Unreleased]\n\n### Fixed\n\n- A link check.\n\n" + "## [1.2.3] - 2026-01-02\n\n### Added\n\n- First release.\n\n" + f"[Unreleased]: {self.ORIGIN}/compare/v1.2.3...HEAD\n" + f"[1.2.3]: {self.ORIGIN}/releases/tag/v1.2.3\n") + rotated = rotate_changelog(old, "1.2.3", "1.3.0", "2026-09-29") + expected = links.expected_release_urls("1.2.3", "1.3.0") + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + + def git(*args): + return subprocess.run(["git", *args], cwd=root, check=True, capture_output=True, text=True).stdout.strip() + + git("init", "-q") + git("config", "user.name", "Test") + git("config", "user.email", "test@example.com") + (root / "catalog.yaml").write_text("schema: v1\nversion: 1.2.3\n") + (root / "CHANGELOG.md").write_text(old) + git("add", ".") + git("-c", "commit.gpgsign=false", "commit", "-qm", "base") + base = git("rev-parse", "HEAD") + (root / "catalog.yaml").write_text("schema: v1\nversion: 1.3.0\n") + (root / "CHANGELOG.md").write_text(rotated) + # An added repeat in a different file must remain an error even after URL deduplication. + (root / "README.md").write_text(f"{self.ORIGIN}/releases/tag/v1.3.0\n") + git("add", ".") + git("-c", "commit.gpgsign=false", "commit", "-qm", "release") + (root / "CHANGELOG.md").write_text("uncommitted text\n") + found = links.occurrences(root, base) + self.assertEqual({item.url for item in found if item.path == "CHANGELOG.md"}, expected) + self.assertEqual(links._expected_pr_release_urls(root, base), expected) + summary = root / "summary" + def missing(current): + return links.Result("error", "HTTP 404", current) + + with mock.patch.object(links, "check", side_effect=missing) as check, \ + mock.patch.dict("os.environ", {"GITHUB_ACTIONS": "true", "GITHUB_STEP_SUMMARY": str(summary)}), \ + mock.patch("sys.stdout", new_callable=io.StringIO) as output: + self.assertEqual(links.main(["--root", str(root), "--base", base]), 1) + self.assertEqual(check.call_count, 2) + self.assertIn("2 notice", output.getvalue()) + self.assertIn("1 error", output.getvalue()) + self.assertIn("::notice file=CHANGELOG.md", output.getvalue()) + self.assertIn("::error file=README.md", output.getvalue()) + self.assertIn("**notice** `CHANGELOG.md:", summary.read_text()) + self.assertIn("**error** `README.md:", summary.read_text()) + + def test_only_exact_unredirected_404_in_changelog_is_notice(self): + expected = links.expected_release_urls("1.2.3", "1.3.0") + release = f"{self.ORIGIN}/releases/tag/v1.3.0" + near = [f"{release}/", f"{release}?query=1", f"{self.ORIGIN}/releases/tag/v1.3.1"] + found = [links.Occurrence("CHANGELOG.md", 1, release), + links.Occurrence("CHANGELOG.md", 2, release), + links.Occurrence("README.md", 3, release)] + found.extend(links.Occurrence("CHANGELOG.md", index + 4, url) for index, url in enumerate(near)) + results = {url: links.Result("error", "HTTP 404", url) for url in [release, *near]} + with mock.patch("sys.stdout", new_callable=io.StringIO) as output: + self.assertTrue(links.emit(found, results, expected=expected)) + self.assertIn("4 error, 0 warning, 2 notice", output.getvalue()) + with mock.patch("sys.stdout", new_callable=io.StringIO) as output: + self.assertFalse(links.emit(found[:2], {release: results[release]}, expected=expected)) + self.assertIn("2 notice", output.getvalue()) + for result in (links.Result("error", "HTTP 410", release), + links.Result("error", "HTTP 404", release, True), + links.Result("error", "HTTP 404", release + "/destination", True), + links.Result("warning", "network error: timeout", release)): + with self.subTest(result=result), mock.patch("sys.stdout", new_callable=io.StringIO) as output: + failed = links.emit([found[0]], {release: result}, expected=expected) + self.assertEqual(failed, result.level == "error") + self.assertIn(f"1 {result.level}", output.getvalue()) + with mock.patch("sys.stdout", new_callable=io.StringIO) as output: + self.assertTrue(links.emit([found[0]], {release: results[release]})) + self.assertIn("1 error", output.getvalue()) + + def test_redirect_to_expected_url_ending_in_404_remains_error(self): + release = f"{self.ORIGIN}/releases/tag/v1.3.0" + with mock.patch.object(links, "public_https", return_value=((socket.AF_INET, socket.SOCK_STREAM, 6, ("1.1.1.1", 443)),)), \ + mock.patch.object(links, "open_pinned", side_effect=[ + http_error(release, 302, "/releases/tag/v1.3.0"), http_error(release, 404)]): + result = links.fetch(release) + self.assertTrue(result.redirected) + with mock.patch("sys.stdout", new_callable=io.StringIO): + self.assertTrue(links.emit([links.Occurrence("CHANGELOG.md", 1, release)], + {release: result}, expected={release})) + + def test_pr_catalog_version_must_be_present_and_strict_at_both_revisions(self): + for invalid in ("schema: v1\n", "version: 1.2.3\nversion: 1.2.3\n", + "version: 01.2.3\n", "version: 1.2\n", "version: v1.2.3\n", + "version: 1.2.3-beta\n"): + with self.subTest(invalid=invalid): + with self.assertRaisesRegex(ValueError, "catalog.yaml"): + links._catalog_version(invalid) + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + + def git(*args): + return subprocess.run(["git", *args], cwd=root, check=True, capture_output=True, text=True).stdout.strip() + + git("init", "-q") + git("config", "user.name", "Test") + git("config", "user.email", "test@example.com") + (root / "catalog.yaml").write_text("version: 1.2.3\n") + git("add", ".") + git("-c", "commit.gpgsign=false", "commit", "-qm", "base") + base = git("rev-parse", "HEAD") + (root / "catalog.yaml").write_text("version: invalid\n") + git("add", ".") + git("-c", "commit.gpgsign=false", "commit", "-qm", "head") + with mock.patch.object(links, "check") as check, mock.patch("sys.stderr", new_callable=io.StringIO) as err: + self.assertEqual(links.main(["--root", str(root), "--base", base]), 2) + self.assertIn("catalog.yaml", err.getvalue()) + check.assert_not_called() + self.assertEqual(links._catalog_version("# version: no\nversion: '1.2.3' # comment\n"), "1.2.3") + with mock.patch.object(links, "_git", side_effect=[b"base\n", b"version: invalid\n"]): + with self.assertRaisesRegex(ValueError, "catalog.yaml"): + links._expected_pr_release_urls(root, base) + + def test_unchanged_version_pr_and_full_sweep_keep_404_errors(self): + release = f"{self.ORIGIN}/releases/tag/v1.2.3" + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + + def git(*args): + return subprocess.run(["git", *args], cwd=root, check=True, capture_output=True, text=True).stdout.strip() + + git("init", "-q") + git("config", "user.name", "Test") + git("config", "user.email", "test@example.com") + (root / "catalog.yaml").write_text("version: 1.2.3\n") + (root / "CHANGELOG.md").write_text("# Changelog\n") + git("add", ".") + git("-c", "commit.gpgsign=false", "commit", "-qm", "base") + base = git("rev-parse", "HEAD") + (root / "CHANGELOG.md").write_text(release + "\n") + git("add", ".") + git("-c", "commit.gpgsign=false", "commit", "-qm", "head") + self.assertEqual(links._expected_pr_release_urls(root, base), set()) + with mock.patch.object(links, "check", return_value=links.Result("error", "HTTP 404", release)), \ + mock.patch("sys.stdout", new_callable=io.StringIO): + self.assertEqual(links.main(["--root", str(root), "--base", base]), 1) + self.assertEqual(links.main(["--root", str(root)]), 1) + + class FakeResponse: def __init__(self, url, body=b"

hi

", content_type="text/html"): self.url = url