Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Bundle Size ReportComparing against baseline from No bundle size changes detected. |
Unlighthouse Performance Comparison — VercelComparing PR preview deployment Unlighthouse scores vs production Unlighthouse scores. Summary ScoreAggregate score across all categories as reported by Unlighthouse.
Category Scores
Core Web Vitals
|
376590d to
3637342
Compare
3637342 to
a514fe5
Compare
|
@jorgemoya How can we verify that a wrangler version bump won't break anything? |
@jordanarldt Have included a build step to validate the build. Where you thinking of something else we need to validate? |
|
@jorgemoya I'm mainly thinking of the deploy pipeline. Like if we update the wrangler version and that somehow breaks the deployment process on native hosting, or makes the storefront stop working. Is that possible to happen? |
Anything is possible 😅 . I consider this rare/low risk, but yes it could happen. Maybe we manually verify the preview deployment you added succeeds before we merge it in? |
|
@jordanarldt Added a step to preview a deployment so we can manually test as well. |
Commerce Hosting builds rest on two versions that live as string literals in the CLI source rather than as manifest entries — `WRANGLER_VERSION`, which is interpolated into `pnpm dlx wrangler@<version>`, and `OPENNEXT_CLOUDFLARE_VERSION`, which is what new projects get pinned to. Dependabot can see neither, and nothing else watched them, so they only moved when someone remembered. One weekly job now maintains both in a single PR. They are not two jobs on purpose: the adapter declares which Wrangler it supports, so Wrangler is pinned to the newest release that adapter allows rather than the newest that exists. Bumping either alone is how they drift out of a supported pair, and separate jobs on separate rolling branches could not see each other's pending bump — an adapter release that raised its Wrangler floor would leave `catalyst build` running a Wrangler the adapter never supported. The job holds both pins, and reports why, rather than guess: when the adapter moves its `next` requirement (judging whether core satisfies a new range needs judgment, and `reconcileOpenNextVersion` refuses to apply a pin core cannot meet), when it needs a Wrangler major this job does not track, or when the current pin sits above a new adapter ceiling. `cloudflare-context-symbol.spec.ts` is why the adapter pin needs more care than a version string. It calls the real `getCloudflareContext()` to prove the symbol key core reads is the one the adapter writes; a release that changed it would leave every native-hosted store silently on an in-process cache. That test exercises whichever adapter is installed, so it only says something about the pin while the two agree — and nothing made them agree. It now asserts that they do, replacing a hand-maintained literal that a lockfile bump could leave behind. Holding that invariant means the peer range has to track the pin, so it moves from `^1.17.3` to `^1.20.6`; the old floor was one nothing targeted or verified. The job runs that spec against the new adapter before opening a PR, which is the only place a bump gets verified, since GitHub will not run checks on a PR pushed with `GITHUB_TOKEN`. `semver` is added at the root, used only to read the adapter's Wrangler peer range. Also runs `test:scripts` in CI. It was defined but invoked by no workflow, so the 139 existing script tests gated nothing. Refs LTRAC-1328 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The unit suites mock Wrangler and OpenNext, so nothing in the job could tell whether new pins actually build. Combined with GitHub declining to run checks on a PR pushed with `GITHUB_TOKEN`, a release that broke the real build would have produced a green bump PR whose first genuine test was `native-hosting.yml` after merge to canary. The job now stands up a real native-hosting project against the new pins and completes the true pipeline — `opennextjs-cloudflare build` followed by `wrangler deploy --dry-run`, which bundles and validates without deploying — before the PR is opened. Two details this depends on. The CLI is built from the branch's own source rather than installed from npm: the published package carries its own pins and would verify nothing about the bump. And `catalyst build` dispatches on project state, so the upstream tree has to be transformed first (middleware.ts swapped in, adapter dependency added) or it falls through to `next build` and never touches either tool. The bump is committed before the build runs and pushed only once it passes, because standing up that project rewrites `core/` and the lockfile, none of which belongs in the bump. Refs LTRAC-1328 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The contract spec in `cloudflare-context-symbol.spec.ts` calls the real `getCloudflareContext()` against whichever `@opennextjs/cloudflare` is installed, so it only says something about OPENNEXT_CLOUDFLARE_VERSION while the installed adapter and the pin agree. Making them agree by raising the peer range worked, but said the wrong thing to consumers: the range is the merchant-facing declaration, and `reconcileOpenNextVersion` deliberately tolerates a project sitting on an older adapter — it offers the upgrade and carries on when declined. A raised floor contradicted that, handing those projects an unmet-peer warning for a setup the CLI still supports. The installed version is now fixed by an exact devDependency instead, and the peer range goes back to the tolerant `^1.17.3` it was. devDependencies are not installed by consumers, so the contract test gets the guarantee it needs and nothing merchant-facing changes at all — which is also why the changeset this replaces is gone. Exact rather than a caret on purpose: a range would let a new 1.x release move the installed version on its own and fail CI before anyone had chosen to adopt it. This does put the adapter in Dependabot's scope for the first time, since it is now a real dependency entry. A Dependabot bump would desync it from the pin and redden the contract test weekly, so it is added to the ignore list — this job owns that version. The job only syncs the devDependency when the adapter version moves, so it will not self-heal a divergence introduced by hand. That is deliberate: the contract spec runs in `CLI Tests` on every PR and fails in either direction, so such a divergence is immediate and loud rather than silent. Refs LTRAC-1328 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A build proves the pins compile and bundle; it does not prove the worker deploys and serves. The job now deploys the bundle it just verified and puts the URL in the PR body, so a bump can be clicked through before it is merged. The merchant-facing preview system is not reusable here. Its action handles only `pull_request` and `issue_comment` — it skips any other event outright — and neither fires for a PR pushed with `GITHUB_TOKEN`. Nothing at the repo root calls that reusable workflow either; `core/.github/workflows/preview-deployment.yml` is the template shipped to merchants, and GitHub only reads workflows from the root. So the deploy happens in the run that already holds the bundle. It uses `--prebuilt`, so what gets deployed is exactly what was verified rather than a rebuild, and it deploys into a project of its own rather than the one `native-hosting.yml` publishes canary to — a weekly bump must not displace that storefront. `BUMP_PREVIEW_PROJECT_UUID` is not set yet, and the step skips with a notice rather than failing when it is absent, so the job keeps working until someone creates the project. The PR body says which of the two happened instead of implying a preview exists. Refs LTRAC-1328 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A preview is evidence about a bump, not a precondition for proposing one, and a failure is as likely to be the preview environment as the pins. Losing the whole PR to it meant losing the build result and the contract check with it. The deploy is now `continue-on-error`, so the PR opens regardless and its body says which of four things happened: deployed with a URL, no preview project configured, deployed but the URL could not be read back, or the deploy failed with a link to the run. A failure reads as something to confirm before merging rather than as a verdict on the bump. The run is still failed afterwards, once the PR exists, so a broken preview is not reported as green. Refs LTRAC-1328 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
896cebb to
9158427
Compare
…face holds Two problems an adversarial review found. The job installs and executes third-party code — dependency lifecycle scripts, an OpenNext build, and, most sharply, an adapter release picked off npm that nobody has reviewed yet, which is the entire point of the job. All of it ran with the checkout's push-capable credential sitting in `.git/config`, so a malicious release would not have needed to break the build to reach the repository's refs; the `wrangler --dry-run` that protects Cloudflare does nothing for that. The checkout now uses `persist-credentials: false` and the token is injected only at the push itself. Separately, a hold reported itself into a job summary on a successful run. The cases that need a human — a moved Next requirement, a Wrangler range no tracked major satisfies — were therefore indistinguishable from "nothing to do", and could have stalled the pins for months behind a green weekly run. Holds now emit a `blocked` output with the reason, and the run fails on it once the PR step has been skipped, so the Actions list and GitHub's scheduled-failure notification both carry it. `setOutput` also collapses newlines now. Every value is single-line by construction, but a multi-line one would corrupt the `key=value` file and could forge a second output. Refs LTRAC-1328 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Linear: LTRAC-1328
Supersedes #3233, which automated the Wrangler pin alone.
What/Why?
Commerce Hosting builds rest on two versions that live as string literals in the CLI source rather than as manifest entries, so Dependabot can't see either and neither moved unless someone remembered:
WRANGLER_VERSIONincommands/build.ts, interpolated intopnpm dlx wrangler@<version>OPENNEXT_CLOUDFLARE_VERSIONinlib/commerce-hosting.ts, what new projects get pinned toOne weekly job now maintains both, in one PR. They're deliberately not two jobs:
@opennextjs/cloudflaredeclares which Wrangler it supports (wrangler: ^4.125.0today), so Wrangler is pinned to the newest release the pinned adapter allows, not the newest that exists. Bumping either in isolation is how they drift out of a supported pair — with separate jobs on separate branches, neither can see the other's pending bump, and an adapter release that raised its Wrangler floor would leavecatalyst buildrunning a Wrangler the adapter doesn't support.A useful side effect of the adapter being the authority: the job is self-healing. If the pins are already mismatched, the next run reconciles Wrangler into the adapter's range rather than just reporting it.
What holds a bump
The job is deliberately conservative — it holds both pins and reports why, rather than guessing:
peerDependencies.nextnextsatisfies a new range needs judgment. Shipping a pin core can't meet is worse than not bumping:reconcileOpenNextVersionrefuses to apply it and tells the merchant to upgrade core first.Even a cosmetically reworded
nextrange holds the bump, because deciding two ranges are equivalent needs range semantics the job doesn't apply to that question. There's a test asserting exactly that.Why the adapter is now a devDependency
cloudflare-context-symbol.spec.tsis the reason the adapter pin needs more care than a version string.corereads the Cloudflare context offSymbol.for('__cloudflare-context__')— an internal detail of the adapter — and the spec calls the realgetCloudflareContext()to prove that key is still the one the adapter writes. If a release changed it, every native-hosted store would fall back to an in-process cache with no error and no signal.That spec exercises whichever adapter is installed, so it only says something about the pin while the two agree — and nothing made them agree. It now asserts that they do, replacing a hand-maintained
expect(OPENNEXT_CLOUDFLARE_VERSION).toBe('1.20.6')literal that a lockfile bump could silently leave behind. Verified it bites: with the constant at1.21.0it fails withexpected '1.20.6' to be '1.21.0'.What fixes the installed version is an exact devDependency on
packages/catalyst, which the job moves in lockstep with the pin. The optionalpeerDependenciesrange stays at the tolerant^1.17.3it has always been.That split matters. An earlier revision of this PR raised the peer range instead, which worked but said the wrong thing: the peer range is the merchant-facing declaration, and
reconcileOpenNextVersiondeliberately tolerates a project on an older adapter — it offers the upgrade and carries on when declined. Raising the floor contradicted that and handed those projects an unmet-peer warning for a setup the CLI still supports. devDependencies aren't installed by consumers, so this version gets the guarantee the spec needs while nothing merchant-facing changes at all — which is why this PR carries no changeset.Exact rather than a caret on purpose: a range would let a new 1.x release move the installed version by itself and fail CI before anyone had chosen to adopt it.
One knock-on: this is the adapter's first appearance as a real dependency entry, so it enters Dependabot's scope. A Dependabot bump would desync it from the pin and redden the contract test every Monday, so it's added to the ignore list — this job owns that version.
The job syncs the devDependency only when the adapter version moves, so it won't self-heal a divergence introduced by hand. Deliberate: the spec runs in
CLI Testson every PR and fails in either direction, so such a divergence is loud and immediate rather than silent.Proving a bump actually builds
The unit suites mock Wrangler and OpenNext, so on their own they cannot say whether new pins build. Combined with GitHub declining to run checks on a PR pushed with
GITHUB_TOKEN, a release that broke the real build would have produced a green bump PR whose first genuine test wasnative-hosting.ymlafter merge.So before opening a PR, the job stands up a real native-hosting project against the new pins and completes the true pipeline:
opennextjs-cloudflare buildfollowed bywrangler deploy --dry-run, which bundles and validates without deploying anything.Two details that make this real rather than decorative:
@bigcommerce/catalystcarries its own pins, so installing it would verify nothing about the bump — the trapnative-hosting.ymlwould fall into, since it installs@bigcommerce/catalyst@alpha.core/has to be transformed first (middleware.ts swapped in, adapter dependency added).catalyst builddispatches on project state, so without that it falls through tonext buildand never touches either tool.The bump is committed before the build runs and pushed only once it passes, because standing up that project rewrites
core/and the lockfile — neither of which belongs in the bump.And that it actually deploys
A build proves the pins compile and bundle; it does not prove the worker deploys and serves. So the job also deploys the bundle it just verified — with
--prebuilt, so what ships is exactly what was tested rather than a rebuild — and puts the URL in the generated PR body.The merchant-facing preview system can't be reused for this, for two independent reasons:
deployment-preview-actionhandles onlypull_requestandissue_comment, and skips any other event outright. Neither fires for a PR pushed withGITHUB_TOKEN.core/.github/workflows/preview-deployment.ymlis the template shipped to merchants, and GitHub only reads workflows from the repo root — so Catalyst's own PRs get Vercel previews, not native-hosting ones.It deploys into a project of its own, never the one
native-hosting.ymlpublishes canary to, so a weekly bump can't displace that storefront.BUMP_PREVIEW_PROJECT_UUIDis configured as a repository secret; everything else reuses the existingNATIVE_HOSTING_*store hash, tokens and auth secret. Previews share one project and serve one deployment at a time, so each weekly bump replaces the previous preview.The deploy is non-blocking. A preview is evidence about a bump, not a precondition for proposing one, and a failure is as likely to be the preview environment as the pins — losing the PR to it would throw away the build result and the contract check too. So the PR always opens, and its body states which of four things happened:
The run is still marked failed after the PR exists, so a broken preview isn't reported as green.
Credentials and hold visibility
Two things an adversarial review caught, both fixed here.
The job runs untrusted code, so it holds no write credential while doing it. It installs dependencies, runs lifecycle scripts, and builds — and the whole point is to install an adapter release off npm that nobody has reviewed yet. All of that previously ran with the checkout's push-capable credential in
.git/config, so a malicious release wouldn't have needed to break the build to reach the repo's refs;wrangler --dry-runprotects Cloudflare, not GitHub. The checkout now setspersist-credentials: false, and the token is injected only at the push.A hold is no longer indistinguishable from a no-op. Holds previously reported into a job summary on a successful run, so the cases that need a human — a moved Next requirement, a Wrangler range no tracked major satisfies — could have stalled the pins for months behind a green weekly run, which is the exact failure this job exists to prevent. Holds now emit a
blockedoutput with the reason and fail the run after the PR step is skipped, so the Actions list and GitHub's scheduled-failure notification both carry it.setOutputalso collapses newlines: every value is single-line by construction, but a multi-line one would corrupt thekey=valuefile and could forge a second output.Also
semveris added at the root, used only to read the adapter's Wrangler peer range. Two earlier attempts were measured and rejected:pnpm updatere-resolved 315 lines of unrelated lockfile tree, and a plainpnpm installafter addingsemverdragged ineslint-plugin-importand friends.--lockfile-onlykeeps it to the lines here.test:scriptsnow runs in CI. It was defined in the rootpackage.jsonbut invoked by no workflow, so 139 existing script tests gated nothing. Added as a step tobasic.yml's existinglint-typecheckjob — the job ID and display name are untouched, since renaming a required check would break branch protection.workflow_dispatchneeds the workflow on the default branch, so dispatch it once after merge; as of writing that produces a Wrangler-only bump to4.136.2(the adapter is already current at1.20.6).Testing
pnpm test:scripts— 178 pass (139 existing + 39 new). Full CLI suite green (46 files, 790 tests). Lint and typecheck clean. Rebased on canary and re-verified, includingpnpm install --frozen-lockfileto confirm the merged lockfile is genuinely in sync.A real native-hosting build was run on this branch, using the CLI built from this source, to confirm the pins still build:
This PR moves neither pin, so that build exercises the same
1.20.6/4.128.0pair canary already ships — it confirms no regression rather than validating a new combination. Validating new combinations is the build gate's job, on each generated PR.The devDependency approach was verified rather than assumed: it resolves to a single importer entry under
devDependencies(no duplicate alongside the optional peer), installs1.20.6, and produces a smaller lockfile diff than the peer-range approach it replaces. The contract spec passes against it.Script paths exercised against the live npm registry:
4.128.0 → 4.136.2, correctly bounded by the adapter's^4.125.0, adapter left at1.20.6, changeset naming only the pin that moved.1.19.0: moved to1.20.6, rewrote the constant and the devDependency together, left the peer range andpeerDependenciesMetauntouched, Next requirement recognised as unchanged.nextrange altered: refused the bump, wrote a> [!WARNING]job summary, touched no files.The preview step was verified without deploying: the PR body renders correctly both with and without a preview URL, and the URL extraction recovers
https://project-30ea75ac.mybigcommerce.comfrom realistic colorized CLI output. It usesperlrather thansedbecause\eisn't portable in BSDsed, and the no-URL path survivesset -euo pipefail.Workflow YAML parses and its embedded shell passes
bash -n. The git/ghplumbing, the build gate and the preview deploy can't run until the workflow is oncanary, so the firstworkflow_dispatchafter merge is what exercises those.Migration
None. No changeset and no consumer-visible change: the only manifest edits are a devDependency and the root
semverdevDependency, neither of which reaches consumers of@bigcommerce/catalyst.🤖 Generated with Claude Code