Fix PR-preview deploys: write metadata after checkout (survives git clean)

[d-morrison]

Problem

PR-preview deploys silently stopped publishing: preview builds pass and the pr-preview-site artifact uploads (~27 MB), but the preview never appears on gh-pages and all checks stay green. Affects every deploy-path (non-closed) PR — e.g. #912 builds fine but https://d-morrison.github.io/rme/pr-preview/pr-912/ 404s, and pr-912/ is absent from gh-pages. The pr-preview/pr-NNN dirs currently on gh-pages are stale (from before this regression).

Root cause

In preview.yml the steps ran:

1. Save PR metadata → mkdir -p _preview_upload/meta, write pr-number.txt / action.txt — before checkout
2. Check out repository → actions/checkout@v5 (default clean: true) runs git clean -ffdx, deleting the untracked _preview_upload/ (incl. meta/)
3. … render …
4. Stage site → recreates _preview_upload/site/ (after checkout, so it survives)
5. Upload pr-preview-site = _preview_upload/ → contains only site/, no meta/

Then preview-deploy.yml's "Read PR metadata" cats the missing files:

cat: _preview_download/meta/pr-number.txt: No such file or directory
cat: _preview_download/meta/action.txt: No such file or directory

→ pr-number/action outputs empty → if: steps.meta.outputs.action == 'deploy' is false → Deploy step skipped, but the failed cat doesn't fail the step, so the job reports success.

Evidence

• Deploy run for Add Course Logistics part: Spring 2026 syllabus and schedule #912's build (27786018099) logs the two cat: … No such file lines, then skips the deploy step; job = success.
• Downloading the pr-preview-site artifact from the build directly shows a top level of site/ only — no meta/.

Fix

Move Save PR metadata to run after "Check out repository", so the files are created after git clean and survive into the artifact. It only needs github.event.pull_request.number, available regardless of checkout. The step stays unguarded, so the closed/remove path (which skips checkout) is unaffected. Added a comment so the load-bearing ordering isn't reverted.

Validation

This PR's own preview build runs the fixed preview.yml (head-branch version), so if the deploy publishes pr-preview/pr-<this-PR>/, the fix is confirmed end-to-end. Note: open PRs like #912 will need main merged in (or a re-run) to pick up the fixed workflow for their own previews.

🤖 Generated with Claude Code

[claude]

Claude finished @d-morrison's task in 2m 15s —— View job

---

Code Review: Fix PR-preview deploys

• Gather context
• Read changed file (preview.yml)
• Read preview-deploy.yml to verify end-to-end fix
• Post review findings

---

Verdict: Correct fix. Approve.

The root cause is accurately diagnosed and the fix is minimal and sound. Here's the full analysis:

---

What changed

.github/workflows/preview.yml — the "Save PR metadata" step moved from before to after "Check out repository", with a detailed explanatory comment added.

---

Correctness analysis

deploy path (non-closed events):

1. actions/checkout@v5 runs with default clean: true → git clean -ffdx wipes untracked files
2. "Save PR metadata" now runs after checkout → _preview_upload/meta/{pr-number.txt,action.txt} created fresh, survive to artifact
3. "Stage site for upload" adds _preview_upload/site/
4. Artifact pr-preview-site = {meta/, site/} ✓

closed/remove path:

• "Check out repository" is skipped (if: github.event.action != 'closed') → no git clean runs
• "Save PR metadata" is unguarded → still runs, writes action.txt = "remove" ✓
• "Upload preview artifact (remove)" uploads _preview_upload/ → contains only meta/, no site/
• preview-deploy.yml reads action = "remove" → Remove PR Preview step fires ✓

Both paths are correct before and after the fix; only the deploy path was broken.

---

Findings

preview.yml:51-60 — Comment is appropriate

The 10-line comment explaining the ordering constraint is justified here. The bug was a subtle ordering dependency with no other in-file signal, and the comment prevents a future "let me clean this up" revert from reintroducing the regression.

---

preview-deploy.yml:61-65 — Pre-existing latent silent-failure risk (not introduced by this PR)

- name: Read PR metadata
  id: meta
  run: |
    echo "pr-number=$(cat _preview_download/meta/pr-number.txt)" >> "$GITHUB_OUTPUT"
    echo "action=$(cat _preview_download/meta/action.txt)" >> "$GITHUB_OUTPUT"

When cat is used inside $(...) in bash with set -e, a missing file produces empty output and exit 0 on the outer echo, so the step passes silently with empty outputs. This is exactly what caused the invisible deploy skips that this PR fixes. After this fix the files will always be present in the happy path, but if the artifact is ever corrupt or partially uploaded, the downstream job will again skip silently rather than fail explicitly.

Consider hardening this in a follow-up:

run: |
  pr_number=$(cat _preview_download/meta/pr-number.txt) || { echo "::error::Missing pr-number.txt"; exit 1; }
  action=$(cat _preview_download/meta/action.txt) || { echo "::error::Missing action.txt"; exit 1; }
  echo "pr-number=$pr_number" >> "$GITHUB_OUTPUT"
  echo "action=$action" >> "$GITHUB_OUTPUT"

Not a blocker for this PR — that file isn't changed here and it's a pre-existing issue.

---

preview.yml:65 — Expression syntax is correct

echo "${{ github.event.action == 'closed' && 'remove' || 'deploy' }}" > _preview_upload/meta/action.txt

GitHub Actions ternary idiom (A && B || C) is correct here. github.event.action is a controlled enum string; github.event.pull_request.number is numeric — no injection risk.

---

Summary

The fix is correct, minimal, and the inline comment prevents the ordering constraint from being accidentally reverted. The one follow-up worth tracking is hardening preview-deploy.yml's metadata-read step to fail explicitly on missing files rather than silently producing empty outputs.

[d-morrison]

Thanks — and the fix is validated end-to-end: this PR's own preview published at https://d-morrison.github.io/rme/pr-preview/pr-913/ (200; pr-913 is in gh-pages), which the old workflow would have silently skipped.

On the one finding:

• preview-deploy.yml "Read PR metadata" silent-failure (pre-existing) — ✅ Addressed in 723c972, rather than deferring. The step now reads each file into a variable with || { echo "::error::…"; exit 1; }, so a missing pr-number.txt/action.txt fails the job loudly instead of producing empty outputs + exit 0. It's small, and it's the deploy-side half of the exact silent-skip pathology this PR fixes, so it belongs here. (Note: preview-deploy.yml runs via workflow_run from main, so this guard takes effect once merged.)
• The two "appropriate"/"correct" notes (the explanatory comment, the &&/|| expression) need no action.

@claude review

[claude]

Claude finished @d-morrison's task in 1m 22s —— View job

---

Code Review: Fix PR-preview deploys (updated)

• Read changed files
• Review preview.yml changes
• Review preview-deploy.yml changes (723c972 hardening)
• Post findings

---

Verdict: Correct and complete. Approve.

The previous finding (silent-failure risk in preview-deploy.yml) is now addressed. Here is the updated analysis covering both commits.

---

preview.yml — metadata-ordering fix

The move of "Save PR metadata" to after "Check out repository" is correct. Key properties verified:

• github.event.pull_request.number and github.event.action are available from the event payload at all times — no dependency on checkout. The step doesn't need the repo contents.
• The step remains unguarded (if: absent), so both paths work:
  • closed/remove: checkout is skipped, metadata step still runs, writes action.txt = "remove".
  • deploy: checkout runs (wiping any prior _preview_upload/), then metadata is written into a clean workspace and survives to the artifact.
• Both upload steps correctly guard by event action, so meta/ + site/ land in the deploy artifact and only meta/ in the remove artifact.

The 10-line explanatory comment is appropriate here — the ordering constraint is non-obvious, caused a real regression, and has no other in-file signal.

---

preview-deploy.yml — explicit failure on missing metadata (723c972)

The new pattern:

pr_number=$(cat _preview_download/meta/pr-number.txt) || { echo "::error::..."; exit 1; }

is correct. In bash, var=$(cmd) propagates cmd's exit code as the statement's exit code, so if cat fails (file absent → exit 1), the || branch fires. This is more reliable than relying on set -e alone, which does not catch $(…) failures inside a larger expression like echo "x=$(cat missing)" — exactly the silent-skip mechanism this PR is recovering from. The comment accurately describes that dynamic.

One minor edge case not guarded against: if pr-number.txt exists but is empty, cat exits 0, pr_number="", and echo "pr-number=" is written to $GITHUB_OUTPUT. This is harmless in practice — the file is always written by the workflow itself with a numeric PR number — but it's worth noting for completeness.

The comment correctly notes that workflow_run jobs run the workflow version from the default branch, so this guard takes effect only after merge.

---

No further findings. The fix is minimal, correct, and end-to-end validated by this PR's own preview publishing at pr-913/.

[github-actions]

👀 Picked up by workflow run #27795652122. R/Quarto/renv setup runs first (~3-5 min); Claude itself responds after that.

[d-morrison]

Re the empty-pr-number.txt completeness note: rebutting — no change. That value is written by this same workflow from github.event.pull_request.number, which is always a non-empty integer on pull_request events, so an empty-but-present file isn't reachable. The added guard already covers the realistic failure (missing file). Guarding the unreachable empty case would be dead defensive code. Treating the review as clean (Approve, no findings).

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.

_{2026-06-19 01:00 UTC}