From 0f192f7dff393ef3cd6a70d42f1a1c6aa27e4ad1 Mon Sep 17 00:00:00 2001 From: Matt McKay Date: Fri, 7 Aug 2026 10:20:45 +1000 Subject: [PATCH] fix(preview-cloudflare): link the stable pr-N alias, not the deployment hash (#14) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PR comment linked the per-deployment hash URL, so every push produced a fresh set of links. The headline URL was the smaller half of the cost: every per-lecture deep link in the comment carried the same hash, so a reviewer could not keep a tab open across a review. The alias is now constructed rather than parsed. `pr-N` is already alias-safe — Cloudflare lowercases branch names and maps non-alphanumerics to `-` — so the URL is fully determined by inputs the action already holds, and there is no output parsing left to go wrong. The README already documented the alias as the behaviour, so this brings the code in line with its own docs. A latent bug went with it: `grep … | head -1` took whichever pages.dev URL came first, and wrangler prints the alias before the hash URL in some versions. So `deploy-url` returned the alias on one wrangler release and the hash on another, with nothing in the action pinning which. Verified both output shapes against the old and new logic before committing. The per-deployment URL is still the only way to view one specific revision, so it becomes a `deployment-url` output — selected by excluding the alias rather than by position — and is named in the comment's Build Info block. Deploy-step inputs also move from `${{ }}` interpolation into `env:`, matching what the comment step in the same file already did. Not covered by the harness: no preview action is, since a real deploy needs Cloudflare credentials. Verified by YAML/bash/JS syntax checks and by running the extraction against three wrangler output shapes. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 19 ++++++++ preview-cloudflare/README.md | 3 +- preview-cloudflare/action.yml | 88 ++++++++++++++++++++++++++--------- 3 files changed, 86 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b410096..5127acc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- **`preview-cloudflare`**: PR comments linked the per-deployment hash URL + (`https://b3a5c314.{project}.pages.dev`) instead of the stable branch alias + (`https://pr-{number}.{project}.pages.dev`), so every push to a PR produced a + fresh set of links and reviewers had to scroll for the newest comment. The cost was larger + than the headline URL: **every per-lecture deep link** in the comment carried the same hash, + so a reviewer could not keep a tab open across a review. The alias is now **constructed** + rather than parsed — `pr-N` is already alias-safe, so the URL is fully determined by inputs + the action already holds, and no output parsing can go wrong. Note the README already + documented the alias as the behaviour, so this brings the code in line with its own docs. + A latent bug went with it: the old `grep … | head -1` took whichever `pages.dev` URL came + first, and wrangler prints the alias *before* the hash URL in some versions — so + `deploy-url` silently returned the alias on one wrangler release and the hash on another. + The per-deployment URL is still worth having, so it is now a separate `deployment-url` + output (extracted by excluding the alias, not by position) and named in the comment's + Build Info block for anyone who needs to look at one specific revision. Inputs in the + deploy step also move from `${{ }}` interpolation into `env:`, matching the discipline the + comment step in the same file already followed. (#14) + ## [0.11.0] - 2026-08-07 ### Added diff --git a/preview-cloudflare/README.md b/preview-cloudflare/README.md index b559f20..0257ead 100644 --- a/preview-cloudflare/README.md +++ b/preview-cloudflare/README.md @@ -50,7 +50,8 @@ To disable changed file detection, set `lectures-dir: ''`. | Output | Description | |--------|-------------| -| `deploy-url` | URL of deployed preview | +| `deploy-url` | Stable preview URL — `https://pr-{number}.{project}.pages.dev`. The branch alias, so it keeps pointing at the newest deployment as the PR gains commits. Use it for anything a human will follow | +| `deployment-url` | Immutable URL of this one deployment — `https://{hash}.{project}.pages.dev`. Pinned to the commit that produced it; use it to look at a specific revision. Empty if wrangler's output did not name it | | `changed-files` | List of changed lecture files | ## Example PR Comment diff --git a/preview-cloudflare/action.yml b/preview-cloudflare/action.yml index 9c292a0..9a5c387 100644 --- a/preview-cloudflare/action.yml +++ b/preview-cloudflare/action.yml @@ -22,8 +22,17 @@ inputs: outputs: deploy-url: - description: 'URL of the deployed preview site' + description: >- + Stable preview URL for this PR — https://pr-{number}.{project}.pages.dev. + This is the branch alias, so it keeps pointing at the newest deployment as + the PR gets more commits. Use it for anything a human will follow. value: ${{ steps.deploy.outputs.deploy-url }} + deployment-url: + description: >- + Immutable URL of this one deployment — https://{hash}.{project}.pages.dev. + Pinned to the commit that produced it, so use it to compare a specific + revision. Empty if wrangler's output did not name it. + value: ${{ steps.deploy.outputs.deployment-url }} changed-files: description: 'List of changed lecture files' value: ${{ steps.detect-changes.outputs.changed-files }} @@ -68,54 +77,79 @@ runs: env: CLOUDFLARE_API_TOKEN: ${{ inputs.cloudflare-api-token }} CLOUDFLARE_ACCOUNT_ID: ${{ inputs.cloudflare-account-id }} + # Read from env rather than interpolated into the script body, matching + # the discipline the comment step below already follows. + PR_NUMBER: ${{ github.event.pull_request.number }} + COMMIT_SHA: ${{ github.event.pull_request.head.sha }} + PROJECT_NAME: ${{ inputs.project-name }} + BUILD_DIR: ${{ inputs.build-dir }} run: | - PR_NUMBER="${{ github.event.pull_request.number }}" - COMMIT_SHA="${{ github.event.pull_request.head.sha }}" SHORT_SHA="${COMMIT_SHA:0:7}" - PROJECT_NAME="${{ inputs.project-name }}" BRANCH_ALIAS="pr-${PR_NUMBER}" - + echo "🔍 Deploying preview for PR #${PR_NUMBER} to Cloudflare Pages..." - + # Deploy to Cloudflare Pages with branch alias - DEPLOY_OUTPUT=$(wrangler pages deploy "${{ inputs.build-dir }}" \ + DEPLOY_OUTPUT=$(wrangler pages deploy "${BUILD_DIR}" \ --project-name="${PROJECT_NAME}" \ --branch="${BRANCH_ALIAS}" \ --commit-hash="${COMMIT_SHA}" \ --commit-message="PR #${PR_NUMBER} (${SHORT_SHA})" \ 2>&1) - + DEPLOY_EXIT_CODE=$? - + echo "::group::Deployment Output" echo "$DEPLOY_OUTPUT" echo "::endgroup::" - + if [ $DEPLOY_EXIT_CODE -ne 0 ]; then echo "❌ Error: Cloudflare Pages deployment failed" exit 1 fi - - # Extract the deployment URL from wrangler output - # Wrangler outputs the URL in format: "Deployment complete! Take a peek over at https://..." - DEPLOY_URL=$(echo "$DEPLOY_OUTPUT" | grep -oE 'https://[a-zA-Z0-9.-]+\.pages\.dev' | head -1) - - # If no URL found, construct it from the branch alias - if [ -z "$DEPLOY_URL" ]; then - DEPLOY_URL="https://${BRANCH_ALIAS}.${PROJECT_NAME}.pages.dev" - echo "⚠️ Could not extract URL from output, using constructed URL: $DEPLOY_URL" - fi - + + # The alias URL is CONSTRUCTED, never parsed (#14). Two reasons: + # + # 1. It is stable. The hash URL names one deployment, so every push to + # the PR produced a different link — including every per-lecture + # deep link in the comment below, which is the bigger cost. The + # alias tracks the newest deployment, so a reviewer can keep one + # tab open across the whole review. + # 2. It needs no parsing at all. `pr-N` is already alias-safe + # (Cloudflare lowercases branch names and maps non-alphanumerics to + # `-`), so the URL is fully determined by inputs we already hold. + # + # Cloudflare creates the alias for any non-production branch, and this + # step only runs on pull_request, so the branch is always `pr-N`. + DEPLOY_URL="https://${BRANCH_ALIAS}.${PROJECT_NAME}.pages.dev" + + # The per-deployment URL is still worth having — it is the only way to + # look at one specific revision — so keep it as a secondary output. + # Match a pages.dev host and drop the alias, rather than taking the + # first hit: wrangler prints both URLs in some versions, so `head -1` + # silently chose one or the other depending on the wrangler release. + DEPLOYMENT_URL=$(echo "$DEPLOY_OUTPUT" \ + | grep -oE 'https://[a-zA-Z0-9.-]+\.pages\.dev' \ + | grep -v "^https://${BRANCH_ALIAS}\." \ + | head -1 || true) + echo "deploy-url=$DEPLOY_URL" >> $GITHUB_OUTPUT - + echo "deployment-url=$DEPLOYMENT_URL" >> $GITHUB_OUTPUT + echo "✅ Deployment successful!" - echo "📍 URL: $DEPLOY_URL" + echo "📍 Preview (stable): $DEPLOY_URL" + if [ -n "$DEPLOYMENT_URL" ]; then + echo "📌 This deployment: $DEPLOYMENT_URL" + else + echo "📌 This deployment: (not named in wrangler output)" + fi - name: Comment on PR if: steps.check-trust.outputs.skip != 'true' && github.event_name == 'pull_request' && steps.deploy.outputs.deploy-url != '' uses: actions/github-script@v9 env: DEPLOY_URL: ${{ steps.deploy.outputs.deploy-url }} + DEPLOYMENT_URL: ${{ steps.deploy.outputs.deployment-url }} CHANGED_FILES: ${{ steps.detect-changes.outputs.changed-files }} LECTURES_DIR: ${{ inputs.lectures-dir }} STRIP_LECTURES_DIR: ${{ steps.detect-changes.outputs.strip-lectures-dir }} @@ -126,6 +160,7 @@ runs: // Untrusted PR data (file paths, deploy URL) is read from env, never // interpolated into the script body (avoids script injection). const deployUrl = process.env.DEPLOY_URL || ''; + const deploymentUrl = process.env.DEPLOYMENT_URL || ''; const changedFiles = process.env.CHANGED_FILES || ''; const lecturesDir = process.env.LECTURES_DIR || ''; const stripLecturesDir = process.env.STRIP_LECTURES_DIR === 'true'; @@ -169,6 +204,13 @@ runs: comment += `---\n`; comment += `
Build Info\n\n`; comment += `- **Workflow:** [${context.workflow}](https://github.com/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId})\n`; + // The preview URL above is the branch alias, which follows the newest + // deployment. Name the immutable one here too, so a reviewer who needs + // to look at exactly this commit still has a link that will not move + // under them when the next push lands. + if (deploymentUrl) { + comment += `- **This deployment:** ${deploymentUrl} (pinned to \`${shortSha}\`; the URL above always tracks the latest push)\n`; + } comment += `
\n`; // Check for existing preview comment and update it