fix: avoid false PR failures for unpublished release links

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

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.

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<quote>['\"]?)(?P<version>(?: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

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"<html><main><h2 id='exists'>hi</h2></main></html>", content_type="text/html"):
         self.url = url