ci: resolve fixtures pin from the PR branch in update-snapshots

.github/workflows/update-snapshots.yml

@@ -3,18 +3,28 @@ on:
   issue_comment:
     types: [created]
 
-# Snapshots are generated against quantecon-book-theme-fixtures at this
-# pinned SHA. Keep FIXTURES_SHA in sync with .github/workflows/ci.yml.
+# Snapshots are generated against quantecon-book-theme-fixtures. The values
+# below are only a FALLBACK: on `issue_comment` events GitHub always runs the
+# copy of this workflow that is on the default branch, so these would pin the
+# fixtures main uses rather than the ones the PR under test uses. Each job
+# re-reads the pin out of the checked-out PR branch's ci.yml instead — see the
+# "Resolve fixtures pin" step.
 env:
   FIXTURES_REPO: QuantEcon/quantecon-book-theme-fixtures
   FIXTURES_SHA: d8ffc17c753ecf45fa25c6062827e1aa9de201b3
 
 jobs:
   # /update-new-snapshots — only creates MISSING snapshots (safe for adding new tests)
   update-new-snapshots:
+    # The author_association check is a privilege gate, not a convenience: these
+    # jobs check out the PR branch and execute its code (`pip install .` runs the
+    # PR's build backend) with a `contents: write` token. Without it, anyone able
+    # to comment on a PR could trigger that.
     if: |
       github.event.issue.pull_request &&
-      contains(github.event.comment.body, '/update-new-snapshots')
+      contains(github.event.comment.body, '/update-new-snapshots') &&
+      contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'),
+               github.event.comment.author_association)
     runs-on: ubuntu-latest
     permissions:
       contents: write
@@ -39,11 +49,29 @@ jobs:
           ref: ${{ steps.pr.outputs.ref }}
           token: ${{ secrets.GITHUB_TOKEN }}
 
+      # Must run after the PR branch is checked out. On `issue_comment` events
+      # the workflow definition comes from the default branch, so the top-level
+      # env pins main's fixtures. A PR that bumps the pin (because it needs a
+      # fixtures change to exercise a new feature) would otherwise regenerate
+      # against the OLD fixtures, produce byte-identical snapshots, and commit
+      # nothing — leaving the PR's visual job failing with no way to fix it.
+      - name: Resolve fixtures pin from the PR branch
+        id: pin
+        run: |
+          CI=.github/workflows/ci.yml
+          SHA=$(grep -oE '^[[:space:]]*FIXTURES_SHA:[[:space:]]*[0-9a-f]{40}' "$CI" \
+                | head -1 | grep -oE '[0-9a-f]{40}' || true)
+          REPO=$(grep -oE '^[[:space:]]*FIXTURES_REPO:[[:space:]]*[^[:space:]]+' "$CI" \
+                | head -1 | awk '{print $2}' || true)
+          echo "sha=${SHA:-$FIXTURES_SHA}" >> "$GITHUB_OUTPUT"
+          echo "repo=${REPO:-$FIXTURES_REPO}" >> "$GITHUB_OUTPUT"
+          echo "Fixtures: ${REPO:-$FIXTURES_REPO}@${SHA:-$FIXTURES_SHA}"
+
       - name: Checkout fixtures
         uses: actions/checkout@v7
         with:
-          repository: ${{ env.FIXTURES_REPO }}
-          ref: ${{ env.FIXTURES_SHA }}
+          repository: ${{ steps.pin.outputs.repo }}
+          ref: ${{ steps.pin.outputs.sha }}
           path: fixtures
 
       - name: Setup Python
@@ -79,15 +107,20 @@ jobs:
           SITE_PATH: fixtures/_build/html
 
       - name: Commit and Push Updated Snapshots
+        id: commit
         run: |
           git config --local user.email "github-actions[bot]@users.noreply.github.com"
           git config --local user.name "github-actions[bot]"
           git add tests/visual/__snapshots__/
           if git diff --staged --quiet; then
             echo "No snapshot changes to commit"
+            echo "committed=false" >> "$GITHUB_OUTPUT"
           else
+            CHANGED=$(git diff --staged --name-only | wc -l | tr -d "[:space:]")
             git commit -m "UPDATE: Visual regression snapshots (new)"
             git push
+            echo "committed=true" >> "$GITHUB_OUTPUT"
+            echo "changed=$CHANGED" >> "$GITHUB_OUTPUT"
           fi
 
       - name: Add reaction to comment
@@ -103,21 +136,34 @@ jobs:
 
       - name: Comment on PR
         uses: actions/github-script@v9
+        env:
+          COMMITTED: ${{ steps.commit.outputs.committed }}
+          CHANGED: ${{ steps.commit.outputs.changed }}
+          FIXTURES: ${{ steps.pin.outputs.repo }}@${{ steps.pin.outputs.sha }}
         with:
           script: |
+            const committed = process.env.COMMITTED === 'true';
+            const built = `\n\nBuilt against fixtures \`${process.env.FIXTURES}\`.`;
+            const body = committed
+              ? `✅ Added ${process.env.CHANGED} missing visual snapshot(s) and committed them to this PR.${built}\n\n` +
+                `⚠️ The bot's commit uses \`GITHUB_TOKEN\`, which does not trigger workflows — push an empty commit to re-run CI against the new baselines.`
+              : `ℹ️ No missing visual snapshots — nothing was committed. Every test already has a baseline, so this PR is unchanged.${built}`;
             await github.rest.issues.createComment({
               owner: context.repo.owner,
               repo: context.repo.repo,
               issue_number: context.issue.number,
-              body: '✅ Missing visual snapshots have been added and committed to this PR.'
+              body
             });
 
   # /update-snapshots — regenerates ALL snapshots (use when styling changes legitimately)
   update-all-snapshots:
+    # See the privilege-gate note on update-new-snapshots above.
     if: |
       github.event.issue.pull_request &&
       contains(github.event.comment.body, '/update-snapshots') &&
-      !contains(github.event.comment.body, '/update-new-snapshots')
+      !contains(github.event.comment.body, '/update-new-snapshots') &&
+      contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'),
+               github.event.comment.author_association)
     runs-on: ubuntu-latest
     permissions:
       contents: write
@@ -142,11 +188,29 @@ jobs:
           ref: ${{ steps.pr.outputs.ref }}
           token: ${{ secrets.GITHUB_TOKEN }}
 
+      # Must run after the PR branch is checked out. On `issue_comment` events
+      # the workflow definition comes from the default branch, so the top-level
+      # env pins main's fixtures. A PR that bumps the pin (because it needs a
+      # fixtures change to exercise a new feature) would otherwise regenerate
+      # against the OLD fixtures, produce byte-identical snapshots, and commit
+      # nothing — leaving the PR's visual job failing with no way to fix it.
+      - name: Resolve fixtures pin from the PR branch
+        id: pin
+        run: |
+          CI=.github/workflows/ci.yml
+          SHA=$(grep -oE '^[[:space:]]*FIXTURES_SHA:[[:space:]]*[0-9a-f]{40}' "$CI" \
+                | head -1 | grep -oE '[0-9a-f]{40}' || true)
+          REPO=$(grep -oE '^[[:space:]]*FIXTURES_REPO:[[:space:]]*[^[:space:]]+' "$CI" \
+                | head -1 | awk '{print $2}' || true)
+          echo "sha=${SHA:-$FIXTURES_SHA}" >> "$GITHUB_OUTPUT"
+          echo "repo=${REPO:-$FIXTURES_REPO}" >> "$GITHUB_OUTPUT"
+          echo "Fixtures: ${REPO:-$FIXTURES_REPO}@${SHA:-$FIXTURES_SHA}"
+
       - name: Checkout fixtures
         uses: actions/checkout@v7
         with:
-          repository: ${{ env.FIXTURES_REPO }}
-          ref: ${{ env.FIXTURES_SHA }}
+          repository: ${{ steps.pin.outputs.repo }}
+          ref: ${{ steps.pin.outputs.sha }}
           path: fixtures
 
       - name: Setup Python
@@ -190,16 +254,20 @@ jobs:
           retention-days: 30
 
       - name: Commit and Push Updated Snapshots
+        id: commit
         run: |
           git config --local user.email "github-actions[bot]@users.noreply.github.com"
           git config --local user.name "github-actions[bot]"
           git add tests/visual/__snapshots__/
           if git diff --staged --quiet; then
             echo "No snapshot changes to commit"
+            echo "committed=false" >> "$GITHUB_OUTPUT"
           else
-            CHANGED=$(git diff --staged --name-only | wc -l)
+            CHANGED=$(git diff --staged --name-only | wc -l | tr -d "[:space:]")
             git commit -m "UPDATE: Regenerate all visual snapshots ($CHANGED files)"
             git push
+            echo "committed=true" >> "$GITHUB_OUTPUT"
+            echo "changed=$CHANGED" >> "$GITHUB_OUTPUT"
           fi
 
       - name: Add reaction to comment
@@ -215,13 +283,25 @@ jobs:
 
       - name: Comment on PR with summary
         uses: actions/github-script@v9
+        env:
+          COMMITTED: ${{ steps.commit.outputs.committed }}
+          CHANGED: ${{ steps.commit.outputs.changed }}
+          FIXTURES: ${{ steps.pin.outputs.repo }}@${{ steps.pin.outputs.sha }}
         with:
           script: |
             const runUrl = `https://github.com/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`;
+            const committed = process.env.COMMITTED === 'true';
+            const body = committed
+              ? `✅ Regenerated visual snapshots and committed ${process.env.CHANGED} changed file(s) to this PR.\n\n` +
+                `Built against fixtures \`${process.env.FIXTURES}\`.\n\n` +
+                `📦 [Download snapshot-update-diff artifact](${runUrl}#artifacts) to review before/after images.\n\n` +
+                `⚠️ The bot's commit uses \`GITHUB_TOKEN\`, which does not trigger workflows — push an empty commit to re-run CI against the new baselines.`
+              : `ℹ️ Snapshots were regenerated but came out identical to the existing baselines, so **nothing was committed** and this PR is unchanged.\n\n` +
+                `Built against fixtures \`${process.env.FIXTURES}\`. If the visual job is still failing, check that this is the fixtures commit you expect — the pin is read from \`.github/workflows/ci.yml\` on this branch.\n\n` +
+                `📦 [Run log](${runUrl})`;
             await github.rest.issues.createComment({
               owner: context.repo.owner,
               repo: context.repo.repo,
               issue_number: context.issue.number,
-              body: '✅ All visual snapshots have been regenerated and committed to this PR.\n\n' +
-                    `📦 [Download snapshot-update-diff artifact](${runUrl}#artifacts) to review before/after images.`
+              body
             });

CHANGELOG.md

@@ -8,6 +8,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 
 ### CI
+- **`/update-snapshots` now reads the fixtures pin from the PR branch** — GitHub always runs the *default branch's* copy of a workflow on `issue_comment` events, so `update-snapshots.yml` was resolving `FIXTURES_SHA` from main rather than from the PR under test. A PR that bumps the pin (because it needs a fixtures change to exercise a new feature) therefore regenerated against the *old* fixtures, produced byte-identical baselines, committed nothing — and still posted "✅ regenerated and committed", leaving the PR's visual job failing with no way to fix it. Both jobs now resolve the pin out of the checked-out branch's `ci.yml`, falling back to the workflow-level value. Both summary comments also report what actually happened (committed vs. unchanged, and which fixtures commit was built against) instead of unconditionally claiming success, and note that the bot's `GITHUB_TOKEN` commit does not itself trigger CI.
+- **`/update-snapshots` is now restricted to trusted actors** — both jobs check out the PR branch and execute its code (`pip install .` runs the PR's build backend) with a `contents: write` token, but had no author-association gate, so anyone able to comment on a PR could trigger that. Both now require `OWNER`, `MEMBER`, or `COLLABORATOR`.
 - **CI Node bumped 20 → 24** — Node 20 reached end-of-life in April 2026, and the grouped npm updates in #400 raised engine floors (`sass-loader` 17 requires Node ≥22.11). Node 24 is the current active LTS (supported to April 2028). Applies to `ci.yml`, `docs.yml`, and `update-snapshots.yml`; `.nvmrc` and the contributor docs (`CONTRIBUTING.md`, `docs/developer/setup.md`) move to Node 24 in step so local dev matches CI.
 - **Stabilized flaky mobile-chrome visual tests on math-heavy pages** — after the Playwright 1.57→1.60 (Chromium) bump in #400, `math`/`proofs`/`cross-references` full-page screenshots intermittently rendered ~60px shorter than baseline, failing Playwright's dimension check before pixel tolerances apply. `waitForReady` now waits for `document.fonts.ready` and polls until the document height holds steady for 750ms, instead of a fixed 500ms sleep after MathJax typesetting.
 - **Cleared open Dependabot security alerts via `npm audit fix`** — `webpack-dev-server` 5.2.2→5.2.4 (GHSA-79cf-xcqc-c78w), `shell-quote` 1.8.1→1.8.4 (GHSA-w7jw-789q-3m8p, critical), `qs` 6.14.2→6.15.2 (GHSA-q8mj-m7cp-5q26), `ws` 8.18.0→8.21.0 (GHSA-58qx-3vcg-4xpx). All transitive dev/build-time only; built theme assets unchanged. The remaining `uuid` alert (GHSA-w5hq-g745-h8pq) was dismissed as not exploitable — `sockjs` only calls `uuid.v4()`, the advisory affects `v3`/`v5`/`v6` with a caller-provided `buf`.