fix(release): restore build before publish and the alpha dist-tag - #650
Conversation
v4.5.1 was published with only package.json, README.md and LICENSE. The package has no JS in it, so every consumer build fails on import while main, module, types and exports all point into dist/. The publish workflow dropped its `npm run build` step when it was rewritten on 15 Sept. Nothing else builds before publish: there is no prepack or prepublishOnly script, and with no `files` field npm falls back to .gitignore, which excludes dist. No build means an empty package. Hooking the build to prepack fixes it for every publish path rather than just the one workflow, so a later workflow edit cannot silently drop it again. A failing build now fails the publish instead of shipping nothing. Co-Authored-By: Claude <noreply@anthropic.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
🔒 Security Scan Results
⏱️ SLA Breach Summary
✅ BUILD PASSED - All security checks passed |
There was a problem hiding this comment.
🟢 Approval recommended
The only review comment is a non-blocking documentation nit.
Pull request overview
Adds an npm prepack hook to build dist before packaging or publishing.
Changes:
- Runs
npm run buildduringnpm packandnpm publish.
File summaries
| File | Description |
|---|---|
package.json |
Adds the prepack lifecycle script. |
Review details
Suppressed comments (1)
package.json:34
- The PR description says there is no
filesfield and that npm therefore falls back to.gitignore, but this package already declares"files": ["dist"]atpackage.json:22-24. The empty tarball is explained bydistnot being built; please correct the rationale so the diagnosis remains accurate.
"prepack": "npm run build",
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Coverage Report
File CoverageNo changed files found. |
hitesh-shetty-cstk
left a comment
hitesh-shetty-cstk
left a comment
There was a problem hiding this comment.
Automated review
Critical journey at risk: publishing the SDK to npm. The failure a consumer sees is npm install succeeding and the import failing, because the package contains no JavaScript.
What this changes: Adds a prepack script so npm run build runs before the tarball is packed. The publish workflow no longer runs a build step, so npm publish packed whatever was on disk, and nothing was.
Business impact: This is the published SDK, so the blast radius is every project that installs it. The public registry currently reports 4.5.1 at 3 files and 12,263 bytes, against 1,907 files and 7.8 MB for 4.5.0, and latest still resolves to 4.5.1. Anyone installing or upgrading fails at import, because main, module, types and exports all resolve into dist/. This change fixes the publish path for the next version. It does nothing for the version already on the registry, which the follow-up list in the description covers.
Security: nothing beyond what the scanners cover. One observation about the surrounding publish job, which this PR does not introduce or change: it runs npm install -g npm@latest at release time in a job holding id-token: write, so an unpinned tool version sits in the provenance path. Worth pinning in its own change.
Flow
flowchart TD
A["Release published"] --> B["checkout at tag"]
B --> C["npm ci"]
C --> D["npm publish --provenance"]
D --> E["prepack: npm run build"]:::changed
E --> F["tsup writes dist/modern and dist/legacy"]
F --> G["pack: files field selects dist"]
G --> H["tarball, about 1900 files"]
D -.->|"before this change"| I["no prepack, dist absent"]
I -.-> J["tarball, 3 files, 12.3 kB"]
classDef changed fill:#fff3cd,stroke:#d39e00
Findings: 0 blocker, 1 should fix, 1 nit. Both are inline on package.json:34. The fix itself is the right shape: prepack covers the publish and publish-alpha jobs and a local npm pack, and neither job currently builds.
Reviewer candidates:
- @csAyushDubey authored 9 of the last 30 commits touching
package.json, though all of them are dependency-upgrade merges from December 2025. - @hitesh-shetty-cstk authored 8 of the last 30, including the
4.4.5,4.4.2,4.4.1and4.2.0release commits. - @Aravind-Kumar-cstk is not requested here, but wrote both 15 September commits to
.github/workflows/npm-publish.yml, which is the other half of this publish path. Worth a walkthrough before merge, given what is at risk.
Not covered: I did not run the build, the tests, or a publish. I checked the tarball figures against the public npm registry (4.4.5 at 1,895 files, 4.5.0 at 1,907, 4.5.1 at 3 files and 12,263 bytes), and confirmed from repository history that develop_v4 and stage_v4 are both ancestors of main, so the branch note at the end of the description holds. What I could not check is whether npm run build succeeds on node 24, which is the nit above.
Automated review by Claude Code. A human review is still required.
Generated by Claude Code
When npm-alpha-publish.yml was folded into this workflow the publish-alpha job came out identical to publish, losing the `--tag alpha` the old workflow had. An alpha release would move the `latest` dist-tag to the prerelease and hand it to every consumer on a caret range. This also restores the alpha channel as a rehearsal for a real publish: cut a prerelease, install it, confirm it resolves, then cut the release, all without touching `latest`. Co-Authored-By: Claude <noreply@anthropic.com>
kirtesh-cstk
changed the title
🔒 Security Scan Results
⏱️ SLA Breach Summary
✅ BUILD PASSED - All security checks passed |
A version tag is mutable, so the action owner or anyone who compromises that account can change what runs in CI without the ref changing here. Pinning to the commit sha behind v2.13.0 keeps the same code and removes that path. Co-Authored-By: Claude <noreply@anthropic.com>
🔒 Security Scan Results
⏱️ SLA Breach Summary
✅ BUILD PASSED - All security checks passed |
Two fixes for the npm publish path, both regressions from the workflow rewrite on 15 Sept.
1.
dist/is missing from the published package4.5.1on npm contains onlypackage.json,README.mdandLICENSE. There is no JS in it, even thoughmain,module,typesandexportsall point intodist/. Anyone who upgrades gets a build failure on import, in any project type. It is not limited to one hosting platform.The publish workflow lost its
npm run buildstep when it was rewritten:Nothing else covers the gap. There is no
prepackorprepublishOnlyscript, sonpm publishnever builds on its own.package.jsondeclares"files": ["dist"], so with nodist/on disk that pattern matches nothing and the tarball is left with only the three files npm always includes regardless. No build means an empty package.The node and npm versions above are the fingerprint: the new workflow pins node 24 and adds
npm install -g npm@latest, the old one pinned node 18.4.5.1was published 12 minutes after that workflow commit, from the same commit that ismaintoday.Fix: one script in
package.json.Rather than restoring the workflow step, because
prepackcovers every publish path including a localnpm publishornpm pack, a later workflow edit cannot silently drop it, and a failing build now fails the publish instead of quietly shipping nothing. It also covers the alpha job below. Restoring the step in the workflow as well would just build twice.2. Alpha releases would overwrite the
latestdist-tagWhen
npm-alpha-publish.ymlwas deleted and folded into this workflow, thepublish-alphajob came out byte-identical topublish. It lost the--tag alphathe standalone workflow had:So a prerelease would move
latestand be handed to every consumer on a caret range. Fix:--tag alphaback on the alpha job.This also restores the alpha channel as a rehearsal for a real publish: cut a prerelease, install it, confirm it resolves, then cut the release, without
latestever moving.Verification
Clean
npm cion this branch, thennpm pack --dry-run:The second figure reproduces the published
4.5.1exactly (3 files, 12263 bytes), which confirms the diagnosis and not only the fix. The first matches4.5.0, anddist/legacy/index.cjs,index.jsandindex.d.tsare all present.The build itself was confirmed on node 24, which no release has used before (
4.5.0built on node 18, and the current workflow builds not at all).tsup.config.js:3usesimport packageJson from './package.json' assert { type: "json" }, which node 24 rejects on its own (SyntaxError: Unexpected identifier 'assert'), but tsup bundles its config through esbuild before evaluating it, so the clause never reaches node's parser.npx tsupon node 24 finishes withBuild successand writesdist/modern/index.jsanddist/legacy/index.cjs.Also exercised in a
node:24container withnpm install -g npm@latest(npm 12.0.2, the same npm that published4.5.1) and a cleannpm cifrom the lockfile:prepackfires and the build runs. npm 12 gates install scripts by default, which does not affect esbuild here because its platform binary ships as its own package. The declaration step was OOM-killed in that container at 8 GB, which is the container limit and not CI. The build asks for a 16 GB heap andubuntu-latesthas 16 GB, which is how4.5.0built. The native run completed the full build including declarations.Follow-ups, not in this PR
4.5.2-alpha.0first as a rehearsal of the real workflow, then4.5.2.4.5.1cannot be republished.4.5.1on npm pointing at4.5.2, so nobody else lands on it.latestcurrently resolves to the broken version.4.5.0. A caret range still resolves to4.5.1. Nothing is lost by doing so: the only change in4.5.1was a dompurify bump, and4.5.0already depends on^3.4.12, which installs the patched release.develop_v4andstage_v4are both strictly behindmain, so they pick both fixes up on their next sync and need no separate change.🤖 Generated with Claude Code