Change type
Planned — normal review → merge
Risk / impact
Low
Security impact assessed?
No — no security impact
Details — description & full context
develop has no build verification. Nothing on it is ever built until release time.
.github/workflows/deploy.yml triggers on exactly two events:
on:
push:
branches:
- main
pull_request:
So a commit gets built when it's proposed (pull_request) and again when it ships (push to main) — but never in between. The moment a PR squash-merges into develop, the resulting commit is unverified, and it stays unverified until someone cuts a release.
This is not theoretical — it just happened. When #131 merged into develop as 2444010, I went looking for the pipeline to watch and found nothing:
$ gh api repos/TurboDocx/Docs/commits/2444010/check-runs --jq '.total_count'
0
Zero check-runs on the merge commit. The first build of that commit was the production deploy 30 minutes later.
Why the gap has real teeth here: a squash-merge produces a commit that no CI run has ever seen. The PR run built the branch head; the squashed commit onto a moved-forward develop is a different tree. Anything that only breaks in that combination — two PRs that are each fine alone but conflict semantically, a lockfile that resolves differently against a newer base — surfaces for the first time at the release deploy, with several PRs stacked on top and no clean bisect.
For this repo that's a build that fans out over 105 doc pages, MDX, an OpenAPI plugin, a patch-package patch, and a git submodule. The recent zero-vulnerability sweep (#131) is a good example of the risk class: it regenerated the entire lockfile, and the only thing standing between that and production was a single PR-time build.
Proposed change — add develop to the push trigger:
on:
push:
branches:
- main
- develop
pull_request:
One line. Every merge to develop gets built, and a breakage is attributable to the single commit that caused it instead of being discovered at release with N commits to bisect.
Worth deciding during review: whether a develop push should also deploy to a Cloudflare Pages preview, or only build. The one-liner above does deploy — cloudflare/pages-action runs unconditionally in the job. If a persistent develop preview URL isn't wanted, gate the deploy step instead and keep the build:
- name: Deploy to Cloudflare Pages
if: github.ref != 'refs/heads/develop'
My recommendation is to let it deploy. A stable staging URL for develop is useful on a docs site — it's how you catch a rendering regression that a green build won't (a broken MDX component that still compiles, a mangled screenshot, a sidebar that lost its ordering). Costs nothing extra beyond the Pages preview that already runs on every PR.
Related: #133 (npm run typecheck is broken and emits .js into src/, which silently breaks the build) is the kind of defect this trigger would have caught earlier — and it's worth noting that typecheck is not in this workflow at all today. Whether to add it is a separate call, and it should not be added until #133 is fixed, since running it currently corrupts the tree.
Component / area
CI/CD — .github/workflows/deploy.yml
Testing & validation
Validation is inherent: after merging, the next push to develop should show a Deploy to Cloudflare Pages run against that commit, where gh api repos/TurboDocx/Docs/commits/<sha>/check-runs currently returns 0.
Rollback plan
Revert the merge commit — remove develop from the push trigger. No runtime state, no infrastructure change; the workflow simply stops firing on develop pushes as it does today.
Breaking change for consumers?
No
Reviewer / approver
PR review approval on the implementing PR.
Change type
Planned — normal review → merge
Risk / impact
Low
Security impact assessed?
No — no security impact
Details — description & full context
develophas no build verification. Nothing on it is ever built until release time..github/workflows/deploy.ymltriggers on exactly two events:So a commit gets built when it's proposed (
pull_request) and again when it ships (pushtomain) — but never in between. The moment a PR squash-merges intodevelop, the resulting commit is unverified, and it stays unverified until someone cuts a release.This is not theoretical — it just happened. When #131 merged into
developas2444010, I went looking for the pipeline to watch and found nothing:Zero check-runs on the merge commit. The first build of that commit was the production deploy 30 minutes later.
Why the gap has real teeth here: a squash-merge produces a commit that no CI run has ever seen. The PR run built the branch head; the squashed commit onto a moved-forward
developis a different tree. Anything that only breaks in that combination — two PRs that are each fine alone but conflict semantically, a lockfile that resolves differently against a newer base — surfaces for the first time at the release deploy, with several PRs stacked on top and no clean bisect.For this repo that's a build that fans out over 105 doc pages, MDX, an OpenAPI plugin, a
patch-packagepatch, and a git submodule. The recent zero-vulnerability sweep (#131) is a good example of the risk class: it regenerated the entire lockfile, and the only thing standing between that and production was a single PR-time build.Proposed change — add
developto the push trigger:One line. Every merge to
developgets built, and a breakage is attributable to the single commit that caused it instead of being discovered at release with N commits to bisect.Worth deciding during review: whether a
developpush should also deploy to a Cloudflare Pages preview, or only build. The one-liner above does deploy —cloudflare/pages-actionruns unconditionally in the job. If a persistentdeveloppreview URL isn't wanted, gate the deploy step instead and keep the build:My recommendation is to let it deploy. A stable staging URL for
developis useful on a docs site — it's how you catch a rendering regression that a green build won't (a broken MDX component that still compiles, a mangled screenshot, a sidebar that lost its ordering). Costs nothing extra beyond the Pages preview that already runs on every PR.Related: #133 (
npm run typecheckis broken and emits.jsintosrc/, which silently breaks the build) is the kind of defect this trigger would have caught earlier — and it's worth noting thattypecheckis not in this workflow at all today. Whether to add it is a separate call, and it should not be added until #133 is fixed, since running it currently corrupts the tree.Component / area
CI/CD —
.github/workflows/deploy.ymlTesting & validation
Validation is inherent: after merging, the next push to
developshould show aDeploy to Cloudflare Pagesrun against that commit, wheregh api repos/TurboDocx/Docs/commits/<sha>/check-runscurrently returns0.Rollback plan
Revert the merge commit — remove
developfrom the push trigger. No runtime state, no infrastructure change; the workflow simply stops firing ondeveloppushes as it does today.Breaking change for consumers?
No
Reviewer / approver
PR review approval on the implementing PR.