From 49f1b623d72f5197fe4bb4b10d9fdca903ed768e Mon Sep 17 00:00:00 2001 From: Tom Cobb Date: Mon, 24 Aug 2026 12:45:01 +0000 Subject: [PATCH 1/2] refactor!: move the gather's decisions into assemble.mjs, add max-prs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review pass over the publish pipeline. The theme is that every decision the gather makes now lives in the tested kernel, and the workflow does IO only. New `select-artifacts` subcommand: given the artifacts list and the open-PR list (with fork approval pre-resolved, since that costs an API call each), it decides which artifact becomes the default branch and which PRs get a preview. That replaces ~50 lines of inline bash — the SHA index, the jq default-branch pick, the eligibility loop and the rank/cap loop — and it fixed two defects the bash had: - The default branch chose its SHA *with* the fork guard, then re-looked-up the artifact in an index built *without* one. A fork's pull_request run executes in the upstream repo's Actions, so a fork PR whose head SHA matched the default branch's could have been staged as site/. The kernel uses the filtered artifact directly and never re-looks-up by SHA. - The release cap and the PR cap broke timestamp ties in opposite directions (`sort -r` kept PR #9 over #10; byDateDesc kept 1.10 over 1.9). Both now share one comparator. `select-releases` likewise emits its own cache key rather than having the workflow re-derive it with awk/sort/sha256sum. The digest is byte-identical to the pipeline it replaces, so existing cache entries stay warm. New `max-prs` input, matching `max-releases`: cap the previews the site carries, ranked by build date, applied after eligibility so an expired artifact or an unapproved fork PR never consumes a slot. This repo's publish.yml now sets the recommended 30/20, making it identical to a consumer's copy but for the `uses:` path — the file people copy is the file that is exercised. Also: - Release-zip downloads are serial again. With the cache the steady state is 0-1 downloads (measured: 0-2s warm), which did not justify a three-pass download-and-stage structure. PR artifacts can never be cached, so those stay parallel. - Removed the site-size guard. actions/deploy-pages already compares the uploaded artifact against 1 GB and warns, reading the stored size — the same number the local pack was approximating. The limit and the recovery are in the docs and in the publish.yml people copy. - Removed _sources entirely: the write went years ago, and the deprecated read fallback has done its job. - The parallel worker takes its row as positional args over a NUL stream, not via `xargs -I{}`, which pasted the row into the script *text* — a branch name may contain `$(...)`, and this workflow holds pages:write. - docs.yml's `version-name` output is gone; nothing consumed it since the injection step, and it implied a coupling that does not exist (both sides derive the name independently, now pinned by a test). - Comments trimmed throughout: history and changelog narration removed, traps and contracts kept. Tests: 48 unit checks (was 34), 79 harness/shape checks (was 71). New coverage for cacheKey, selectArtifacts, the version-name contract, the cache-key outputs, and the worker's argument wiring — each mutation-tested. Co-Authored-By: Claude Opus 5 --- .github/CONTRIBUTING.md | 12 +- .github/workflows/_lint.yml | 6 +- .github/workflows/_test.yml | 6 +- .github/workflows/ci.yml | 4 +- .github/workflows/docs.yml | 43 +-- .github/workflows/publish-gh-pages.yml | 450 +++++++++-------------- .github/workflows/publish.yml | 39 +- CLAUDE.md | 151 +++++--- assemble/assemble.mjs | 360 ++++++++++++++---- blueapi-issue.md | 147 -------- docs/explanations/architecture.md | 75 ++-- docs/how-to/keep-the-site-small.md | 79 ++-- docs/reference/workflows.md | 70 +++- docs/tutorials/adding-to-a-fresh-repo.md | 12 +- scripts/migrate.sh | 8 +- test/test-assemble.mjs | 190 +++++++++- test/workflow-harness/run.py | 42 ++- test/workflow-harness/test_gather.py | 205 ++++++++--- test/workflow-harness/test_shape.py | 52 +-- 19 files changed, 1161 insertions(+), 790 deletions(-) delete mode 100644 blueapi-issue.md diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index f289b3b..c96dfb6 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -33,8 +33,10 @@ git push origin --tags (`--tags` pushes every local tag; on a clean clone that's just the new one. Use `git push origin vX.Y.Z` to push only that tag.) -CI runs lint + tests + the docs build; `release.yml` creates the GitHub Release with -`version-switcher.mjs` + the tag's `docs.zip` attached (via `gh`); and the nested -publish (tag re-dispatch) reconstructs + deploys the site including the new tag. The plugin URL -and the `uses:` refs for the reusable workflows all resolve to the same tag, so one -tag versions both halves. +CI runs lint + tests + the docs build, and `release.yml` creates the GitHub Release with +`version-switcher.mjs` + the tag's `docs.zip` attached (via `gh`). When that CI run +completes, `publish.yml` picks it up on `workflow_run` and reconstructs + deploys the site +including the new tag — the release asset is already attached by then, because +`release.yml` ran inside the run publish is waiting on. The plugin URL and the `uses:` +refs for the reusable workflows all resolve to the same tag, so one tag versions both +halves. diff --git a/.github/workflows/_lint.yml b/.github/workflows/_lint.yml index 44aadff..2abfb14 100644 --- a/.github/workflows/_lint.yml +++ b/.github/workflows/_lint.yml @@ -14,10 +14,8 @@ jobs: run: npx prek run --all-files --show-diff-on-failure # Workflow EXPRESSIONS are validated only by GitHub, and a bad one rejects the - # whole workflow before any job starts — no logs, no annotations, no failing step. - # That is how `|| "manual dispatch"` (expressions take single-quoted literals only) - # took the site's deploy down after it had passed YAML parsing, `bash -n`, and the - # workflow harness. actionlint catches exactly that class. + # whole workflow before any job starts — no logs, no annotations, no failing step, + # so nothing local catches it. actionlint does. # # Two errors are suppressed: actionlint's `job` context schema is stale and does not # know `job.workflow_repository` / `job.workflow_sha`, both of which are real and diff --git a/.github/workflows/_test.yml b/.github/workflows/_test.yml index 1503694..6672e08 100644 --- a/.github/workflows/_test.yml +++ b/.github/workflows/_test.yml @@ -9,9 +9,9 @@ jobs: - name: Unit tests (widget logic + make-switcher parity) run: npm test - # publish.yml's gather steps are the most intricate code in the repo and used to - # be verifiable only by merging and watching a real deploy. The harness loads them - # out of the YAML and runs them against a mock gh, so they cannot drift. + # publish.yml's gather steps are the most intricate code in the repo, and a real + # deploy is the only other way to exercise them. The harness loads them out of the + # YAML and runs them against a mock gh, so the tests cannot drift from the workflow. - name: Install uv uses: astral-sh/setup-uv@v8.2.0 # full version: setup-uv uses immutable releases with: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fd710df..c0b13e0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,8 +7,8 @@ name: CI # It does NOT publish the site. publish.yml picks this run up on `workflow_run` once it # completes and deploys from there, so the site's reconstruction cost stays out of every # PR's critical path and a Pages failure never shows as a red check against an author who -# cannot act on it. That also means this workflow needs no elevated permissions, and its -# `name:` below is load-bearing — publish.yml matches on it. +# cannot act on it. This workflow therefore needs no elevated permissions, and its `name:` +# below is load-bearing — publish.yml matches on it. on: pull_request: push: diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 2dacba4..50e1000 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -12,22 +12,17 @@ on: required: false default: docs/_build/html type: string - outputs: - version-name: - description: The version name this build was served at (pr- | | ). - value: ${{ jobs.build.outputs.version-name }} # REUSABLE docs build — the UNPRIVILEGED half, shared across MyST projects. Builds # the docs at the versioned BASE_URL and uploads this build's `docs` artifact # (docs.zip, a single html/ root dir). It runs for every event — PRs (including forks), # pushes to the default branch, and tags — but NEVER publishes. publish.yml (the -# privileged half) deploys, called separately by the consumer's ci.yml on internal -# events, injecting THIS build's artifact for the current version name. +# privileged half) picks the artifact up once the run completes. # -# This workflow OWNS the build/publish contract so it cannot drift: the version -# name, the BASE_URL, the docs.zip single-root-dir shape, and the `docs` artifact name -# all live here. The consumer supplies only HOW to turn sources into HTML, via -# `build-command`. +# This workflow owns one half of the build/publish contract: the docs.zip +# single-root-dir shape and the `docs` artifact NAME (publish.yml gathers cross-run +# artifacts by that name via the artifacts API, never by workflow filename). The +# consumer supplies only HOW to turn sources into HTML, via `build-command`. # # Environment given to build-command: Node/npx (preinstalled on the runner) and uv # (always installed; honours a committed .python-version). So build-command can be @@ -42,8 +37,6 @@ permissions: jobs: build: runs-on: ubuntu-latest - outputs: - version-name: ${{ steps.ver.outputs.version-name }} steps: # fetch-depth: 0 brings the full history *and tags* — a build-command may # derive a version from `git describe` (this repo's docs do, to pin the @@ -68,8 +61,14 @@ jobs: enable-cache: false # Version name = the site sub-dir this build is served at, and the BASE_URL it - # must be built with. pr- for PRs; otherwise the ref name (default branch or - # a tag without `/`). No sanitisation: every version name is URL/path-safe. + # must be built with: pr- for PRs, otherwise the ref name (the default branch, + # or a tag without `/`). No sanitisation — every version name is URL/path-safe. + # + # This is the OTHER half of the contract, and it is not passed anywhere: the + # gather independently re-derives the same name for each source it stages (a + # release's tag, a PR's number, the repo's default_branch) and unpacks the zip at + # site/. Both sides compute the same string from the same facts, so + # this rule and assemble.mjs's dest rule must change together. - name: Compute version name id: ver run: | @@ -82,9 +81,8 @@ jobs: echo "version-name=$name" >> "$GITHUB_OUTPUT" # BASE_URL must match the versioned sub-path the build is served at, or its - # root-absolute assets 404. assemble files this build's artifact at the same - # version name (site/, site/, site/pr-), so the two - # cannot drift. VERSION_NAME is the bare version token, exported for builds + # root-absolute assets 404 — see the version-name contract above. + # VERSION_NAME is the bare version token, exported for builds # that need it directly — e.g. a Sphinx conf.py setting pydata-sphinx-theme's # switcher `version_match` — without parsing it back out of BASE_URL. # build-command comes through an env var (not interpolated into the script) @@ -100,10 +98,9 @@ jobs: # Pack the site as docs.zip with a single root directory — the durable contract # both publish.yml's gather and release.yml's asset rely on. Stage into html/ - # first so any html-dir name yields the same root. publish.yml's extract keys on - # there being exactly ONE root dir, not on it being called html/ (release assets - # are durable and may have been packed by another pipeline), but everything this - # workflow produces is html/-rooted. + # first so any html-dir name yields the same root. The extract side keys on there + # being exactly ONE root dir, not on its name, but everything this workflow + # produces is html/-rooted. - name: Pack docs.zip (single html/ root) env: HTML_DIR: ${{ inputs.html-dir }} @@ -123,7 +120,3 @@ jobs: path: ${{ runner.temp }}/docs.zip compression-level: 0 - # NOTE: nothing here tells a fork PR that its docs will not be published. There is - # no longer anywhere sensible to say it: publish.yml excludes forks before the - # engine is reached, so no job runs in that case at all. This build job stays - # purely about building + uploading the docs artifact. diff --git a/.github/workflows/publish-gh-pages.yml b/.github/workflows/publish-gh-pages.yml index f1d34bb..edca5d2 100644 --- a/.github/workflows/publish-gh-pages.yml +++ b/.github/workflows/publish-gh-pages.yml @@ -3,37 +3,26 @@ name: Publish to GitHub Pages # Reconstruct the WHOLE versioned docs site from durable sources (the consumer's # default-branch build, release docs.zip assets, open-PR build artifacts) and deploy it # directly to GitHub Pages — no gh-pages branch. The PRIVILEGED half, and a reusable -# workflow: its only caller is a `publish.yml` in the consumer's repo (this repo has one -# too, identical but for the pin), triggered by `workflow_run` when their CI completes. +# workflow: its only caller is a `publish.yml` in the consumer's repo, triggered by +# `workflow_run` when their CI completes. # -# WHY workflow_run, and why this file is so much smaller than it used to be. Pages stamps -# every deployment with `pages_build_version = GITHUB_SHA` and silently no-ops a repeat -# (actions/deploy-pages#383) — but only for SOME events. `workflow_dispatch` and -# `workflow_run` both force a re-serve. That was proven here on 2026-08-21, not reasoned: -# four consecutive deploys at the identical build version b24237484c3b445469c2db4ef161410a185fcdbc -# (a main push, a tag cut on that same commit, and two pushes to one PR) EVERY ONE of -# which updated the live origin. +# Two properties of that trigger shape the code below: # -# Because a tag's deploy re-serves directly, the old `re-dispatch` trampoline is gone, -# and with it the `publish-dispatch.yml` shim that existed only to give the trampoline a -# locally-dispatchable file. Because `workflow_run` fires after the triggering run has -# COMPLETED, that run's `docs` artifact is discoverable by the ordinary gather, so the -# `version-name` injection is gone too. And because a fork's CI never reaches this -# workflow (the caller guards on it), the read-only `warn` job is gone. +# - GITHUB_SHA and github.ref are ALWAYS the default branch's HEAD, never the commit +# that was built — a PR-triggered deploy reports `refs/heads/main`. Anything asking +# "was this the default branch?" must read `github.event.workflow_run.head_branch`. +# - Pages stamps each deployment with `pages_build_version = GITHUB_SHA` and silently +# no-ops a repeat (actions/deploy-pages#383), EXCEPT on `workflow_run` and +# `workflow_dispatch`, which force a re-serve. So a tag cut on an already-deployed +# commit still updates the origin, with no re-dispatch machinery. # -# One caveat that shapes the code below: under `workflow_run`, GITHUB_SHA and github.ref -# are ALWAYS the default branch's HEAD, never the commit that was built. Anything that -# needs the real branch must read `github.event.workflow_run.head_branch`. -# -# The assemble logic is THIS repo's assemble.mjs (the pure kernel); the IO plumbing -# lives as inline steps in this file. To version-match assemble.mjs to this workflow, -# we check out the reusable workflow's OWN repo at its OWN commit via the `job` context: -# unlike github.workflow_* (which resolve to the CALLER's entry workflow), +# The pure logic is assemble.mjs; the IO plumbing is inline steps here. To version-match +# assemble.mjs to this workflow we check out its OWN repo at its OWN commit via the `job` +# context: unlike github.workflow_* (which resolve to the CALLER's entry workflow), # `job.workflow_repository` + `job.workflow_sha` resolve to the file that defines THIS -# job — i.e. the reusable workflow at the ref a consumer pinned it to. So assemble.mjs -# always matches the workflow version, with no hardcoded repo and no manual release-time -# bump. assemble.mjs runs `git tag` in the cwd, so the CONSUMER's repo is checked out at -# the root (with tags) to supply their version list; assemble.mjs comes from .mvs. +# job — the reusable workflow at the ref a consumer pinned it to. assemble.mjs runs +# `git tag` in the cwd, so the CONSUMER's repo is checked out at the root (with tags) to +# supply their version list; assemble.mjs itself comes from .mvs. on: workflow_call: inputs: @@ -47,55 +36,50 @@ on: Publish only the N most recent releases (by the tagged commit's date), or 0 for all of them. The site is deployed as ONE Pages artifact and Pages caps that at 1 GB, so an old project with hundreds of released docs.zips will eventually - fail to deploy at all — a repo with 131 of them was already at 452 MB. Set this - in your publish.yml's `with:` block so it applies on every path. + fail to deploy at all. It also bounds the release-zip cache entry, which + counts against the repo's 10 GB Actions cache quota. Set this in your + publish.yml's `with:` block so it applies on every path. + required: false + default: "0" + type: string + max-prs: + description: >- + Publish previews for only the N most recently built open PRs, or 0 for all of + them. Same 1 GB ceiling as max-releases: a repo with dozens of open PRs carries + a preview for each on every deploy. Set this in your publish.yml's `with:` + block so it applies on every path. required: false default: "0" type: string jobs: - # The only job. The caller (publish.yml in the consumer's repo) has already gated on - # the triggering run having SUCCEEDED and not being from a fork, so there is nothing - # left to branch on here. + # The only job. The caller has already gated on the triggering run having SUCCEEDED and + # not being from a fork, so there is nothing left to branch on here. deploy: runs-on: ubuntu-latest permissions: contents: read # checkout + read release assets - actions: read # gather cross-run docs artifacts (the retry-dispatch that needed write is gone) + actions: read # gather cross-run docs artifacts pages: write # deploy to Pages id-token: write # deploy-pages OIDC statuses: write # set the preview-approved status on a fork PR head SHA environment: name: github-pages url: ${{ steps.deployment.outputs.page_url }} - # Cancelling is safe now, and worth having: every deploy reconstructs the whole site - # from durable sources, so a superseded one has nothing unique in it — the next deploy - # gathers everything the cancelled one would have. This was NOT safe while publish was - # nested in ci.yml, where cancelling marked an unrelated PR's CI run as cancelled. + # Cancelling a superseded deploy is safe: every deploy reconstructs the whole site + # from durable sources, so the next one gathers everything the cancelled one would + # have. concurrency: group: pages cancel-in-progress: true env: REPO: ${{ github.repository }} - # The branch that was actually BUILT. Under workflow_run, github.ref and GITHUB_SHA - # are always the default branch's HEAD regardless of what triggered it, so github.ref - # is useless for "was this the default branch?" — it is true for a PR-triggered - # deploy too. head_branch is the honest answer; ref_name covers a manual dispatch, - # where there is no triggering run. + # The branch that was actually BUILT — see the header: github.ref is the default + # branch even for a PR-triggered deploy. ref_name covers a manual dispatch, where + # there is no triggering run. BUILT_BRANCH: ${{ github.event.workflow_run.head_branch || github.ref_name }} DEFAULT: ${{ github.event.repository.default_branch }} SEED_TAG: pages-default-seed - # Site-size guard thresholds, both on the deployed artifact's scale. PROBE is the - # uncompressed size past which it is worth actually packing the tree to find out - # (the pack costs real seconds); WARN is the packed size that earns a warning, - # 700 MB against Pages' hard 1 GB. Overridable so the guard is testable and so a - # site with unusual compressibility can be tuned. - SIZE_PROBE_BYTES: "1610612736" - SIZE_WARN_BYTES: "734003200" - # DEPRECATED: the in-site durable store. Nothing writes it any more (the default - # branch's zip lives in the Actions cache); this is only still read as a one-shot - # fallback for a site last deployed by an older publish.yml. - SOURCES_DIR: _sources steps: # The CONSUMER's repo, at the root cwd: assemble.mjs runs `git tag` here to # order that repo's deployed versions, so it needs their tags (fetch-depth: 0). @@ -130,8 +114,8 @@ jobs: -f state=success -f context=preview-approved \ -f description="Fork docs preview approved" - # BUILT_BRANCH vs github.ref is the one thing a post-mortem here always trips over, - # so state both up front rather than leaving someone to infer it from a cache miss. + # BUILT_BRANCH vs github.ref is the thing a post-mortem here always trips over, so + # state both up front rather than leaving someone to infer it from a cache miss. # # Every context value comes through `env:`, never interpolated into the script. # That keeps arbitrary values out of the shell, and it avoids nesting quotes: an @@ -154,9 +138,9 @@ jobs: 'github.sha' "$GH_SHA (→ pages_build_version)" \ 'triggered by' "${TRIGGER_URL:-manual dispatch}" - # The live Pages base URL, from the Pages API — respects a custom domain - # (CNAME); falls back to the conventional github.io URL if the API call - # fails. Used by the _sources restore below and baked into switcher.json. + # The live Pages base URL, from the Pages API — respects a custom domain (CNAME); + # falls back to the conventional github.io URL if the API call fails. Baked into + # switcher.json's entry URLs. - name: Resolve the Pages URL env: GH_TOKEN: ${{ github.token }} @@ -174,7 +158,9 @@ jobs: # # Selection is assemble.mjs's job (pure, unit-tested); this step only does IO. # It runs BEFORE any download so the gather fetches only what will actually be - # published, and so the cache key describes exactly that set. + # published. It writes the decision TSV to --out and prints the cache key for + # that exact selection on stdout — the key is derived from the selection, so it + # is derived where the selection is made. # # Cached zips are named by ASSET ID, not by tag: an asset id identifies its bytes # unambiguously, so a re-cut release can never be served from a stale cache entry. @@ -186,16 +172,15 @@ jobs: run: | set -euo pipefail gh api --paginate "repos/$REPO/releases" | jq -rs 'add // []' > "$RUNNER_TEMP/releases.json" - node "$GITHUB_WORKSPACE/.mvs/assemble/assemble.mjs" select-releases \ + key=$(node "$GITHUB_WORKSPACE/.mvs/assemble/assemble.mjs" select-releases \ --default-branch "$DEFAULT" --seed-tag "$SEED_TAG" \ --max-releases "${MAX_RELEASES:-0}" \ - < "$RUNNER_TEMP/releases.json" > "$RUNNER_TEMP/releases.tsv" - # Key on the exact set of asset ids we intend to publish, so any change to - # that set misses and re-saves. restore-keys then falls back to the previous - # set, which shares nearly every zip — only the new release is downloaded. - digest=$(awk -F'\t' '$2 != "-" {print $5}' "$RUNNER_TEMP/releases.tsv" \ - | sort | sha256sum | cut -c1-32) - echo "cache-key=mvs-relzips-v1-$digest" >> "$GITHUB_OUTPUT" + --out "$RUNNER_TEMP/releases.tsv" \ + < "$RUNNER_TEMP/releases.json") + echo "cache-key=$key" >> "$GITHUB_OUTPUT" + # The bare namespace, for restore-keys: a prefix match returns the previous + # selection, which shares every zip but the newly cut one. + echo "cache-prefix=${key%-*}-" >> "$GITHUB_OUTPUT" - name: Restore cached release zips id: relcache @@ -203,72 +188,54 @@ jobs: with: path: ${{ runner.temp }}/relcache key: ${{ steps.releases.outputs.cache-key }} - restore-keys: mvs-relzips-v1- + restore-keys: ${{ steps.releases.outputs.cache-prefix }} # Releases are permanent sources; the seed release (if present) stands in for the # default branch until the durable copy is established — store it as $DEFAULT.zip # so the priority-overwrite chain below can supersede it with a fresher source. # # Prints a decision for every release the API listed, so a version missing from - # the site can be explained from this log alone. Downloads run in parallel: a repo - # with a hundred releases spent minutes here doing them one at a time. + # the site can be explained from this log alone. Downloads are serial: the cache + # above makes the steady state 0 or 1 of them, and a cold cache is a rare, + # off-critical-path event that max-releases already bounds. - name: Gather release artifacts env: GH_TOKEN: ${{ github.token }} run: | set -euo pipefail relcache="$RUNNER_TEMP/relcache"; mkdir -p "$relcache" - todo="$RUNNER_TEMP/relcache-todo"; : > "$todo" - failed="$RUNNER_TEMP/relcache-failed"; mkdir -p "$failed" + warnings="$RUNNER_TEMP/relcache-warnings"; : > "$warnings" + hits=0; downloads=0 - # Pass 1 — account for every release, and queue the cache misses. + # Warnings are buffered to after the table rather than printed mid-loop, so + # the decision table stays readable. printf '%-28s %-21s %-9s %s\n' TAG CREATED SIZE DECISION + # NB tab is IFS whitespace, so `read` COLLAPSES consecutive tabs — an empty + # field would shift every column after it. assemble.mjs emits "-", never "". while IFS=$'\t' read -r tag dest created size assetid decision; do [ -n "$tag" ] || continue if [ "$dest" != "-" ]; then if [ -s "$relcache/$assetid.zip" ]; then + hits=$((hits + 1)) decision="$decision (cache hit)" + elif gh release download "$tag" --repo "$REPO" -p docs.zip \ + -O "$relcache/$assetid.zip" 2>"$RUNNER_TEMP/relcache-download.err"; then + downloads=$((downloads + 1)) + decision="$decision (downloaded)" else - # Space-separated is safe: a git refname can never contain a space. - echo "$tag $assetid" >> "$todo" - decision="$decision (downloading)" - fi - fi - printf '%-28s %-21s %-9s %s\n' "$tag" "$created" "$size" "$decision" - # NB tab is IFS whitespace, so `read` COLLAPSES consecutive tabs — an empty - # field would shift every column after it. assemble.mjs emits "-", never "". - done < "$RUNNER_TEMP/releases.tsv" - - # Pass 2 — fetch the misses in parallel. Each worker is independent and - # records its own failure, so one bad asset cannot abort the rest. - n=$(grep -c . "$todo" || true) - echo "release zips: $(( $(awk -F'\t' '$2 != "-"' "$RUNNER_TEMP/releases.tsv" | wc -l) - n )) cached, $n to download" - if [ "$n" -gt 0 ]; then - export REPO relcache failed - # shellcheck disable=SC2016 - xargs -a "$todo" -P 8 -n 2 bash -c ' - set -uo pipefail - tag="$1"; assetid="$2" - if ! gh release download "$tag" --repo "$REPO" -p docs.zip \ - -O "$relcache/$assetid.zip" 2>"$failed/$assetid.err"; then + # Leave no truncated zip behind for the next run's cache to trust. rm -f "$relcache/$assetid.zip" - touch "$failed/$assetid" + decision="$decision (DOWNLOAD FAILED — skipped)" + echo "::warning::release $tag: docs.zip download failed — skipping. $(head -1 "$RUNNER_TEMP/relcache-download.err" 2>/dev/null)" >> "$warnings" + fi + if [ -s "$relcache/$assetid.zip" ]; then + cp "$relcache/$assetid.zip" "$RUNNER_TEMP/gather/$dest.zip" fi - ' _ - fi - - # Pass 3 — stage into the gather dir, in the TSV's order so the log is - # deterministic whatever order the downloads finished in. - while IFS=$'\t' read -r tag dest created size assetid decision; do - [ "$dest" = "-" ] && continue - if [ -f "$failed/$assetid" ]; then - echo "::warning::release $tag: docs.zip download failed — skipping. $(head -1 "$failed/$assetid.err" 2>/dev/null)" - elif [ -s "$relcache/$assetid.zip" ]; then - cp "$relcache/$assetid.zip" "$RUNNER_TEMP/gather/$dest.zip" - else - echo "::warning::release $tag: docs.zip missing after download — skipping" fi + printf '%-28s %-21s %-9s %s\n' "$tag" "$created" "$size" "$decision" done < "$RUNNER_TEMP/releases.tsv" + echo "release zips: $hits cached, $downloads downloaded" + cat "$warnings" # Prune anything no longer selected, so a capped cache stays capped. keep=$(awk -F'\t' '$2 != "-" {print $5 ".zip"}' "$RUNNER_TEMP/releases.tsv" | sort -u) @@ -277,11 +244,13 @@ jobs: grep -qxF "$(basename "$f")" <<<"$keep" || rm -f "$f" done - # Save only when the DEFAULT BRANCH was what got built. A cache written while a PR - # is the trigger is scoped to that PR and never reused by anything else, so saving - # there would just churn the 10 GB repo quota. This must test the BUILT branch, not - # github.ref — see BUILT_BRANCH above. Skipped on an exact key hit: the entry is - # already there and cache entries are immutable. + # Save only when the DEFAULT BRANCH was what got built. Entries are scoped to the + # writing ref, and a deploy dispatched from some other ref would write one only that + # ref can read — invisible to every ordinary deploy, and dead weight against the + # 10 GB quota. Test the BUILT branch, not github.ref, which under workflow_run is + # the default branch even for a PR-triggered deploy (so a workflow_run save is + # always default-branch-scoped, whatever triggered it). Skipped on an exact key + # hit: cache entries are immutable, so it is already there. - name: Save release zips to the cache if: >- (github.event.workflow_run.head_branch || github.ref_name) == github.event.repository.default_branch && @@ -292,116 +261,86 @@ jobs: key: ${{ steps.releases.outputs.cache-key }} # Branch CI artifacts overwrite the seed (higher priority). Found via the - # artifacts API by NAME (`docs` — the contract), never by workflow filename, - # so the consumer's entry workflow can be called anything. An artifact's - # workflow_run carries head_branch/head_sha/head_repository_id but no run - # conclusion — existence is the signal (docs.yml only uploads after a - # successful build). Default branch: newest non-expired docs artifact built - # from THIS repo's code on that branch (head_repository_id keeps a same-named - # fork branch out). PRs: keyed by head SHA — internal always; external forks - # only when the SHA carries a preview-approved status. No - # $ARTIFACT_VERSION_NAME skip — the current build (staged next) overwrites - # anything gathered here with the same name. + # artifacts API by NAME (`docs` — the contract), never by workflow filename, so the + # consumer's entry workflow can be called anything. An artifact's workflow_run + # carries head_branch/head_sha/head_repository_id but no run conclusion — existence + # is the signal, since docs.yml only uploads after a successful build. # - # The artifact list is paginated ONCE and indexed by head SHA. It used to be - # re-paginated per open PR, which on a busy repo (26 open PRs, thousands of - # artifacts) was ~175 s of identical API calls. + # This step is IO only: paginate the artifacts API ONCE (re-paginating it per open + # PR costs minutes on a busy repo), resolve each fork PR's approval, then hand both + # lists to assemble.mjs, which decides what lands where. Unlike the release zips + # these artifacts cannot be cached — a PR's head SHA changes on every push — so + # every deploy pays 1 + open-PR downloads, and those run in parallel. - name: Gather branch CI artifacts env: GH_TOKEN: ${{ github.token }} + MAX_PRS: ${{ inputs.max-prs }} run: | set -euo pipefail repo_id=$(gh api "repos/$REPO" -q .id) - arts="$RUNNER_TEMP/artifacts.json" gh api --paginate "repos/$REPO/actions/artifacts?name=docs&per_page=100" \ - | jq -rs '[.[].artifacts[] | select(.expired | not)]' > "$arts" - echo "$(jq length "$arts") non-expired 'docs' artifact(s) indexed" + | jq -rs '[.[].artifacts[]]' > "$RUNNER_TEMP/artifacts.json" - # Ascending by created_at so the last write per SHA is the newest artifact. - declare -A BY_SHA - while IFS=$'\t' read -r sha id created size run; do - [ -n "$sha" ] && BY_SHA["$sha"]="$id"$'\t'"$created"$'\t'"$size"$'\t'"$run" - done < <(jq -r 'sort_by(.created_at) | .[] - | [.workflow_run.head_sha, .id, .created_at, .size_in_bytes, .workflow_run.id] | @tsv' "$arts") - - # want.tsv: dest artifact-id label — resolved before any download, - # so the downloads can all run at once. - want="$RUNNER_TEMP/artifact-want.tsv"; : > "$want" - queue() { - local dest="$1" sha="$2" label="$3" meta="${BY_SHA[$2]:-}" - if [ -z "$meta" ]; then - echo " $label → no non-expired 'docs' artifact for ${sha:0:8} — skipping" - return 0 - fi - local id created size run - IFS=$'\t' read -r id created size run <<<"$meta" - echo " $label → artifact $id ($size B, built $created) from $GITHUB_SERVER_URL/$REPO/actions/runs/$run @ ${sha:0:8}" - printf '%s\t%s\t%s\n' "$dest" "$id" "$label" >> "$want" - } - - echo "default branch:" - dsha=$(jq -r --arg b "$DEFAULT" --argjson r "$repo_id" ' - [.[] | select(.workflow_run.head_branch == $b and .workflow_run.head_repository_id == $r)] - | sort_by(.created_at) | last | .workflow_run.head_sha // empty' "$arts") - if [ -n "$dsha" ]; then - queue "$DEFAULT" "$dsha" "default-branch CI ($DEFAULT)" - else - echo " default-branch CI ($DEFAULT) → no non-expired 'docs' artifact — skipping" - fi - - prs=$(gh pr list --repo "$REPO" --state open --limit 200 \ - --json number,headRefOid,isCrossRepository -q '.[] | [.number, .headRefOid, .isCrossRepository] | @tsv') - echo "open PRs: $(grep -c . <<<"$prs" || true)" + # Fork approval is an API call per cross-repo PR, so settle it here and pass + # the answer in — assemble.mjs stays free of network IO. + prs="$RUNNER_TEMP/prs.tsv"; : > "$prs" while IFS=$'\t' read -r num sha cross; do - [ -z "$num" ] && continue + [ -n "$num" ] || continue + approved=true if [ "$cross" = "true" ]; then approved=$(gh api "repos/$REPO/commits/$sha/statuses" \ -q 'any(.[]; .context=="preview-approved" and .state=="success")') - if [ "$approved" != "true" ]; then - echo " PR #$num → fork PR, head ${sha:0:8} not preview-approved — skipping" - continue - fi fi - queue "pr-$num" "$sha" "PR #$num" - done <<< "$prs" + printf '%s\t%s\t%s\n' "$num" "$sha" "$approved" >> "$prs" + done < <(gh pr list --repo "$REPO" --state open --limit 200 \ + --json number,headRefOid,isCrossRepository \ + -q '.[] | [.number, .headRefOid, .isCrossRepository] | @tsv') + # The API serves an artifact as a zip-of-the-artifact, with docs.zip inside. - # Three failure modes are reported distinctly because they need different - # fixes: download failure (transient), no docs.zip inside (an artifact built - # by a docs.yml older than the packing contract — re-run that build), and the - # not-found case already handled above. - if [ -s "$want" ]; then - export REPO - gather_dir="$RUNNER_TEMP/gather"; export gather_dir - # shellcheck disable=SC2016 - xargs -a "$want" -P 8 -d '\n' -I{} bash -c ' - set -uo pipefail - IFS=$'"'"'\t'"'"' read -r dest id label <<<"{}" - tmp=$(mktemp -d) - if ! gh api "repos/$REPO/actions/artifacts/$id/zip" > "$tmp/artifact.zip" 2>/dev/null; then - echo "::warning::$label: artifact $id download failed — skipping" - elif ! unzip -q "$tmp/artifact.zip" docs.zip -d "$tmp" >/dev/null 2>&1 || [ ! -f "$tmp/docs.zip" ]; then - # The FILE test, not unzip'"'"'s exit status: unzip exits 0 with only a - # "caution: filename not matched" note when the member is absent. - echo "::warning::$label: artifact $id contains no docs.zip — it predates the docs.zip packing contract. Re-run that build to refresh it. Skipping." - else - mv "$tmp/docs.zip" "$gather_dir/$dest.zip" - fi - rm -rf "$tmp" - ' - fi + # Two failure modes are reported distinctly because they need different fixes: + # a download failure (transient), and no docs.zip inside — which has two very + # different causes, so the warning names both. + # assemble.mjs decides; this pipeline only fetches. It prints three + # NUL-separated fields per artifact, which xargs hands to the worker as + # POSITIONAL ARGS — never -I{}, which pastes the row into the script TEXT where + # a label containing $(...) would be executed rather than printed. -r so an + # empty selection runs nothing. + # + # `gather-artifact` after the script is the worker's $0: `bash -c` takes the + # arg AFTER the script as $0, so without something there the first field would + # land in $0 and every other shift by one. bash prefixes its errors with $0, so + # naming it makes a failing worker identify itself in the log. + export REPO + export gather_dir="$RUNNER_TEMP/gather" + # shellcheck disable=SC2016 + node "$GITHUB_WORKSPACE/.mvs/assemble/assemble.mjs" select-artifacts \ + --default-branch "$DEFAULT" --repo-id "$repo_id" \ + --prs "$prs" --max-prs "${MAX_PRS:-0}" \ + --repo "$REPO" --server-url "$GITHUB_SERVER_URL" \ + < "$RUNNER_TEMP/artifacts.json" \ + | xargs -0 -r -P 8 -n 3 bash -c ' + set -uo pipefail + dest=$1 id=$2 label=$3 + tmp=$(mktemp -d) + if ! gh api "repos/$REPO/actions/artifacts/$id/zip" > "$tmp/artifact.zip" 2>/dev/null; then + echo "::warning::$label: artifact $id download failed — skipping" + elif ! unzip -q "$tmp/artifact.zip" docs.zip -d "$tmp" >/dev/null 2>&1 || [ ! -f "$tmp/docs.zip" ]; then + # The FILE test, not the exit status: unzip exits 0 with only a + # "caution: filename not matched" note when the member is absent. + echo "::warning::$label: artifact $id has no docs.zip inside — skipping. Either it was built before docs.yml packed docs.zip (re-run that build to refresh it), or another workflow in this repo uploads an unrelated artifact also named docs (rename it — the artifact NAME is how versions are found)." + else + mv "$tmp/docs.zip" "$gather_dir/$dest.zip" + fi + rm -rf "$tmp" + ' gather-artifact # The default branch's last known-good docs.zip. Releases are durable and PRs are # ephemeral by design, but the DEFAULT BRANCH has no permanent source: it is # gathered from its latest CI artifact, which expires, after which it would drop - # out and the guard below would hard-fail. - # - # This used to be solved by publishing the zip into the site itself - # (`_sources/.zip`) — permanent, but it shipped a multi-megabyte zip to - # every visitor's Pages artifact forever. The Actions cache holds it instead. The - # trade is real and deliberate: cache entries are evicted after 7 days without a - # read, and by LRU past 10 GB. A repo that goes quiet for over a week AND whose - # main CI artifact has expired loses this rung, where `_sources/` was forever. + # out and the guard below would hard-fail. The Actions cache holds a copy instead + # — evicted after 7 days without a read and by LRU past 10 GB, so a repo quiet for + # over a week AND whose CI artifact has expired loses the rung and fails loudly. # # Key is a bare prefix so it never matches exactly; restore-keys then supplies the # most recent entry. @@ -412,9 +351,9 @@ jobs: key: mvs-default-v1- restore-keys: mvs-default-v1- - # Fallback chain for $DEFAULT.zip: current build → release/seed → branch CI (all - # above) → the cache → the deprecated in-site copy. Hard-fail if none of them - # produced it: a deploy missing the default branch publishes a hole. + # Fallback chain for $DEFAULT.zip: release/seed → branch CI (both above) → the + # cache. Hard-fail if none of them produced it: a deploy missing the default + # branch publishes a hole. - name: Ensure default-branch zip id: ensure-default env: @@ -429,15 +368,9 @@ jobs: cp "$cache/default.zip" "$zip" echo "::notice::$DEFAULT restored from the Actions cache ($(stat -c%s "$zip") B) \ — its CI artifact has expired, so this deploy is serving the last cached build" - # DEPRECATED rung, kept so a site deployed by an older publish.yml still has a - # source on its first upgraded deploy. Nothing writes _sources/ any more, so it - # disappears from the site on that very deploy; this read is one-shot insurance. - elif curl -fsSL "$PAGES_URL/$SOURCES_DIR/$DEFAULT.zip" -o "$zip" 2>/dev/null; then - echo "::notice::$DEFAULT restored from the deprecated in-site copy \ - ($PAGES_URL/$SOURCES_DIR/$DEFAULT.zip) — it will be cached from here on" else - echo "::error::$DEFAULT: not provided by current build, releases, branch CI, \ - the Actions cache, or the deprecated in-site copy — deploy aborted" + echo "::error::$DEFAULT: not provided by releases, branch CI or the Actions \ + cache — deploy aborted" exit 1 fi # Re-save only when the bytes actually changed: cache entries are immutable, so @@ -450,8 +383,8 @@ jobs: echo "cache-key=mvs-default-v1-$new" >> "$GITHUB_OUTPUT" fi - # Same rule as the release-zip cache, and the same trap: test the BUILT branch, not - # github.ref, which under workflow_run is always the default branch. + # Same rule and the same trap as the release-zip cache above. Also gated on the + # content hash having changed, so an unchanged default branch adds no entry. - name: Save the default branch's durable docs.zip if: >- steps.ensure-default.outputs.cache-key != '' && @@ -465,13 +398,11 @@ jobs: # # THE ZIP-SHAPE CONTRACT: a docs.zip must unzip to exactly ONE top-level # directory, whose CONTENTS become site/. The directory's NAME is - # ignored. docs.yml packs `html/`, but release assets are durable and long-lived - # — a repo migrating from python-copier-template's `_release.yml` has immutable - # releases carrying zips rooted at the TAG NAME (`mv html $GITHUB_REF_NAME; - # zip -r docs.zip $GITHUB_REF_NAME`), which cannot be re-cut. Keying on `html/` - # silently dropped every one of those releases from the switcher. Anything else - # — no entries, several entries, or files loose at the zip root — is malformed - # (nothing is known to produce it) and warns + skips rather than guessing. + # ignored, because release assets are durable and may have been packed by another + # pipeline — python-copier-template's `_release.yml` roots its zip at the TAG NAME, + # and an immutable release cannot be re-cut. Anything else — no entries, several + # entries, or files loose at the zip root — is malformed and warns + skips rather + # than guessing. - name: Extract artifacts into site run: | set -euo pipefail @@ -495,34 +426,13 @@ jobs: fi rm -rf "$tmp" done - # upload-pages-artifact tars the whole site and GitHub Pages rejects the - # artifact over 1 GB. That ceiling arrives silently — deploys just start - # failing — so warn while there is still room to act. `max-releases` is the - # lever: it is unlimited by default, which is safe for a young project and a - # slow-motion footgun for an old one. - # - # The cap is on the COMPRESSED artifact, which is nothing like the size of the - # tree: this repo's own site is 734 MiB on disk but packs to 223 MiB, because - # HTML and JS compress ~3x. Comparing `du` against 1 GB would cry wolf at a - # third of the real usage. So measure the tree cheaply, and only pay for an - # actual pack when the tree is big enough for the answer to matter. - # - # -h matches upload-pages-artifact's --dereference: the `stable/` symlink is - # inflated to a full copy of a release in the real artifact, so a measurement - # that stored it as a symlink would under-read by a whole version. - bytes=$(du -sb "$RUNNER_TEMP/site" | cut -f1) - echo "assembled site: $(numfmt --to=iec --suffix=B "$bytes") uncompressed across \ + # Report the assembled size. Pages rejects an artifact over 1 GB and + # deploy-pages warns when the upload exceeds it; `max-releases` / `max-prs` are + # the levers (see docs/how-to/keep-the-site-small.md). This is the tree on + # disk, so it reads high — the cap applies to the compressed upload, and + # HTML/JS packs around 3x. + echo "assembled site: $(du -sh "$RUNNER_TEMP/site" | cut -f1) on disk across \ $(find "$RUNNER_TEMP/site" -mindepth 1 -maxdepth 1 -type d | wc -l) version dir(s)" - if [ "$bytes" -gt "$SIZE_PROBE_BYTES" ]; then - packed=$(tar -ch -C "$RUNNER_TEMP/site" . | gzip -1 -c | wc -c) - echo "packed artifact: ~$(numfmt --to=iec --suffix=B "$packed") (Pages rejects over 1 GB)" - if [ "$packed" -gt "$SIZE_WARN_BYTES" ]; then - echo "::warning title=Docs site approaching the Pages 1 GB limit::The \ - deployed artifact is ~$(numfmt --to=iec --suffix=B "$packed"), against a hard \ - 1 GB cap. Set the \`max-releases\` input in your publish.yml to publish only the \ - N most recent releases." - fi - fi # `generate` orders versions, writes switcher.json + index.html, and prints the # stable-alias source dir (the newest release), or "" when no release is deployed yet. @@ -544,36 +454,22 @@ jobs: with: path: ${{ runner.temp }}/site - # Deploy the uploaded artifact. deploy-pages stamps the deployment with - # pages_build_version = GITHUB_SHA and has no input to change it; the value is - # server-validated against the OIDC commit claim, so it CANNOT be overridden - # (a custom value 404s — actions/deploy-pages#383). The Pages backend silently - # DROPS a *second* deploy of an already-deployed SHA on a non-`workflow_dispatch` - # event (it reports success but the origin keeps serving the first artifact), - # EXCEPT a `workflow_dispatch` deploy of that SHA, which forces a re-serve. A - # release tag shares the default branch's just-deployed SHA, so an inline tag - # deploy would be dropped — which is why tags go through the `re-dispatch` job - # below (re-fired as workflow_dispatch) instead of here. main/internal-PR - # deploys are the FIRST of their SHA, so they serve fine inline. The Verify step - # below backstops any residual stale-origin case. + # deploy-pages stamps the deployment with pages_build_version = GITHUB_SHA and has + # no input to change it; the value is server-validated against the OIDC commit + # claim, so a custom one 404s (actions/deploy-pages#383). Under this workflow's + # triggers a repeat SHA still re-serves — see the header. - name: Deploy to GitHub Pages id: deployment uses: actions/deploy-pages@v5 - # deploy-pages reports success once the deployment RECORD flips active — even - # if the Pages backend silently fails to update the SERVED origin (a wedged - # site can serve a stale artifact indefinitely; see the 2026-06 incident where - # a gh-pages→Actions cutover froze the origin while every deploy reported OK). - # The re-dispatch (deploy via workflow_dispatch) prevents the tag-after-merge - # drop; this is the backstop that turns any *other* stale-origin failure into a - # red check instead of silently serving old docs. Confirm the live switcher.json - # byte-matches the one we just assembled, polling up to ~6 min — the window is - # generous because a release deploys TWICE in quick succession on the same SHA (the - # merge-to-default-branch push, then the tag re-dispatch), and the second (dispatch) - # re-serve can take a few minutes to propagate to the origin behind the first - # (observed ~4 min on 2026-06-25). The loop exits the instant the origin matches, - # so a fast/first-of-SHA deploy isn't slowed. On failure, dump the origin headers - # (a frozen etag/last-modified is the wedge signature). + # deploy-pages reports success once the deployment RECORD flips active, even if the + # Pages backend fails to update the SERVED origin — a wedged site can serve a stale + # artifact indefinitely while every deploy reports OK. This turns that into a red + # check instead of silently serving old docs: confirm the live switcher.json + # byte-matches the one just assembled, polling up to ~6 min because a re-serve can + # take minutes to reach the origin. The loop exits the instant it matches, so a + # fast deploy isn't slowed. On failure, dump the origin headers (a frozen + # etag/last-modified is the wedge signature). - name: Verify the deployed origin matches what we assembled id: verify env: diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 6d17214..1812f53 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -1,24 +1,15 @@ name: Publish # The one file each consuming repo carries, and the single place they pin . It does -# nothing but call the engine (publish-gh-pages.yml) when CI finishes. -# -# This repo's copy is IDENTICAL to a consumer's but for the `uses:` path — theirs pins +# nothing but call the engine (publish-gh-pages.yml) when CI finishes. This repo's copy is +# IDENTICAL to a consumer's but for the `uses:` path — theirs pins # DiamondLightSource/myst-version-switcher-plugin/.github/workflows/publish-gh-pages.yml@, # this one uses the local file so the working tree is what gets tested. # -# WHY workflow_run rather than a job in ci.yml. Publishing is O(the whole site) — it -# reconstructs every version on every run — so nesting it in CI put a constant, sizeable -# cost inside every PR's critical path, which is exactly why the first consumer switched -# PR previews off a day after adopting. On workflow_run it runs after CI, on its own time, -# and a failed deploy no longer shows up as a red check against an author who has no -# control over the Pages infrastructure. -# -# It also DELETED machinery rather than adding any: because workflow_run fires once the -# triggering run has COMPLETED, that run's `docs` artifact is discoverable by the ordinary -# gather, so there is no in-run artifact to inject; and because a tag's deploy re-serves -# on this event (see the engine's header), there is no trampoline and so no need for a -# separately dispatchable shim. +# Publishing lives here rather than as a job in ci.yml because it is O(the whole site) and +# independent of the change, so nesting it put a large constant on every PR's critical +# path; and because a wedged Pages origin is not the PR author's to fix, so its red check +# was noise on their PR. on: # `workflows:` matches the entry workflow by its `name:`, NOT its filename — if you # rename your CI workflow, change it here too or publishing silently stops. @@ -51,7 +42,7 @@ jobs: # head SHA — so a later push to the PR drops the preview until re-approved. # # A manual dispatch has no `workflow_run` payload, so both terms are empty and the - # first clause carries it. + # first clause carries it. Removing either guard fails OPEN. if: >- github.event_name == 'workflow_dispatch' || (github.event.workflow_run.conclusion == 'success' && @@ -60,15 +51,13 @@ jobs: with: # Set (dispatch only) → approve and preview that fork PR's head SHA. pr: ${{ inputs.pr }} - # Publish only the N most recent releases; 0 = all of them. A literal, because this - # is site-size policy and must hold on every path. The site deploys as ONE Pages - # artifact against a hard 1 GB cap — see docs/how-to/keep-the-site-small.md. - # - # This repo deliberately differs from the "20" the tutorial ships: our own site is - # ~223 MB packed across 24 releases, a fifth of the cap, so capping would drop old - # versions for no gain. The capped path is covered by the unit tests and the - # workflow harness instead. - max-releases: "0" + # The site deploys as ONE Pages artifact against a hard 1 GB cap, and every release + # and open PR is a full copy of the docs. These cap what gets published; 0 = no + # limit. Both are LITERALS because this is site-size policy and must hold on every + # path — this workflow is the engine's single caller, so a literal here cannot be + # bypassed by a manual dispatch. See docs/how-to/keep-the-site-small.md. + max-releases: "30" + max-prs: "20" permissions: contents: read actions: read # gather cross-run docs artifacts diff --git a/CLAUDE.md b/CLAUDE.md index 8c54a83..e06dd49 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -47,57 +47,97 @@ default-branch deploy keeps a copy of its `docs.zip` in the Actions cache (content-addressed key `mvs-default-v1-`, so an unchanged branch adds no entry), and a deploy whose fresh artifact is gone restores from there. -This used to be published *into the site* at `_sources/.zip`. That was -permanent, but it shipped a multi-megabyte zip inside every Pages artifact forever on a -site already pressing the 1 GB cap (see "The site has a hard size ceiling"). The cache -is evicted after 7 days unread and by LRU past 10 GB, so a repo quiet for >1 week whose +The alternative — publishing it *into the site* at `_sources/.zip` — is +permanent, but ships a multi-megabyte zip inside every Pages artifact forever on a site +already pressing the 1 GB cap (see "The site has a hard size ceiling"). The cache is +evicted after 7 days unread and by LRU past 10 GB, so a repo quiet for >1 week whose main artifact ALSO expired loses the rung and hard-fails loudly — accepted trade. -`_sources` is no longer written; the READ survives as a deprecated one-shot rung so a -site last deployed by an older `publish.yml` still has a source on its first upgraded -deploy (fetched from `$PAGES_URL` — the live Pages URL from the Pages API, so custom -domains work; the same URL roots the `switcher.json` entries). +`_sources` is gone entirely (no write, no read fallback). `$PAGES_URL` — the live Pages +URL from the Pages API, so custom domains work — now only roots the `switcher.json` +entries. Before the branch ever builds docs (mid gh-pages migration) a final rung reads a **published seed release** (`pages-default-seed`, created by `scripts/migrate.sh` from the old gh-pages `/` tree) — so a repo can cut over to the reusable workflow in one PR, before `docs→main`. Drafts can't be used: a `contents:read` deploy token can't read them (verified), so the seed is a published release on a sentinel tag, deleted at -finalize. `migrate.sh --delete-gh-pages`'s guard now requires the default branch to be -live in the new site AND to have a non-expired `docs` artifact of its own (it used to -probe `_sources/.zip`, which passed even while the content came from the seed -— i.e. while gh-pages was still the only real copy). +finalize. `migrate.sh --delete-gh-pages`'s guard requires the default branch to be live +in the new site AND to have a non-expired `docs` artifact of its own — being live alone +is still satisfied while the content comes from the seed, i.e. while gh-pages is the only +real copy. -### The site has a hard size ceiling — cap the releases +### The site has a hard size ceiling — cap the releases and PRs `upload-pages-artifact` tars the WHOLE site into one artifact and Pages rejects it over -**1 GB**. blueapi at 131 released `docs.zip`s was at 452 MB, +~5 MB/release: a few years -from deploys simply failing, silently, with no warning on the way. The engine takes -**`max-releases`** (default `0` = unlimited, so upgrading never silently deletes -versions); the tutorial ships `20`. Set it as a LITERAL in `publish.yml`'s `with:` — -that is the single caller on every path, so a literal there can't be bypassed by a -manual dispatch. A deploy past 700 MB warns — measured on -the PACKED artifact (the cap is on the gzipped tar; this repo's site is 734 MiB on disk -and 223 MiB deployed, so `du` would cry wolf at a third of real usage). The tree is only -packed to measure once it exceeds `SIZE_PROBE_BYTES`, since packing costs seconds; `tar --ch` matches upload-pages-artifact's `--dereference`, which inflates `stable/` into a -full copy. - -Selection is `assemble.mjs`'s `selectReleases` (pure, unit-tested; it replaced an -untested bash `case`), ranked by **`created_at`** — NOT `published_at`, which lies when +**1 GB**. blueapi at 131 released `docs.zip`s was at 452 MB, +~5 MB/release. The engine +takes **`max-releases`** and **`max-prs`** (both default `0` = unlimited, so upgrading +never silently deletes versions); the paved path ships `30`/`20`, and this repo's +`publish.yml` is now IDENTICAL to a consumer's but for the `uses:` path. Set them as +LITERALS in `publish.yml`'s `with:` — that is the single caller on every path, so a +literal there can't be bypassed by a manual dispatch. The caps also bound the release-zip +cache entry, which competes with the repo's other caches for a 10 GB quota (blueapi is at +7.5 GB of 62 caches before adopting; an uncapped entry there would be 458 MB per release +set). + +There is NO size guard in the workflow. `actions/deploy-pages` already compares the +uploaded artifact against 1 GB and warns (`ONE_GIGABYTE` in `src/internal/deployment.js`, +reading the artifact's stored/compressed size), so packing the tree to re-measure it was +duplicated work with a worse number. The engine just reports `du -sh` of the assembled +tree; the recovery — lower the caps, re-run, nothing is deleted — lives in +docs/how-to/keep-the-site-small.md and in the tutorial's copy-paste `publish.yml`. + +Selection is `assemble.mjs`'s `selectReleases` (pure, unit-tested), ranked by +**`created_at`** — NOT `published_at`, which lies when an old release is re-published (blueapi's `1.3.2-a9`: created 2025-10, published 2026-07, and under `published_at` it outranked the newer `1.11.3`). No version-number parsing: tags are too inconsistent across repos. The seed release is exempt from the cap. `getSortedTags()`/`orderVersions()` are untouched — that's switcher DISPLAY order, a separate concern. -### The gather is cached, indexed and parallel +### The gather is cached and indexed The deploy's cost is O(whole site), not O(the change) — identical work whatever fired it. On blueapi that was 654 s against a 27–45 s docs build. Fixed by: an `actions/cache` entry of release zips keyed on the exact set of asset ids to publish (assets are immutable; files named by ASSET ID so a re-cut release can't serve stale bytes; pruned to -the published set), the artifacts API paginated ONCE and indexed newest-per-head-SHA -(it was re-paginated per open PR — 27× for blueapi, ~175 s), and parallel downloads -(`xargs -P 8`). **Caches are saved only on the default branch**: a cache written on a PR -ref is scoped to that PR and unreadable elsewhere. +the published set), and the artifacts API paginated ONCE and indexed newest-per-head-SHA +(it was re-paginated per open PR — 27× for blueapi, ~175 s). **Caches are saved only when +the default branch was BUILT**: entries are scoped to the writing ref, so one written by a +deploy dispatched from another ref is unreadable by every ordinary deploy. + +The gather's two selection steps are IO-only: they call `gh`, then hand the payloads to +`assemble.mjs`. `select-artifacts` decides the default-branch artifact and the PR +previews; `select-releases` decides the releases. Both rank with the SAME comparator +(`byDateDesc`), which is why the caps can't drift on tie-breaks — they did: the old inline +bash used `sort -r`, keeping PR #9 over #10, while releases kept `1.10` over `1.9`. + +`select-artifacts` carries a security boundary: the default branch is the newest artifact +matching `head_branch == DEFAULT && head_repository_id == `, and it uses THAT +artifact — never a re-lookup by head SHA, which is how the old bash did it and which could +return a fork's artifact for the same commit (a fork's `pull_request` run executes in the +UPSTREAM repo's Actions, so its artifacts are listed here). PR lookups deliberately have +no repo filter — a fork PR's artifact belongs to the fork, and `preview-approved` is the +gate. + +`select-releases` writes the decision TSV to `--out` and prints the release-zip +**cache key on stdout** (`cacheKey()` + `RELZIPS_CACHE_PREFIX`, both unit-tested): the key +is a digest of the selection, so it is derived where the selection is made rather than by +re-deriving it with `awk`/`sha256sum` in the workflow. The workflow also emits +`cache-prefix` (the bare namespace) so `restore-keys` doesn't repeat the literal. + +**Neither key holds a branch or a commit.** The branch is already the cache's SCOPE (all +entries here read `refs/heads/main`). Both keys are CONTENT hashes: `mvs-relzips-v1-` and `mvs-default-v1-`. A commit +SHA would mint a fresh multi-MB entry on every push even when the docs are byte-identical. +Consequence: the default-branch RESTORE can't name its key (you don't know the hash until +you have the content), so `key:` is the bare prefix `mvs-default-v1-` — which can never +match exactly — and `restore-keys` returns the newest entry. That looks like a bug; it +isn't. + +**Release downloads are SERIAL; PR-artifact downloads are PARALLEL** (`xargs -P 8`), and +the asymmetry is deliberate. With the cache the release gather fetches 0–1 zips on almost +every deploy (measured here: 0–2 s warm), so parallelising it bought only a rare cold +start in exchange for a three-pass download-and-stage structure. PR artifacts can never be +cached — a head SHA changes on every push — so that gather pays 1 + open-PR downloads on +EVERY deploy (~0.8 s each; 27 of them at blueapi scale). ### Reconstruct from durable sources, publish the whole tree Every deploy rebuilds the **complete** site from authoritative inputs — `main`'s @@ -134,12 +174,20 @@ SHA exactly as `workflow_dispatch` does — four consecutive deploys at identica - the **`warn` job**, `retry-until` and the wedged-origin retry-dispatch are gone, and the engine no longer needs `actions: write`. +### The version name is computed twice, never passed +`docs.yml` derives `pr-` / the ref name for `BASE_URL`; the gather independently +derives the same string for each source's `site/` (a release's tag, a PR's number, +the repo's `default_branch`). Nothing is threaded between them — `docs.yml` has no +`version-name` output any more (it existed only for the deleted injection step). The two +rules must change together. + ### THE workflow_run TRAP: `github.ref` is always the default branch In a `workflow_run` run `GITHUB_SHA` and `github.ref` are **always the default branch's HEAD**, never the built commit (a PR-triggered deploy reports `refs/heads/main`). Anything asking "was this the default branch?" must read `github.event.workflow_run.head_branch`. -Both cache-save steps got this wrong initially — gating on `github.ref` would have every -PR-triggered deploy writing caches scoped to a PR nothing can read. +Both cache-save steps ask this question — gated on `github.ref` they would fire on every +deploy, including one dispatched from a non-default ref, whose entry only that ref can +read. `test/workflow-harness/test_shape.py` asserts this, plus the caller's two guards (`conclusion == 'success'`, `head_repository == this repo`). All three **fail OPEN**, and @@ -171,8 +219,8 @@ env: run: cd docs && myst build --html ``` Without this, assets and links break under the versioned GitHub Pages sub-path. The -version name is computed in `docs.yml` and is exactly the `site/` dir -`assemble` files this build's artifact at, so the two cannot drift. +version name is computed in `docs.yml` and re-derived identically by the gather as the +`site/` dir — see "The version name is computed twice, never passed". ### `assemble` degrades gracefully on first deploy With no releases and no other branches, `assemble` produces a single-entry @@ -203,17 +251,18 @@ Two top-level workflows: `ci.yml` builds, `publish.yml` listens for it finishing - `publish.yml` — **the file each consumer carries**, and the one place `` is pinned. `workflow_run` (CI completing) + `workflow_dispatch` (fork-PR preview via `pr`, manual re-deploy) → `publish-gh-pages.yml`. Its job `if:` carries the two guards that - fail open (`conclusion == 'success'`, `head_repository == this repo`) and a - `max-releases` literal. This repo's copy is identical to a consumer's but for the - `uses:` path (local vs pinned) and `max-releases: "0"` vs the tutorial's `"20"` — our - site is ~223 MB packed across 24 releases, a fifth of the cap. + fail open (`conclusion == 'success'`, `head_repository == this repo`) and the + `max-releases`/`max-prs` literals. This repo's copy is identical to a consumer's but + for the `uses:` path (local vs pinned) — same `30`/`20` caps, so the file people copy + is the file that is actually exercised. - `publish-gh-pages.yml` — **assemble + deploy ENGINE, privileged.** `workflow_call` ONLY. **One** job, `deploy`: sparse-checks out `assemble.mjs` at `job.workflow_sha` so it matches the pinned ref (see "Self-referencing assemble.mjs"), runs inline gather + extract, then `assemble.mjs generate`, `upload-pages-artifact`, `deploy-pages` and the origin verify — carrying the `github-pages` environment, perms, and `concurrency: {group: pages, cancel-in-progress: true}`. No event branching: the - caller already established success + non-fork. Inputs are just `pr` and `max-releases`. + caller already established success + non-fork. Inputs are `pr`, `max-releases` and + `max-prs`. Sub-workflows of `ci.yml`: - `_lint.yml` — biome @@ -223,13 +272,13 @@ Sub-workflows of `ci.yml`: input) with `BASE_URL` + `VERSION_NAME` set (the latter for builds that need the bare token, e.g. a Sphinx conf.py setting pydata's switcher `version_match`) → pack `docs.zip` (single root dir `html/`, staged so any `html-dir` works) → upload the `docs` - artifact. No deploy; `contents: read` only (the fork-PR hint lives in publish.yml's - `warn` job). Installs uv unconditionally and relies on the runner's - preinstalled Node, so `build-command` can be `make docs` / `npx … myst build` / - `tox -e docs` regardless of project. This repo passes `npm ci && npm run docs`. It - OWNS the build↔publish contract (version name, BASE_URL, docs.zip's single root - dir, `docs` artifact name — publish.yml gathers cross-run artifacts by that NAME via - the artifacts API, never by workflow filename) so consumers only choose a command. + artifact. No deploy; `contents: read` only. Installs uv unconditionally and relies on + the runner's preinstalled Node, so `build-command` can be `make docs` / + `npx … myst build` / `tox -e docs` regardless of project. This repo passes + `npm ci && npm run docs`. It defines the build↔publish contract (version-name rule, + BASE_URL, docs.zip's single root dir, `docs` artifact name — publish.yml gathers + cross-run artifacts by that NAME via the artifacts API, never by workflow filename) so + consumers only choose a command. It has no outputs: nothing is passed to publish. - `release.yml` — **PUBLIC reusable, tag-only.** Downloads every artifact in the run and attaches them to the tag's GitHub Release via `gh` — `gh release create` if no Release exists yet (draft→upload→publish atomically, so immutable-safe), else @@ -240,8 +289,8 @@ Sub-workflows of `ci.yml`: **Publish flow.** `ci.yml` builds and stops. When that run *completes*, `publish.yml` fires on `workflow_run` and calls the engine, which gathers `main`, the newest -`max-releases` releases, and every open PR's artifact from durable sources, then deploys -the whole site. Same path for every event — a PR, a `main` push and a tag all just +`max-releases` releases, and the newest `max-prs` open PRs' artifacts from durable +sources, then deploys the whole site. Same path for every event — a PR, a `main` push and a tag all just produce a completed CI run. A **tag** additionally has `release.yml` attach its `docs.zip` first, within that same CI run, so by the time publish gathers, the asset is already there (this is why the old tag/asset race is gone rather than fixed). An @@ -338,7 +387,7 @@ jobs: (github.event.workflow_run.conclusion == 'success' && github.event.workflow_run.head_repository.full_name == github.repository) uses: DiamondLightSource/myst-version-switcher-plugin/.github/workflows/publish-gh-pages.yml@ - with: { max-releases: "20" } + with: { pr: "${{ inputs.pr }}", max-releases: "30", max-prs: "20" } permissions: { pages: write, id-token: write, contents: read, actions: read, statuses: write } ``` diff --git a/assemble/assemble.mjs b/assemble/assemble.mjs index b5dc9f2..fe0052c 100644 --- a/assemble/assemble.mjs +++ b/assemble/assemble.mjs @@ -5,14 +5,22 @@ * dir, then the extract step unzips them all into the site tree. This file is * the final step: given the populated site tree it orders the versions, writes * switcher.json + index.html, and prints the stable-alias source dir on stdout. - * Exposed as two subcommands, one at each end of the gather: + * Exposed as three subcommands: two that decide what the gather should fetch, and one + * that renders the assembled tree. * - * node assemble.mjs select-releases --default-branch [--seed-tag ] - * [--max-releases ] < releases.json - * → one TSV row per listed release deciding whether (and where) it lands in - * the site. Runs BEFORE the gather, so publish.yml downloads only what it + * node assemble.mjs select-releases --default-branch --out + * [--seed-tag ] [--max-releases ] < releases.json + * → write one TSV row per listed release to , deciding whether (and + * where) it lands in the site; print the cache key for that exact selection + * on stdout. Runs BEFORE the gather, so publish.yml downloads only what it * will actually publish. * + * node assemble.mjs select-artifacts --default-branch --repo-id + * [--prs ] [--max-prs ] < artifacts.json + * → print `dest \0 artifact-id \0 label \0` for every CI artifact to download + * (the default branch's, plus each eligible open PR's), ready for + * `xargs -0 -n 3`; a decision for every candidate goes to stderr. + * * node assemble.mjs generate --site-dir --base-url [--required ] * → write switcher.json + index.html into ; print the stable-alias * source dir (the newest deployed release) on stdout, or nothing. Also @@ -23,6 +31,7 @@ * `generate` file writes touch IO. */ import { execFileSync } from "node:child_process"; +import { createHash } from "node:crypto"; import { readdirSync, readFileSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { parseArgs } from "node:util"; @@ -41,13 +50,35 @@ export const DOCS_ZIP = "docs.zip"; export const SEED_TAG = "pages-default-seed"; /** - * The DEPRECATED in-site durable store (`_sources/.zip`). publish.yml used - * to persist the default branch's docs.zip here every deploy; it now caches it - * instead, so newly assembled trees never contain this directory. The exclusion - * stays as insurance — it costs nothing, and a stray `_sources` dir must never be - * mistaken for a version. + * Namespace for the release-zip `actions/cache` entries. Bump the version segment + * whenever a change here would make an existing entry mean something different, so + * old entries are abandoned rather than misread. `restore-keys` uses this bare + * prefix, so the workflow takes it from `select-releases` too rather than repeating + * the literal. */ -export const SOURCES_DIR = "_sources"; +export const RELZIPS_CACHE_PREFIX = "mvs-relzips-v1-"; + +/** + * The cache key for a selection: the namespace plus a digest of the asset ids it + * intends to publish. Content-addressed on the SET, so cutting a release misses (and + * `restore-keys` then supplies the previous set, which shares every other zip), while + * a deploy that changes nothing re-uses the entry exactly. + * + * Asset ids, not tags: an id names immutable bytes, so a re-cut release can never be + * served from a stale entry. Skipped and capped-out releases contribute nothing, which + * is what keeps a capped site's cache capped. + */ +export function cacheKey(rows = []) { + const ids = rows + .filter((row) => row.dest !== null) + .map((row) => String(row.assetId)) + .sort(); + const digest = createHash("sha256") + .update(ids.map((id) => `${id}\n`).join("")) + .digest("hex") + .slice(0, 32); + return `${RELZIPS_CACHE_PREFIX}${digest}`; +} /** Run a git command and return its non-empty stdout lines. */ function gitLines(args) { @@ -64,10 +95,7 @@ export function discoverVersions(siteDir) { return []; } return entries - .filter( - (d) => - d.isDirectory() && d.name !== STABLE_ALIAS && d.name !== SOURCES_DIR, - ) + .filter((d) => d.isDirectory() && d.name !== STABLE_ALIAS) .map((d) => d.name); } @@ -85,9 +113,8 @@ export function getSortedTags() { /** * Order the gathered versions: `master`, `main`, then tags newest-first, then any - * leftover directories (e.g. feature-branch previews) alphabetically. `tags` must - * already be newest-first. Versions are the directory names under `site/` — the - * current build is already among them, so there is no `add` parameter. + * leftover directories (e.g. PR previews) alphabetically. `tags` must already be + * newest-first; `builds` are the directory names under `site/`. */ export function orderVersions(builds, tags) { const remaining = new Set(builds); @@ -110,14 +137,18 @@ export function orderVersions(builds, tags) { } /** - * Newest first. Releases with no date sort last, and ties break on tag name so - * the selection is deterministic (two releases can share a timestamp). + * Comparator: newest first, undated last. Timestamp ties break on `keyOf` ascending + * — arbitrary, but a total order, which is what matters: an unstable ranking would + * change the published set (and so the cache key) on identical inputs. Shared by the + * release cap and the PR cap so the two cannot drift apart. */ -function byDateDesc(a, b) { - const ta = a.date ? Date.parse(a.date) : Number.NEGATIVE_INFINITY; - const tb = b.date ? Date.parse(b.date) : Number.NEGATIVE_INFINITY; - if (ta !== tb) return tb - ta; - return a.tag.localeCompare(b.tag); +function byDateDesc(keyOf) { + return (a, b) => { + const ta = a.date ? Date.parse(a.date) : Number.NEGATIVE_INFINITY; + const tb = b.date ? Date.parse(b.date) : Number.NEGATIVE_INFINITY; + if (ta !== tb) return tb - ta; + return String(keyOf(a)).localeCompare(String(keyOf(b))); + }; } /** @@ -132,17 +163,14 @@ function byDateDesc(a, b) { * number. Tags are wildly inconsistent across repos and a misparse silently drops * a release; the API already knows the order. * - * The key is `created_at` (the tagged commit's date), NOT `published_at`. - * `published_at` records when the release record was last published, so - * re-publishing an old release makes it look new: blueapi has `1.3.2-a9` created - * 2025-10-01 but published 2026-07-30, which under `published_at` outranks the - * genuinely newer `1.11.3`. `created_at` is immune to that. + * The key is `created_at` (the tagged commit's date), NOT `published_at`, which + * records when the release record was last published — re-publishing an old + * release makes it look newer than releases that genuinely followed it. * - * `maxReleases` caps how many releases the site publishes. It exists because the - * deploy uploads the WHOLE site as one artifact and GitHub Pages caps that at 1 GB: - * a repo with 131 released docs.zips was at 452 MB and climbing ~5 MB per release. - * 0 means unlimited. The seed release is never capped — it is not a version, it - * stands in for the default branch. + * `maxReleases` caps how many releases the site publishes, because the deploy + * uploads the WHOLE site as one artifact against a 1 GB Pages limit. 0 means + * unlimited. The seed release is never capped — it is not a version, it stands in + * for the default branch. * * @param {object[]} releases raw release objects * @param {{defaultBranch: string, seedTag?: string, maxReleases?: number}} opts @@ -189,7 +217,7 @@ export function selectReleases( // never consumes a slot. Everything past the cap is demoted in place. const ranked = rows .filter((row) => row.dest !== null && row.tag !== seedTag) - .sort(byDateDesc); + .sort(byDateDesc((row) => row.tag)); for (const row of ranked.slice(cap)) { row.dest = null; row.decision = `skip: beyond max-releases=${cap} (dated ${row.date ?? "unknown"})`; @@ -198,6 +226,125 @@ export function selectReleases( return rows; } +/** + * Decide which CI artifacts contribute a version, and why. + * + * Pure: takes the flattened `GET /repos/{repo}/actions/artifacts?name=docs` list plus + * the open-PR list with fork approval already resolved (that costs an API call per fork + * PR, so the caller settles it), and returns one row per candidate — the default branch + * and every PR — in a stable order, so the workflow can print a table accounting for all + * of them. `dest` is the `site/` the artifact becomes, or null when skipped. + * + * Two different lookups, deliberately: + * + * - The DEFAULT BRANCH takes the newest artifact built from THIS repo on that branch. + * `repoId` is a security boundary, not an optimisation: a fork's pull_request run + * executes in the upstream repo's Actions, so without it a fork branch named `main` + * could land in `site/main`. The chosen artifact is that filtered one — never a + * re-lookup by SHA, which could hand back a fork's artifact for the same commit. + * - A PR takes the newest artifact at its head SHA with NO repo filter, because a fork + * PR's artifact legitimately belongs to the fork. `approved` is what gates those. + * + * `maxPrs` caps how many previews the site carries, ranked by build date. As with + * releases, the cap applies AFTER eligibility, so a PR with no artifact or an + * unapproved fork PR never consumes a slot. 0 means unlimited. + * + * @param {object[]} artifacts flattened artifact objects (expired ones are dropped here) + * @param {{defaultBranch: string, repoId: number|string, + * prs?: {num: string, sha: string, approved: boolean}[], + * maxPrs?: number}} opts + * @returns {{dest: string|null, label: string, artifactId: number|null, sha: string, + * date: string|null, size: number, runId: number|null, decision: string}[]} + */ +export function selectArtifacts( + artifacts = [], + { defaultBranch, repoId, prs = [], maxPrs = 0 } = {}, +) { + const cap = Number(maxPrs) > 0 ? Number(maxPrs) : 0; + const live = artifacts.filter((a) => a && !a.expired); + + const meta = (artifact) => ({ + artifactId: artifact?.id ?? null, + date: artifact?.created_at ?? null, + size: artifact?.size_in_bytes ?? 0, + runId: artifact?.workflow_run?.id ?? null, + }); + // byDateDesc reads `.date`; artifacts carry `created_at`, so map before sorting + // rather than silently comparing undefined against undefined. + const newest = (candidates) => + candidates.length === 0 + ? null + : candidates + .map((a) => ({ date: a?.created_at ?? null, key: String(a?.id), a })) + .sort(byDateDesc((x) => x.key))[0].a; + + // Newest artifact per head SHA, for the PR lookups. + const bySha = new Map(); + for (const artifact of live) { + const sha = artifact?.workflow_run?.head_sha; + if (!sha) continue; + const best = bySha.get(sha); + if ( + !best || + Date.parse(artifact.created_at) >= Date.parse(best.created_at) + ) { + bySha.set(sha, artifact); + } + } + + const rows = []; + + const branchArtifact = newest( + live.filter( + (a) => + a?.workflow_run?.head_branch === defaultBranch && + String(a?.workflow_run?.head_repository_id) === String(repoId), + ), + ); + rows.push({ + dest: branchArtifact ? defaultBranch : null, + label: `default-branch CI (${defaultBranch})`, + sha: branchArtifact?.workflow_run?.head_sha ?? "", + ...meta(branchArtifact), + decision: branchArtifact + ? "gather" + : "skip: no non-expired 'docs' artifact for the default branch", + }); + + const prRows = prs.map((pr) => { + const artifact = pr.approved ? bySha.get(pr.sha) : undefined; + const row = { + dest: null, + label: `PR #${pr.num}`, + num: pr.num, + sha: pr.sha, + ...meta(artifact), + decision: "", + }; + if (!pr.approved) { + row.decision = `skip: fork PR, head ${pr.sha.slice(0, 8)} not preview-approved`; + } else if (!artifact) { + row.decision = `skip: no non-expired 'docs' artifact for ${pr.sha.slice(0, 8)}`; + } else { + row.dest = `pr-${pr.num}`; + row.decision = "gather"; + } + return row; + }); + + if (cap > 0) { + const ranked = prRows + .filter((row) => row.dest !== null) + .sort(byDateDesc((row) => row.num)); + for (const row of ranked.slice(cap)) { + row.dest = null; + row.decision = `skip: beyond max-prs=${cap} (built ${row.date ?? "unknown"})`; + } + } + rows.push(...prRows); + return rows; +} + /** * Is `tag` a prerelease? Mirrors `release.yml`'s test (a PEP 440-style `a`, * `b`, or `rc` marker *following a digit*, e.g. `1.0a1`/`2.0rc1`) so "stable" @@ -229,7 +376,7 @@ export function preferredVersion(versions, tags) { * a `stable/` alias pointing at it and the root redirects to the constant * `stable/` URL. Before the first release (preferred is `main`/`master`/a * leftover, or a prerelease-only fallback) there is no `stable/` and the root - * redirects straight to that fallback version, as today. + * redirects straight to that fallback version. * * @returns {{preferred: string|null, stableSrc: string|null, redirectTarget: string|null}} * `stableSrc` is the version dir to alias as `stable/` (or null); `redirectTarget` @@ -245,13 +392,10 @@ export function stablePlan(versions, tags) { /** * Required branches that did not end up in the assembled site. Pure. `versions` - * are the discovered site dirs; a required branch is present iff its name is - * among them. By the time `generate` runs, the current - * ref and every gathered branch are already dirs, so this needs no separate - * "present" bookkeeping. The action gathers a preview for every branch with a - * recent CI build (dumb bash); this only guards that the required branches - * (default: the repo's default branch) didn't silently vanish. `generate` - * hard-fails on a non-empty result. + * are the discovered site dirs; a required branch is present iff its name is among + * them. Guards that a branch the site cannot do without (default: the repo's + * default branch) did not silently vanish — `generate` hard-fails on a non-empty + * result rather than publishing a hole. * * @param {string[]} [required] branches that must be present (raw names) * @param {string[]} [versions] discovered site dir names @@ -279,7 +423,7 @@ export function switcherStruct(baseUrl, versions, preferred) { }); } -/** Serialise the switcher exactly as the Python tool did (2-space JSON). */ +/** Serialise the switcher as pydata-style 2-space JSON. */ export function renderSwitcher(baseUrl, versions, preferred) { return JSON.stringify(switcherStruct(baseUrl, versions, preferred), null, 2); } @@ -337,12 +481,14 @@ function cmdSelectReleases(rest) { "default-branch": { type: "string" }, "seed-tag": { type: "string" }, "max-releases": { type: "string" }, + out: { type: "string" }, }, }); const defaultBranch = values["default-branch"]; - if (!defaultBranch) { + const out = values.out; + if (!defaultBranch || !out) { throw new Error( - "usage: assemble.mjs select-releases --default-branch [--seed-tag ] [--max-releases ] < releases.json", + "usage: assemble.mjs select-releases --default-branch --out [--seed-tag ] [--max-releases ] < releases.json", ); } const raw = readFileSync(0, "utf8").trim(); @@ -360,29 +506,107 @@ function cmdSelectReleases(rest) { `${rows.length} release(s) listed, ${gathered.length} selected` + (cap > 0 ? ` (max-releases=${cap})` : " (max-releases unlimited)"), ); - if (cap > 0 && rows.length - gathered.length > 0) { - const dropped = rows.filter((row) => - row.decision.startsWith("skip: beyond"), + const dropped = rows.filter((row) => row.decision.startsWith("skip: beyond")); + if (dropped.length > 0) { + console.error( + `${dropped.length} release(s) dropped by the cap — oldest kept: ` + + `${gathered.map((r) => r.tag).slice(-1)[0] ?? "(none)"}`, + ); + } + + const tsv = rows + .map((row) => + [ + field(row.tag), + field(row.dest), + field(row.date), + field(row.size), + field(row.assetId), + field(row.decision), + ].join("\t"), + ) + .map((line) => `${line}\n`) + .join(""); + writeFileSync(out, tsv, "utf8"); + + // The only stdout: the cache key for exactly this selection. + console.log(cacheKey(rows)); +} + +/** + * `select-artifacts --default-branch --repo-id [--prs ] + * [--max-prs ] [--server-url ] [--repo ]` + * — read the flattened artifacts JSON on stdin (`gh api --paginate … | jq -rs + * '[.[].artifacts[]]'`) and print, for every artifact to download, three NUL-separated + * fields: dest, artifact id, label. That feeds `xargs -0 -n 3` directly, which hands + * them to the worker as positional args — data, never script text. NUL because a label + * carries spaces and a delimiter the payload cannot contain is the only kind that + * cannot be spoofed by it. Decisions go to stderr, so the log stays readable. + * + * `--prs` is TSV: `number head-sha approved`, with fork approval already + * resolved by the caller (it costs an API call per fork PR). + */ +function cmdSelectArtifacts(rest) { + const { values } = parseArgs({ + args: rest, + options: { + "default-branch": { type: "string" }, + "repo-id": { type: "string" }, + "max-prs": { type: "string" }, + prs: { type: "string" }, + "server-url": { type: "string" }, + repo: { type: "string" }, + }, + }); + const { "default-branch": defaultBranch, "repo-id": repoId } = values; + if (!defaultBranch || !repoId) { + throw new Error( + "usage: assemble.mjs select-artifacts --default-branch --repo-id [--prs ] [--max-prs ] [--server-url ] [--repo ] < artifacts.json", ); - if (dropped.length > 0) { - console.error( - `${dropped.length} release(s) dropped by the cap — oldest kept: ` + - `${gathered.map((r) => r.tag).slice(-1)[0] ?? "(none)"}`, - ); - } } + const raw = readFileSync(0, "utf8").trim(); + const artifacts = raw ? JSON.parse(raw) : []; + + const prs = (values.prs ? readFileSync(values.prs, "utf8") : "") + .split("\n") + .filter(Boolean) + .map((line) => { + const [num, sha, approved] = line.split("\t"); + return { num, sha: sha ?? "", approved: approved === "true" }; + }); + + const cap = Number(values["max-prs"] ?? 0) || 0; + const rows = selectArtifacts(artifacts, { + defaultBranch, + repoId, + prs, + maxPrs: cap, + }); + const wanted = rows.filter((row) => row.dest !== null); + console.error( + `${artifacts.filter((a) => a && !a.expired).length} non-expired 'docs' artifact(s), ` + + `${prs.length} open PR(s), ${wanted.length} to gather` + + (cap > 0 ? ` (max-prs=${cap})` : ""), + ); + const runUrl = (row) => + values.repo && row.runId + ? ` from ${values["server-url"] || "https://github.com"}/${values.repo}/actions/runs/${row.runId}` + : ""; for (const row of rows) { - const cells = [ - field(row.tag), - field(row.dest), - field(row.date), - field(row.size), - field(row.assetId), - field(row.decision), - ]; - process.stdout.write(`${cells.join("\t")}\n`); + console.error( + row.dest === null + ? ` ${row.label} → ${row.decision}` + : ` ${row.label} → artifact ${row.artifactId} (${row.size} B, built ${row.date})${runUrl(row)} @ ${row.sha.slice(0, 8)}`, + ); } + + // The only stdout: NUL-separated triples for `xargs -0 -n 3`. + process.stdout.write( + wanted + .map((row) => `${row.dest}\0${row.artifactId}\0${row.label}\0`) + .join(""), + ); } /** @@ -457,8 +681,12 @@ export function main(argv = process.argv.slice(2)) { return cmdGenerate(rest); case "select-releases": return cmdSelectReleases(rest); + case "select-artifacts": + return cmdSelectArtifacts(rest); default: - throw new Error("usage: assemble.mjs generate|select-releases ..."); + throw new Error( + "usage: assemble.mjs generate|select-releases|select-artifacts ...", + ); } } diff --git a/blueapi-issue.md b/blueapi-issue.md deleted file mode 100644 index c7ac8d2..0000000 --- a/blueapi-issue.md +++ /dev/null @@ -1,147 +0,0 @@ -# Docs site: slow CI, permanently one release behind, and heading for a hard wall - -Three separate problems in blueapi's Pages setup, with one shared root cause. Two are -fixed upstream in -[myst-version-switcher-plugin](https://github.com/DiamondLightSource/myst-version-switcher-plugin) -and need a pin bump plus one config line here; one is still open. - -Measured on blueapi's own runs, 2026-08-21. - ---- - -## 1. The publish job was slow, which is why it got switched off - -[PR #1614](https://github.com/DiamondLightSource/blueapi/pull/1614) narrowed `publish` -to tags only, one day after adoption, because it was adding many minutes to every PR. -That was the right call on the evidence. The evidence: - -| step, PR run [31081562099](https://github.com/DiamondLightSource/blueapi/actions/runs/31081562099) | time | -|---|---| -| `Gather release artifacts` — 188 releases listed, 131 `docs.zip` downloaded **one at a time** | 114 s | -| `Gather branch CI artifacts` — 26 open PRs, one **full artifacts-API pagination each** | 175 s | -| `Upload Pages artifact` — 452,549,958 bytes | 36 s | -| `Deploy to GitHub Pages` | 280 s | -| **deploy job total** | **654 s** | - -The docs *build* is 27–45 s. Everything else in CI finishes by ~200 s. Publish alone took -the run from ~200 s to ~750 s. - -The cause is that the deploy's cost is **O(whole site)**, not O(what changed): every -event re-downloaded ~453 MB of immutable release assets and re-uploaded a 452 MB -artifact, doing identical work regardless of which PR triggered it. - -**Fixed upstream.** Release assets are cached (`actions/cache`, keyed on the exact set of -asset ids, so a new release downloads exactly one zip); the artifacts API is paginated -once and indexed by head SHA instead of once per open PR; downloads run in parallel. The -deploy should land near ~100 s, which starts after `docs` (~50 s) and finishes inside -your 195 s `system-test` — off the critical path in wall-clock terms. - -## 2. The site is permanently one release behind — **still open** - -`https://diamondlightsource.github.io/blueapi/` is missing 1.18.1 right now: the switcher -lists 1.18.0 as `stable` and `/1.18.1/` 404s. This is not a one-off — **every** release -has been missing itself, appearing only when the *next* release is cut. - -### Root cause 2a — the tag re-dispatch races the `docs.zip` upload - -`publish.yml`'s `re-dispatch` job waits for the release **record**, but the deploy's -gather selects releases by their **`docs.zip` asset**. blueapi publishes releases from -the **GitHub UI**, so the record exists *before* the tag push even starts CI — the wait -is a no-op. Timeline for 1.18.1: - -| time (UTC) | event | -|---|---| -| 09:34:14 | release 1.18.1 published (UI) | -| 09:34:16 | tag CI run [32122292712](https://github.com/DiamondLightSource/blueapi/actions/runs/32122292712) starts | -| 09:34:50 | `re-dispatch` sees the release, dispatches immediately | -| 09:35:02 | dispatched deploy [32122349268](https://github.com/DiamondLightSource/blueapi/actions/runs/32122349268) lists releases — **1.18.1 has no assets yet** | -| 09:35:57 | `release / release` attaches `docs.zip` — 55 s too late | - -That run logged `Sorted versions: ["main","1.18.0",…]` and `Stable alias source: 1.18.0`. -1.18.0 missed itself the same way and only landed 11 days later, carried in by 1.18.1's -deploy. - -**Status: not yet fixed.** The obvious fix (wait for the asset, not the record) is -written and working, but it is on hold behind a larger question — whether publishing -should move off the inline CI path onto a `workflow_run` trigger, which would make this -race disappear entirely (`workflow_run` fires after the tag's CI run, `release.yml` -included, so the asset is already attached). That is being settled by a live experiment -upstream. Either way blueapi gets a fix; this note is so nobody re-debugs it meanwhile. - -### Root cause 2b — nothing ever self-heals - -```yaml - publish: - if: github.repository == 'DiamondLightSource/blueapi' && github.ref_type == 'tag' -``` - -Every PR and every push to `main` logs `publish → skipped`, so the *only* deploys are tag -re-dispatches — the exact path broken by 2a. A deploy that misses a source stays missed -until the next tag. - -**Suggested change**, keeping #1614's intent (no deploy on every PR push) while restoring -self-heal on every merge: - -```yaml - publish: - needs: [docs] - # don't publish a pages site in the fork's org - if: >- - github.repository == 'DiamondLightSource/blueapi' && - (github.ref_type == 'tag' || github.ref == 'refs/heads/main') -``` - -PR previews still work: **any** deploy gathers every open PR's build artifact, so PR docs -still appear at `/pr-/`, just refreshed on the next `main` deploy rather than on every -push. With problem 1 fixed you may not need to narrow it at all. - -## 3. The site is at 45% of a hard limit nobody warns you about - -`upload-pages-artifact` tars the **whole** site into one artifact, and GitHub Pages -rejects that artifact over **1 GB**. blueapi's last deploy uploaded **452,549,958 bytes** -across 139 versions, growing ~5 MB per release. That is roughly 110 more releases — -a couple of years — until deploys simply start failing, with no signal on the way. - -**Fixed upstream** by a new `max-releases` input. It defaults to `0` (unlimited) so -upgrading changes nothing on its own, and any deploy past 700 MB now warns. - -To opt in, set it in `.github/workflows/publish-dispatch.yml`, in the `with:` block — -**not** as an input threaded from `ci.yml`, so it applies on every path (inline publish, -tag re-dispatch, fork preview, manual re-deploy): - -```yaml -jobs: - publish: - uses: DiamondLightSource/myst-version-switcher-plugin/.github/workflows/publish.yml@ - with: - version-name: ${{ inputs.version-name }} - max-releases: "20" # ← add this - pr: ${{ inputs.pr }} - dispatch-workflow: publish-dispatch.yml - retry-until: ${{ inputs.retry-until }} -``` - -For blueapi that takes the released half from **453 MB to 73 MB**. Releases are ranked by -the tagged commit's date, and nothing is deleted — every release keeps its `docs.zip` -asset, so raising the cap brings the old versions straight back on the next deploy. - -Worth knowing when picking N: prereleases occupy slots. Your newest 20 releases include -`1.11.5-a1/a2/a3`, `1.11.1-a3` and `1.14.1-dev.1`, so `max-releases: 20` publishes 15 -real versions. - ---- - -## Actions for blueapi - -1. **Bump the pins** to the new tag once it is cut — currently `@v0.24` in three places: - `docs.yml` and `release.yml` in `ci.yml`, and `publish.yml` in `publish-dispatch.yml`. -2. **Restore self-heal**: the `ci.yml` `if:` change in 2b. -3. **Set `max-releases`** in `publish-dispatch.yml` (suggest `"20"`). -4. **Backfill 1.18.1 now** — its asset exists, so a manual re-deploy picks it up: - ```bash - gh workflow run publish-dispatch.yml --repo DiamondLightSource/blueapi - ``` -5. **Optional cleanup**: six PRs (#1246, #1504, #1512, #1552, #1553, #1561) have `docs` - artifacts predating the `docs.zip` packing contract, so they log - `artifact unavailable — skipping`. Harmless; re-running CI on those PRs clears it. The - upstream warning is now specific about this case rather than a catch-all. diff --git a/docs/explanations/architecture.md b/docs/explanations/architecture.md index d36797c..16f8bd0 100644 --- a/docs/explanations/architecture.md +++ b/docs/explanations/architecture.md @@ -21,11 +21,11 @@ artifacts and silently drop if the artifact expires and nothing rebuilds — fin it), so each deploy keeps a copy of its `docs.zip` in the **Actions cache**; a deploy whose fresh artifact has expired restores the branch from there. -That copy used to be published into the site itself, at `_sources/.zip`. That -was permanent, which the cache is not — but it also shipped a multi-megabyte zip inside -every Pages artifact, forever, on a site already pressing against a hard 1 GB ceiling -(see [below](#why-the-site-has-a-size-limit)). The cache is evicted after 7 days without -a read, and by LRU past 10 GB, so a repo that goes quiet for over a week *and* whose +The alternative — publishing that zip into the site itself — is permanent, which the +cache is not, but it ships a multi-megabyte zip inside every Pages artifact forever, on +a site already pressing against a hard 1 GB ceiling (see +[below](#why-the-site-has-a-size-limit)). The cache is evicted after 7 days without a +read, and by LRU past 10 GB, so a repo that goes quiet for over a week *and* whose default-branch CI artifact has also expired loses the rung and the deploy hard-fails — loudly, and fixed by one push. That is the accepted trade. (A gh-pages migration keeps its old `/main/` content only until the default branch builds docs under the new @@ -42,23 +42,25 @@ with nothing on the way to say so. Two things follow. -The engine takes a **`max-releases`** input: publish only the N most recent releases, -ranked by the tagged commit's date. It defaults to `0` (unlimited), so upgrading never -silently deletes versions from an existing site — but the paved path in the -[tutorial](../tutorials/adding-to-a-fresh-repo.md) sets it, and a deploy whose *packed* -artifact passes 700 MB says so in a warning. Packed, not the tree on disk: HTML and JS -compress around 3×, so measuring the directory would cry wolf at a third of real usage. Older releases keep their `docs.zip` assets and -come back the moment you raise the cap. See -[keep-the-site-small](../how-to/keep-the-site-small.md). +The engine takes **`max-releases`** and **`max-prs`** inputs: publish only the N most +recent releases (ranked by the tagged commit's date) and the N most recently built open +PRs. Both default to `0` (unlimited), so upgrading never silently deletes versions from +an existing site — but the paved path in the +[tutorial](../tutorials/adding-to-a-fresh-repo.md) sets them. `actions/deploy-pages` +warns when the uploaded artifact exceeds the limit, and the fix is not destructive: +older releases keep their `docs.zip` assets and come back the moment you raise the cap. +See [keep-the-site-small](../how-to/keep-the-site-small.md). Ranking is on the release's `created_at`, never on parsing the version number: tags are too inconsistent across repos for a parser to be safe, and `published_at` lies whenever an old release is re-published (blueapi has a `1.3.2-a9` created in 2025 and published in 2026, which under `published_at` outranks the genuinely newer `1.11.3`). -The same pressure is why the default branch's durable copy moved out of the site and into -the Actions cache, and why the gather caches release assets rather than re-downloading -~450 MB of immutable zips on every event. +The same pressure is why the default branch's durable copy lives in the Actions cache +rather than in the site, and why the gather caches release assets rather than +re-downloading ~450 MB of immutable zips on every event. The caps bound that cache too: +its entry is as large as the published release set, and it competes with the repo's +build caches for a 10 GB quota. ### Why this replaced the `gh-pages` + `keep_files` model @@ -193,8 +195,8 @@ HEAD** — never the commit that was built. A PR-triggered deploy reports `refs/ That is mostly harmless (it is why the same-SHA question mattered at all), but it silently breaks anything that asks "was this the default branch?". Both of the engine's cache-save -steps did exactly that, and gating on `github.ref` would have had every PR-triggered deploy -writing caches scoped to a PR that nothing else can read. They test +steps ask exactly that: gated on `github.ref` they would fire on every deploy, including +one dispatched from another ref, whose entry only that ref could ever read. They test `github.event.workflow_run.head_branch` instead. Because that lives in an `if:` expression rather than a shell script, the gather harness @@ -303,24 +305,39 @@ The `stable` segment name is a fixed convention, hardcoded in the widget. listed in the switcher if gathered. - **Concurrency:** `concurrency: { group: pages, cancel-in-progress: true }`. Cancelling a superseded deploy is safe *because* every deploy reconstructs the whole site — the - one that replaces it gathers everything the cancelled one would have. This was unsafe - while publish was nested in CI, where cancelling marked an unrelated PR's run cancelled. + one that replaces it gathers everything the cancelled one would have. ## The release-layer cache -Re-downloading and unzipping every release's `docs.zip` on every deploy was the one -recurring cost that scaled with the number of releases, and it stopped being theoretical -once a consumer reached 131 of them: 114 seconds per deploy, on every event, fetching the -same immutable bytes. +Re-downloading every release's `docs.zip` on every deploy is the one recurring cost that +scales with the number of releases, and it is not theoretical: at 131 releases it was 114 +seconds per deploy, on every event, fetching the same immutable bytes. -The engine now caches them (`actions/cache`, keyed on the exact set of asset ids it +So the engine caches them — `actions/cache`, keyed on the exact set of asset ids it intends to publish, files named by **asset id** so a re-cut release cannot be served from -a stale entry, and pruned to the published set so a capped site keeps a capped cache). +a stale entry, and pruned to the published set so a capped site keeps a capped cache. Steady state is a total hit; cutting a release downloads exactly one zip. -Caches are written only when the **default branch** was what got built — one saved while -a PR is the trigger is scoped to that PR and unreadable by anything else, so saving there -would only churn the 10 GB repo quota. +Caches are written only when the **default branch** was what got built. Entries are +scoped to the ref that wrote them, so one written by a deploy dispatched from some other +ref is invisible to every ordinary deploy — dead weight against the 10 GB quota. (Under +`workflow_run` the writing ref is always the default branch whatever triggered the run, +which is why the guard has to read `head_branch`, not `github.ref`.) + +Neither key contains a branch or a commit. The branch is already in the *scope*, and both +keys are **content hashes**: the release cache on the set of asset ids it intends to +publish, the default-branch cache on the bytes of `docs.zip` itself. A commit SHA would +mint a new multi-megabyte entry on every push even when the built docs are identical. The +default-branch restore therefore uses a bare `key: mvs-default-v1-` that can never match +exactly, letting `restore-keys` return the most recent entry — you cannot name a content +hash before you have the content. + +That cache is also why the release downloads are **serial** while the PR-artifact +downloads are **parallel**. With the cache in place the release gather fetches nothing on +almost every deploy, so parallelising it buys a rare, off-critical-path cold start at the +cost of a three-pass download-and-stage structure. PR artifacts cannot be cached at all — +a PR's head SHA changes with every push — so that gather pays one download per open PR on +*every* deploy, which is where `xargs -P 8` earns its keep. ## Key resolved decisions diff --git a/docs/how-to/keep-the-site-small.md b/docs/how-to/keep-the-site-small.md index d9c01e1..94f3649 100644 --- a/docs/how-to/keep-the-site-small.md +++ b/docs/how-to/keep-the-site-small.md @@ -1,30 +1,33 @@ # How-to: keep the published site under the Pages size limit Every deploy uploads the **whole** site as one artifact, and GitHub Pages rejects an -artifact over **1 GB**. Nothing warns you as you approach it; deploys simply start -failing once you arrive. - -The cap is on the **packed** artifact, not the directory tree — HTML and JS compress -around 3×, so a site that is 734 MiB on disk deploys as about 223 MiB. +artifact over **1 GB**. The cap is on the **compressed** upload, not the directory tree — +HTML and JS pack around 3×, so a site that is 734 MiB on disk deploys as about 223 MiB. A docs build is a few megabytes, so this only bites long-lived projects — but it bites them hard. [blueapi](https://github.com/DiamondLightSource/blueapi), with 131 released -`docs.zip`s, was at **452 MB** and adding ~5 MB per release: a few years of headroom, and -no signal on the way. +`docs.zip`s, was at **452 MB** and adding ~5 MB per release. -## Check where you are +## What happens when you hit it -Every deploy prints the size of the tree it assembled: +`actions/deploy-pages` compares the uploaded artifact against the limit and warns: ``` -assembled site: 734MB uncompressed across 31 version dir(s) +Uploaded artifact size of 1103175680 bytes exceeds the allowed size of 1 GB. Deployment might fail. ``` -Once that passes 1.5 GiB it also packs the tree to measure what will really be -uploaded, and warns past 700 MB of *packed* bytes: +That warning only appears once you are already over, so treat a deploy that emits it as +needing action now. Nothing is lost when it happens: cap the site with the two inputs +below and re-run the deploy — every release still has its `docs.zip` asset, so raising +the cap again brings the old versions straight back. + +## Check where you are + +Every deploy prints the size of the tree it assembled (on disk, so roughly 3× what will +actually be uploaded): ``` -packed artifact: ~712MB (Pages rejects over 1 GB) +assembled site: 734M on disk across 31 version dir(s) ``` Or measure the released half directly: @@ -34,52 +37,60 @@ gh api --paginate repos/ORG/REPO/releases \ | jq -rs '[.[][].assets[] | select(.name=="docs.zip") | .size] | add / 1048576 | floor' ``` -## Cap the number of published releases +## Cap what gets published -Set `max-releases` in your `publish.yml`, in the `with:` block: +Two inputs, both set in your `publish.yml`'s `with:` block: ```yaml jobs: publish: uses: DiamondLightSource/myst-version-switcher-plugin/.github/workflows/publish-gh-pages.yml@ with: - max-releases: "20" # ← publish only the 20 most recent releases pr: ${{ inputs.pr }} + max-releases: "30" # ← publish only the 30 most recent releases + max-prs: "20" # ← and previews for only the 20 most recently built PRs ``` :::{important} -A **literal**, not an input threaded in from elsewhere. `publish.yml` is the single caller +**Literals**, not inputs threaded in from elsewhere. `publish.yml` is the single caller of the engine on every path — the `workflow_run` deploy, a fork preview, a manual re-deploy — so a literal here is the one place the policy cannot be bypassed. ::: -The default is `0`, meaning unlimited, so upgrading the pinned tag never removes versions -from a site that already has them. You opt in. +Both default to `0`, meaning unlimited, so upgrading the pinned tag never removes +versions from a site that already has them. You opt in. + +### How the two caps rank + +- **`max-releases`** ranks by the **tagged commit's date** (`created_at`), newest first. + Nothing parses your version numbers — tags vary far too much between projects for that + to be safe, and `published_at` is unreliable because re-publishing an old release + stamps it as new. +- **`max-prs`** ranks by when each PR's docs were last **built**, newest first, so an + abandoned PR's preview drops off before an active one's. -### What the cap does and doesn't do +Both cap only what is *eligible*: a release with no `docs.zip` asset, a PR whose build +artifact has expired, and an unapproved fork PR never consume a slot. -Releases are ranked by the **tagged commit's date** (`created_at`), newest first, and the -top N are published. Nothing parses your version numbers — tags vary far too much between -projects for that to be safe, and `published_at` is unreliable because re-publishing an -old release stamps it as new. +Capping only affects **what the site serves** — nothing is deleted. Dropped versions +leave the switcher and their URLs 404, so pick N so the versions people actually link to +stay in. Note that prereleases occupy slots too: blueapi's newest 20 releases include 5 +prereleases, so `max-releases: 20` would publish only 15 real ones. -Capping only affects **what the site serves**. Every release keeps its `docs.zip` asset on -the GitHub Release, so raising the cap brings the old versions straight back on the next -deploy. Nothing is deleted. +### It also bounds the Actions cache -Dropped versions leave the switcher and their URLs 404. Pick N so that the versions people -actually link to stay in — and note that prereleases occupy slots too: blueapi's newest 20 -releases include 5 prereleases, so `max-releases: 20` publishes only 15 real ones. +The engine caches release zips so a deploy re-downloads only what changed. That cache +entry is as big as the published release set, and each new release writes a *fresh* +entry, all of it counting against the repo's **10 GB** Actions cache quota — which is +shared with your build caches, and evicted by LRU when full. At blueapi's 458 MB of +release zips, an uncapped cache entry would push their other caches out within a few +releases. `max-releases: 30` keeps it near 100 MB. ## What isn't capped - **The default branch** — always published; it is the one required version. - **The migration seed release** (`pages-default-seed`) — it stands in for the default branch rather than being a version of its own. -- **Open-PR previews** — naturally bounded by how many PRs you have open, and they leave - as PRs close. blueapi's 26 open PRs are about 91 MB. -- **Releases with no `docs.zip` asset** — never published in the first place, and they do - not consume a slot. ## Related diff --git a/docs/reference/workflows.md b/docs/reference/workflows.md index 323bada..0da5376 100644 --- a/docs/reference/workflows.md +++ b/docs/reference/workflows.md @@ -25,7 +25,9 @@ jobs: github.event.workflow_run.head_repository.full_name == github.repository) uses: DiamondLightSource/myst-version-switcher-plugin/.github/workflows/publish-gh-pages.yml@ with: - max-releases: "20" + pr: ${{ inputs.pr }} + max-releases: "30" + max-prs: "20" permissions: pages: write id-token: write @@ -46,10 +48,27 @@ and uploads the `docs` artifact. Declares `contents: read` only — it never holds a write token. Installs `uv` unconditionally and relies on the runner's preinstalled Node, so `build-command` can be `make` / `npx` / `tox` / `npm` driven. +| input | required | default | meaning | +|---|---|---|---| +| `build-command` | **yes** | — | Command that builds the HTML site. Run with `BASE_URL` and `VERSION_NAME` set. Fold any project setup (`npm ci`, `cp CONFIG`, apt deps) into it. | +| `html-dir` | no | `docs/_build/html` | Directory the build writes the site to. Its contents are staged into `docs.zip`'s `html/` root, so any name works. | + +## `publish-gh-pages.yml` — assemble + deploy (privileged) + +`workflow_call` only, one `deploy` job: gather → extract → `assemble.mjs generate` → +`upload-pages-artifact` → `deploy-pages` → verify the served origin. Call it from a +`publish.yml` that triggers on your CI workflow completing. + | input | required | default | meaning | |---|---|---|---| | `pr` | no | `""` | Fork PR number to approve (pins its head SHA as `preview-approved`) and preview. Set on the caller's `workflow_dispatch` path. | -| `max-releases` | no | `"0"` | Publish only the N most recent releases, ranked by the tagged commit's date (`created_at`); `0` = all of them. The site deploys as **one** Pages artifact against a hard **1 GB** cap, which a long-lived project will eventually hit. Set it as a **literal** in `publish.yml`'s `with:` block so it applies on every path. See [keep-the-site-small](../how-to/keep-the-site-small.md). | +| `max-releases` | no | `"0"` | Publish only the N most recent releases, ranked by the tagged commit's date (`created_at`); `0` = all of them. | +| `max-prs` | no | `"0"` | Publish previews for only the N most recently built open PRs; `0` = all of them. | + +Both caps exist because the site deploys as **one** Pages artifact against a hard +**1 GB** limit, which a long-lived project will eventually hit. Set them as **literals** +in `publish.yml`'s `with:` block so they apply on every path. See +[keep-the-site-small](../how-to/keep-the-site-small.md). ## What the engine gathers @@ -59,7 +78,7 @@ Every deploy rebuilds the complete tree from authoritative inputs: |---|---|---| | default branch (e.g. `main`) | newest `docs` artifact built from that branch → the **Actions cache** copy (hard-fail if neither exists) | cached — re-saved on each default-branch deploy; evicted after 7 days unread | | released tags | the `docs.zip` asset attached to each **GitHub Release**, newest `max-releases` of them (the migration seed release, if present, seeds the default-branch zip and is never capped) | permanent | -| open PRs (`pr-`) | each PR's `docs` artifact, keyed by current head SHA — internal always, fork PRs only when the SHA carries a `preview-approved` status | ephemeral — drops when the PR merges/closes | +| open PRs (`pr-`) | each PR's `docs` artifact, keyed by current head SHA, newest `max-prs` of them — internal always, fork PRs only when the SHA carries a `preview-approved` status | ephemeral — drops when the PR merges/closes | Branch and PR artifacts are found via the **artifacts API by name** (`docs` — the artifact name is the contract), not by workflow filename, so the consumer's entry @@ -67,10 +86,15 @@ workflow can be called anything. The URLs baked into `switcher.json` use the sit live Pages URL from the **Pages API**, so a custom domain (CNAME) works. Release assets are immutable, so the gather caches them (`actions/cache`, keyed on the -exact set of asset ids it intends to publish) and downloads only what it has not seen; -downloads run in parallel, and the artifacts API is paginated **once** and indexed by -head SHA rather than re-queried per PR. Caches are written only from the default branch, -because a cache saved on a PR ref is scoped to that PR and unreadable by any other run. +exact set of asset ids it intends to publish) and downloads only what it has not seen — +usually nothing, or one zip for a new release. PR artifacts cannot be cached (a head SHA +changes on every push), so those downloads run in parallel and the artifacts API is +paginated **once** and indexed by head SHA rather than re-queried per PR. + +Both caches are keyed on a **content hash** — the set of asset ids to publish, and +`docs.zip`'s own bytes — never on a commit, so an unchanged input adds no entry. The +branch is not in either key because GitHub scopes entries to the ref that wrote them, and +saves are gated on the default branch having been the thing that was built. A version no longer gathered (a merged/closed PR, a deleted release) is correctly dropped, because `deploy-pages` replaces the *entire* site. Sources are gathered in @@ -80,7 +104,7 @@ fresher source always wins when names collide. ## The `docs.zip` / version-name contracts Two contracts let the build (`docs.yml`) and the reconstruction (`assemble`) agree -without coordination — `docs.yml` owns both: +without passing anything between them: - **`docs.zip` unzips to exactly one top-level directory.** `docs.yml` packs it once, rooted at `html/`, and it is delivered the *same file* two ways: uploaded verbatim as @@ -93,15 +117,27 @@ without coordination — `docs.yml` owns both: is skipped with a warning rather than guessed at. - **The version name is the site sub-dir *and* the `BASE_URL`.** It is `pr-` for PRs, else the ref name (the default branch, or a tag without `/`). `docs.yml` sets - `BASE_URL=/REPO/` and `assemble` files the artifact at - `site/` — the same literal name on both sides, so assets never 404. - There is **no sanitisation**: version names are clean by construction (the - `tags: ['*']` trigger never builds `/`-tags). + `BASE_URL=/REPO/`; nothing passes that name onward, because the gather + re-derives the identical string from the same facts (a release's tag, a PR's number, + the repo's `default_branch`) and unpacks the zip at `site/`. Same name + on both sides, so assets never 404. There is **no sanitisation**: version names are + clean by construction (the `tags: ['*']` trigger never builds `/`-tags). ## Internals: `assemble.mjs` -The pure logic (ordering, prerelease detection, `switcher.json`/redirect rendering, -the required-branch guard) lives in `assemble/assemble.mjs` and is unit-tested -(`npm test`) without git, the network, or the filesystem. The `gh`/`unzip`/`mv` IO -plumbing lives as inline bash steps in `publish.yml`'s `deploy` job — named and -separated so step timing and failures are visible in the GH Actions UI. +The pure logic lives in `assemble/assemble.mjs` and is unit-tested (`npm test`) without +git, the network, or the filesystem. Three subcommands, all deciding *what* while the +workflow does the IO: + +- **`select-releases`** — which releases contribute a version, and the `actions/cache` + key for exactly that set (so a capped site cannot end up with an uncapped cache entry). +- **`select-artifacts`** — which CI artifact becomes the default branch, and which open + PRs get a preview, with fork approval resolved by the caller beforehand. +- **`generate`** — version ordering, prerelease detection, `switcher.json` and the root + redirect, the `stable/` alias source, and the required-branch guard. + +Both `select-*` commands rank with the same comparator, so the release cap and the PR cap +cannot drift apart on how they break a timestamp tie. The `gh`/`unzip`/`mv` IO +plumbing lives as inline bash steps in `publish-gh-pages.yml`'s `deploy` job — named +and separated so step timing and failures are visible in the GH Actions UI, and covered +by a harness that loads those steps out of the YAML and runs them against a mock `gh`. diff --git a/docs/tutorials/adding-to-a-fresh-repo.md b/docs/tutorials/adding-to-a-fresh-repo.md index f25d2a8..6b60046 100644 --- a/docs/tutorials/adding-to-a-fresh-repo.md +++ b/docs/tutorials/adding-to-a-fresh-repo.md @@ -135,10 +135,14 @@ jobs: uses: DiamondLightSource/myst-version-switcher-plugin/.github/workflows/publish-gh-pages.yml@__LATEST_TAG__ with: pr: ${{ inputs.pr }} - # Publish only the N most recent releases; 0 = all. A LITERAL, so it holds on every - # path — the site deploys as one Pages artifact against a hard 1 GB cap. See - # docs/how-to/keep-the-site-small.md. - max-releases: "20" + # The whole site deploys as ONE Pages artifact, and Pages rejects it over 1 GB. + # Every release and every open PR is a full copy of the docs, so these cap what + # gets published (0 = no limit). LITERALS, so the policy holds on every path into + # the engine — including a manual dispatch. If a deploy ever warns that the + # uploaded artifact exceeds 1 GB, lower these and re-run it: nothing is deleted, + # and raising them again brings the old versions back. + max-releases: "30" + max-prs: "20" permissions: contents: read actions: read diff --git a/scripts/migrate.sh b/scripts/migrate.sh index f6193ca..766e2ca 100755 --- a/scripts/migrate.sh +++ b/scripts/migrate.sh @@ -281,11 +281,9 @@ verify() { # repo — i.e. its CI really produces docs.zip now. # # (1) alone is not enough: it is still satisfied while // is coming from the -# migration seed, which is exactly the state where gh-pages is still the only real -# copy. This guard used to probe the in-site `_sources/.zip` instead; that -# file is no longer published (the default branch's durable copy lives in the Actions -# cache, which has no public URL to probe and is evictable), so the guard now waits for -# the stronger and more honest condition it always should have used. +# migration seed, which is exactly the state where gh-pages is still the only real copy. +# (2) is what rules that out. The default branch's durable copy lives in the Actions +# cache, which has no public URL to probe, so the artifact is the thing to check. guard_default_durable() { local default url code repo_id arts deadline default=$(gh repo view "$REPO" --json defaultBranchRef -q .defaultBranchRef.name) diff --git a/test/test-assemble.mjs b/test/test-assemble.mjs index b904591..36231aa 100644 --- a/test/test-assemble.mjs +++ b/test/test-assemble.mjs @@ -11,13 +11,16 @@ import { mkdirSync, mkdtempSync, symlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { + cacheKey, discoverVersions, isPrerelease, missingRequired, orderVersions, preferredVersion, + RELZIPS_CACHE_PREFIX, renderRedirect, renderSwitcher, + selectArtifacts, selectReleases, stablePlan, switcherStruct, @@ -77,11 +80,8 @@ for (const d of ["main", "2.1", "2.0"]) mkdirSync(join(site, d)); writeFileSync(join(site, "switcher.json"), "[]"); // file, ignored writeFileSync(join(site, "index.html"), "x"); // file, ignored symlinkSync("2.1", join(site, "stable")); // the alias, excluded -mkdirSync(join(site, "_sources")); // durable default-branch store, excluded assert.deepEqual(discoverVersions(site).sort(), ["2.0", "2.1", "main"]); -ok( - "discoverVersions returns dirs only, excluding files, the stable alias, and _sources", -); +ok("discoverVersions returns dirs only, excluding files and the stable alias"); assert.deepEqual(discoverVersions(join(site, "does-not-exist")), []); ok("discoverVersions returns [] for a missing dir"); @@ -321,6 +321,188 @@ assert.equal( ); ok("selectReleases carries the docs.zip asset id through for cache keying"); +// --- selectArtifacts: which CI artifact lands where --- +const art = (id, sha, created, extra = {}) => ({ + id, + created_at: created, + size_in_bytes: 4096, + expired: false, + workflow_run: { + id: 900, + head_sha: sha, + head_branch: extra.branch ?? null, + head_repository_id: extra.repoId ?? 42, + }, +}); +const pr = (num, sha, approved = true) => ({ num: String(num), sha, approved }); +const destOf = (rows, label) => + rows.find((r) => r.label === label)?.dest ?? null; +const rowOf = (rows, label) => rows.find((r) => r.label === label); + +// The newest artifact on the branch wins — compared on created_at, not artifact id. +// A comparator reading the wrong field ties every artifact and silently returns the +// oldest, which is a stale site nobody notices. +const branchRows = selectArtifacts( + [ + art(9000, "mainsha", "2026-04-01T00:00:00Z", { branch: "main" }), + art(9001, "mainsha", "2026-05-01T00:00:00Z", { branch: "main" }), + ], + { defaultBranch: "main", repoId: 42 }, +); +assert.equal(rowOf(branchRows, "default-branch CI (main)").artifactId, 9001); +ok("selectArtifacts takes the newest default-branch artifact by build date"); + +// A fork's pull_request run executes in the UPSTREAM repo's Actions, so a fork branch +// called `main` is visible here. Landing it in site/main would serve fork content as +// the default branch. +const forkBranch = selectArtifacts( + [ + art(9100, "mainsha", "2026-05-01T00:00:00Z", { branch: "main" }), + art(9200, "forksha", "2026-06-01T00:00:00Z", { + branch: "main", + repoId: 777, + }), + ], + { defaultBranch: "main", repoId: 42 }, +); +assert.equal(rowOf(forkBranch, "default-branch CI (main)").artifactId, 9100); +ok( + "selectArtifacts excludes a fork-owned branch of the same name, even when newer", +); + +// ...including when the fork's artifact shares the default branch's head SHA, which a +// SHA-keyed re-lookup would hand back despite the repo filter. +const forkSameSha = selectArtifacts( + [ + art(9300, "mainsha", "2026-05-01T00:00:00Z", { branch: "main" }), + art(9400, "mainsha", "2026-06-01T00:00:00Z", { + branch: "main", + repoId: 777, + }), + ], + { defaultBranch: "main", repoId: 42 }, +); +assert.equal(rowOf(forkSameSha, "default-branch CI (main)").artifactId, 9300); +ok( + "selectArtifacts keeps a fork out of the default branch at an identical head SHA", +); + +// A PR's artifact is looked up with NO repo filter: a fork PR's build legitimately +// belongs to the fork, and `approved` is what gates it. +const prRows = selectArtifacts( + [art(9500, "forkpr", "2026-05-01T00:00:00Z", { repoId: 777 })], + { defaultBranch: "main", repoId: 42, prs: [pr(7, "forkpr")] }, +); +assert.equal(destOf(prRows, "PR #7"), "pr-7"); +ok("selectArtifacts previews an approved fork PR's own artifact"); + +const unapproved = selectArtifacts( + [art(9600, "forkpr", "2026-05-01T00:00:00Z", { repoId: 777 })], + { defaultBranch: "main", repoId: 42, prs: [pr(7, "forkpr", false)] }, +); +assert.equal(destOf(unapproved, "PR #7"), null); +assert.match(rowOf(unapproved, "PR #7").decision, /not preview-approved/); +ok("selectArtifacts skips an unapproved fork PR"); + +// Expired artifacts are not a source at all. +const expired = selectArtifacts( + [{ ...art(9700, "prsha", "2026-05-01T00:00:00Z"), expired: true }], + { defaultBranch: "main", repoId: 42, prs: [pr(7, "prsha")] }, +); +assert.equal(destOf(expired, "PR #7"), null); +ok("selectArtifacts ignores expired artifacts"); + +// The cap ranks by build date and applies AFTER eligibility, so an ineligible PR +// never costs a slot — same rule as selectReleases. +const cappedPrs = selectArtifacts( + [ + art(1, "a", "2026-05-01T00:00:00Z"), + art(2, "b", "2026-05-03T00:00:00Z"), + art(3, "c", "2026-05-02T00:00:00Z"), + ], + { + defaultBranch: "main", + repoId: 42, + maxPrs: 2, + prs: [pr(20, "a"), pr(21, "b"), pr(22, "c"), pr(23, "nothing")], + }, +); +assert.deepEqual( + cappedPrs.filter((r) => r.dest).map((r) => r.dest), + ["pr-21", "pr-22"], +); +assert.match(rowOf(cappedPrs, "PR #20").decision, /beyond max-prs=2/); +assert.match(rowOf(cappedPrs, "PR #23").decision, /no non-expired/); +ok("selectArtifacts caps PRs by build date, after eligibility"); + +assert.equal( + selectArtifacts([], { defaultBranch: "main", repoId: 42 }).length, + 1, +); +ok("selectArtifacts still reports the default branch when nothing was found"); + +// --- cacheKey: the release-zip cache key is derived from the SELECTION --- +const keyRows = selectReleases( + [rel("3.0", "2026-03-01T00:00:00Z"), rel("2.0", "2026-02-01T00:00:00Z")], + { defaultBranch: "main" }, +); +assert.match(cacheKey(keyRows), /^mvs-relzips-v1-[0-9a-f]{32}$/); +ok("cacheKey is the namespace prefix plus a 32-char digest"); + +assert.equal(cacheKey(keyRows), cacheKey(keyRows.slice().reverse())); +ok("cacheKey is order-independent — it names a set, not a listing"); + +// Cutting a release must MISS, or the new version never gets downloaded. +const withNew = selectReleases( + [ + rel("4.0", "2026-04-01T00:00:00Z"), + rel("3.0", "2026-03-01T00:00:00Z"), + rel("2.0", "2026-02-01T00:00:00Z"), + ], + { defaultBranch: "main" }, +); +assert.notEqual(cacheKey(withNew), cacheKey(keyRows)); +ok("cacheKey changes when a release joins the published set"); + +// ...and capping one out must miss too, so the entry stays pruned to what is served. +const threeReleases = [ + rel("4.0", "2026-04-01T00:00:00Z"), + rel("3.0", "2026-03-01T00:00:00Z"), + rel("2.0", "2026-02-01T00:00:00Z"), +]; +assert.notEqual( + cacheKey( + selectReleases(threeReleases, { defaultBranch: "main", maxReleases: 1 }), + ), + cacheKey( + selectReleases(threeReleases, { defaultBranch: "main", maxReleases: 2 }), + ), +); +ok("cacheKey changes when max-releases changes the published set"); + +// A release contributing no zip must not perturb the key, or a repo with one +// asset-less release would miss on every single deploy. +assert.equal( + cacheKey([ + ...keyRows, + { + tag: "junk", + dest: null, + date: null, + assetId: null, + size: 0, + decision: "", + }, + ]), + cacheKey(keyRows), +); +ok("cacheKey ignores rows that contribute nothing to the site"); + +assert.equal(RELZIPS_CACHE_PREFIX, "mvs-relzips-v1-"); +ok( + "the cache namespace is exported so the workflow need not repeat the literal", +); + // A release with no created_at at all must not win the cap, and must not throw. const undated = selectReleases( [ diff --git a/test/workflow-harness/run.py b/test/workflow-harness/run.py index 57c10c9..8df4c8d 100644 --- a/test/workflow-harness/run.py +++ b/test/workflow-harness/run.py @@ -1,31 +1,36 @@ """ -Harness for the bash inside publish-gh-pages.yml's `deploy` job. +Harness for the bash inside the publish workflows. -The gather steps are the workflow's most intricate logic — release selection, -an asset cache, a SHA-indexed artifact lookup, parallel downloads, and four -distinct failure modes — and they used to be testable only by merging and -watching a real deploy. This loads the steps straight out of the YAML (so the -tests cannot drift from the workflow), substitutes the ${{ }} expressions, and -runs them against a mock `gh` and synthetic release/artifact payloads. +The gather steps are the most intricate logic in the repo — release selection, an asset +cache, a SHA-indexed artifact lookup, parallel downloads, and several distinct failure +modes — and a real deploy is the only other way to exercise them. This loads the steps +straight out of the YAML (so the tests cannot drift from the workflow), substitutes the +${{ }} expressions, and runs them against a mock `gh` and synthetic payloads. Run with `npm run test:workflows`. """ import yaml, re, subprocess, os, sys, json, shutil, pathlib H = os.path.dirname(os.path.abspath(__file__)) REPO_DIR = os.path.dirname(os.path.dirname(H)) -d = yaml.safe_load(open(f"{REPO_DIR}/.github/workflows/publish-gh-pages.yml")) -steps = {s.get("name"): s for s in d["jobs"]["deploy"]["steps"] if s.get("run")} +WF = f"{REPO_DIR}/.github/workflows" -WANT = ["Select releases to publish", "Gather release artifacts", "Gather branch CI artifacts"] +EXPR = re.compile(r"\$\{\{\s*([^}]*?)\s*\}\}") -def run(step_name, env, exprs): - script = steps[step_name]["run"] - script = re.sub(r"\$\{\{\s*([^}]*?)\s*\}\}", lambda m: exprs.get(m.group(1), ""), script) +def steps_of(workflow, job): + """Every `run:` step of one job, by name.""" + d = yaml.safe_load(open(f"{WF}/{workflow}")) + return {s.get("name"): s for s in d["jobs"][job]["steps"] if s.get("run")} + +steps = steps_of("publish-gh-pages.yml", "deploy") + +def run(step_name, env, exprs, *, from_steps=None): + step = (from_steps or steps)[step_name] + sub = lambda text: EXPR.sub(lambda m: exprs.get(m.group(1), ""), text) # step-level env: from the step's own `env:` block, after expression substitution senv = dict(env) - for k, v in (steps[step_name].get("env") or {}).items(): - senv[k] = re.sub(r"\$\{\{\s*([^}]*?)\s*\}\}", lambda m: exprs.get(m.group(1), ""), str(v)) - p = subprocess.run(["bash", "-c", script], env=senv, capture_output=True, text=True) + for k, v in (step.get("env") or {}).items(): + senv[k] = sub(str(v)) + p = subprocess.run(["bash", "-c", sub(step["run"])], env=senv, capture_output=True, text=True) return p def setup(tmp, releases, artifacts, prs, *, approved=(), dlfail=(), artfail=(), artnodocs=()): @@ -56,10 +61,7 @@ def setup(tmp, releases, artifacts, prs, *, approved=(), dlfail=(), artfail=(), "GITHUB_OUTPUT": os.path.join(tmp, "out.txt"), "GITHUB_SERVER_URL": "https://github.com", "REPO": "acme/widget", "DEFAULT": "main", - "SEED_TAG": "pages-default-seed", "SOURCES_DIR": "_sources", - # Real defaults are 1.5 GiB / 700 MB; a test fixture that size is impractical, - # so the guard reads them from env and the size tests dial them down. - "SIZE_PROBE_BYTES": "1610612736", "SIZE_WARN_BYTES": "734003200", + "SEED_TAG": "pages-default-seed", }) open(env["GITHUB_OUTPUT"], "w").close() open(env["MOCK_CALLS"], "w").close() diff --git a/test/workflow-harness/test_gather.py b/test/workflow-harness/test_gather.py index 45ddc82..7274745 100644 --- a/test/workflow-harness/test_gather.py +++ b/test/workflow-harness/test_gather.py @@ -1,10 +1,10 @@ """Behaviour tests for publish-gh-pages.yml's gather steps. See run.py for how they load.""" -import os, sys, tempfile +import os, re, sys, tempfile sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) -from fixtures import ARTIFACTS, PRS, RELEASES -from run import run, setup +from fixtures import ARTIFACTS, PRS, RELEASES, art, rel +from run import run, setup, steps_of fails = [] @@ -15,16 +15,23 @@ def check(name, cond, detail=""): fails.append(name) -def do(tmp, maxrel="0", **kw): +def do(tmp, maxrel="0", maxprs="0", **kw): """Run the three gather steps in order against one synthetic repo.""" env, rt, data = setup(tmp, RELEASES, ARTIFACTS, PRS, **kw) - exprs = {"github.token": "t", "inputs.max-releases": maxrel, "runner.temp": rt} + exprs = {"github.token": "t", "inputs.max-releases": maxrel, + "inputs.max-prs": maxprs, "runner.temp": rt} r1 = run("Select releases to publish", env, exprs) assert r1.returncode == 0, r1.stderr r2 = run("Gather release artifacts", env, exprs) r3 = run("Gather branch CI artifacts", env, exprs) return env, rt, data, r1, r2, r3 + +def log(r): + """Everything the step wrote to the workflow log: assemble.mjs decisions land on + stderr, the download warnings on stdout.""" + return r.stdout + r.stderr + print("\n-- unlimited: all valid releases gathered, junk skipped --") with tempfile.TemporaryDirectory() as tmp: env, rt, data, r1, r2, r3 = do(tmp) @@ -44,9 +51,40 @@ def do(tmp, maxrel="0", **kw): check("artifacts API paginated exactly ONCE (was once per PR)", calls.count("actions/artifacts?name=docs") == 1, f"count={calls.count('actions/artifacts?name=docs')}") - check("newest artifact per SHA wins", "artifacts/9001/zip" in calls and "artifacts/9000/zip" not in calls) + check("newest artifact per SHA wins", + "artifacts/9001/zip" in calls and "artifacts/9000/zip" not in calls, calls) check("fork-owned same-named branch excluded", "artifacts/9002/zip" not in calls) +print("\n-- the cache key comes from the selection, via GITHUB_OUTPUT --") +# The restore step reads BOTH of these: `key` for an exact hit, `restore-keys` for the +# prefix fallback to the previous selection. A missing/misspelt output silently degrades +# the cache to a permanent miss, which nothing else here would notice. +def cache_outputs(tmp, maxrel): + env, rt, data = setup(tmp, RELEASES, [], []) + ex = {"github.token": "t", "inputs.max-releases": maxrel, "runner.temp": rt} + r = run("Select releases to publish", env, ex) + assert r.returncode == 0, r.stderr + return dict(l.split("=", 1) for l in open(env["GITHUB_OUTPUT"]).read().splitlines() if l) + +with tempfile.TemporaryDirectory() as tmp: + out = cache_outputs(tmp, "0") + check("the step emits a cache-key", + bool(re.fullmatch(r"mvs-relzips-v1-[0-9a-f]{32}", out.get("cache-key", ""))), + out) + check("the step emits the bare namespace for restore-keys", + out.get("cache-prefix") == "mvs-relzips-v1-", out) + check("the key sits under the prefix", + out.get("cache-key", "").startswith(out.get("cache-prefix", "\0")), out) + +with tempfile.TemporaryDirectory() as tmp: + capped = cache_outputs(tmp, "2") + with tempfile.TemporaryDirectory() as tmp2: + uncapped = cache_outputs(tmp2, "0") + check("capping changes the key but not the namespace", + capped["cache-key"] != uncapped["cache-key"] + and capped["cache-prefix"] == uncapped["cache-prefix"], + f'{capped} vs {uncapped}') + print("\n-- cache hit: second run downloads nothing --") with tempfile.TemporaryDirectory() as tmp: env, rt, data, *_ = do(tmp) @@ -57,7 +95,7 @@ def do(tmp, maxrel="0", **kw): check("second run exits 0", r.returncode == 0, r.stderr) check("no release downloaded on a full cache hit", "release download" not in open(env["MOCK_CALLS"]).read()) - check("log reports all cached", "4 cached, 0 to download" in r.stdout, r.stdout[:400]) + check("log reports all cached", "4 cached, 0 downloaded" in r.stdout, r.stdout[:400]) print("\n-- cap: only the newest N, seed exempt, cache pruned --") with tempfile.TemporaryDirectory() as tmp: @@ -69,6 +107,39 @@ def do(tmp, maxrel="0", **kw): sorted(os.listdir(f"{rt}/relcache")) == ["1.zip", "201.zip", "301.zip"], os.listdir(f"{rt}/relcache")) check("cap decision is explained in the log", "beyond max-releases=2" in r2.stdout, r2.stdout[:300]) +print("\n-- max-prs: newest builds win, an ineligible PR costs no slot --") +# A dedicated fixture: the shared one has only one eligible PR, so no cap could bite. +CAP_PRS = [(20, "sha20", False), (21, "sha21", False), (22, "sha22", False), + (23, "sha23", False), (24, "sha24", True)] +CAP_ARTS = [art(1020, "sha20", "2026-05-01T00:00:00Z"), + art(1021, "sha21", "2026-05-03T00:00:00Z"), + art(1022, "sha22", "2026-05-02T00:00:00Z"), + # 23 has no artifact at all; 24 is an unapproved fork. + art(1024, "sha24", "2026-05-04T00:00:00Z")] + +def cap_prs(tmp, maxprs): + env, rt, data = setup(tmp, [], CAP_ARTS, CAP_PRS) + ex = {"github.token": "t", "inputs.max-prs": maxprs, "runner.temp": rt} + return run("Gather branch CI artifacts", env, ex), rt + +with tempfile.TemporaryDirectory() as tmp: + r, rt = cap_prs(tmp, "2") + got = sorted(os.listdir(f"{rt}/gather")) + check("cap keeps the two most recently built PRs", got == ["pr-21.zip", "pr-22.zip"], got) + check("the capped PR is explained", "PR #20 → skip: beyond max-prs=2" in log(r), log(r)) + check("a PR with no artifact does not consume a slot", + "PR #23 → skip: no non-expired" in log(r), log(r)) + check("an unapproved fork PR does not consume a slot", + "PR #24 → skip: fork PR" in log(r) and "pr-24.zip" not in got, log(r)) + check("the cap is reported alongside the PR count", "(max-prs=2)" in log(r), log(r)) + +with tempfile.TemporaryDirectory() as tmp: + r, rt = cap_prs(tmp, "0") + got = sorted(os.listdir(f"{rt}/gather")) + check("max-prs 0 is unlimited", + got == ["pr-20.zip", "pr-21.zip", "pr-22.zip"], got) + check("no cap is announced when unlimited", "max-prs=" not in log(r), log(r)) + print("\n-- failures are isolated, not fatal --") with tempfile.TemporaryDirectory() as tmp: env, rt, data, r1, r2, r3 = do(tmp, dlfail=["2.0"], artfail=[9100], artnodocs=[9001]) @@ -79,8 +150,10 @@ def do(tmp, maxrel="0", **kw): check("the failed release is absent", "2.0.zip" not in got, got) check("failed download is warned", "::warning::release 2.0" in r2.stdout, r2.stdout[-400:]) check("failed release is NOT left in the cache", "201.zip" not in os.listdir(f"{rt}/relcache")) - check("artifact download failure warned", "artifact 9100 download failed" in r3.stdout, r3.stdout[-500:]) - check("artifact without docs.zip warned distinctly", "contains no docs.zip" in r3.stdout, r3.stdout[-500:]) + check("artifact download failure warned", "artifact 9100 download failed" in log(r3), log(r3)[-500:]) + check("artifact without docs.zip warned distinctly", "has no docs.zip inside" in log(r3), log(r3)[-500:]) + check("that warning names both causes, not just the historical one", + "re-run that build" in log(r3) and "also named docs" in log(r3), log(r3)[-600:]) check("main falls back to the seed release when its artifact is bad", "main.zip" in got, got) print("\n-- empty repo: no releases, no artifacts, no PRs --") @@ -90,6 +163,8 @@ def do(tmp, maxrel="0", **kw): a = run("Select releases to publish", env, exprs) b = run("Gather release artifacts", env, exprs) c = run("Gather branch CI artifacts", env, exprs) + # Also the guard on xargs -r: without it, an empty selection runs the worker once + # with no args, `set -u` trips on $1, and pipefail fails the whole deploy. check("all three steps survive an empty repo", a.returncode == 0 and b.returncode == 0 and c.returncode == 0, a.stderr + b.stderr + c.stderr) @@ -98,11 +173,10 @@ def do(tmp, maxrel="0", **kw): print("\n-- default-branch fallback chain --") import subprocess -def ensure(tmp, *, gathered=None, cached=None, in_site=None): +def ensure(tmp, *, gathered=None, cached=None): """Run Ensure-default-branch-zip with a chosen rung populated.""" env, rt, data = setup(tmp, [], [], []) ex = {"github.token": "t", "runner.temp": rt} - env["PAGES_URL"] = in_site or "http://127.0.0.1:1" # unroutable => the curl rung fails for path, marker in ((f"{rt}/gather/main.zip", gathered), (f"{rt}/defaultcache/default.zip", cached)): if marker: @@ -132,7 +206,6 @@ def ensure(tmp, *, gathered=None, cached=None, in_site=None): # Same bytes already cached => no new immutable cache entry requested. env, rt, data = setup(tmp, [], [], []) ex = {"github.token": "t", "runner.temp": rt} - env["PAGES_URL"] = "http://127.0.0.1:1" os.makedirs(f"{rt}/defaultcache", exist_ok=True) subprocess.run([f"{data}/mkzip.sh", f"{rt}/gather/main.zip", "same"], check=True) subprocess.run([f"{data}/mkzip.sh", f"{rt}/defaultcache/default.zip", "same"], check=True) @@ -144,7 +217,78 @@ def ensure(tmp, *, gathered=None, cached=None, in_site=None): r.returncode == 0 and "cache-key=" not in open(env["GITHUB_OUTPUT"]).read(), open(env["GITHUB_OUTPUT"]).read()) -print("\n-- extract: zip shape contract and the size guard --") +print("\n-- the parallel download passes rows as data, not as script text --") +# The worker used to be `xargs -I{}`, which pastes each row into the script TEXT. Git +# permits $ and () in a branch name, so a branch could smuggle a command substitution +# into the label. Positional args make it inert; assert it stays that way. +with tempfile.TemporaryDirectory() as tmp: + hostile = "br$(id -u)x" + env, rt, data = setup(tmp, [], [art(5000, "sha5000", "2026-05-01T00:00:00Z", branch=hostile)], + [], artfail=[5000]) + env["DEFAULT"] = hostile + r = run("Gather branch CI artifacts", env, {"github.token": "t", "runner.temp": rt}) + check("the branch name reaches the warning verbatim", hostile in log(r), log(r)[-400:]) + check("the command substitution in it is never executed", + f"br{os.getuid()}x" not in log(r), log(r)[-400:]) + +# The worker's argument wiring: `bash -c` takes the arg AFTER the script as $0, so if +# that placeholder is ever dropped, dest lands in $0 and every field shifts by one. A +# space-bearing label is the case that shows it — the row still parses, it just lands in +# the wrong place. +with tempfile.TemporaryDirectory() as tmp: + env, rt, data = setup(tmp, [], [art(6000, "sha6000", "2026-05-01T00:00:00Z", branch="main")], []) + r = run("Gather branch CI artifacts", env, {"github.token": "t", "runner.temp": rt}) + check("a row whose label contains spaces still stages at its own dest", + os.listdir(f"{rt}/gather") == ["main.zip"], + f'{os.listdir(f"{rt}/gather")} — label was {"default-branch CI (main)"!r}') + +print("\n-- the version-name contract: both sides derive the same string --") +# docs.yml builds at BASE_URL=/repo/ and the gather independently stages +# each source at site/. Nothing is passed between them, so the ONLY thing +# keeping a build's assets from 404ing is that the two rules agree. Pin them together. +DOCS = steps_of("docs.yml", "build") + +def docs_yml_version_name(tmp, event, *, ref_name="", pr_number=""): + """Run docs.yml's own 'Compute version name' step and read its output.""" + out = os.path.join(tmp, "ver.txt") + open(out, "w").close() + env = dict(os.environ, GITHUB_OUTPUT=out) + exprs = {"github.event_name": event, "github.ref_name": ref_name, + "github.event.pull_request.number": pr_number} + r = run("Compute version name", env, exprs, from_steps=DOCS) + assert r.returncode == 0, r.stderr + return open(out).read().strip().split("=", 1)[1] + +with tempfile.TemporaryDirectory() as tmp: + # A tag: docs.yml uses the ref name; the gather uses the release's tag. + built = docs_yml_version_name(tmp, "push", ref_name="1.2.3") + env, rt, data = setup(tmp, [rel("1.2.3", "2026-01-01T00:00:00Z", 555)], [], []) + ex = {"github.token": "t", "inputs.max-releases": "0", "runner.temp": rt} + assert run("Select releases to publish", env, ex).returncode == 0 + dest = open(f"{rt}/releases.tsv").read().split("\t")[1] + check(f"a tag builds and is staged at the same name ({built!r})", built == dest, + f"docs.yml says {built!r}, the gather stages at {dest!r}") + +with tempfile.TemporaryDirectory() as tmp: + # A PR: docs.yml uses pr-; the gather uses the PR number from `gh pr list`. + built = docs_yml_version_name(tmp, "pull_request", pr_number="7") + env, rt, data = setup(tmp, [], [art(7001, "pr7sha", "2026-01-01T00:00:00Z")], + [(7, "pr7sha", False)]) + r = run("Gather branch CI artifacts", env, {"github.token": "t", "runner.temp": rt}) + staged = [f[:-4] for f in os.listdir(f"{rt}/gather")] + check(f"a PR builds and is staged at the same name ({built!r})", staged == [built], + f"docs.yml says {built!r}, the gather staged {staged}") + +with tempfile.TemporaryDirectory() as tmp: + # The default branch: docs.yml uses the ref name, the gather uses default_branch. + built = docs_yml_version_name(tmp, "push", ref_name="main") + env, rt, data = setup(tmp, [], [art(8001, "mainsha", "2026-01-01T00:00:00Z", branch="main")], []) + r = run("Gather branch CI artifacts", env, {"github.token": "t", "runner.temp": rt}) + staged = [f[:-4] for f in os.listdir(f"{rt}/gather")] + check(f"the default branch builds and is staged at the same name ({built!r})", + staged == [built], f"docs.yml says {built!r}, the gather staged {staged}") + +print("\n-- extract: the zip-shape contract --") with tempfile.TemporaryDirectory() as tmp: env, rt, data = setup(tmp, [], [], []) ex = {"github.token": "t", "runner.temp": rt} @@ -158,42 +302,9 @@ def ensure(tmp, *, gathered=None, cached=None, in_site=None): got = sorted(os.listdir(f"{rt}/site")) check("extract exits 0 despite malformed zips", r.returncode == 0, r.stderr) check("only well-formed zips become version dirs", got == ["1.0", "main"], got) - check("_sources is no longer published into the site", "_sources" not in got, got) check("an unreadable zip warns", "could not unzip" in r.stdout, r.stdout) check("a two-root zip warns", "exactly one top-level directory" in r.stdout, r.stdout) - check("the site size is reported", "assembled site:" in r.stdout, r.stdout) - check("a small site does not trip the 1 GB warning", - "approaching the Pages 1 GB limit" not in r.stdout) - check("a small site is not packed just to measure it", - "packed artifact:" not in r.stdout, r.stdout) - -# The guard's whole point is the warning, and it compares the PACKED size — measuring -# the tree with du would cry wolf, since HTML/JS packs ~3x (this repo's own site is -# 734 MiB on disk and 223 MiB deployed). Dial the thresholds down rather than build a -# multi-gigabyte fixture. -with tempfile.TemporaryDirectory() as tmp: - env, rt, data = setup(tmp, [], [], []) - ex = {"github.token": "t", "runner.temp": rt} - subprocess.run([f"{data}/mkzip.sh", f"{rt}/gather/main.zip", "main"], check=True) - env["SIZE_PROBE_BYTES"] = "1" # always pack - env["SIZE_WARN_BYTES"] = "1" # always warn - r = run("Extract artifacts into site", env, ex) - check("past the probe threshold the tree is actually packed", - "packed artifact:" in r.stdout, r.stdout) - check("past the warn threshold a warning is emitted", - "::warning title=Docs site approaching the Pages 1 GB limit" in r.stdout, r.stdout) - check("the warning names max-releases as the lever", "max-releases" in r.stdout, r.stdout) - -with tempfile.TemporaryDirectory() as tmp: - env, rt, data = setup(tmp, [], [], []) - ex = {"github.token": "t", "runner.temp": rt} - subprocess.run([f"{data}/mkzip.sh", f"{rt}/gather/main.zip", "main"], check=True) - env["SIZE_PROBE_BYTES"] = "1" # pack it... - env["SIZE_WARN_BYTES"] = str(10**12) # ...but it is nowhere near the cap - r = run("Extract artifacts into site", env, ex) - check("a packed site under the cap reports but does not warn", - "packed artifact:" in r.stdout and "approaching the Pages 1 GB limit" not in r.stdout, - r.stdout) + check("the assembled size is reported", "assembled site:" in r.stdout, r.stdout) print(f"\n{'FAILURES: ' + ', '.join(fails) if fails else 'all harness checks passed'}") sys.exit(1 if fails else 0) diff --git a/test/workflow-harness/test_shape.py b/test/workflow-harness/test_shape.py index 357c2f1..0912932 100644 --- a/test/workflow-harness/test_shape.py +++ b/test/workflow-harness/test_shape.py @@ -2,9 +2,8 @@ Structural invariants of the publish workflows. The gather bash is covered by test_gather.py. What that harness cannot reach are the -`if:` expressions, which are evaluated by GitHub, not by a shell — and those are where -the workflow_run migration is easiest to get quietly wrong. Each assertion here stands -for a specific way it has broken or could break. +`if:` expressions and the wiring between the three files, which GitHub evaluates rather +than a shell. Each assertion here stands for a way that wiring can break silently. """ import os @@ -38,24 +37,21 @@ def load(name): print("\n-- engine: publish-gh-pages.yml --") check("is workflow_call only", list(engine_on) == ["workflow_call"], list(engine_on)) -check("exposes only pr + max-releases", - sorted(engine_on["workflow_call"]["inputs"]) == ["max-releases", "pr"], +check("exposes exactly pr + max-releases + max-prs", + sorted(engine_on["workflow_call"]["inputs"]) == ["max-prs", "max-releases", "pr"], sorted(engine_on["workflow_call"]["inputs"])) -check("has a single deploy job (no re-dispatch, no warn)", - list(engine["jobs"]) == ["deploy"], list(engine["jobs"])) +check("is a single deploy job", list(engine["jobs"]) == ["deploy"], list(engine["jobs"])) steps = engine["jobs"]["deploy"]["steps"] names = [s.get("name", s.get("uses", "")) for s in steps] -check("no in-run artifact injection remains", - not any("this run's docs artifact" in n or "Stage current build" == n for n in names), names) -check("no wedged-origin retry remains", not any("Retry via" in n for n in names), names) -check("still verifies the deployed origin", any("Verify" in n for n in names), names) - -# THE trap this migration introduces. Under workflow_run, github.ref is ALWAYS the -# default branch — even when a PR's CI triggered the run — so gating a cache SAVE on -# github.ref would have every PR-triggered deploy writing caches that nothing can read. +check("verifies the deployed origin", any("Verify" in n for n in names), names) + +# THE workflow_run trap. github.ref is ALWAYS the default branch — even when a PR's CI +# triggered the run — so a save gated on it would fire for every deploy, including one +# dispatched from another ref, whose entry only that ref can read. head_branch is the +# only honest answer to "was this the default branch?". saves = [s for s in steps if "cache/save" in str(s.get("uses", ""))] -check("there are cache-save steps to check", len(saves) == 2, f"found {len(saves)}") +check("both caches have a save step", len(saves) == 2, f"found {len(saves)}") for s in saves: cond = str(s.get("if", "")) check(f"{s['name']!r} does not gate on github.ref", @@ -66,8 +62,9 @@ def load(name): check("deploy cancels superseded runs", engine["jobs"]["deploy"]["concurrency"].get("cancel-in-progress") is True, engine["jobs"]["deploy"]["concurrency"]) -check("deploy no longer needs actions:write", - engine["jobs"]["deploy"]["permissions"]["actions"] == "read", +check("deploy holds no write token beyond what it deploys with", + engine["jobs"]["deploy"]["permissions"]["actions"] == "read" + and engine["jobs"]["deploy"]["permissions"]["contents"] == "read", engine["jobs"]["deploy"]["permissions"]) print("\n-- caller: publish.yml --") @@ -78,18 +75,23 @@ def load(name): f'{caller_on["workflow_run"]["workflows"]} vs ci name {ci["name"]!r}') guard = str(caller["jobs"]["publish"]["if"]) -# workflow_run hands a WRITE token to a job triggered by fork-PR code. Losing either -# half of this guard is the pwn-request footgun the old read-only warn job existed to -# avoid, and it would fail open rather than loudly. +# workflow_run hands a WRITE token to a job triggered by fork-PR code. Losing either half +# of this guard is a pwn-request, and it fails open rather than loudly. check("requires the triggering run to have succeeded", "conclusion == 'success'" in guard, guard) check("excludes forks", "head_repository.full_name == github.repository" in guard, guard) -check("still allows a manual dispatch", - "workflow_dispatch" in guard, guard) +check("still allows a manual dispatch", "workflow_dispatch" in guard, guard) + +# Site-size policy has to hold on EVERY path into the engine, and this workflow is its +# only caller — an expression here could be emptied by a manual dispatch. +with_block = caller["jobs"]["publish"]["with"] +for cap in ("max-releases", "max-prs"): + val = str(with_block.get(cap, "")) + check(f"{cap} is set as a literal", val.isdigit(), repr(val)) print("\n-- entry: ci.yml --") -check("no longer publishes", "publish" not in ci["jobs"], list(ci["jobs"])) -check("still uploads the docs artifact CI's consumers gather", "docs" in ci["jobs"], list(ci["jobs"])) +check("builds without publishing", "publish" not in ci["jobs"], list(ci["jobs"])) +check("uploads the docs artifact the engine gathers", "docs" in ci["jobs"], list(ci["jobs"])) print("\n" + ("FAILURES: " + ", ".join(fails) if fails else "all shape checks passed")) sys.exit(1 if fails else 0) From 8bf59014c82e541bfdf38ba0dd431f663a307bf3 Mon Sep 17 00:00:00 2001 From: Tom Cobb Date: Mon, 24 Aug 2026 12:53:56 +0000 Subject: [PATCH 2/2] feat: link the published docs from the commit that triggered the deploy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On success the engine posts a `docs-preview` commit status on the triggering commit, with the published URL as its target — so a PR carries a link to its own preview, and a branch or tag build links the version it produced. Restores the one thing the workflow_run migration cost that was worth having, without the thing it was fixing. Two properties, both asserted in test_shape.py: - `if: success()` and `state=success` only. Publishing lives off the PR's critical path because a wedged Pages origin is not the author's to fix; a status that could turn red would hand that straight back. A missing status means "not published (yet)". - Fails CLOSED. The dest is resolved from the workflow_run payload (or from `inputs.pr` for a dispatched fork preview) and posted only if that version dir is actually in the tree just deployed — no link is better than one that 404s. The Approve step now also outputs the head SHA it pinned, so a fork preview links back to the same commit it was approved against, and takes `inputs.pr` through `env:` rather than interpolating it into the script — the rule the rest of this file already follows. Also adds BUILT_BRANCH to the harness env, which mirrors the real job. Co-Authored-By: Claude Opus 5 --- .github/workflows/publish-gh-pages.yml | 50 +++++++++++++++++++++++- CLAUDE.md | 9 +++++ docs/reference/workflows.md | 14 +++++++ test/workflow-harness/run.py | 3 ++ test/workflow-harness/test_gather.py | 53 ++++++++++++++++++++++++++ test/workflow-harness/test_shape.py | 12 ++++++ 6 files changed, 140 insertions(+), 1 deletion(-) diff --git a/.github/workflows/publish-gh-pages.yml b/.github/workflows/publish-gh-pages.yml index edca5d2..de84248 100644 --- a/.github/workflows/publish-gh-pages.yml +++ b/.github/workflows/publish-gh-pages.yml @@ -104,15 +104,19 @@ jobs: # drops the preview until re-approved. Gated on `pr` alone, which the caller # threads in from its workflow_dispatch input. - name: Approve fork PR head SHA + id: approve if: inputs.pr != '' env: GH_TOKEN: ${{ github.token }} + PR: ${{ inputs.pr }} run: | set -euo pipefail - sha=$(gh pr view "${{ inputs.pr }}" --repo "$REPO" --json headRefOid -q .headRefOid) + sha=$(gh pr view "$PR" --repo "$REPO" --json headRefOid -q .headRefOid) gh api --method POST "repos/$REPO/statuses/$sha" \ -f state=success -f context=preview-approved \ -f description="Fork docs preview approved" + # Reused below to link the published preview back to this same commit. + echo "sha=$sha" >> "$GITHUB_OUTPUT" # BUILT_BRANCH vs github.ref is the thing a post-mortem here always trips over, so # state both up front rather than leaving someone to infer it from a cache miss. @@ -504,3 +508,47 @@ jobs: fi sleep 5 done + + # Link the published docs from the commit that produced them, so a PR carries a + # preview URL without carrying the deploy's failure modes. + # + # Deliberately success()-only and state=success only. Publishing moved off the PR's + # critical path precisely because a wedged Pages origin is not the author's to fix; + # a status that could go red would put that right back. A missing status means "not + # published (yet)", which is the honest signal and costs the author nothing. + # + # Fails closed: if the version dir is not in the tree we just deployed — a fork PR + # whose number we cannot resolve, a payload without `pull_requests` — say so and + # post nothing, rather than publish a link that 404s. + - name: Link the published docs from the triggering commit + if: success() + env: + GH_TOKEN: ${{ github.token }} + PR_INPUT: ${{ inputs.pr }} + APPROVED_SHA: ${{ steps.approve.outputs.sha }} + TRIGGER_EVENT: ${{ github.event.workflow_run.event }} + TRIGGER_SHA: ${{ github.event.workflow_run.head_sha }} + TRIGGER_PR: ${{ github.event.workflow_run.pull_requests[0].number }} + PAGE_URL: ${{ steps.deployment.outputs.page_url }} + run: | + set -euo pipefail + if [ -n "${APPROVED_SHA:-}" ]; then + # Fork preview: the maintainer named the PR, and we pinned its head SHA. + sha="$APPROVED_SHA"; dest="pr-$PR_INPUT" + elif [ "${TRIGGER_EVENT:-}" = pull_request ] && [ -n "${TRIGGER_PR:-}" ]; then + sha="$TRIGGER_SHA"; dest="pr-$TRIGGER_PR" + elif [ -n "${TRIGGER_SHA:-}" ]; then + # A branch or tag build: BUILT_BRANCH is the version name it was served at. + sha="$TRIGGER_SHA"; dest="$BUILT_BRANCH" + else + echo "manual re-deploy — no triggering commit to link"; exit 0 + fi + if [ ! -d "$RUNNER_TEMP/site/$dest" ]; then + echo "::notice::no $dest/ in the deployed site — not linking ${sha:0:8}" + exit 0 + fi + url="${PAGE_URL%/}/$dest/" + gh api --method POST "repos/$REPO/statuses/$sha" \ + -f state=success -f context=docs-preview \ + -f target_url="$url" -f description="Docs published at $dest/" + echo "linked ${sha:0:8} → $url" diff --git a/CLAUDE.md b/CLAUDE.md index e06dd49..3d94d1a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -181,6 +181,15 @@ the repo's `default_branch`). Nothing is threaded between them — `docs.yml` ha `version-name` output any more (it existed only for the deleted injection step). The two rules must change together. +### The PR gets a link, never a red check +A successful deploy posts a `docs-preview` commit status on +`workflow_run.head_sha` (or, for a dispatched fork preview, the SHA the Approve step +pinned) with the published URL as `target_url`. `if: success()` plus `state=success` only: +the whole reason publishing moved off the PR's critical path is that a wedged Pages origin +is not the author's to fix, so a status that could go red would undo it. The step also +fails CLOSED — no `site/` dir, no status — rather than posting a 404 link. +`test_shape.py` asserts both properties. + ### THE workflow_run TRAP: `github.ref` is always the default branch In a `workflow_run` run `GITHUB_SHA` and `github.ref` are **always the default branch's HEAD**, never the built commit (a PR-triggered deploy reports `refs/heads/main`). Anything diff --git a/docs/reference/workflows.md b/docs/reference/workflows.md index 0da5376..07cc29f 100644 --- a/docs/reference/workflows.md +++ b/docs/reference/workflows.md @@ -70,6 +70,20 @@ Both caps exist because the site deploys as **one** Pages artifact against a har in `publish.yml`'s `with:` block so they apply on every path. See [keep-the-site-small](../how-to/keep-the-site-small.md). +## What it reports back + +On a successful deploy the engine posts a `docs-preview` commit status on the commit that +triggered it, with the published URL as its target — so a PR carries a link to its own +preview, and a default-branch or tag build links the version it produced. + +It is posted **only on success, and only ever as a success**. Publishing runs off the PR's +critical path because a wedged Pages origin is not the author's to fix; a status that could +turn red would hand that straight back to them. A missing status means "not published +(yet)". If the version directory is not in the deployed tree — a fork PR whose number +cannot be resolved, say — nothing is posted rather than a link that 404s. + +Requires `statuses: write`, which the caller already grants for fork-preview approval. + ## What the engine gathers Every deploy rebuilds the complete tree from authoritative inputs: diff --git a/test/workflow-harness/run.py b/test/workflow-harness/run.py index 8df4c8d..587701d 100644 --- a/test/workflow-harness/run.py +++ b/test/workflow-harness/run.py @@ -61,6 +61,9 @@ def setup(tmp, releases, artifacts, prs, *, approved=(), dlfail=(), artfail=(), "GITHUB_OUTPUT": os.path.join(tmp, "out.txt"), "GITHUB_SERVER_URL": "https://github.com", "REPO": "acme/widget", "DEFAULT": "main", + # Job-level env in the real workflow: the branch that was actually BUILT, which + # under workflow_run is NOT github.ref. Tests override it to exercise the others. + "BUILT_BRANCH": "main", "SEED_TAG": "pages-default-seed", }) open(env["GITHUB_OUTPUT"], "w").close() diff --git a/test/workflow-harness/test_gather.py b/test/workflow-harness/test_gather.py index 7274745..cf506a5 100644 --- a/test/workflow-harness/test_gather.py +++ b/test/workflow-harness/test_gather.py @@ -306,5 +306,58 @@ def docs_yml_version_name(tmp, event, *, ref_name="", pr_number=""): check("a two-root zip warns", "exactly one top-level directory" in r.stdout, r.stdout) check("the assembled size is reported", "assembled site:" in r.stdout, r.stdout) +print("\n-- linking the published docs back to the triggering commit --") +def link(tmp, *, dirs=(), built_branch="main", **exprs): + env, rt, data = setup(tmp, [], [], []) + env["BUILT_BRANCH"] = built_branch + for d in dirs: + os.makedirs(f"{rt}/site/{d}", exist_ok=True) + ex = {"github.token": "t", "runner.temp": rt, + "steps.deployment.outputs.page_url": "https://acme.github.io/widget/"} + ex.update(exprs) + r = run("Link the published docs from the triggering commit", env, ex) + return r, open(env["MOCK_CALLS"]).read() + +with tempfile.TemporaryDirectory() as tmp: + r, calls = link(tmp, dirs=["pr-7"], + **{"github.event.workflow_run.event": "pull_request", + "github.event.workflow_run.head_sha": "deadbeef", + "github.event.workflow_run.pull_requests[0].number": "7"}) + check("a PR build is linked at its own version dir", + "statuses/deadbeef" in calls and "https://acme.github.io/widget/pr-7/" in calls, + calls) + check("the status is a success, never a failure", + "state=success" in calls and "state=failure" not in calls, calls) + +with tempfile.TemporaryDirectory() as tmp: + # A payload with no pull_requests falls through to the branch name, which is not a + # version dir — post nothing rather than a link that 404s. + r, calls = link(tmp, dirs=["pr-7"], built_branch="feature/x", + **{"github.event.workflow_run.event": "pull_request", + "github.event.workflow_run.head_sha": "deadbeef"}) + check("an unresolvable PR posts no status at all", + r.returncode == 0 and "statuses/" not in calls, calls + log(r)) + check("and says why", "not linking" in log(r), log(r)) + +with tempfile.TemporaryDirectory() as tmp: + r, calls = link(tmp, dirs=["main"], + **{"github.event.workflow_run.event": "push", + "github.event.workflow_run.head_sha": "cafe1234"}) + check("a default-branch build is linked too", + "statuses/cafe1234" in calls and "widget/main/" in calls, calls) + +with tempfile.TemporaryDirectory() as tmp: + # A manual re-deploy has no triggering commit to hang a status on. + r, calls = link(tmp, dirs=["main"]) + check("a manual re-deploy posts nothing and still succeeds", + r.returncode == 0 and "statuses/" not in calls, calls + log(r)) + +with tempfile.TemporaryDirectory() as tmp: + # Fork preview: the maintainer named the PR, so inputs.pr and the pinned SHA win. + r, calls = link(tmp, dirs=["pr-42"], + **{"inputs.pr": "42", "steps.approve.outputs.sha": "f0rk5haa"}) + check("a dispatched fork preview is linked at the approved SHA", + "statuses/f0rk5haa" in calls and "widget/pr-42/" in calls, calls) + print(f"\n{'FAILURES: ' + ', '.join(fails) if fails else 'all harness checks passed'}") sys.exit(1 if fails else 0) diff --git a/test/workflow-harness/test_shape.py b/test/workflow-harness/test_shape.py index 0912932..6ff3a41 100644 --- a/test/workflow-harness/test_shape.py +++ b/test/workflow-harness/test_shape.py @@ -59,6 +59,18 @@ def load(name): check(f"{s['name']!r} gates on the branch that was built", "workflow_run.head_branch" in cond, cond) +# The PR-visible status must never be able to go red: publishing lives off the PR's +# critical path because a wedged Pages origin is not the author's to fix, and a status +# that could fail would hand that back to them. +link = next((s for s in steps if s.get("name", "").startswith("Link the published")), None) +check("there is a step linking the docs back to the triggering commit", link is not None) +if link: + check("it only runs when the deploy succeeded", "success()" in str(link.get("if", "")), + link.get("if")) + check("it can only ever post a success state", + "state=success" in link["run"] and "state=failure" not in link["run"] + and "state=error" not in link["run"]) + check("deploy cancels superseded runs", engine["jobs"]["deploy"]["concurrency"].get("cancel-in-progress") is True, engine["jobs"]["deploy"]["concurrency"])