Skip to content

fix(preview-cloudflare): link the stable pr-N alias, not the deployment hash (#14) - #131

Merged
mmcky merged 1 commit into
mainfrom
fix/14-cloudflare-alias-url
Aug 7, 2026
Merged

mmcky merged 1 commit into
mainfrom
fix/14-cloudflare-alias-url

Conversation

@mmcky

@mmcky mmcky commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Closes #14. PLAN backlog item 5.

The change

The PR comment linked the per-deployment hash URL (https://b3a5c314.{project}.pages.dev); it now links the branch alias (https://pr-{number}.{project}.pages.dev), which tracks the newest deployment.

The headline URL turned out to be the smaller half of the cost. Every per-lecture deep link in the comment was built from the same base, so all of them changed on every push too — which is what actually forces a reviewer to go hunting for the newest comment instead of keeping one tab open.

The alias is constructed, not 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. This step only runs on pull_request, so the branch is always pr-N and the alias always exists.

Worth noting the README already documented pr-{number}.{project}.pages.dev as the behaviour, in the feature list, the example comment, and a "Preview URL Structure" table. So this is the code catching up with its own docs rather than a change in intent.

A latent bug found on the way

The old extraction was grep -oE 'https://…\.pages\.dev' | head -1, which takes whichever pages.dev URL appears 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. Since the action installs wrangler@latest on every run, that could have changed under the project without any commit here.

I checked all three output shapes against both the old and new logic before committing:

wrangler output old head -1 new
hash URL only hash hash
alias line, then hash alias hash
neither (no files changed) empty empty

Keeping the pinned URL

The per-deployment URL is the only way to view one specific revision, so it is not discarded — it becomes a separate deployment-url output, selected by excluding the alias rather than by position, and is named in the comment's Build Info block with the short SHA it belongs to. deploy-url keeps its name and its "the preview URL" meaning; only its stability changes.

Also

Deploy-step inputs move from ${{ }} interpolation into env:, matching the discipline the github-script comment step in the same file already followed.

Testing

Not covered by the harness, and cannot be — no preview action is, because a real deploy needs Cloudflare credentials and a live project. Verified by YAML, bash -n and node --check on the changed script bodies, plus the extraction table above. The real check is the next PR preview on a consumer repo: the comment should show pr-N.… and the Build Info block should carry the hash URL.

🤖 Generated with Claude Code

…nt hash (#14)

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) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 7, 2026 00:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the preview-cloudflare composite action so PR comments link the stable Cloudflare Pages branch-alias URL (pr-{number}.{project}.pages.dev) rather than a per-deployment hash URL, improving reviewer experience by keeping preview/deep links consistent across pushes. It also preserves access to the immutable hash URL by exposing it as a separate output and including it in the PR comment’s Build Info section.

Changes:

  • Construct deploy-url directly from pr-{number} + project-name (no parsing), ensuring a stable preview link.
  • Add a new deployment-url output extracted from wrangler output (excluding the alias) and surface it in the PR comment Build Info block.
  • Align deploy-step variable handling with the existing pattern by moving ${{ }} interpolations into env:.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
preview-cloudflare/README.md Documents the stable deploy-url vs immutable deployment-url outputs.
preview-cloudflare/action.yml Switches deploy-url to constructed alias URL, adds deployment-url, and updates PR comment content accordingly.
CHANGELOG.md Records the fix and notes the prior wrangler-output parsing variability.

@mmcky
mmcky merged commit 0a8e818 into main Aug 7, 2026
18 checks passed
@mmcky mmcky mentioned this pull request Aug 7, 2026
mmcky added a commit that referenced this pull request Aug 7, 2026
Patch: one bug fix, no behaviour added.

The judgement call worth recording — v0.11.0 was a minor because it added an
input, and this adds an output (`deployment-url`), which is strictly the same
category of additive surface growth. Called a patch anyway on two grounds: the
changelog files the change under Fixed because the new output is incidental to
the fix rather than a new capability (it preserves information the fix would
otherwise have discarded), and `deploy-url`'s previous value was
non-deterministic — alias or hash depending on the wrangler release — so no
consumer could safely have depended on it.

Contents: preview-cloudflare links the stable pr-N alias rather than the
per-deployment hash (#14, #131).

CONTRIBUTING step 5: no @v0 workarounds outstanding — the @v0 mentions in
test-actions.yml are comments describing the structural limitation.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@mmcky
mmcky deleted the fix/14-cloudflare-alias-url branch August 7, 2026 04:05
mmcky added a commit that referenced this pull request Aug 11, 2026
Five stale facts, all overtaken by v0.11.1 (#131/#132):

- latest release read v0.11.0
- backlog item 5 (#14, the Cloudflare alias URL) was still listed as open
  although #131 shipped it; its disposition row still said "Still valid"
- the dependency policy still pinned the baseline to anaconda=2025.12,
  four months after #95 migrated both images to 2026.06
- the rollout section still showed lecture-dp and lecture-python.myst on
  @v0.8.0, contradicting the "every consumer now tracks @v0" line above it

The disposition table covered 8 issues while 21 are open, so it was no
longer usable as a status view. Adds a second table for everything opened
after the July review, including #135.

Item 3's "=2025.12" is left as-is: it narrates the misconception that
produced #86/#87, not the current baseline.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mmcky added a commit that referenced this pull request Aug 11, 2026
#137 corrected the consumers table and added release gating; this clears
what it did not touch, found while triaging all 20 open issues:

- backlog item 5 (#14, the Cloudflare alias URL) was still listed as open
  although #131 shipped it in v0.11.1; its disposition row still read
  "Still valid"
- the dependency policy still pinned the baseline to anaconda=2025.12,
  a month after #95 migrated both images to 2026.06
- rollout Phase 1 still put lecture-dp on @v0.8.0, contradicting the
  consumers table above it

The disposition table covered 8 issues while 23 are open, so it had stopped
working as a status view. Adds a second table for everything opened after
the July review.

Item 3's "=2025.12" is left alone: it narrates the misconception that
produced #86/#87, not the current baseline.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mmcky added a commit that referenced this pull request Aug 11, 2026
…ker (#139)

#137 corrected the consumers table and added release gating; this clears
what it did not touch, found while triaging all 20 open issues:

- backlog item 5 (#14, the Cloudflare alias URL) was still listed as open
  although #131 shipped it in v0.11.1; its disposition row still read
  "Still valid"
- the dependency policy still pinned the baseline to anaconda=2025.12,
  a month after #95 migrated both images to 2026.06
- rollout Phase 1 still put lecture-dp on @v0.8.0, contradicting the
  consumers table above it

The disposition table covered 8 issues while 23 are open, so it had stopped
working as a status view. Adds a second table for everything opened after
the July review.

Item 3's "=2025.12" is left alone: it narrates the misconception that
produced #86/#87, not the current baseline.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

preview-cloudflare: Use branch alias URL instead of hash URL in PR comments

2 participants