diff --git a/tests/test_pr_preview.py b/tests/test_pr_preview.py index f7c136c9..fff2bc1b 100644 --- a/tests/test_pr_preview.py +++ b/tests/test_pr_preview.py @@ -47,14 +47,14 @@ def fake_sphinx(source: Path, output: Path, entry: dict, catalog: dict, *, review_only: bool = False) -> None: assert review_only output.mkdir(parents=True) - (output / "index.html").write_text("Review manual") + (output / "index.html").write_text("Review manual") target = output / "webhooks" target.mkdir() (target / "index.html").write_text( - 'Event') - (target / "new_event.html").write_text("Native event") + 'Event') + (target / "new_event.html").write_text("Native event") (output / "api").mkdir() - (output / "api/openapi.html").write_text("OpenAPI") + (output / "api/openapi.html").write_text("OpenAPI") def test_future_pages_are_built_and_linked_without_filename_registration(self): with tempfile.TemporaryDirectory() as tmp: diff --git a/tests/test_seo.py b/tests/test_seo.py new file mode 100644 index 00000000..47975801 --- /dev/null +++ b/tests/test_seo.py @@ -0,0 +1,118 @@ +# SPDX-FileCopyrightText: 2026 LibreCode coop and contributors +# SPDX-License-Identifier: CC-BY-3.0 + +"""Regression coverage for permanent URLs and indexable documentation.""" +import json +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +from versioning.build import legacy_redirects +from versioning.seo import ( + PUBLIC_ORIGIN, apply_seo, canonical_url, mark_preview_noindex, + read_legacy_routes, verify_seo, +) + + +class DocumentationSeoTests(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + self.root = Path(self.tmp.name) + self.site = self.root / "site" + self.site.mkdir() + self.catalog = {"versions": [{"slug": "15"}, {"slug": "main"}], + "current_stable": "15"} + + def page(self, name: str, body: str = "

Article

"): + path = self.site / name + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(f"Documentation" + f"{body}", encoding="utf-8") + return path + + def test_historic_routes_cover_all_existing_manual_pages(self): + root = Path(__file__).resolve().parent.parent + routes = read_legacy_routes(root) + source = {f"{manual}/{p.relative_to(root / manual).with_suffix('.html')}" + for manual in ("user_manual", "admin_manual", "developer_manual") + for p in (root / manual).rglob("*.rst")} + self.assertEqual(set(routes), source) + self.assertEqual(len(routes), 46) + self.assertEqual(routes["user_manual/functionality.html"], + "main/user_manual/functionality.html") + self.assertEqual(routes["admin_manual/installation.html"], + "main/admin_manual/installation.html") + self.assertEqual(routes["developer_manual/architecture.html"], + "main/developer_manual/architecture.html") + + def test_legacy_redirects_keep_individual_targets_across_new_stable(self): + self.page("15/index.html") + for name in ("15/user_manual/index.html", + "main/user_manual/functionality.html"): + self.page(name) + (self.site / "versions.json").write_text( + json.dumps(self.catalog), encoding="utf-8") + self.page("index.html", '
') + with patch("versioning.build.version_overview", return_value=self.page("index.html").read_text()): + legacy_redirects(self.site, "15", routes={ + "user_manual/index.html": "15/user_manual/index.html", + "user_manual/functionality.html": "main/user_manual/functionality.html", + }) + redirect = (self.site / "user_manual/functionality.html").read_text() + self.assertIn("url=/main/user_manual/functionality.html", redirect) + self.assertNotIn("url=/15/user_manual/index.html", redirect) + + def test_missing_redirect_destination_fails_before_deploy(self): + self.page("15/index.html") + (self.site / "versions.json").write_text(json.dumps(self.catalog)) + with self.assertRaisesRegex(ValueError, "Missing historical destination"): + legacy_redirects(self.site, "15", routes={ + "user_manual/functionality.html": "main/user_manual/missing.html" + }) + + def test_published_sitemap_and_canonical_are_exact(self): + self.page("index.html") + self.page("15/index.html") + self.page("15/user_manual/index.html") + self.page("15/user_manual/guide.html") + self.page("main/index.html") + self.page("15/user_manual/search.html") + self.page("15/user_manual/404.html") + apply_seo(self.site, self.catalog) + self.assertEqual(verify_seo(self.site, self.catalog), []) + self.assertIn(f"{PUBLIC_ORIGIN}/15/user_manual/guide.html", + (self.site / "sitemap.xml").read_text()) + self.assertNotIn("search.html", (self.site / "sitemap.xml").read_text()) + self.assertIn('content="noindex,follow"', + (self.site / "15/user_manual/search.html").read_text()) + + def test_preview_content_is_not_indexable(self): + self.page("index.html") + self.page("15/user_manual/guide.html") + self.page("_review/16/developer_manual/index.html") + apply_seo(self.site, self.catalog, preview=True) + mark_preview_noindex(self.site / "_review") + self.assertEqual(verify_seo(self.site, self.catalog, preview=True), []) + self.assertFalse((self.site / "sitemap.xml").exists()) + + def test_unsafe_and_generic_legacy_destinations_are_rejected(self): + source = self.root / "src" + (source / "versioned").mkdir(parents=True) + for bad in ("15/user_manual/index.html", "../../private.html"): + (source / "versioned/legacy-routes.json").write_text(json.dumps({ + "user_manual/functionality.html": bad + })) + with self.assertRaises(ValueError): + read_legacy_routes(source) + + def test_canonical_has_stable_directory_semantics(self): + self.assertEqual(canonical_url(Path("15/user_manual/index.html")), + f"{PUBLIC_ORIGIN}/15/user_manual/") + self.assertEqual(canonical_url(Path("15/user_manual/guide.html")), + f"{PUBLIC_ORIGIN}/15/user_manual/guide.html") + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_versioning.py b/tests/test_versioning.py index c36719fe..420bbe20 100644 --- a/tests/test_versioning.py +++ b/tests/test_versioning.py @@ -98,7 +98,8 @@ def test_updating_15_preserves_existing_13_14_and_main(self): (incoming / "versions.json").write_text(json.dumps(updated)) (incoming / "15").mkdir() (incoming / "15/index.html").write_text("15-updated") - result_catalog = assemble(incoming, result, "15", published) + with patch("versioning.publish.legacy_redirects"), patch("versioning.publish.apply_seo"): + result_catalog = assemble(incoming, result, "15", published) self.assertEqual((result / "13/saved.html").read_text(), "do-not-erase") self.assertEqual((result / "14/index.html").read_text(), "14-original") self.assertEqual((result / "main/index.html").read_text(), "main-original") @@ -204,7 +205,7 @@ def test_preview_review_link_is_valid_during_initial_site_verification(self): for filename in ("home.css", "home-theme.js", "logo.png"): (root / filename).write_text("asset") with patch.dict("os.environ", {"LIBRESIGN_PREVIEW_PR": "127"}): - legacy_redirects(root, "15") + legacy_redirects(root, "15", routes={}) self.assertEqual(inspect_links(root, root / "index.html"), []) self.assertTrue((root / "_review/index.html").is_file()) @@ -219,7 +220,7 @@ def test_production_does_not_create_review_page(self): (root / "15/index.html").parent.mkdir() (root / "15/index.html").write_text("Stable") with patch.dict("os.environ", {"LIBRESIGN_PREVIEW_PR": ""}): - legacy_redirects(root, "15") + legacy_redirects(root, "15", routes={}) self.assertFalse((root / "_review/index.html").exists()) self.assertNotIn('href="_review/"', (root / "index.html").read_text()) diff --git a/versioned/legacy-routes.json b/versioned/legacy-routes.json new file mode 100644 index 00000000..1d8a66ab --- /dev/null +++ b/versioned/legacy-routes.json @@ -0,0 +1,48 @@ +{ + "admin_manual/configuration.html": "main/admin_manual/configuration.html", + "admin_manual/index.html": "15/admin_manual/index.html", + "admin_manual/installation.html": "main/admin_manual/installation.html", + "admin_manual/maintenance.html": "main/admin_manual/maintenance.html", + "admin_manual/security.html": "main/admin_manual/security.html", + "developer_manual/api/external-email-signers.html": "15/developer_manual/api/external-email-signers.html", + "developer_manual/api/guide-api.html": "main/developer_manual/api/guide-api.html", + "developer_manual/api/index.html": "15/developer_manual/api/index.html", + "developer_manual/api/openapi.html": "15/developer_manual/api/openapi.html", + "developer_manual/architecture.html": "main/developer_manual/architecture.html", + "developer_manual/development-workflow.html": "main/developer_manual/development-workflow.html", + "developer_manual/error-handling.html": "main/developer_manual/error-handling.html", + "developer_manual/getting-started/branch-policies.html": "main/developer_manual/getting-started/branch-policies.html", + "developer_manual/getting-started/commits.html": "main/developer_manual/getting-started/commits.html", + "developer_manual/getting-started/development-environment/coding-standards.html": "main/developer_manual/getting-started/development-environment/coding-standards.html", + "developer_manual/getting-started/development-environment/frontend.html": "main/developer_manual/getting-started/development-environment/frontend.html", + "developer_manual/getting-started/development-environment/index.html": "main/developer_manual/getting-started/development-environment/index.html", + "developer_manual/getting-started/development-environment/setup.html": "main/developer_manual/getting-started/development-environment/setup.html", + "developer_manual/getting-started/development-environment/updating.html": "main/developer_manual/getting-started/development-environment/updating.html", + "developer_manual/getting-started/index.html": "main/developer_manual/getting-started/index.html", + "developer_manual/getting-started/tests.html": "main/developer_manual/getting-started/tests.html", + "developer_manual/index.html": "15/developer_manual/index.html", + "developer_manual/integration/application.html": "15/developer_manual/integration/application.html", + "developer_manual/integration/deployment.html": "15/developer_manual/integration/deployment.html", + "developer_manual/integration/identity.html": "15/developer_manual/integration/identity.html", + "developer_manual/integration/index.html": "15/developer_manual/integration/index.html", + "developer_manual/integration/lifecycle.html": "15/developer_manual/integration/lifecycle.html", + "developer_manual/prologue/code-of-conduct.html": "main/developer_manual/prologue/code-of-conduct.html", + "developer_manual/prologue/community-support.html": "main/developer_manual/prologue/community-support.html", + "developer_manual/prologue/index.html": "main/developer_manual/prologue/index.html", + "developer_manual/prologue/reporting-bugs.html": "main/developer_manual/prologue/reporting-bugs.html", + "developer_manual/release-process.html": "main/developer_manual/release-process.html", + "developer_manual/release-process/configuration.html": "main/developer_manual/release-process/configuration.html", + "developer_manual/release-process/manual.html": "main/developer_manual/release-process/manual.html", + "developer_manual/release-process/milestones.html": "main/developer_manual/release-process/milestones.html", + "developer_manual/release-process/preparing.html": "main/developer_manual/release-process/preparing.html", + "developer_manual/release-process/publishing.html": "main/developer_manual/release-process/publishing.html", + "developer_manual/release-process/reusable-tooling.html": "main/developer_manual/release-process/reusable-tooling.html", + "developer_manual/release-process/versioning.html": "main/developer_manual/release-process/versioning.html", + "developer_manual/requesting-features.html": "main/developer_manual/requesting-features.html", + "developer_manual/translation.html": "main/developer_manual/translation.html", + "user_manual/faq.html": "main/user_manual/faq.html", + "user_manual/functionality.html": "main/user_manual/functionality.html", + "user_manual/index.html": "15/user_manual/index.html", + "user_manual/start.html": "main/user_manual/start.html", + "user_manual/suport.html": "main/user_manual/suport.html" +} diff --git a/versioning/build.py b/versioning/build.py index 905b0e93..27683bce 100644 --- a/versioning/build.py +++ b/versioning/build.py @@ -148,7 +148,7 @@ def version_overview(catalog: dict) -> str: ) -def legacy_redirects(output: Path, current: str) -> None: +def legacy_redirects(output: Path, current: str, *, routes: dict[str, str] | None = None) -> None: destination = output / current if not (destination / "index.html").is_file(): raise ValueError("Cannot create legacy redirects without stable landing page") @@ -168,26 +168,19 @@ def legacy_redirects(output: Path, current: str) -> None: "

Release-specific review pages are generated after validation.

", encoding="utf-8", ) - # Previous URLs did not include a version prefix. Preserve page paths - # when the same page exists in current stable; otherwise go to that - # manual's landing page, never silently to a development-only page. - for manual in MANUALS[1:]: - base = output / manual - base.mkdir(parents=True, exist_ok=True) - main_source = output / "main" / manual - for page in main_source.rglob("*.html"): - relative = page.relative_to(main_source) - target = destination / manual / relative - if not target.is_file(): - target = destination / manual / "index.html" - redirect = base / relative - redirect.parent.mkdir(parents=True, exist_ok=True) - redirect.write_text( - redirect_html("/" + str(target.relative_to(output)).replace(os.sep, "/")), - encoding="utf-8", - ) - (base / "index.html").write_text( - redirect_html(f"/{current}/{manual}/"), encoding="utf-8", + # The previous unversioned URLs are a permanent migration interface. + # Do not move their destinations when a future stable line is published. + from versioning.seo import read_legacy_routes + + for old, target in (read_legacy_routes(ROOT) if routes is None else routes).items(): + destination_page = output / target + if not destination_page.is_file(): + raise ValueError(f"Missing historical destination: {old} => {target}") + legacy_page = output / old + legacy_page.parent.mkdir(parents=True, exist_ok=True) + legacy_page.write_text( + redirect_html("/" + target), + encoding="utf-8", ) diff --git a/versioning/pr_preview.py b/versioning/pr_preview.py index 914ff71b..471867e8 100644 --- a/versioning/pr_preview.py +++ b/versioning/pr_preview.py @@ -16,6 +16,7 @@ from versioning.build import ROOT, run_sphinx, source_tree from versioning.verify import inspect_links +from versioning.seo import mark_preview_noindex PREVIEW_MARKER = "" REVIEW_MANUALS = ("user_manual", "admin_manual", "developer_manual") @@ -138,6 +139,7 @@ def preview_unreleased(site: Path, preview_root: str) -> list[str]: local /= "index.html" if not local.is_file(): raise ValueError(f"Preview route is missing: {route}") + mark_preview_noindex(review) (review / "routes.txt").write_text( "\n".join(unique_routes) + "\n", encoding="utf-8" ) diff --git a/versioning/publish.py b/versioning/publish.py index cd7a6e3e..0656cceb 100644 --- a/versioning/publish.py +++ b/versioning/publish.py @@ -3,10 +3,12 @@ import argparse import json +import os import shutil from pathlib import Path from versioning.build import legacy_redirects +from versioning.seo import apply_seo ROOT = Path(__file__).resolve().parent.parent @@ -62,6 +64,7 @@ def assemble(built: Path, destination: Path, selected: str, legacy_redirects(destination, output_catalog["current_stable"]) (destination / "CNAME").write_text("docs.libresign.coop\n", encoding="utf-8") (destination / ".nojekyll").touch() + apply_seo(destination, output_catalog, preview=bool(os.environ.get("LIBRESIGN_PREVIEW_PR"))) return output_catalog diff --git a/versioning/seo.py b/versioning/seo.py new file mode 100644 index 00000000..43b8fd9a --- /dev/null +++ b/versioning/seo.py @@ -0,0 +1,178 @@ +# SPDX-FileCopyrightText: 2026 LibreCode coop and contributors +# SPDX-License-Identifier: CC-BY-3.0 + +"""Canonical URLs, migration routes, and sitemap checks for published docs.""" +from __future__ import annotations + +import json +import re +import xml.etree.ElementTree as ET +from html import escape +from pathlib import Path + +PUBLIC_ORIGIN = "https://docs.libresign.coop" +SITEMAP_NS = "http://www.sitemaps.org/schemas/sitemap/0.9" +AUXILIARY = {"404.html", "genindex.html", "search.html"} +MANUALS = {"user_manual", "admin_manual", "developer_manual"} + + +def canonical_url(path: Path) -> str: + """Return the single public URL used in HTML canonical and sitemap.""" + parts = path.parts + if not parts or path.is_absolute() or ".." in parts: + raise ValueError(f"Invalid published path: {path}") + if path.name == "index.html": + parent = path.parent.as_posix() + return PUBLIC_ORIGIN + ("/" if parent == "." else f"/{parent}/") + return f"{PUBLIC_ORIGIN}/{path.as_posix()}" + + +def _published_pages(site: Path, versions: set[str]) -> tuple[list[Path], list[Path]]: + """Separate indexable content pages from generated Sphinx utility pages.""" + content, auxiliary = [], [] + for page in sorted(site.rglob("*.html")): + path = page.relative_to(site) + if path == Path("index.html"): + content.append(page) + elif path.parts[0] in versions and not any(part.startswith("_") for part in path.parts): + if path.name in AUXILIARY or "swagger-ui" in path.parts: + auxiliary.append(page) + else: + content.append(page) + return content, auxiliary + + +def _add_head_tag(page: Path, tag: str, *, guard: str) -> None: + markup = page.read_text(encoding="utf-8") + if re.search(guard, markup, re.IGNORECASE): + if tag not in markup: + raise ValueError(f"Conflicting SEO metadata in {page}") + return + if "" not in markup: + raise ValueError(f"Missing HTML head in {page}") + page.write_text(markup.replace("", f" {tag}\n", 1), encoding="utf-8") + + +def _add_canonical(page: Path, url: str) -> None: + tag = f'' + _add_head_tag(page, tag, guard=r']*\brel=["\']canonical["\']') + + +def _add_noindex(page: Path) -> None: + tag = '' + _add_head_tag(page, tag, guard=r']*\bname=["\']robots["\']') + + +def mark_preview_noindex(directory: Path) -> None: + for page in directory.rglob("*.html"): + _add_noindex(page) + + +def apply_seo(site: Path, catalog: dict, *, preview: bool = False) -> None: + """Publish canonical URLs only after all versions and redirects are assembled.""" + versions = {entry["slug"] for entry in catalog["versions"]} + content, auxiliary = _published_pages(site, versions) + if preview: + mark_preview_noindex(site) + (site / "sitemap.xml").unlink(missing_ok=True) + (site / "robots.txt").unlink(missing_ok=True) + return + + urls = [] + for page in content: + url = canonical_url(page.relative_to(site)) + _add_canonical(page, url) + urls.append(url) + for page in auxiliary: + _add_noindex(page) + + ET.register_namespace("", SITEMAP_NS) + root = ET.Element(f"{{{SITEMAP_NS}}}urlset") + for url in sorted(set(urls)): + ET.SubElement(ET.SubElement(root, f"{{{SITEMAP_NS}}}url"), + f"{{{SITEMAP_NS}}}loc").text = url + ET.ElementTree(root).write(site / "sitemap.xml", encoding="utf-8", + xml_declaration=True) + (site / "robots.txt").write_text( + f"User-agent: *\nAllow: /\nSitemap: {PUBLIC_ORIGIN}/sitemap.xml\n", + encoding="utf-8", + ) + + +def read_legacy_routes(root: Path) -> dict[str, str]: + """The historic unversioned routes are an immutable migration contract.""" + routes = json.loads((root / "versioned/legacy-routes.json").read_text(encoding="utf-8")) + if not isinstance(routes, dict) or not routes: + raise ValueError("Legacy URL mapping must be a nonempty object") + for old, target in routes.items(): + for value in (old, target): + if not isinstance(value, str) or not value.endswith(".html"): + raise ValueError(f"Unsafe legacy mapping: {old} => {target}") + path = Path(value) + if (path.is_absolute() or ".." in path.parts or + "//" in value or "\\" in value or + any(not component for component in path.parts)): + raise ValueError(f"Unsafe legacy mapping: {old} => {target}") + if len(Path(old).parts) < 2 or len(Path(target).parts) < 3: + raise ValueError(f"Invalid legacy route shape: {old} => {target}") + if Path(old).parts[0] not in MANUALS: + raise ValueError(f"Legacy source is not a manual: {old}") + if Path(target).parts[0] != "main" and not re.fullmatch(r"[1-9][0-9]*", Path(target).parts[0]): + raise ValueError(f"Legacy destination is not pinned: {target}") + if old.split("/")[0] != target.split("/")[1]: + raise ValueError(f"Legacy destination changes manual: {old} => {target}") + if Path(old).name != "index.html" and Path(target).name == "index.html": + raise ValueError(f"Legacy article redirects to generic index: {old}") + return routes + + +def verify_seo(site: Path, catalog: dict, *, preview: bool = False, + root: Path | None = None) -> list[str]: + """Check the generated artifact, not just its source configuration.""" + versions = {entry["slug"] for entry in catalog["versions"]} + content, auxiliary = _published_pages(site, versions) + errors = [] + if preview: + for page in site.rglob("*.html"): + if 'name="robots" content="noindex,follow"' not in page.read_text(encoding="utf-8"): + errors.append(f"Preview page can be indexed: {page.relative_to(site)}") + if (site / "sitemap.xml").exists(): + errors.append("Preview publishes production sitemap") + return errors + + expected = {canonical_url(page.relative_to(site)) for page in content} + if not (site / "sitemap.xml").is_file(): + errors.append("Missing sitemap.xml") + else: + try: + tree = ET.parse(site / "sitemap.xml") + actual = [item.text for item in tree.findall(f".//{{{SITEMAP_NS}}}loc")] + if set(actual) != expected or len(actual) != len(expected): + errors.append("Sitemap URLs differ from indexable content pages") + except ET.ParseError as exc: + errors.append(f"Invalid sitemap XML: {exc}") + if not (site / "robots.txt").is_file() or ( + f"Sitemap: {PUBLIC_ORIGIN}/sitemap.xml" not in + (site / "robots.txt").read_text(encoding="utf-8") + ): + errors.append("robots.txt must disclose the canonical sitemap") + for page in content: + expected_tag = f'' + markup = page.read_text(encoding="utf-8") + if markup.count(expected_tag) != 1: + errors.append(f"Missing or duplicated canonical: {page.relative_to(site)}") + if 'name="robots" content="noindex' in markup: + errors.append(f"Production content has noindex: {page.relative_to(site)}") + for page in auxiliary: + if 'name="robots" content="noindex,follow"' not in page.read_text(encoding="utf-8"): + errors.append(f"Utility page is indexable: {page.relative_to(site)}") + if root is not None: + for source, destination in read_legacy_routes(root).items(): + old_page, new_page = site / source, site / destination + if not old_page.is_file() or not new_page.is_file(): + errors.append(f"Missing legacy mapping destination: {source} => {destination}") + continue + markup = old_page.read_text(encoding="utf-8") + if f"url=/{destination}" not in markup or 'http-equiv=\'refresh\'' not in markup: + errors.append(f"Incorrect legacy redirect: {source} => {destination}") + return errors diff --git a/versioning/verify.py b/versioning/verify.py index cb0b06d5..d8dc0239 100644 --- a/versioning/verify.py +++ b/versioning/verify.py @@ -11,6 +11,7 @@ from urllib.parse import unquote, urlsplit from versioning.catalog import fetch_source, operations, validate_contract +from versioning.seo import verify_seo REQUIRED_MANUALS = ("user_manual", "admin_manual", "developer_manual") @@ -149,6 +150,9 @@ def verify_site(output: Path, versions: list[str] | None = None, for slug in versions: for page in (output / slug).rglob("*.html"): errors.extend(inspect_links(output, page)) + if (output / ".nojekyll").exists(): + errors.extend(verify_seo(output, catalog, + preview=bool(os.environ.get("LIBRESIGN_PREVIEW_PR")))) if errors: raise ValueError("\n".join(errors[:60])) return {