Uh oh!
There was an error while loading. Please reload this page.
ci: build the website on pull requests, deploy only on main - #38
Conversation
www.yml only triggered on push to main, so the first execution of any change to the site or its toolchain was the one that deployed it to production. Not theoretical: #12 bumped actions/setup-node v5 -> v7 and edited www.yml itself. Every check on that PR was green and not one of them had run this job — merging it would have run setup-node@v7 for the first time ever, straight to prod. Adds a pull_request trigger on the same path filter, splits the single "build & deploy" step in two, and gates the deploy on the event not being a pull_request. The build needs no Cloudflare credentials — verified by running `opennextjs-cloudflare build` with both unset — so the PR job runs without secrets, which is also the right posture for fork PRs. `deploy` acts on an already-built app ("Deploy a *built* OpenNext app"), so splitting the steps doesn't build twice. Two knock-on fixes: * concurrency is now keyed by ref. It was a single global "deploy-www" group with cancel-in-progress, so a PR build would have cancelled an in-flight deploy of main. * the job was named "Build & deploy to Cloudflare", which is a lie on a PR. Renamed rather than made conditional: a check name that varies by event is useless as a required status check (#21). `npm run deploy` is unchanged for local use; the workflow uses the new build:worker script plus a bare deploy. Closes#16
z33b0t
commented
Jul 29, 2026
@z33b0t ready to merge. All 7 checks green — including the new website build, which ran on this PR itself. That's the self-validating bit: a Suggest merging this before #34 (the CSP headers PR). #34 currently has no website build on it at all, for exactly the reason this PR fixes; merging this first and rebasing #34 gets that change a real build before it deploys. Two knock-ons flagged in the body: |
z33b0t
commented
Jul 29, 2026
@z33b0t Green and ready, but I can't merge this one — my Either merge it yourself, or grant the scope and I'll finish: (Oddly, #30 and #37 modified workflows and merged fine before this started refusing — so the restriction isn't applying consistently. Worth knowing in case you see it again.) |
Dependabot proposed 7.0.2 (#43), which does not build. Next 16 rejects it: TypeScript 7.0.2 does not provide the compiler API required by Next.js. Enable experimental.useTypeScriptCli in your Next.js config to use the TypeScript CLI, or install TypeScript 6 instead. Worth being precise about what's wrong, because the obvious reading is wrong: `tsc --noEmit` passes clean on 7.0.2, so the source is not the problem. TS 7 is the native port and drops the JS compiler API that `next build` links against for its type-check pass. Nothing in this repo can fix that from our side short of opting into experimental.useTypeScriptCli. 6.0.3 is the newest version Next accepts, and it builds and type-checks clean. Verified: `tsc --noEmit` clean, and a full `opennextjs-cloudflare build` succeeds. Note none of this is checked by CI yet — www.yml has no pull_request trigger until #38 lands (#16), which is exactly how a broken toolchain bump would otherwise have reached production unnoticed. Closes#43 Co-authored-by: z33b0t <z33b0t@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
www.ymlhad nopull_requesttrigger, so the first execution of any change to the site or its toolchain was the one that deployed it. This PR is its own demonstration: it touches.github/workflows/www.yml, so the new job should appear on this PR — the first time the website build has ever run pre-merge.The issue's open question, answered
It does succeed. Ran it locally with both explicitly unset (
env -u …) — clean build,Worker saved in .open-next/worker.js. So the PR job needs no secrets at all, which is the right posture for fork PRs too. No dummy values needed.Also confirmed
deploydoesn't rebuild — its own help says "Deploy a built OpenNext app" — so splitting build and deploy into two steps doesn't build twice.Two knock-on fixes I'd flag
concurrencywas a footgun. It was a single globaldeploy-wwwgroup withcancel-in-progress: true. Adding a PR trigger without touching it would mean a PR build cancelling an in-flight production deploy ofmain. Now keyed bygithub.ref.Build & deploy to Cloudflareisn't what happens on a PR. I renamed it toBuild website (deploys to Cloudflare on main)rather than making it conditional — a check name that varies by event can't be used as a required status check, which matters for main ruleset requires a PR but not passing status checks #21.Note the rename means this check appears under a new name; if you've got anything pinned to the old one, it'll need updating.
Related
This is also why the CSP work in #34 shows only 4 checks — the site build never ran on it. Merging this first and rebasing #34 would get that PR a real build.
Closes#16