From 2870bcceff1b1a5d469f57d76536f17462e0a7be Mon Sep 17 00:00:00 2001 From: Chelsea Kelly-Reif <3114598+ChelseaKR@users.noreply.github.com> Date: Sun, 13 Sep 2026 08:40:55 -0700 Subject: [PATCH] The sitemap said all 53 pages changed today, every day mkdocs fills from get_build_date(), so every entry carried the day the docs were built. Measured live 2026-09-13: 53 of 53 read 2026-09-13, including a page whose source had not moved since 2026-08-16. Each page is now dated from the commit that last changed its own source, and a page with no such commit carries no element at all. A shallow checkout is refused outright rather than consulted: `git log -1 -- ` answers there for every path with the tip's date, which would have reinstated the build date under a new name. The build prints how many of how many pages it could date and names the rest; `sprout site-check` fails on a malformed, impossible or future date, and on a sitemap that dates nothing at all. The live sentinel no longer strips before comparing, so the deployed dates are now compared byte for byte. --- .github/workflows/ci.yml | 8 + .github/workflows/live-integrity.yml | 8 + .github/workflows/pages.yml | 8 + CHANGELOG.md | 56 ++++++ docs_hooks/sitemap_lastmod.py | 253 +++++++++++++++++++++++++++ mkdocs.yml | 9 + scripts/verify_live_site.py | 58 +++--- src/sprout/cli.py | 11 +- src/sprout/site_meta.py | 105 ++++++++++- tests/test_site_meta.py | 125 ++++++++++++- tests/test_sitemap_lastmod.py | 169 ++++++++++++++++++ theme_overrides/sitemap.xml | 26 +++ 12 files changed, 794 insertions(+), 42 deletions(-) create mode 100644 docs_hooks/sitemap_lastmod.py create mode 100644 tests/test_sitemap_lastmod.py create mode 100644 theme_overrides/sitemap.xml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b3e7ae9..3c5e41f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -321,6 +321,14 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 with: persist-credentials: false + # The sitemap's is the date of the commit that last changed each + # page (docs_hooks/sitemap_lastmod.py). A depth-1 checkout cannot answer that + # and does not fail trying: `git log -1 -- ` returns the tip commit's + # date for every path, because relative to no parent that commit introduces + # the whole tree. The hook refuses a shallow checkout rather than publish 53 + # identical dates, so without full history the sitemap ships no at + # all and `sprout site-check` fails. + fetch-depth: 0 - uses: astral-sh/setup-uv@bec219d24cd3e171d82865faccec33120bb574f4 # v10.1.0 with: enable-cache: true diff --git a/.github/workflows/live-integrity.yml b/.github/workflows/live-integrity.yml index e2ae571..a3b07b6 100644 --- a/.github/workflows/live-integrity.yml +++ b/.github/workflows/live-integrity.yml @@ -56,6 +56,14 @@ jobs: with: ref: main persist-credentials: false + # The sitemap's is the date of the commit that last changed each + # page (docs_hooks/sitemap_lastmod.py). A depth-1 checkout cannot answer that + # and does not fail trying: `git log -1 -- ` returns the tip commit's + # date for every path, because relative to no parent that commit introduces + # the whole tree. The hook refuses a shallow checkout rather than publish 53 + # identical dates, so without full history the sitemap ships no at + # all and `sprout site-check` fails. + fetch-depth: 0 - uses: astral-sh/setup-uv@bec219d24cd3e171d82865faccec33120bb574f4 # v10.1.0 with: diff --git a/.github/workflows/pages.yml b/.github/workflows/pages.yml index ddb0b84..7f5d1ad 100644 --- a/.github/workflows/pages.yml +++ b/.github/workflows/pages.yml @@ -25,6 +25,14 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false + # The sitemap's is the date of the commit that last changed each + # page (docs_hooks/sitemap_lastmod.py). A depth-1 checkout cannot answer that + # and does not fail trying: `git log -1 -- ` returns the tip commit's + # date for every path, because relative to no parent that commit introduces + # the whole tree. The hook refuses a shallow checkout rather than publish 53 + # identical dates, so without full history the sitemap ships no at + # all and `sprout site-check` fails. + fetch-depth: 0 # No caching in this deploy job: it holds pages: write + id-token: write, and a # poisoned/stale cache is the standard's canonical cache-poisoning exposure for a # credentialed deploy job (CI-CD-STANDARD §8c, CICD-24) — same reasoning as release.yml. diff --git a/CHANGELOG.md b/CHANGELOG.md index e6c395d..12cd9c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -118,6 +118,62 @@ fixes. Security entries reference the advisory (GHSA) per the portfolio release that triggered the run instead of creating one, and `publish-pypi` names the environment's url so the `pypi` required reviewer can see what they are approving. +- **The sitemap told every crawler that all 53 pages changed today, every day.** mkdocs + fills `` from `get_build_date()`, so a docs site rebuilt from every push to + `main` published the day of the build as the day of the content: measured live on + 2026-09-13, all 53 of 53 entries read `2026-09-13`, including `/PUBLICATION-READINESS/`, + whose source had not been touched since 2026-08-16. `` is the one element in a + sitemap that is a factual claim about the content, and this one was a claim about the + CI runner — the same "absence rendered as a value" shape as a failed read published as a + measurement, here as *"we did not measure this per page"* published as today's date. + + `docs_hooks/sitemap_lastmod.py` now dates each page from the commit that last changed + its own source (`git log -1 --format=%cs`), and `theme_overrides/sitemap.xml` emits the + element **only** for a page it could date. The same build now publishes 52 dates across + 16 distinct days from 2026-06-22 to 2026-09-07, and none of them is the build's own. + + **`/` is deliberately left undated.** `docs-pages` copies `web-static/public/` over the + mkdocs output, so the document served at the root is the zero-server reference surface + and not `docs/index.md`. Dating that URL from `docs/index.md`'s history would publish a + change date for a document nobody receives, which is the defect wearing different + clothes. + + **A shallow checkout is refused rather than trusted.** `git log -1 -- ` does not + fail in a depth-1 clone: it answers for *every* path with the tip commit's date, because + relative to no parent that single commit introduces the whole tree. Nothing about the + reply is malformed, so a reader that asks and then sanity-checks the answer would have + reinstated the build date under a new name. The hook therefore asks + `git rev-parse --is-shallow-repository` first, once, and publishes nothing at all from a + checkout that cannot answer. `tests/test_sitemap_lastmod.py` proves both halves against a + real depth-1 clone: that the naive question does come back with the wrong date there, and + that the hook refuses it. + + **The count is checked against the file rather than trusted.** The hook computes + dates and the template publishes them, and the first cannot see the second. Measured + while writing this: with the override removed and the hook left in place, the build + printed `52 of 53 pages state when they last changed` while the file it had just + written carried the build date on all 53 — a reassuring number beside the exact + artifact it was wrong about. `on_post_build` now reads the written sitemap back and + fails the build (a warning, and `mkdocs build --strict` is how the docs are built) + unless its dates are precisely the derived ones, compared as multisets so a fallback + that repeats one date cannot pass on a count. + + **Two voices, because an omission is honest and silent.** The build prints + `52 of 53 pages state when they last changed` and names every page it could not date; + `sprout site-check` — a merge gate, and a step of `docs-pages` — fails when a published + `` is malformed, impossible, or in the future, and when *none* of the entries + carries one at all, which is exactly what a lost `fetch-depth: 0` produces. The three + jobs that build the docs (`ci.yml`'s `docs`, `pages.yml`, `live-integrity.yml`) now + check out with full history, and say in place why. + + **And the live check can now see it.** `scripts/verify_live_site.py` used to strip every + `` before comparing the deployed `sitemap.xml`, because those elements were the + build's own date and moved on their own. They are now a property of the commit, so the + file is compared byte for byte, dates included — the elements the old exclusion removed + are precisely the ones this change corrects, and a sentinel blind to them could not have + told an honest deployed sitemap from a build-stamped one. `sitemap.xml.gz` keeps its one + documented exclusion (a gzip MTIME) and is still checked by decompressing it. + - **The only required status check could not name the gate that failed, and could pass having checked nothing.** `ci-gate` is the single required check for branch protection, so it is the one place a reader looks when a merge is blocked. It joined diff --git a/docs_hooks/sitemap_lastmod.py b/docs_hooks/sitemap_lastmod.py new file mode 100644 index 0000000..5435e3d --- /dev/null +++ b/docs_hooks/sitemap_lastmod.py @@ -0,0 +1,253 @@ +"""State when each page last changed, or state nothing. + +mkdocs fills every ```` in ``sitemap.xml`` from +``mkdocs.utils.get_build_date()`` -- the day the build ran. On a site rebuilt from +every push to ``main`` that makes all 53 entries read today's date, on every page, +every day: a document untouched since August advertises itself as changed this +morning, and will advertise itself as changed tomorrow morning too. ``lastmod`` is +the one element in a sitemap that is a factual claim about the content, and that +claim was about the CI runner. + +This hook replaces it with the date of the commit that last changed the page's own +source file, and -- where there is no such commit -- with nothing at all. +```` is optional in the sitemaps protocol, so an omitted element is a +valid sitemap that says nothing false, which a stamped-today one is not. + +It pairs with ``theme_overrides/sitemap.xml``, which emits the element only for a +page this hook could date. Nothing here writes a date; every date it publishes is +one git already recorded. + +WHY IT ASKS WHETHER THE CHECKOUT CAN ANSWER, NOT WHETHER AN ANSWER ARRIVED + +``git log -1 --format=%cs -- `` does not fail in a depth-1 clone. It answers, +for *every* path, with the tip commit's date: relative to no parent, the single +grafted commit introduces the whole tree. Nothing about that reply is malformed, so +a reader that asks and then sanity-checks what came back cannot tell it from a real +answer -- it would quietly reinstate the build date under a new name, on every page +at once, which is the defect this hook exists to remove. So the first question is +whether this checkout has the history to answer at all +(``git rev-parse --is-shallow-repository``), asked once, before any date is read. + +TWO VOICES + +An omission is the honest artifact and a silent one. So the build says out loud how +many entries it could date and names the routes it could not, and +``sprout site-check`` (a merge gate, and a step of ``docs-pages``) reads the +published sitemap and fails when it carries no date at all -- which is what a +shallow checkout, or a lost ``fetch-depth: 0``, actually produces. + +AND THE COUNT IS CHECKED AGAINST THE FILE, NOT TRUSTED + +This hook computes dates; ``theme_overrides/sitemap.xml`` publishes them. Those are +two things, and the first cannot see the second. Measured while writing this: with +the template override removed and the hook left in place, the build still printed +``52 of 53 pages state when they last changed`` while the file it had just written +carried the build date on all 53 -- a reassuring number beside the exact artifact it +was wrong about. So ``on_post_build`` reads the sitemap back and fails the build +unless the dates in it are precisely the dates computed here. + +Registered as a mkdocs hook in ``mkdocs.yml``. It runs at build time only, shells +out to git only, and touches nothing at runtime. +""" + +from __future__ import annotations + +import logging +import re +import subprocess +from pathlib import Path +from typing import Any + +log = logging.getLogger("mkdocs.hooks.sitemap_lastmod") + +#: What a published ```` is allowed to look like. ``git log --format=%cs`` +#: emits exactly this, and ``sprout site-check`` refuses anything else, so a page +#: that declares its own ``lastmod`` in front matter is held to the same shape. +_ISO_DATE = re.compile(r"^\d{4}-\d{2}-\d{2}$") + +#: Source pages whose built HTML is not what the site serves at that address, so +#: their source's history is not the history of the published document. +#: +#: ``docs-pages`` (and ``make site-check``) copy ``web-static/public/`` over the +#: mkdocs output after the build, so ``/`` is served by the zero-server reference +#: surface and not by ``docs/index.md``. Dating that URL from ``docs/index.md`` +#: would publish a change date for a document nobody receives. The reference +#: surface is assembled from the TypeScript sources *and* the exported corpus +#: bundle, so no single source file's history describes it either: the honest +#: answer for this one URL is no answer. +_NOT_THE_PUBLISHED_DOCUMENT = frozenset({"index.md"}) + +#: Timeout on any single git call. A hung git must not hang a docs build. +_GIT_TIMEOUT_SECONDS = 30.0 + +#: Populated in ``on_files`` and read in ``on_page_markdown``: ``src_uri`` -> date. +#: Reset on every build so ``mkdocs serve``'s rebuilds cannot carry a stale map. +_DATES: dict[str, str] = {} + +#: Every documentation page this build considered, dated or not. Kept so the count +#: printed at the end has a denominator that is the pages, not a second walk of the +#: docs directory that could disagree with the one mkdocs actually built from. +_PAGES: list[str] = [] + +#: Why no page could be dated, or ``None`` when at least one could. Reported once, +#: in ``on_post_build``, so the build says it rather than leaving a silent gap. +_REFUSAL: str | None = None + + +def _git(repo: Path, *args: str) -> str | None: + """One git command's stdout, or ``None`` when git could not answer it.""" + try: + completed = subprocess.run( + ["git", *args], + cwd=repo, + capture_output=True, + text=True, + timeout=_GIT_TIMEOUT_SECONDS, + check=False, + ) + except (OSError, subprocess.SubprocessError): + return None + if completed.returncode != 0: + return None + return completed.stdout.strip() + + +def history_refusal(repo: Path) -> str | None: + """Why this checkout cannot date a page, or ``None`` when it can. + + Asked once per build, before any date is read, for the reason in this module's + header: in a shallow clone the per-path question has a confident wrong answer + rather than no answer. + """ + inside = _git(repo, "rev-parse", "--is-inside-work-tree") + if inside != "true": + return ( + "this tree is not a git checkout, so no page has a recorded change date. " + "The sitemap will carry no at all." + ) + if _git(repo, "rev-parse", "--is-shallow-repository") != "false": + return ( + "this is a shallow (depth-limited) checkout. `git log -1 -- ` answers " + "for every path in one, with the tip commit's date, so asking it here would " + "stamp today's date on every page under a different name. Check out with " + "`fetch-depth: 0` (actions/checkout) or `git fetch --unshallow`. The sitemap " + "will carry no at all until then." + ) + return None + + +def last_changed(repo: Path, source: Path) -> str | None: + """The committer date of the commit that last changed ``source``. + + Committer date rather than author date: it is when the change reached this + branch, which is when the page a reader fetches actually moved. A file with no + commit touching it -- newly added and not yet committed, or generated into the + docs tree by a build step -- has no change date here, and gets none. + """ + answer = _git(repo, "log", "-1", "--format=%cs", "--", str(source)) + if not answer or not _ISO_DATE.match(answer): + return None + return answer + + +def on_files(files: Any, config: Any) -> Any: + """Date every documentation page once, before any of them is rendered.""" + global _REFUSAL + _DATES.clear() + _PAGES.clear() + docs_dir = Path(config["docs_dir"]) + repo = docs_dir.parent + _REFUSAL = history_refusal(repo) + _PAGES.extend(sorted(file.src_uri for file in files.documentation_pages())) + if _REFUSAL is not None: + return files + for source in _PAGES: + if source in _NOT_THE_PUBLISHED_DOCUMENT: + continue + date = last_changed(repo, docs_dir / source) + if date is not None: + _DATES[source] = date + return files + + +def on_page_markdown(markdown: str, page: Any, config: Any, files: Any) -> str: + """Hand the page its own change date, if this checkout knows one. + + A page that states its own ``lastmod`` in front matter keeps it: the page is + closer to its content than its file's history is, and the published value is + held to the same format by ``sprout site-check`` either way. + """ + if not page.meta.get("lastmod"): + date = _DATES.get(page.file.src_uri) + if date is not None: + page.meta["lastmod"] = date + return markdown + + +#: Pulls every ```` out of the written sitemap. A regex and not an XML +#: parser, for the reason ``sprout.site_meta`` gives: this is a file the build just +#: wrote, and a gate that parses XML is still a gate that parses XML. +_PUBLISHED_LASTMOD = re.compile(r"\s*([^<]*?)\s*") + + +def published_dates(site_dir: Path) -> list[str] | None: + """Every ```` in the sitemap this build wrote, or ``None`` if there is none.""" + sitemap = site_dir / "sitemap.xml" + if not sitemap.is_file(): + return None + return _PUBLISHED_LASTMOD.findall(sitemap.read_text(encoding="utf-8")) + + +def on_post_build(config: Any) -> None: + """Say how many entries carry a date, name the ones that do not, and check the file. + + A number with no denominator beside it is the shape this whole change exists to + remove, so the count of pages that could be dated is printed next to the count of + pages there were -- and then read back out of the artifact, because the count and + the artifact are produced by different code and only one of them is published. + """ + if _REFUSAL is not None: + log.warning( + "sitemap : 0 of %d pages state when they last changed -- %s", + len(_PAGES), + _REFUSAL, + ) + else: + log.info( + "sitemap : %d of %d pages state when they last changed", + len(_DATES), + len(_PAGES), + ) + for source in _PAGES: + if source not in _DATES: + log.info("sitemap : no recorded change date for %s", source) + + published = published_dates(Path(config["site_dir"])) + if published is None: + log.warning("sitemap : no sitemap.xml was written, so nothing was checked") + return + for complaint in disagreements(published, sorted(_DATES.values())): + log.warning("sitemap : %s", complaint) + + +def disagreements(published: list[str], computed: list[str]) -> list[str]: + """Every way the written sitemap's dates are not the dates this hook derived. + + Compared as multisets, which is the comparison that catches the failure actually + observed: a template that ignores this hook and falls back to the build date + publishes one date, repeated once per URL, while the hook holds many. Comparing + only the counts would miss it on a day when the two happen to be equal in number. + """ + if sorted(published) == sorted(computed): + return [] + extra = sorted(set(published) - set(computed)) + missing = sorted(set(computed) - set(published)) + complaint = ( + f"the sitemap this build wrote carries {len(published)} date(s) but this hook " + f"derived {len(computed)}; the published dates are not the derived ones" + ) + if extra: + complaint += f". Published and not derived: {', '.join(extra)}" + if missing: + complaint += f". Derived and not published: {', '.join(missing)}" + return [complaint] diff --git a/mkdocs.yml b/mkdocs.yml index 7a33d8d..40caa17 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -7,6 +7,10 @@ copyright: Copyright 2026 Chelsea Kelly-Reif — Apache-2.0 theme: name: material + # Holds one file: sitemap.xml. mkdocs' own template stamps every with + # the build date; this one emits the date docs_hooks/sitemap_lastmod.py derived + # from the page's own history, and omits the element where there is none. + custom_dir: theme_overrides palette: primary: green accent: green @@ -18,8 +22,13 @@ theme: # carried one identical sentence. The hook gives each page the description its own # opening paragraph already states; it writes no new copy and leaves a page that # declares its own description alone. +# Every sitemap entry carried {build date}, so a page untouched since +# August advertised itself as changed this morning, on all 53 URLs, every day. The +# hook dates each page from the commit that last changed its source and publishes +# no date where there is none to publish. hooks: - docs_hooks/page_description.py + - docs_hooks/sitemap_lastmod.py # Google Analytics 4 on every handbook page, with the footer opt-out (ADR 0023). The # loader itself is web-static/public/analytics.js, published at the site root. - docs_hooks/analytics.py diff --git a/scripts/verify_live_site.py b/scripts/verify_live_site.py index 084bbcf..9016ef8 100644 --- a/scripts/verify_live_site.py +++ b/scripts/verify_live_site.py @@ -18,31 +18,35 @@ WHAT IS NOT COMPARED BYTE FOR BYTE, AND WHY -Two files, both stamped by mkdocs with the day the build ran rather than with -anything about the content: - - * `sitemap.xml` carries a `` per URL, which mkdocs fills from - `get_build_date()`. Every one of the 52 entries reads today's date on a page - untouched for weeks. It is compared with those elements removed, so the URL - set, the change frequencies and the priorities are all still gated and only - the build stamp is not. +One file, stamped by mkdocs with the day the build ran rather than with anything +about the content: + * `sitemap.xml.gz` embeds a gzip MTIME field, so its bytes move whenever a build crosses a UTC midnight. It is not compared as bytes. Instead its live body must decompress, and what it decompresses to must be the live `sitemap.xml` byte for byte, which is the property the file exists to have. -Both are day-granular build stamps, and forcing byte equality on them would make -the check fail for a reason that is not drift. Everything else is exact: the -corpus index, the config bundle, every ES module, the shell, the 52 rendered -pages, the theme assets, the search index and all seven audit artifacts. +`sitemap.xml` used to be the second one. Its `` elements were mkdocs' +`get_build_date()`, so every entry read the day of the build and the check had to +strip them to compare anything at all. They are now the date of the commit that +last changed each page (`docs_hooks/sitemap_lastmod.py`), which is a property of +the commit and not of the run, so the file is byte-stable for a given commit and +is compared exactly -- including the dates. That matters: the elements the old +exclusion removed are precisely the ones the fix changed, and a check blind to +them could not have told whether the deployed sitemap carried the honest dates or +the build stamp. + +Everything else is exact too: the corpus index, the config bundle, every ES +module, the shell, the 52 rendered pages, the theme assets, the search index and +all seven audit artifacts. Vacuity is the failure mode a check like this is most exposed to, so four things are refused outright instead of being reported as a pass: * a build tree below the file floor, because a sentinel that compares nothing and prints OK is worse than no sentinel at all (`--minimum`); - * a build tree in which neither excluded file is present, which would mean the - two exclusions above had silently become the whole story; + * a build tree in which the one excluded file is not present, which would mean + the exclusion above had silently stopped describing this build; * any fetch that does not return HTTP 200, an unreachable host included; * an origin that answers a guaranteed-missing path with anything but 404, which is how a catch-all would make every matching comparison meaningless. @@ -57,7 +61,6 @@ import gzip import hashlib import http.client -import re import secrets import ssl import sys @@ -76,7 +79,6 @@ # The two files mkdocs stamps with the build date. See the note above. SITEMAP = "sitemap.xml" SITEMAP_GZ = "sitemap.xml.gz" -LASTMOD = re.compile(rb"\s*[^<]*") MAXIMUM_FILE_BYTES = 16 * 1024 * 1024 EXIT_DIFFERS = 1 @@ -202,15 +204,10 @@ def compare(origin: Origin, inventory: dict[str, bytes], nonce: str) -> list[str continue if relative == SITEMAP: live_sitemap = response.body - live = LASTMOD.sub(b"", response.body) - expected = LASTMOD.sub(b"", expected) - label = " (with the build-stamped lastmod elements removed)" - else: - live = response.body - label = "" + live = response.body if live != expected: differences.append( - f"{relative}{label}: live sha256 {short(live)} ({len(live)} bytes) is " + f"{relative}: live sha256 {short(live)} ({len(live)} bytes) is " f"not the built {short(expected)} ({len(expected)} bytes)" ) differences.extend(compare_sitemap_gz(origin, nonce, live_sitemap)) @@ -298,12 +295,11 @@ def main(argv: list[str] | None = None) -> int: root = REPO / root inventory = built_inventory(root) refuse_an_empty_comparison(len(inventory), args.minimum, "the built tree") - excluded = {name for name in (SITEMAP, SITEMAP_GZ) if name in inventory} - if not excluded: + if SITEMAP_GZ not in inventory: raise LiveSiteError( - f"neither {SITEMAP} nor {SITEMAP_GZ} is in the built tree, so the two " - f"documented exclusions no longer describe this build. Re-read them " - f"before trusting what is left." + f"{SITEMAP_GZ} is not in the built tree, so the one documented " + f"exclusion no longer describes this build. Re-read it before " + f"trusting what is left." ) origin = Origin(args.url, timeout_seconds=args.timeout_seconds) nonce = secrets.token_hex(16) @@ -340,9 +336,9 @@ def main(argv: list[str] | None = None) -> int: total = sum(len(payload) for payload in inventory.values()) print( f"{origin.url} serves exactly what this checkout builds: {len(inventory)} " - f"file(s), {total} bytes. {SITEMAP} was compared without its build-stamped " - f"lastmod elements and {SITEMAP_GZ} was checked by decompressing it, both " - f"for the reasons in this file's header." + f"file(s), {total} bytes. {SITEMAP} was compared byte for byte, its " + f" dates included; {SITEMAP_GZ} was checked by decompressing it, " + f"for the reason in this file's header." ) return 0 diff --git a/src/sprout/cli.py b/src/sprout/cli.py index 76f1430..c4cb1f9 100644 --- a/src/sprout/cli.py +++ b/src/sprout/cli.py @@ -42,7 +42,7 @@ from .config import Config, load_config from .models import Answer from .offline_shell import OfflineShellError, check_offline_shell -from .site_meta import check_site +from .site_meta import check_site, lastmod_coverage from .slo import check_all, covered_files app = typer.Typer(add_completion=False, help="Sprout — grounded, evaluated plant-care assistant.") @@ -688,7 +688,14 @@ def site_check( for problem in problems: typer.echo(f" - {problem}", err=True) raise typer.Exit(1) - typer.echo(f"{root}: every published page's address checks out") + # Both numbers on the success line, not just the reassuring one: a sitemap that + # dated four of its fifty-three pages passes this gate, correctly, and a reader + # of the green line should be able to see that is what happened. + dated, total = lastmod_coverage(root) + typer.echo( + f"{root}: every published page's address checks out; " + f"{dated} of {total} sitemap entries state when the page last changed" + ) @app.command("offline-check") diff --git a/src/sprout/site_meta.py b/src/sprout/site_meta.py index c3a4ae5..265f478 100644 --- a/src/sprout/site_meta.py +++ b/src/sprout/site_meta.py @@ -23,7 +23,13 @@ origin in full -- names a file the build actually wrote; * ``robots.txt`` exists and advertises the sitemap at the origin; * every ```` in ``sitemap.xml`` resolves to a file the build actually - wrote, and every built page appears in the sitemap. + wrote, and every built page appears in the sitemap; +* every ```` in ``sitemap.xml`` is a well-formed ``YYYY-MM-DD`` date that + is not in the future, and at least one entry carries one. ```` is + optional, so an entry without one is fine and a page this build could not date + is meant to have none -- but a sitemap in which *nothing* is dated means the + build had no history to read (a shallow checkout, a lost ``fetch-depth: 0``), + and that is a silent regression to exactly the state this gate was added for. It reads only the built tree. It makes no network call, here or anywhere. """ @@ -32,11 +38,12 @@ import re from collections import defaultdict +from datetime import date from html import unescape as html_unescape from pathlib import Path, PurePosixPath from urllib.parse import urlsplit -__all__ = ["check_site", "page_url"] +__all__ = ["check_site", "lastmod_coverage", "page_url"] # Read with a regex rather than an XML parser. The file is one this build just # wrote, but a gate that parses XML is still a gate that parses XML, and the @@ -44,6 +51,14 @@ # the addresses in a urlset. _URLSET = re.compile(r"\s*(.*?)\s*", re.IGNORECASE | re.DOTALL) +_URL_ENTRY = re.compile(r"(.*?)", re.IGNORECASE | re.DOTALL) +_LASTMOD = re.compile(r"\s*(.*?)\s*", re.IGNORECASE | re.DOTALL) + +#: The one shape a published ```` may take here. The sitemaps protocol +#: allows a full W3C datetime as well, and the builder emits a date, so anything +#: else in the file is a value that came from somewhere this gate does not know +#: about and has not checked. +_ISO_DATE = re.compile(r"^\d{4}-\d{2}-\d{2}$") _TITLE = re.compile(r"]*>(.*?)", re.IGNORECASE | re.DOTALL) # Both of these are quote-aware on purpose. A description is prose, and prose has @@ -290,14 +305,87 @@ def _check_robots(root: Path, origin: str) -> list[str]: return problems -def _check_sitemap(root: Path, origin: str, pages: list[Path]) -> list[str]: +def _entries(text: str) -> list[tuple[str, str | None]]: + """Each ```` in a sitemap as its address and its ````, if any.""" + found: list[tuple[str, str | None]] = [] + for entry in _URL_ENTRY.finditer(text): + block = entry.group(1) + loc = _LOC.search(block) + if loc is None: + continue + stamp = _LASTMOD.search(block) + found.append( + (html_unescape(loc.group(1)), html_unescape(stamp.group(1)) if stamp else None) + ) + return found + + +def lastmod_coverage(root: Path) -> tuple[int, int]: + """How many published sitemap entries state when the page last changed, of how many. + + Both numbers, never just the first. A build that dated nothing prints the same + reassuring line as one that dated everything unless the denominator is beside it. + """ + sitemap = root / "sitemap.xml" + if not sitemap.is_file(): + return (0, 0) + entries = _entries(sitemap.read_text(encoding="utf-8")) + return (sum(1 for _, stamp in entries if stamp), len(entries)) + + +def _check_lastmod(entries: list[tuple[str, str | None]], today: date) -> list[str]: + """Every way a published ```` is not a fact about the content. + + ```` is optional, and that is the point: a page whose change date this + build could not read is supposed to carry none, because an omitted element says + nothing while a stamped one is a claim. So a missing element is never a problem + here and a malformed or impossible one always is. + + A date in the future is treated as broken rather than fresh, for the reason a + future timestamp always is: it is a wrong clock or a wrong record, and every + "is this recent" comparison it is fed into answers yes forever. + """ + problems: list[str] = [] + dated = 0 + for url, stamp in entries: + if stamp is None: + continue + dated += 1 + if not _ISO_DATE.match(stamp): + problems.append( + f"sitemap lastmod for {url} is {stamp!r}, which is not a YYYY-MM-DD date" + ) + continue + try: + stamped = date.fromisoformat(stamp) + except ValueError: + problems.append(f"sitemap lastmod for {url} is {stamp!r}, which is not a real date") + continue + if stamped > today: + problems.append( + f"sitemap lastmod for {url} is {stamp}, which is in the future. " + "A page cannot have changed after now; that is a clock or a record, " + "not freshness." + ) + if entries and dated == 0: + problems.append( + f"not one of the {len(entries)} sitemap entries says when its page last " + "changed. is optional per entry on purpose, but none at all means " + "the build could read no history -- a shallow checkout, or a lost " + "`fetch-depth: 0` -- rather than a site with nothing to date." + ) + return problems + + +def _check_sitemap(root: Path, origin: str, pages: list[Path], today: date) -> list[str]: sitemap = root / "sitemap.xml" if not sitemap.is_file(): return ["sitemap.xml was not published"] text = sitemap.read_text(encoding="utf-8") if not _URLSET.search(text): return ["sitemap.xml is not a urlset"] - listed = [html_unescape(loc) for loc in _LOC.findall(text)] + entries = _entries(text) + listed = [url for url, _ in entries] if not listed: return ["sitemap.xml lists no URL at all"] @@ -308,6 +396,7 @@ def _check_sitemap(root: Path, origin: str, pages: list[Path]) -> list[str]: problems.append(f"sitemap lists {url}, which this build did not write") for url in sorted(built - set(listed)): problems.append(f"{url} was built but is in neither the sitemap nor a noindex") + problems.extend(_check_lastmod(entries, today)) return problems @@ -328,13 +417,17 @@ def _indexable(pages: list[Path], root: Path) -> list[Path]: return kept -def check_site(root: Path, origin: str) -> list[str]: +def check_site(root: Path, origin: str, *, today: date | None = None) -> list[str]: """Every problem with the published metadata of the built site at ``root``. ``origin`` is a bare https origin with no trailing slash. Returns an empty list when the site's claims about where its pages live are all true. + + ``today`` is the day the sitemap's dates are read against, and exists so a test + can pin it. It defaults to the real one. """ origin = origin.rstrip("/") + today = today or date.today() built = sorted(root.rglob("*.html")) pages = _indexable(built, root) if not pages: @@ -374,5 +467,5 @@ def check_site(root: Path, origin: str) -> list[str]: problems.extend(_check_links(built, root, origin)) problems.extend(_check_robots(root, origin)) - problems.extend(_check_sitemap(root, origin, pages)) + problems.extend(_check_sitemap(root, origin, pages, today)) return problems diff --git a/tests/test_site_meta.py b/tests/test_site_meta.py index 7116b6b..7056997 100644 --- a/tests/test_site_meta.py +++ b/tests/test_site_meta.py @@ -9,14 +9,25 @@ import importlib.util import re +from datetime import date from pathlib import Path import pytest -from sprout.site_meta import check_site, page_url +from sprout.site_meta import check_site, lastmod_coverage, page_url _ORIGIN = "https://sprout.example" +#: The day the fixture site is read against. Pinned, because "is this date in the +#: future" is a question whose answer moves on its own otherwise. +_TODAY = date(2026, 9, 13) + +#: When each fixture page last changed. Deliberately neither equal to each other +#: nor to ``_TODAY``: a per-page date that happened to be the build date would pass +#: a gate that only checks the shape of the value. +_CHANGED_HOME = "2026-05-01" +_CHANGED_DOCS = "2026-06-22" + _HOOK = importlib.util.spec_from_file_location( "page_description", Path(__file__).resolve().parent.parent / "docs_hooks" / "page_description.py", @@ -63,17 +74,32 @@ def site(tmp_path: Path) -> Path: (root / "robots.txt").write_text( f"User-agent: *\nAllow: /\n\nSitemap: {_ORIGIN}/sitemap.xml\n", encoding="utf-8" ) + # Two different dates, both months before the day this fixture is built, because + # that gap is the whole subject: a sitemap whose dates are the build's own date + # is indistinguishable from an honest one until the content is older than the + # build, and every page here is. (root / "sitemap.xml").write_text( '\n' '' - f"{_ORIGIN}/" - f"{_ORIGIN}/docs/" + f"{_ORIGIN}/{_CHANGED_HOME}" + f"{_ORIGIN}/docs/{_CHANGED_DOCS}" "\n", encoding="utf-8", ) return root +def _resitemap(site: Path, *entries: str) -> None: + """Replace the fixture's sitemap with these raw ```` entries.""" + (site / "sitemap.xml").write_text( + '\n' + '' + + "".join(entries) + + "\n", + encoding="utf-8", + ) + + def test_a_site_with_nothing_wrong_reports_nothing(site: Path) -> None: assert check_site(site, _ORIGIN) == [] @@ -229,6 +255,99 @@ def test_the_error_page_is_neither_indexed_nor_expected_in_the_sitemap(site: Pat assert check_site(site, _ORIGIN) == [] +# ---------------------------------------------------------------------------------- +# : the one element in a sitemap that is a claim about the content +# ---------------------------------------------------------------------------------- + + +def test_dates_that_are_the_pages_own_pass(site: Path) -> None: + """The fixture's dates are months older than the day it is read on, and stand.""" + assert check_site(site, _ORIGIN, today=_TODAY) == [] + assert lastmod_coverage(site) == (2, 2) + + +def test_a_sitemap_that_dates_nothing_at_all_is_reported(site: Path) -> None: + """The shallow-checkout shape: history unreadable, so every element is dropped. + + Omitting on a page whose change date is unknown is the right answer, + which is exactly why omitting it on *every* page has to be reported: the two are + the same file, and only the denominator tells them apart. + """ + _resitemap( + site, + f"{_ORIGIN}/", + f"{_ORIGIN}/docs/", + ) + assert lastmod_coverage(site) == (0, 2) + assert any( + "not one of the 2 sitemap entries says when its page last changed" in problem + for problem in check_site(site, _ORIGIN, today=_TODAY) + ) + + +def test_one_page_without_a_date_is_not_a_problem(site: Path) -> None: + """A page this build genuinely could not date is meant to carry no element.""" + _resitemap( + site, + f"{_ORIGIN}/", + f"{_ORIGIN}/docs/{_CHANGED_DOCS}", + ) + assert check_site(site, _ORIGIN, today=_TODAY) == [] + assert lastmod_coverage(site) == (1, 2) + + +def test_a_date_in_the_future_is_reported(site: Path) -> None: + """A future date is a broken clock, and it satisfies "is this fresh" forever.""" + _resitemap( + site, + f"{_ORIGIN}/2026-09-14", + f"{_ORIGIN}/docs/{_CHANGED_DOCS}", + ) + assert any( + "which is in the future" in problem for problem in check_site(site, _ORIGIN, today=_TODAY) + ) + + +def test_todays_date_is_not_in_the_future(site: Path) -> None: + """A page really changed today reads as today. The boundary is not a failure.""" + _resitemap( + site, + f"{_ORIGIN}/{_TODAY.isoformat()}", + f"{_ORIGIN}/docs/{_CHANGED_DOCS}", + ) + assert check_site(site, _ORIGIN, today=_TODAY) == [] + + +def test_a_misshapen_date_is_reported(site: Path) -> None: + _resitemap( + site, + f"{_ORIGIN}/2026-5-1", + f"{_ORIGIN}/docs/{_CHANGED_DOCS}", + ) + assert any( + "which is not a YYYY-MM-DD date" in problem + for problem in check_site(site, _ORIGIN, today=_TODAY) + ) + + +def test_a_well_shaped_date_that_is_not_a_day_is_reported(site: Path) -> None: + """`2026-02-30` has the right shape and is not a date.""" + _resitemap( + site, + f"{_ORIGIN}/2026-02-30", + f"{_ORIGIN}/docs/{_CHANGED_DOCS}", + ) + assert any( + "which is not a real date" in problem for problem in check_site(site, _ORIGIN, today=_TODAY) + ) + + +def test_coverage_of_an_unpublished_sitemap_is_zero_of_zero(tmp_path: Path) -> None: + """Absent is reported as absent, never as full coverage of nothing.""" + (tmp_path / "empty").mkdir() + assert lastmod_coverage(tmp_path / "empty") == (0, 0) + + def test_an_empty_tree_says_it_checked_nothing(tmp_path: Path) -> None: """A gate that runs over nothing must not report success for having done so.""" (tmp_path / "empty").mkdir() diff --git a/tests/test_sitemap_lastmod.py b/tests/test_sitemap_lastmod.py new file mode 100644 index 0000000..e48360e --- /dev/null +++ b/tests/test_sitemap_lastmod.py @@ -0,0 +1,169 @@ +"""The sitemap's ```` hook, and the one way it could quietly lie again. + +mkdocs stamped every entry with the build date. The hook replaces that with the +date of the commit that last changed each page. The failure mode worth testing is +not "git returned nothing" -- that case is visible and handled -- but the one where +git returns something confident and wrong: in a shallow clone ``git log -1 -- `` +answers for every path with the tip commit's date, because relative to no parent the +single grafted commit introduces the whole tree. + +So the control here is a real depth-1 clone of a real repository whose file was last +changed months before its tip. It asserts both halves: that the naive per-path +question does come back with the tip's date in that clone (the trap is live, not +hypothetical), and that the hook refuses to date anything there instead of publishing +it. +""" + +from __future__ import annotations + +import importlib.util +import shutil +import subprocess +from pathlib import Path + +import pytest + +_MODULE = importlib.util.spec_from_file_location( + "sitemap_lastmod", + Path(__file__).resolve().parent.parent / "docs_hooks" / "sitemap_lastmod.py", +) +assert _MODULE is not None and _MODULE.loader is not None +sitemap_lastmod = importlib.util.module_from_spec(_MODULE) +_MODULE.loader.exec_module(sitemap_lastmod) + +#: When the page changed, and when the repository last did. Two different days, so a +#: date derived from the tip is distinguishable from a date derived from the page. +PAGE_CHANGED = "2026-05-01" +REPO_TIP = "2026-09-07" + +_IDENTITY = { + "GIT_AUTHOR_NAME": "Test", + "GIT_AUTHOR_EMAIL": "test@example.invalid", + "GIT_COMMITTER_NAME": "Test", + "GIT_COMMITTER_EMAIL": "test@example.invalid", +} + + +def _run(repo: Path, *args: str, when: str | None = None) -> None: + env = dict(_IDENTITY) + if when is not None: + env["GIT_AUTHOR_DATE"] = f"{when}T12:00:00+00:00" + env["GIT_COMMITTER_DATE"] = f"{when}T12:00:00+00:00" + subprocess.run( + ["git", *args], + cwd=repo, + check=True, + capture_output=True, + env={**_environ(), **env}, + ) + + +def _environ() -> dict[str, str]: + import os + + return dict(os.environ) + + +@pytest.fixture +def repo(tmp_path: Path) -> Path: + """A repository whose one page last changed months before its tip commit.""" + if shutil.which("git") is None: # pragma: no cover - git is present in CI and locally + pytest.skip("git is not available") + root = tmp_path / "repo" + (root / "docs").mkdir(parents=True) + _run(root, "init", "-q", "-b", "main") + (root / "docs" / "page.md").write_text("# Page\n", encoding="utf-8") + _run(root, "add", "docs/page.md") + _run(root, "commit", "-q", "-m", "add the page", when=PAGE_CHANGED) + # A later commit that does not touch the page. This is what makes the shallow + # answer wrong rather than accidentally right. + (root / "README.md").write_text("unrelated\n", encoding="utf-8") + _run(root, "add", "README.md") + _run(root, "commit", "-q", "-m", "something else entirely", when=REPO_TIP) + return root + + +def test_a_page_is_dated_from_the_commit_that_changed_it(repo: Path) -> None: + assert sitemap_lastmod.history_refusal(repo) is None + assert sitemap_lastmod.last_changed(repo, repo / "docs" / "page.md") == PAGE_CHANGED + + +def test_an_uncommitted_page_has_no_date_rather_than_a_guess(repo: Path) -> None: + new = repo / "docs" / "draft.md" + new.write_text("# Draft\n", encoding="utf-8") + assert sitemap_lastmod.last_changed(repo, new) is None + + +def test_a_tree_that_is_not_a_checkout_is_refused(tmp_path: Path) -> None: + plain = tmp_path / "plain" + (plain / "docs").mkdir(parents=True) + refusal = sitemap_lastmod.history_refusal(plain) + assert refusal is not None + assert "not a git checkout" in refusal + + +def test_a_shallow_clone_answers_with_the_wrong_date_and_is_refused( + repo: Path, tmp_path: Path +) -> None: + """The control. Both halves of it matter and neither implies the other.""" + shallow = tmp_path / "shallow" + subprocess.run( + ["git", "clone", "--quiet", "--depth", "1", repo.as_uri(), str(shallow)], + check=True, + capture_output=True, + env=_environ(), + ) + + # Half one: the trap is real. Asked directly, git answers for a path it has no + # history for, with the tip's date, and nothing about the reply is malformed. + naive = subprocess.run( + ["git", "log", "-1", "--format=%cs", "--", "docs/page.md"], + cwd=shallow, + check=True, + capture_output=True, + text=True, + ).stdout.strip() + assert naive == REPO_TIP != PAGE_CHANGED + + # Half two: the hook asks whether the checkout can answer before it asks anything + # else, so that reply never reaches a sitemap. + refusal = sitemap_lastmod.history_refusal(shallow) + assert refusal is not None + assert "shallow" in refusal + assert "fetch-depth: 0" in refusal + + +def test_the_written_sitemap_is_compared_against_the_derived_dates(tmp_path: Path) -> None: + """The failure this comparison exists for, measured rather than imagined. + + With the template override removed and the hook left in place, the build printed + `52 of 53 pages state when they last changed` while the file it had just written + carried the build date on all 53. The count was right about the hook and wrong + about the artifact, and only the artifact is published. + """ + build_date = "2026-09-13" + derived = [PAGE_CHANGED, REPO_TIP] + complaints = sitemap_lastmod.disagreements([build_date] * 3, derived) + assert len(complaints) == 1 + assert "carries 3 date(s) but this hook derived 2" in complaints[0] + assert f"Published and not derived: {build_date}" in complaints[0] + + +def test_dates_that_match_are_not_a_complaint() -> None: + assert sitemap_lastmod.disagreements([REPO_TIP, PAGE_CHANGED], [PAGE_CHANGED, REPO_TIP]) == [] + + +def test_an_unwritten_sitemap_is_not_read_as_agreement(tmp_path: Path) -> None: + """A missing artifact is not a matching one; it is nothing to compare.""" + assert sitemap_lastmod.published_dates(tmp_path) is None + + +def test_published_dates_are_read_back_out_of_the_written_file(tmp_path: Path) -> None: + (tmp_path / "sitemap.xml").write_text( + "" + f"https://example.invalid/{PAGE_CHANGED}" + "https://example.invalid/undated/" + "", + encoding="utf-8", + ) + assert sitemap_lastmod.published_dates(tmp_path) == [PAGE_CHANGED] diff --git a/theme_overrides/sitemap.xml b/theme_overrides/sitemap.xml new file mode 100644 index 0000000..8a0312f --- /dev/null +++ b/theme_overrides/sitemap.xml @@ -0,0 +1,26 @@ +{#- + mkdocs' own sitemap template, with one element changed. + + Upstream emits `{{ file.page.update_date }}`, and + `Page.update_date` is `mkdocs.utils.get_build_date()` -- the day the build ran. + That put today's date on all 53 URLs, every day, including pages untouched for + months. Here the element is emitted only for a page `docs_hooks/sitemap_lastmod.py` + could date from the commit that last changed its source, and omitted entirely for + one it could not: `` is optional in the sitemaps protocol, and an absent + element is a valid sitemap that says nothing false. + + `update_date` itself is deliberately left alone. mkdocs also reads it to set the + gzip MTIME on sitemap.xml.gz, and `max()` over an empty date string there is a + ValueError, so the build date keeps that job and only the published claim changes. +-#} + + +{%- for file in pages -%} + {% if not file.page.is_link and (file.page.abs_url or file.page.canonical_url) %} + + {% if file.page.canonical_url %}{{ file.page.canonical_url|e }}{% else %}{{ file.page.abs_url|e }}{% endif %}{% if file.page.meta.lastmod %} + {{ file.page.meta.lastmod|e }}{% endif %} + + {%- endif -%} +{% endfor %} +