From 2226acc045676cf45947fa04a6f29056663c629a Mon Sep 17 00:00:00 2001 From: Yernat Yestekov <2068106+doublewhy@users.noreply.github.com> Date: Sun, 11 Oct 2026 11:31:11 -0700 Subject: [PATCH 1/2] ci(docs): check that in-repo GitHub links in docs/public resolve docs/public/conf.py keeps every link into this repository out of the Sphinx link check and says local checks cover those links, but no check did. A public page could link a blob or tree URL to a path that does not exist, and both docs-local and docs-links passed. tools/public_docs_links.py finds links under the prefixes that linkcheck_ignore skips (github.com/OpenRAE/rae and the former RAESystem owner) that use the blob or tree route on main, dev or HEAD. It resolves each path, percent-decoded and without its query string, fragment or trailing slash, against the files git ls-files lists. A blob link must name a tracked file and a tree link a directory that holds one. Other refs, other repositories and other repository pages are skipped. A failure names the page, the line and the URL, and the check fails closed when git cannot list the tracked files. tools/check_public_docs.py runs the check with its other source checks, so it runs in the docs lane's existing "public source boundary" stage. The conf.py comment now says what the local check covers. Every such link in docs/public resolves, so no page changes. --- docs/public/conf.py | 8 +- ...est_issue_1494_public_docs_github_links.py | 204 ++++++++++++++++++ tools/check_public_docs.py | 5 + tools/public_docs_links.py | 103 +++++++++ 4 files changed, 317 insertions(+), 3 deletions(-) create mode 100644 implementations/python/tests/test_issue_1494_public_docs_github_links.py create mode 100644 tools/public_docs_links.py diff --git a/docs/public/conf.py b/docs/public/conf.py index c90b0b313..653647b43 100644 --- a/docs/public/conf.py +++ b/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 = [ diff --git a/implementations/python/tests/test_issue_1494_public_docs_github_links.py b/implementations/python/tests/test_issue_1494_public_docs_github_links.py new file mode 100644 index 000000000..34a38852c --- /dev/null +++ b/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'README', + 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" + ] diff --git a/tools/check_public_docs.py b/tools/check_public_docs.py index b85c76c20..a1e7978ee 100644 --- a/tools/check_public_docs.py +++ b/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)) diff --git a/tools/public_docs_links.py b/tools/public_docs_links.py new file mode 100644 index 000000000..d4c93d8cf --- /dev/null +++ b/tools/public_docs_links.py @@ -0,0 +1,103 @@ +"""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//`` or ``tree//``, 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 ``:``, 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 //rae///; REPOSITORY_LINK fixes the route. + route, ref, *segments = urlsplit(url).path.split("/")[3:] + if ref not in CHECKED_REFS: + return None + path = PurePosixPath(unquote("/".join(segments))) + expected = ROUTE_KINDS[route] + actual = kinds.get(path) + if 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}/" From d7e8db2302c519196361a47e941636af38e8705f Mon Sep 17 00:00:00 2001 From: Yernat Yestekov <2068106+doublewhy@users.noreply.github.com> Date: Sun, 11 Oct 2026 16:30:15 -0700 Subject: [PATCH 2/2] refactor(docs): return at most three times from the repository link check SonarCloud's python:S1142 allows three returns per function, and _link_problem had four: one for a link on a ref the check skips and one for a link that resolves. Both mean the link has no problem, so one condition now covers them. The check reads the path before it tests the ref, which cannot fail: REPOSITORY_LINK only matches the blob and tree routes, so ROUTE_KINDS has the route. Behavior is unchanged. --- tools/public_docs_links.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tools/public_docs_links.py b/tools/public_docs_links.py index d4c93d8cf..dcbde5422 100644 --- a/tools/public_docs_links.py +++ b/tools/public_docs_links.py @@ -91,12 +91,11 @@ def _line_problems(line: str, kinds: Mapping[PurePosixPath, str]) -> Iterator[st def _link_problem(url: str, kinds: Mapping[PurePosixPath, str]) -> str | None: # The URL path is //rae///; REPOSITORY_LINK fixes the route. route, ref, *segments = urlsplit(url).path.split("/")[3:] - if ref not in CHECKED_REFS: - return None path = PurePosixPath(unquote("/".join(segments))) expected = ROUTE_KINDS[route] actual = kinds.get(path) - if actual == expected: + # 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}"