From 1435e3a8445f1625a10e695832c98683b96eb99d Mon Sep 17 00:00:00 2001 From: lmoresi Date: Thu, 13 Aug 2026 19:41:24 +1000 Subject: [PATCH 1/3] Comment the preview link when the pull request opens, and stop duplicating it Two defects in how the preview gets announced. The link is the only way to reach a preview -- the directory is a hash of the branch name and nothing links to it -- so a missing notification is a missing preview. FIRST: preview.yml comments on PUSH, and only if a pull request already exists for the branch. Push, then open the pull request -- the natural order, and the one the worktree flow encourages -- and the comment step correctly finds no pull request and exits. The preview is built and serving the whole time. That happened to UWTN 2026-012 and had to be recovered by computing the URL by hand. preview-link.yml now fires on pull_request opened/reopened and comments the link WITHOUT rebuilding: it reads the same URL, confirms it is really serving first, and says nothing if it is not, leaving the push-triggered run to comment when its build lands. It takes the path from preview_mark.preview_path rather than reimplementing the hash, and reads head.ref rather than GITHUB_REF_NAME, which on a pull_request event is "/merge" and would key the URL to the wrong directory. SECOND: the dedupe never matched. It looked for a body starting "**Preview:**" while the variant that carries links starts "**Preview" with no colon -- so the common case matched nothing and every push appended another comment. PR #7 has thirteen. Both workflows now match on the prefix both variants share. Underworld development team with AI support from Claude Code --- .github/workflows/preview-link.yml | 77 ++++++++++++++++++++++++++++++ .github/workflows/preview.yml | 2 +- tests/test_migration.py | 27 +++++++++++ 3 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/preview-link.yml diff --git a/.github/workflows/preview-link.yml b/.github/workflows/preview-link.yml new file mode 100644 index 0000000..13e9256 --- /dev/null +++ b/.github/workflows/preview-link.yml @@ -0,0 +1,77 @@ +name: preview-link + +# Put the preview link on a pull request the moment it is opened. +# +# preview.yml builds on PUSH and comments only if a pull request already exists +# for that branch. Push first and open the pull request afterwards -- which is +# the natural order, and the one the worktree flow encourages -- and the comment +# step correctly finds nothing and exits. The preview is built and serving; only +# the notification is missing, and there is no other way to find the URL: the +# directory is a hash of the branch name and nothing links to it. +# +# So this comments when the pull request is opened, WITHOUT rebuilding. It reads +# the same URL preview.yml would have published to and checks that it is really +# serving before saying anything, which is the same discipline preview.yml uses. +# If the preview is not up -- push and open in quick succession, and the build +# takes minutes -- it says nothing, and the push-triggered run will comment when +# it finishes. + +on: + pull_request: + types: [opened, reopened] + +permissions: + contents: read + pull-requests: write + +jobs: + link: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Comment the preview link, if there is a preview + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + # head.ref, NOT GITHUB_REF_NAME: on a pull_request event that is + # "/merge", and the preview directory is keyed on the BRANCH. + BRANCH: ${{ github.event.pull_request.head.ref }} + PR: ${{ github.event.pull_request.number }} + run: | + # One definition of the path, imported rather than reimplemented, so + # it cannot drift from what preview_mark.py actually publishes to. + HASH=$(python3 -c "import sys; sys.path.insert(0, 'scripts'); \ + import preview_mark; print(preview_mark.preview_path('$BRANCH'))") + URL="https://underworld-technical-notes.github.io/underworldcode.org-preview/${HASH}/" + + CODE=$(curl -s -o /dev/null -w '%{http_code}' "$URL" || echo 000) + if [ "$CODE" != "200" ]; then + echo "no preview serving at $URL (HTTP $CODE) -- the push build will comment" + exit 0 + fi + + # Link the notes this pull request touches, the same as preview.yml. + LINKS="" + for SLUG in $(gh pr diff "$PR" --name-only \ + | sed -n 's|^articles/\([^/]*\)/.*|\1|p' | sort -u); do + [ -f "articles/${SLUG}/metadata.yml" ] || continue + TITLE=$(sed -n 's/^title: *//p' "articles/${SLUG}/metadata.yml" | head -1) + LINKS="${LINKS}- [${TITLE:-$SLUG}](${URL}${SLUG}/)"$'\n' + done + + if [ -n "$LINKS" ]; then + BODY=$(printf '**Preview**\n\n%s\nOr the [whole site](%s).\n\nShows notes at draft and review, which the published site withholds. Not indexed, no comments, and not the citable version.' "$LINKS" "$URL") + else + BODY=$(printf '**Preview:** %s\n\nNo article changed on this branch. Shows notes at draft and review, which the published site withholds. Not indexed, no comments, and not the citable version.' "$URL") + fi + + # Update the existing comment rather than adding another. Matching on + # "**Preview" and not "**Preview:**": the linked variant has no colon, + # which is why one pull request collected thirteen of these. + EXISTING=$(gh api "repos/${GITHUB_REPOSITORY}/issues/${PR}/comments" \ + --jq '.[] | select(.body | startswith("**Preview")) | .id' | head -1) + if [ -n "$EXISTING" ]; then + gh api -X PATCH "repos/${GITHUB_REPOSITORY}/issues/comments/${EXISTING}" -f body="$BODY" + else + gh api -X POST "repos/${GITHUB_REPOSITORY}/issues/${PR}/comments" -f body="$BODY" + fi diff --git a/.github/workflows/preview.yml b/.github/workflows/preview.yml index ecf3f0e..54bb173 100644 --- a/.github/workflows/preview.yml +++ b/.github/workflows/preview.yml @@ -190,7 +190,7 @@ jobs: BODY=$(printf '**Preview:** %s\n\nNo article changed on this branch. Built from `%s`. Shows notes at draft and review, which the published site withholds. Not indexed, no comments, and not the citable version.' "$URL" "${GITHUB_SHA::7}") fi EXISTING=$(gh api "repos/${GITHUB_REPOSITORY}/issues/${PR}/comments" \ - --jq '.[] | select(.body | startswith("**Preview:**")) | .id' | head -1) + --jq '.[] | select(.body | startswith("**Preview")) | .id' | head -1) if [ -n "$EXISTING" ]; then gh api -X PATCH "repos/${GITHUB_REPOSITORY}/issues/comments/${EXISTING}" -f body="$BODY" else diff --git a/tests/test_migration.py b/tests/test_migration.py index 354de78..8de96bd 100644 --- a/tests/test_migration.py +++ b/tests/test_migration.py @@ -1937,3 +1937,30 @@ def test_the_deposit_stops_while_identifiers_are_unrecorded(): first_live = config.index("--live") assert guard < first_live, \ "the guard must come before the first step that can deposit" + + +def test_a_pull_request_gets_its_preview_link_even_if_opened_later(): + """preview.yml comments on PUSH, and only if a PR already exists. + + Push the branch, open the pull request afterwards -- the natural order -- + and the comment step finds no PR and exits. The preview is built and + serving, but nothing links to it and the directory is a hash of the branch + name, so it cannot be found. That happened to UWTN 2026-012. + + Also guards the dedupe prefix. The linked variant of the body starts + "**Preview" with no colon, so a rule matching "**Preview:**" never finds + it: PR #7 collected thirteen preview comments before this was noticed. + """ + link = (ROOT / ".github" / "workflows" / "preview-link.yml").read_text(encoding="utf-8") + config = "\n".join(l for l in link.splitlines() if not l.lstrip().startswith("#")) + assert "types: [opened, reopened]" in config, "it has to fire when the PR appears" + assert "pull_request.head.ref" in config, \ + "GITHUB_REF_NAME is '/merge' here; the preview path keys on the branch" + assert "preview_mark" in config, "import the path, do not reimplement it" + assert "myst build" not in config and "preview_build" not in config, \ + "this comments on an existing preview; it must not rebuild one" + + for name in ("preview.yml", "preview-link.yml"): + text = (ROOT / ".github" / "workflows" / name).read_text(encoding="utf-8") + assert 'startswith("**Preview")' in text, \ + "%s dedupe must match the linked variant, which has no colon" % name From d1503ee5320e70bdb982dbc03ba8543cc2a9b5d7 Mon Sep 17 00:00:00 2001 From: lmoresi Date: Thu, 13 Aug 2026 19:56:27 +1000 Subject: [PATCH 2/3] Only preview when something renderable changed Louis: 'It's odd that there is a preview action that holds up merging 13 and 12 when those have nothing to preview.' It was not actually holding anything up -- only `test` is a required check, so those merge whatever preview is doing -- but spending six minutes republishing an unchanged site for a branch that touches workflows and tests is waste, and it makes the checks list look like it is blocking when it is not. The push trigger now filters on the paths that change what a reader sees. scripts/ is in the list deliberately: those build the site, so a change there can alter every page without touching an article. Safe as a paths filter ONLY because preview is not required. A required check skipped this way stays 'expected' and blocks the merge forever rather than passing, so the test says so next to the assertion. Underworld development team with AI support from Claude Code --- .github/workflows/preview.yml | 27 ++++++++++++++++++++++----- tests/test_migration.py | 18 ++++++++++++++++++ 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/.github/workflows/preview.yml b/.github/workflows/preview.yml index 54bb173..43af409 100644 --- a/.github/workflows/preview.yml +++ b/.github/workflows/preview.yml @@ -20,11 +20,28 @@ name: preview on: push: branches-ignore: [main] - # A deposit-queue branch changes this one file and nothing a reader would - # look at. Publishing a whole preview site for it costs minutes and tells - # nobody anything; deposit-pdf.yml builds the PDF instead, which is the - # artefact that is actually under review at that point. - paths-ignore: ['deposit-queue.txt'] + # Only build when something a reader would actually SEE has changed. A + # branch that touches workflows or tests has nothing to preview, and a + # six-minute build to publish an unchanged site helps nobody -- including + # a deposit-queue branch, whose one-line change is answered by the PDF that + # deposit-pdf.yml builds instead. + # + # Safe as a filter because `preview` is NOT a required status check -- only + # `test` is. A paths filter on a required check would leave the run + #remaining "expected" and block the merge forever rather than skipping it. + # If that ever changes, this has to change with it. + # + # scripts/ is included deliberately: those build the site, so a change + # there can alter every page without touching an article. + paths: + - 'articles/**' + - 'pages-src/**' + - 'static/**' + - 'templates/**' + - 'scripts/**' + - 'myst.yml' + - 'authors.yml' + - 'classification.yml' workflow_dispatch: concurrency: diff --git a/tests/test_migration.py b/tests/test_migration.py index 8de96bd..8ffc921 100644 --- a/tests/test_migration.py +++ b/tests/test_migration.py @@ -1964,3 +1964,21 @@ def test_a_pull_request_gets_its_preview_link_even_if_opened_later(): text = (ROOT / ".github" / "workflows" / name).read_text(encoding="utf-8") assert 'startswith("**Preview")' in text, \ "%s dedupe must match the linked variant, which has no colon" % name + + +def test_the_preview_only_runs_when_there_is_something_to_preview(): + """Six minutes to republish an unchanged site helps nobody. + + A branch that touches only workflows or tests has nothing to render, and + the preview used to build for it anyway. This is safe as a paths filter + ONLY because `preview` is not a required status check -- a required check + skipped by a paths filter stays "expected" and blocks the merge forever + instead of passing. If preview is ever made required, the filter has to go. + """ + text = (ROOT / ".github" / "workflows" / "preview.yml").read_text(encoding="utf-8") + config = "\n".join(l for l in text.splitlines() if not l.lstrip().startswith("#")) + assert "paths:" in config, "the preview should not build for unrenderable changes" + for needed in ("'articles/**'", "'scripts/**'", "'myst.yml'"): + assert needed in config, ( + "%s changes what the site looks like; leaving it out means no " + "preview when one is wanted" % needed) From da590d58b60692a60f7d4e5b416d443f322db6ed Mon Sep 17 00:00:00 2001 From: lmoresi Date: Thu, 13 Aug 2026 19:57:29 +1000 Subject: [PATCH 3/3] Assert the outcome, not the mechanism The deposit-preview test pinned 'paths-ignore' as the way a deposit-queue branch is kept out of the preview. Replacing it with a positive paths list -- which excludes deposit-queue.txt more strongly, by not naming it -- made that test fail on a change that improved the thing it guards. Also: I committed the previous change with that test failing, because the command piped pytest into tail and the pipe's exit status masked pytest's. Second time this session. Run it, check $?, then commit. --- tests/test_migration.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/test_migration.py b/tests/test_migration.py index 8ffc921..7e40f1b 100644 --- a/tests/test_migration.py +++ b/tests/test_migration.py @@ -1901,8 +1901,17 @@ def test_a_deposit_pull_request_gets_a_pdf_not_a_preview(): builds the archival PDF and attaches it to the run instead. """ preview = (ROOT / ".github" / "workflows" / "preview.yml").read_text(encoding="utf-8") - assert "paths-ignore" in preview and "deposit-queue.txt" in preview, \ + # Assert the OUTCOME, not the mechanism: this began as a paths-ignore on + # deposit-queue.txt and became a positive paths list, which excludes it by + # not naming it. Either satisfies the point; pinning the mechanism made + # this fail on a change that strengthened it. + prev_cfg = "\n".join(l for l in preview.splitlines() + if not l.lstrip().startswith("#")) + assert "paths:" in prev_cfg or "paths-ignore:" in prev_cfg, \ "a deposit-queue branch must not trigger a full preview build" + assert "deposit-queue.txt" not in prev_cfg.split("jobs:")[0] \ + or "paths-ignore:" in prev_cfg, \ + "deposit-queue.txt must not be a path that triggers the preview" pdf = (ROOT / ".github" / "workflows" / "deposit-pdf.yml").read_text(encoding="utf-8") config = "\n".join(l for l in pdf.splitlines() if not l.lstrip().startswith("#"))