ci: the PR gate never built, and never loaded what it built - #79
Open
VickyXAI wants to merge 1 commit into
Open
Conversation
#77 landed the pull-request gate on main, which is the important half — until then publish.yml was the only thing running build, typecheck and test, and only on push to main, i.e. after a merge. But the gate it landed runs typecheck, test and brand-numbers. It never builds, and never loads the thing it builds. Build is added because publish.yml runs it alongside typecheck and test, and this gate exists to move that same trio earlier — the header comment says as much. Without it a tsup/dts break still only surfaces on main, mid-release. A CLI load smoke is added because 0.32.3 shipped with sharp imported at the top level. sharp is an optionalDependency, so wherever it is absent — musl, unusual arch, offline CI, --no-optional — ESM resolution failed before main() ran and all 19 tools went with it. Build, typecheck and test were green the whole time. Loading the built entrypoint is the only step that catches that class, and it is the class this repo has actually shipped. The smoke deletes node_modules/sharp rather than installing with --omit=optional. That was tried first and fails for an unrelated reason: it also strips rollup's platform binary, so tsup cannot run at all — a failure of the build toolchain, not of the package under test, and a permanent false red. Verified by replaying the whole sequence against the current main baseline in a clean worktree: npm ci, typecheck, build, 254/254 tests, brand-numbers up to date, and the sharp-absent smoke returning the package.json version.
VickyXAI
force-pushed
the
ci/gate-pull-requests
branch
from
July 26, 2026 05:38
41d5bda to
ed655b6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#77 landed the pull-request gate on main, which was the important half — until
then
publish.ymlwas the only thing running build, typecheck and test, and onlyon push to main, i.e. after a merge. (That gap was live earlier today: #78
was opened against main and no check ran at all.)
But the gate that landed runs typecheck, test and brand-numbers. It never
builds, and it never loads what it builds.
buildpublish.ymlrunsnpm run buildalongside typecheck and test, and this gateexists to move that same trio earlier — the header comment in the workflow says
as much. Without it, a tsup/dts break still only surfaces on main, mid-release,
which is the exact failure mode the gate was written to prevent.
CLI load smoke
0.32.3 shipped with
sharpimported at the top level.sharpis anoptionalDependency, so wherever it is absent — musl, unusual arch, offline CI,--no-optional— ESM resolution failed beforemain()ran and all 19 tools wentwith it. Build, typecheck and test were green the entire time. Loading the
built entrypoint is the only step that catches that class — and it is a class
this repo has actually shipped, not a hypothetical.
The smoke deletes
node_modules/sharprather than installing with--omit=optional. That was tried first and fails for an unrelated reason: italso strips rollup's platform binary (
@rollup/rollup-darwin-arm64), sotsupcannot run at all. That is a failure of the build toolchain rather than of the
package under test, and it would have made the step a permanent false red.
Verification
The whole sequence was replayed against the current main baseline in a clean
worktree before this was opened — shipping an unrun CI config is precisely what
this gate exists to prevent:
npm cinpm run typechecknpm run buildnpm testsync-brand-numbers.mjs --checkpackage.jsonversion, exit 0Rebased onto main after #77 merged; the brand-numbers step is kept as-is, so
this is purely additive.