fix(ci): ship the napi loader as a build artifact - #86
Open
jamesyong-42 wants to merge 1 commit into
Open
Conversation
Regression from #85, caught by the release gate before anything reached npm. Gitignoring crates/spaghetti-napi/index.js assumed every consumer builds first. Two in napi-build.yml do not: - `Test bindings` checks out, downloads the .node, and require()s ./index.js. All 6 targets failed with MODULE_NOT_FOUND, which skipped publish and aborted the release — working as designed. - `Publish to npm` also never builds. That one would have failed silently: `files` in package.json skips entries missing on disk, so it would have published a package whose `main` points at nothing. napi has no loader-only generator — `napi build` is the sole producer and needs the Rust toolchain — so the loader now travels with the binary it belongs to. The build job uploads it beside the .node (same path prefix, so the archive root is unchanged), the test job picks it up from the artifact download, and the publish job lifts it out before packing. This is strictly better than committing it: the published loader now carries the released version in its per-platform guards. The committed copy never did, which is why history has a manual "regenerate loader for 0.5.20" chore. Two guards, since the failure mode is silent: the publish job hard-fails if no loader is in the artifacts, then require()s the assembled package before publishing. ubuntu-latest matches the linux-x64-gnu binary that `napi artifacts` places next to it, so that exercises the real pair. Corrects RELEASING.md, which claimed every publish path builds first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Fixes the 0.5.22 release failure. Regression from #85, caught by the release gate — nothing reached npm.
What broke
Gitignoring
crates/spaghetti-napi/index.jsassumed every consumer builds first. Two jobs innapi-build.ymldon't:Test bindingschecks out, downloads the.node, andrequire()s./index.js. All 6 targets failed withMODULE_NOT_FOUND, which skipped publish and aborted the release. Working exactly as designed.Publish to npmalso never builds. That one would have failed silently:filesinpackage.jsonskips entries missing on disk, so it would have published a package whosemainpoints at a file that isn't in it.The Test gate stopped it before the publish job ran. All three packages are still on 0.5.21.
The fix
napihas no loader-only generator —napi buildis the sole producer and needs the Rust toolchain — so the loader travels with the binary it belongs to:.node(same path prefix, so the archive root is unchanged and*.nodeconsumers are unaffected)This is strictly better than re-committing it: the published loader now carries the released version in its per-platform guards. A committed copy never did — hence the manual
chore(napi): regenerate loader for 0.5.20 after native rebuildin history.Because the failure mode is silent, two guards: the publish job hard-fails if no loader is in the artifacts, then
require()s the assembled package before publishing (ubuntu-latest matches the linux-x64-gnu binarynapi artifactsplaces next to it, so it exercises the real loader/binary pair).Verification
Full 6-target matrix dispatched with
publish=false(the gate from #83), run 30393214444 — 6/6 builds, 6/6 Test bindings green, publish skipped.Downloaded a real artifact to check the publish job's precondition rather than assume it:
Both at the archive root, so
find artifacts -name index.jsmatches.Version tracking confirmed from both sides: built pre-rebase (
package.json0.5.21) the loader stamped 0.5.21; rebased onto main (0.5.22) it stamped 0.5.22.Limit: the two new publish-job steps only execute under
publish=true, which publishes — they cannot be exercised without a real release. What is verified is their precondition and that both failure modes are now loud instead of silent.Also
Corrects
RELEASING.md, which claimed "every publish path builds first" — the false premise behind the regression — and now documents that neither the test nor publish job builds.After merge
napi-buildgets re-dispatched against the existingspaghetti-v0.5.22tag to complete the release. That remains the first genuine exercise of the retargeted npm OIDC trusted publishers.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.