ci(docs): check that in-repo GitHub links in docs/public resolve

docs/public/conf.py

@@ -54,9 +54,11 @@ def setup(app):
 redirects = json.loads((Path(__file__).parent / "redirects.json").read_text(encoding="utf-8"))
 
 # Bound remote I/O and avoid connection-pool stalls during the required
-# repository-wide link check. Links back into this repository are covered by
-# local policy and schema checks, so do not spend unauthenticated GitHub quota
-# rechecking them over the network.
+# repository-wide link check. Do not spend unauthenticated GitHub quota on links
+# back into this repository. Instead, tools/check_public_docs.py checks that blob
+# and tree links on main, dev or HEAD name paths that git tracks in the checkout
+# (tools/public_docs_links.py). Links to issues, pull requests and other
+# repository pages stay unchecked.
 linkcheck_timeout = 15
 linkcheck_workers = 1
 linkcheck_ignore = [

implementations/python/tests/test_issue_1494_public_docs_github_links.py

@@ -0,0 +1,204 @@
+"""In-repository GitHub links on public pages must resolve (issue 1494).
+
+``docs/public/conf.py`` keeps links into this repository out of the Sphinx link check.
+``tools/check_public_docs.py`` checks their ``blob`` and ``tree`` links on ``main``,
+``dev`` and ``HEAD`` against the files git tracks instead.
+"""
+
+from __future__ import annotations
+
+import json
+import subprocess
+import sys
+from pathlib import Path
+
+import pytest
+from tools import check_public_docs
+from tools.check_public_docs import REQUIRED_PUBLIC_PAGES, REQUIRED_PUBLIC_REDIRECTS
+from tools.public_docs_links import RULE_ID, evaluate_repository_links, repository_link_failures
+
+PAGE = "docs/public/page.md"
+REPO = "https://github.com/OpenRAE/rae"
+TRACKED = ("README.md", "docs/guide.md", "docs/two words.md", "specs/formal/model.md")
+MISSING_FILE = f"{REPO}/blob/main/docs/missing.md"
+
+
+def _write(path: Path, content: str) -> None:
+    path.parent.mkdir(parents=True, exist_ok=True)
+    path.write_text(content, encoding="utf-8")
+
+
+def _git(root: Path, *args: str) -> None:
+    subprocess.run(["git", *args], cwd=root, check=True, capture_output=True)
+
+
+def _failures(line: str) -> list[tuple[str, str | None, str]]:
+    return [
+        (failure.rule_id, failure.path, failure.message) for failure in repository_link_failures({PAGE: line}, TRACKED)
+    ]
+
+
+@pytest.mark.parametrize(
+    "line",
+    [
+        f"See {REPO}/blob/main/README.md.",
+        f"Read the [guide]({REPO}/blob/dev/docs/guide.md).",
+        f"<{REPO}/tree/main/specs/formal>",
+        f"`Model <{REPO}/blob/HEAD/specs/formal/model.md>`_",
+        f'<a href="{REPO}/blob/main/README.md">README</a>',
+        f"[specs]({REPO}/tree/main/specs/)",
+        f"[repository root]({REPO}/tree/main)",
+        f"[install]({REPO}/blob/main/docs/guide.md#install)",
+        f"[source]({REPO}/blob/main/README.md?plain=1#L3)",
+        f"[spaced]({REPO}/blob/main/docs/two%20words.md)",
+        "[former owner](https://github.com/RAESystem/rae/blob/main/README.md)",
+    ],
+)
+def test_links_to_tracked_paths_pass(line: str) -> None:
+    assert _failures(line) == []
+
+
+@pytest.mark.parametrize(
+    ("line", "message"),
+    [
+        (
+            f"[missing]({MISSING_FILE})",
+            f"{MISSING_FILE} does not resolve: this checkout tracks no file at docs/missing.md",
+        ),
+        (
+            f"[missing]({REPO}/tree/dev/docs/missing)",
+            f"{REPO}/tree/dev/docs/missing does not resolve: this checkout tracks no directory at docs/missing",
+        ),
+        (
+            f"[case]({REPO}/blob/HEAD/readme.md)",
+            f"{REPO}/blob/HEAD/readme.md does not resolve: this checkout tracks no file at readme.md",
+        ),
+        (
+            "[former owner](https://github.com/RAESystem/rae/blob/main/docs/missing.md)",
+            "https://github.com/RAESystem/rae/blob/main/docs/missing.md does not resolve: "
+            "this checkout tracks no file at docs/missing.md",
+        ),
+        (
+            f"[spaced]({REPO}/blob/main/docs/two%20word.md)",
+            f"{REPO}/blob/main/docs/two%20word.md does not resolve: this checkout tracks no file at docs/two word.md",
+        ),
+        (
+            f"`git show {MISSING_FILE}`",
+            f"{MISSING_FILE} does not resolve: this checkout tracks no file at docs/missing.md",
+        ),
+        (
+            f"[{MISSING_FILE}]({MISSING_FILE})",
+            f"{MISSING_FILE} does not resolve: this checkout tracks no file at docs/missing.md",
+        ),
+        (
+            f"[specs]({REPO}/blob/main/specs)",
+            f"{REPO}/blob/main/specs names a directory: link specs with /tree/, not /blob/",
+        ),
+        (
+            f"[README]({REPO}/tree/main/README.md)",
+            f"{REPO}/tree/main/README.md names a file: link README.md with /blob/, not /tree/",
+        ),
+    ],
+)
+def test_links_that_do_not_resolve_fail_once_with_the_url(line: str, message: str) -> None:
+    assert _failures(line) == [(RULE_ID, f"{PAGE}:1", message)]
+
+
+@pytest.mark.parametrize(
+    "line",
+    [
+        f"[tag]({REPO}/blob/v6.0.1/docs/missing.md)",
+        f"[commit]({REPO}/blob/3011197c/docs/missing.md)",
+        f"[branch]({REPO}/tree/release/docs/missing)",
+        "[other repository](https://github.com/OpenRAE/env-packs/blob/main/docs/missing.md)",
+        f"[issue]({REPO}/issues/813)",
+        f"[template]({REPO}/issues/new?template=missing.md)",
+        "[plain HTTP](http://github.com/OpenRAE/rae/blob/main/docs/missing.md)",
+        "[other host](https://example.com/OpenRAE/rae/blob/main/docs/missing.md)",
+        "[relative](../missing.md)",
+    ],
+)
+def test_links_outside_the_checked_scope_are_ignored(line: str) -> None:
+    assert _failures(line) == []
+
+
+@pytest.mark.integration
+def test_only_files_that_git_tracks_resolve(tmp_path: Path) -> None:
+    _write(tmp_path / "README.md", "# Tracked\n")
+    _git(tmp_path, "init", "-q")
+    _git(tmp_path, "add", "README.md")
+    _write(tmp_path / "docs" / "draft.md", "# On disk, never added\n")
+    page = f"[README]({REPO}/blob/main/README.md)\n[draft]({REPO}/blob/main/docs/draft.md)\n"
+
+    failures = evaluate_repository_links(tmp_path, {PAGE: page})
+
+    assert [(failure.path, failure.message) for failure in failures] == [
+        (
+            f"{PAGE}:2",
+            f"{REPO}/blob/main/docs/draft.md does not resolve: this checkout tracks no file at docs/draft.md",
+        )
+    ]
+
+
+@pytest.mark.integration
+def test_a_checkout_that_git_cannot_list_fails_closed(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
+    monkeypatch.setenv("GIT_CEILING_DIRECTORIES", str(tmp_path.parent))
+
+    failures = evaluate_repository_links(tmp_path, {PAGE: "No links here.\n"})
+
+    assert [(failure.rule_id, failure.path) for failure in failures] == [(RULE_ID, None)]
+    assert failures[0].message.startswith("cannot list the files git tracks: ")
+
+
+@pytest.mark.integration
+@pytest.mark.parametrize(
+    ("dead_link", "status", "output"),
+    [
+        ("", 0, []),
+        (
+            f"See the [missing page]({MISSING_FILE}).\n",
+            1,
+            [
+                f"[{RULE_ID}] docs/public/quickstart.md:4: {MISSING_FILE} does not resolve: "
+                "this checkout tracks no file at docs/missing.md"
+            ],
+        ),
+    ],
+    ids=["live-link", "dead-link"],
+)
+def test_checker_reports_dead_links_by_page_line_and_url(
+    tmp_path: Path,
+    monkeypatch: pytest.MonkeyPatch,
+    capsys: pytest.CaptureFixture[str],
+    dead_link: str,
+    status: int,
+    output: list[str],
+) -> None:
+    public_root = tmp_path / "docs" / "public"
+    for relative_path in REQUIRED_PUBLIC_PAGES:
+        _write(public_root / relative_path, f"# {Path(relative_path).stem.title()}\n")
+    _write(public_root / "redirects.json", json.dumps(REQUIRED_PUBLIC_REDIRECTS))
+    _write(tmp_path / "README.md", "# RAES\n")
+    quickstart = f"# Quickstart\n\nRead the [README]({REPO}/blob/main/README.md).\n{dead_link}"
+    _write(public_root / "quickstart.md", quickstart)
+    _git(tmp_path, "init", "-q")
+    _git(tmp_path, "add", "-A")
+    monkeypatch.setattr(check_public_docs, "REPO_ROOT", tmp_path)
+    monkeypatch.setattr(sys, "argv", ["check_public_docs.py"])
+
+    assert check_public_docs.main() == status
+    assert capsys.readouterr().out.splitlines() == output
+
+
+def test_checker_skips_the_link_check_when_the_public_root_is_missing(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
+) -> None:
+    # Without a git repository, a link check that ran would add a "cannot list" failure.
+    monkeypatch.setenv("GIT_CEILING_DIRECTORIES", str(tmp_path.parent))
+    monkeypatch.setattr(check_public_docs, "REPO_ROOT", tmp_path)
+    monkeypatch.setattr(sys, "argv", ["check_public_docs.py"])
+
+    assert check_public_docs.main() == 1
+    assert capsys.readouterr().out.splitlines() == [
+        "[public-docs-root] docs/public: the curated public documentation root is missing or unsafe"
+    ]

tools/check_public_docs.py

@@ -19,6 +19,7 @@
     sys.path.insert(0, str(REPO_ROOT))
 
 from tools.policy.common import PolicyFailure, failures_to_json, safe_repo_path  # noqa: E402
+from tools.public_docs_links import evaluate_repository_links  # noqa: E402
 from tools.public_docs_markup import directive_targets, markdown_link_targets  # noqa: E402
 
 PUBLIC_DOCS_ROOT = Path("docs/public")
@@ -471,6 +472,10 @@ def main() -> int:
     args = parser.parse_args()
 
     failures = [*evaluate_public_sources(REPO_ROOT), *evaluate_public_boundary(REPO_ROOT)]
+    public_root = _public_path(REPO_ROOT)
+    if public_root is not None and public_root.is_dir():
+        pages = {_relative(REPO_ROOT, source): _bounded_text(source) for source in _source_paths(public_root)}
+        failures.extend(evaluate_repository_links(REPO_ROOT, pages))
     if not failures and args.output is not None:
         failures.extend(evaluate_public_output(REPO_ROOT, args.output))
         failures.extend(evaluate_published_assets(REPO_ROOT, args.output))

tools/public_docs_links.py

@@ -0,0 +1,102 @@
+"""Check that public pages link to paths that exist in this repository.
+
+``docs/public/conf.py`` keeps links into this repository out of the Sphinx link check,
+so ``tools/check_public_docs.py`` runs this check on the links that name repository
+content instead. A checked link starts with a prefix that ``linkcheck_ignore`` skips,
+``https://github.com/OpenRAE/rae/`` or ``https://github.com/RAESystem/rae/``, continues
+with ``blob/<ref>/`` or ``tree/<ref>/``, and names ``main``, ``dev`` or ``HEAD`` as the ref.
+Those branches carry this tree: ``dev`` takes it when the change merges, and ``main``
+(``HEAD``, the default branch) at the next release. A ``blob`` link must therefore name a
+file that git tracks in this checkout, and a ``tree`` link a directory that holds one.
+Git's list matches case exactly, as GitHub does, and leaves out untracked files.
+
+The check skips links on other refs (commit SHAs, tags, other branches), because they can
+name content that this checkout does not have. It ignores fragments and query strings, and
+it does not check links to issues, pull requests or other repository pages. It reads every
+line of a source, code included, so it errs toward false positives, like the directive scan
+in ``tools/public_docs_markup.py``.
+"""
+
+from __future__ import annotations
+
+import re
+import subprocess
+from collections.abc import Iterable, Iterator, Mapping
+from pathlib import Path, PurePosixPath
+from urllib.parse import unquote, urlsplit
+
+from tools.policy.common import PolicyFailure
+
+RULE_ID = "public-docs-repository-link"
+# The prefixes that linkcheck_ignore in docs/public/conf.py skips, then a route. A URL ends
+# where whitespace or the Markdown, MyST or reStructuredText markup around it begins.
+REPOSITORY_LINK = re.compile(r"https://github\.com/(?:OpenRAE|RAESystem)/rae/(?:blob|tree)/[^\s<>()\[\]{}\"'`|\\*]*")
+# Punctuation that ends a sentence after a bare URL rather than belonging to it.
+TRAILING_PUNCTUATION = ".,;:!?"
+CHECKED_REFS = frozenset({"dev", "main", "HEAD"})
+ROUTE_KINDS = {"blob": "file", "tree": "directory"}
+KIND_ROUTES = {kind: route for route, kind in ROUTE_KINDS.items()}
+
+
+def evaluate_repository_links(repo_root: Path, pages: Mapping[str, str]) -> list[PolicyFailure]:
+    """Return a failure for each checked link in ``pages`` that git's tracked files do not resolve.
+
+    ``pages`` maps the repository path of each public source to its text. The check fails
+    closed when git cannot list the files it tracks under ``repo_root``.
+    """
+
+    try:
+        tracked = _tracked_files(repo_root)
+    except (OSError, subprocess.CalledProcessError, UnicodeDecodeError) as exc:
+        return [PolicyFailure(RULE_ID, f"cannot list the files git tracks: {exc}")]
+    return repository_link_failures(pages, tracked)
+
+
+def repository_link_failures(pages: Mapping[str, str], tracked: Iterable[str]) -> list[PolicyFailure]:
+    """Return a failure, located at ``<page>:<line>``, for each checked link that ``tracked`` does not resolve.
+
+    A ``blob`` link resolves to a path in ``tracked``. A ``tree`` link resolves to a directory
+    above one, and an empty path names the repository root.
+    """
+
+    kinds = _path_kinds(tracked)
+    failures = (
+        PolicyFailure(RULE_ID, problem, f"{page}:{number}")
+        for page, text in pages.items()
+        for number, line in enumerate(text.split("\n"), start=1)
+        for problem in _line_problems(line, kinds)
+    )
+    return list(dict.fromkeys(failures))
+
+
+def _tracked_files(repo_root: Path) -> list[str]:
+    listing = subprocess.run(["git", "ls-files", "-z"], cwd=repo_root, check=True, capture_output=True).stdout
+    return [path for path in listing.decode("utf-8").split("\0") if path]
+
+
+def _path_kinds(tracked: Iterable[str]) -> dict[PurePosixPath, str]:
+    files = [PurePosixPath(path) for path in tracked]
+    kinds = {directory: "directory" for path in files for directory in path.parents}
+    kinds.update(dict.fromkeys(files, "file"))
+    return kinds
+
+
+def _line_problems(line: str, kinds: Mapping[PurePosixPath, str]) -> Iterator[str]:
+    for match in REPOSITORY_LINK.finditer(line):
+        problem = _link_problem(match.group().rstrip(TRAILING_PUNCTUATION), kinds)
+        if problem is not None:
+            yield problem
+
+
+def _link_problem(url: str, kinds: Mapping[PurePosixPath, str]) -> str | None:
+    # The URL path is /<owner>/rae/<route>/<ref>/<repository path>; REPOSITORY_LINK fixes the route.
+    route, ref, *segments = urlsplit(url).path.split("/")[3:]
+    path = PurePosixPath(unquote("/".join(segments)))
+    expected = ROUTE_KINDS[route]
+    actual = kinds.get(path)
+    # Skip a link on another ref, and pass one whose path git tracks as the kind that its route names.
+    if ref not in CHECKED_REFS or actual == expected:
+        return None
+    if actual is None:
+        return f"{url} does not resolve: this checkout tracks no {expected} at {path}"
+    return f"{url} names a {actual}: link {path} with /{KIND_ROUTES[actual]}/, not /{route}/"