Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .github/workflows/docs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,15 @@ jobs:
set -euo pipefail
if [ "$EVENT" = pull_request ]; then
name="pr-$PR_NUMBER"
elif [ "$EVENT" = merge_group ]; then
# merge_group's ref_name is GitHub's internal
# gh-readonly-queue/<base-branch>/pr-<n>-<sha> — it contains '/' (and, via
# the base branch, whatever else a branch name allows) so it fails the check
# below outright. This build is never gathered: the site-side gather only
# reads the default branch's HEAD artifact, open PRs' artifacts and release
# zips, never a merge queue's temporary branch, so the name just needs to be
# a valid path segment, not a parse of GitHub's ref.
name="merge-queue"
else
name="$REF_NAME"
fi
Expand Down Expand Up @@ -164,7 +173,14 @@ jobs:
( cd "$staging" && zip -rq "$RUNNER_TEMP/docs.zip" html )

# compression-level 0: docs.zip is already compressed.
#
# Skipped on merge_group: that build's version name is the fixed literal
# "merge-queue" (see the step above), and the site-side gather never looks for
# it — only the default branch's HEAD artifact, open PRs' artifacts and release
# zips are gathered. Uploading it anyway would be pure storage churn against the
# repo's Actions quota, once per queue entry.
- name: Upload docs artifact
if: github.event_name != 'merge_group'
uses: actions/upload-artifact@v7
with:
name: docs
Expand Down
15 changes: 11 additions & 4 deletions .github/workflows/publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ on:

jobs:
publish:
# Two guards, both load-bearing:
# Three guards, all load-bearing:
#
# conclusion == 'success' — a failed CI run has no usable docs artifact, and
# workflow_run fires on failures too.
Expand All @@ -49,12 +49,19 @@ jobs:
# dispatches this workflow with `pr`, which records an approval against that exact
# 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. Removing either guard fails OPEN.
# event != 'merge_group' — a merge-queue CI run's docs.yml build is never uploaded
# (its version name is the fixed "merge-queue" literal, and the gather never looks
# for it), so gathering here would only reassemble and redeploy a site that changes
# nothing — once per queue entry, on top of the deploy the subsequent push to the
# base branch already triggers.
#
# A manual dispatch has no `workflow_run` payload, so all three terms are empty and the
# first clause carries it. Removing any of the three guards fails OPEN.
if: >-
github.event_name == 'workflow_dispatch' ||
(github.event.workflow_run.conclusion == 'success' &&
github.event.workflow_run.head_repository.full_name == github.repository)
github.event.workflow_run.head_repository.full_name == github.repository &&
github.event.workflow_run.event != 'merge_group')
uses: ./.github/workflows/publish-gh-pages.yml
with:
# Set (dispatch only) → approve and preview that fork PR's head SHA.
Expand Down
22 changes: 12 additions & 10 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,9 @@ Both cache-save steps ask this question — gated on `github.ref` they would fir
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
they live in `if:` expressions the gather harness can't reach.
`test/workflow-harness/test_shape.py` asserts this, plus the caller's three guards
(`conclusion == 'success'`, `head_repository == this repo`, `event != 'merge_group'`).
All four **fail OPEN**, and they live in `if:` expressions the gather harness can't reach.

**The fork guard is not optional:** `workflow_run` runs with a WRITE token even when a
fork's PR triggered it (pwn-request). Fork builds reach the site only via a maintainer
Expand Down Expand Up @@ -281,9 +281,10 @@ Two top-level workflows: `ci.yml` builds, `publish.yml` listens for it finishing
publishing.
- `publish.yml` — **the file each consumer carries**, and the one place `<tag>` 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 the
`max-releases`/`max-prs` literals. This repo's copy is identical to a consumer's but
manual re-deploy) → `publish-gh-pages.yml`. Its job `if:` carries the three guards that
fail open (`conclusion == 'success'`, `head_repository == this repo`,
`event != 'merge_group'`) 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`
Expand Down Expand Up @@ -419,16 +420,17 @@ jobs:
if: >-
github.event_name == 'workflow_dispatch' ||
(github.event.workflow_run.conclusion == 'success' &&
github.event.workflow_run.head_repository.full_name == github.repository)
github.event.workflow_run.head_repository.full_name == github.repository &&
github.event.workflow_run.event != 'merge_group')
uses: DiamondLightSource/myst-version-switcher-plugin/.github/workflows/publish-gh-pages.yml@<tag>
with: { pr: "${{ inputs.pr }}", max-releases: "30", max-prs: "20" }
permissions: { pages: write, id-token: write, contents: read, actions: read, statuses: write }
```

`ci.yml` has **no** publish job. Copy `publish.yml` verbatim from the
[tutorial](docs/tutorials/adding-to-a-fresh-repo.md) rather than hand-rolling it — both
`if:` guards fail OPEN, and `workflows: [CI]` matches the entry workflow by NAME, so
renaming it silently stops publishing.
[tutorial](docs/tutorials/adding-to-a-fresh-repo.md) rather than hand-rolling it — all
three `if:` guards fail OPEN, and `workflows: [CI]` matches the entry workflow by NAME,
so renaming it silently stops publishing.

## Upstreaming

Expand Down
12 changes: 11 additions & 1 deletion docs/explanations/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ one dispatched from another ref, whose entry only that ref could ever read. They

Because that lives in an `if:` expression rather than a shell script, the gather harness
cannot reach it; `test/workflow-harness/test_shape.py` asserts it structurally, along with
the caller's two guards — all of which fail *open*, and so would never announce themselves.
the caller's three guards — all of which fail *open*, and so would never announce themselves.

### Fork PRs still cannot deploy themselves

Expand All @@ -212,6 +212,16 @@ never reaches the engine automatically. A maintainer publishes a preview by disp
`publish.yml` with the PR number, which records approval against that exact head SHA; a
later push to the PR drops the preview until re-approved.

### merge_group runs are excluded too

A merge-queue CI run gets a valid but fixed version name (`merge-queue`, see `docs.yml`)
so the build itself does not hard-fail — but that build's artifact is deliberately never
uploaded, and the gather never looks for it. Without a third guard term,
`github.event.workflow_run.event != 'merge_group'`, the caller's other two guards both
still pass for such a run, so `publish.yml` would fire the engine anyway: a full
reassemble-and-deploy that changes nothing, once per queue entry, on top of the deploy the
subsequent push to the base branch already triggers.

## The inline-bash / JS split inside `assemble`

The `assemble` logic is split between `publish-gh-pages.yml` inline steps and
Expand Down
9 changes: 7 additions & 2 deletions docs/how-to/choose-publish-events.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ jobs:
github.event_name == 'workflow_dispatch' ||
(github.event.workflow_run.conclusion == 'success' &&
github.event.workflow_run.head_repository.full_name == github.repository &&
github.event.workflow_run.event != 'merge_group' &&
github.event.workflow_run.event != 'pull_request')
```

Expand All @@ -54,7 +55,7 @@ release's deploy was the only chance that release ever had to reach the site.

## What you must not remove

The two guards in the shipped `if:` are not style:
The three guards in the shipped `if:` are not style:

- **`conclusion == 'success'`** — `workflow_run` fires on failed runs too, and a failed
run has no usable `docs` artifact.
Expand All @@ -63,8 +64,12 @@ The two guards in the shipped `if:` are not style:
untrusted code a privileged path to your live site. Fork builds reach the site only
when a maintainer dispatches `publish.yml` with a `pr` number, which records approval
against that exact head SHA.
- **`event != 'merge_group'`** — a merge-queue build's `docs` artifact is never uploaded
and never gathered (see `docs.yml`'s version-name step), so without this term every
queue entry triggers a full site reassemble-and-redeploy that changes nothing, on top
of the deploy the subsequent push to the base branch already causes.

Both fail *open* if removed — the site keeps deploying and nothing looks wrong — so
All three fail *open* if removed — the site keeps deploying and nothing looks wrong — so
`test/workflow-harness/test_shape.py` asserts them in this repo. Consider the same if you
edit your copy.

Expand Down
3 changes: 2 additions & 1 deletion docs/reference/workflows.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ jobs:
if: >-
github.event_name == 'workflow_dispatch' ||
(github.event.workflow_run.conclusion == 'success' &&
github.event.workflow_run.head_repository.full_name == github.repository)
github.event.workflow_run.head_repository.full_name == github.repository &&
github.event.workflow_run.event != 'merge_group')
uses: DiamondLightSource/myst-version-switcher-plugin/.github/workflows/publish-gh-pages.yml@<tag>
with:
pr: ${{ inputs.pr }}
Expand Down
15 changes: 10 additions & 5 deletions docs/tutorials/adding-to-a-fresh-repo.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,19 @@ on:

jobs:
publish:
# Both guards matter. `conclusion` because workflow_run fires on failed runs too, and
# a failed run has no usable docs artifact. `head_repository` because workflow_run
# gets a WRITE token even when a fork's PR triggered it — without this, untrusted
# code reaches your live site. Both fail OPEN if removed.
# All three guards matter. `conclusion` because workflow_run fires on failed runs
# too, and a failed run has no usable docs artifact. `head_repository` because
# workflow_run gets a WRITE token even when a fork's PR triggered it — without this,
# untrusted code reaches your live site. `event != 'merge_group'` because a
# merge-queue build is never gathered (it has no artifact to gather), so without
# this term every queue entry triggers a full, no-op site redeploy on top of the one
# the subsequent push to the base branch already causes. All three fail OPEN if
# removed.
if: >-
github.event_name == 'workflow_dispatch' ||
(github.event.workflow_run.conclusion == 'success' &&
github.event.workflow_run.head_repository.full_name == github.repository)
github.event.workflow_run.head_repository.full_name == github.repository &&
github.event.workflow_run.event != 'merge_group')
uses: DiamondLightSource/myst-version-switcher-plugin/.github/workflows/publish-gh-pages.yml@__LATEST_TAG__
with:
pr: ${{ inputs.pr }}
Expand Down
11 changes: 11 additions & 0 deletions test/workflow-harness/test_gather.py
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,17 @@ def docs_yml_version_name(tmp, event, **kw):
check(f"an unusable version name is rejected ({bad!r})",
r.returncode != 0 and "not usable as a URL path segment" in log(r), log(r))

with tempfile.TemporaryDirectory() as tmp:
# merge_group's ref_name is GitHub's own gh-readonly-queue/<base>/pr-<n>-<sha> —
# it contains '/' and would otherwise fail the same check as "feature/x" above.
# This build is never gathered (the gather never reads a merge queue's temporary
# branch), so a fixed, valid name is enough — no need to parse GitHub's ref.
built = docs_yml_version_name(
tmp, "merge_group",
ref_name="gh-readonly-queue/main/pr-1645-579cb18403ab661103b5e1093396e45b2e91939f")
check(f"a merge_group build gets a fixed, valid name ({built!r})",
built == "merge-queue", built)

print("\n-- linking the published docs back to the triggering commit --")
def link(tmp, *, dirs=(), built_branch="main", **exprs):
env, rt, data = setup(tmp, [], [], [])
Expand Down
15 changes: 15 additions & 0 deletions test/workflow-harness/test_shape.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ def load(name):
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("excludes merge_group runs (never gathered, so a no-op redeploy)",
"workflow_run.event != 'merge_group'" 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
Expand Down Expand Up @@ -152,5 +154,18 @@ def load(name):
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-- reusable: docs.yml --")
docs_wf = load("docs.yml")
docs_steps = docs_wf["jobs"]["build"]["steps"]
upload = next((s for s in docs_steps if s.get("name") == "Upload docs artifact"), None)
check("has an 'Upload docs artifact' step", upload is not None, [s.get("name") for s in docs_steps])
if upload:
# merge_group's version name is a fixed literal shared by every PR in the batch, so
# publishing it would overwrite an unrelated version's site — see the code comment
# above this step for why the build still runs but the upload is skipped.
check("skips uploading on merge_group",
"github.event_name != 'merge_group'" in str(upload.get("if", "")),
upload.get("if"))

print("\n" + ("FAILURES: " + ", ".join(fails) if fails else "all shape checks passed"))
sys.exit(1 if fails else 0)