Skip to content

fix(ci): stop generated files from failing every release PR - #85

Merged
jamesyong-42 merged 1 commit into
mainfrom
fix/release-version-drift
Jul 28, 2026
Merged

fix(ci): stop generated files from failing every release PR#85
jamesyong-42 merged 1 commit into
mainfrom
fix/release-version-drift

Conversation

@jamesyong-42

Copy link
Copy Markdown
Member

Release PR #80 (0.5.22) fails CI on check (ubuntu-latest) at the Generated files are current step. This fixes the cause.

What's actually wrong

release-please bumps package.json and Cargo.toml but cannot run a build, so the two generated files that embed the version go stale the moment it opens a release PR. pnpm build then regenerates them and git diff --exit-code fails. Both are in #80's failing diff:

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/crates/spaghetti-napi/index.js b/crates/spaghetti-napi/index.js

Version stamping landed in a6c88cf (#79), which is in this release — so 0.5.22 is the first release PR that could hit this, and every future one would too. main passes because there the versions agree; only the bumped release branch diverges.

Not caused by the org transfer.

Cargo.lock — bumped by release-please

A lockfile must stay committed, so it gets a toml extra-file entry.

The jsonpath matches on @.name.value, not @.name. The updater parses TOML into position-annotated nodes ({start, end, value}), so the intuitive form matches nothing and silently no-ops with only a No entries modified warning — it would have looked fixed and failed the next release anyway. I caught this by running release-please's GenericToml updater directly against the real lockfile:

jsonpath result
$.package[?(@.name==='spaghetti-napi')].version 0 matches, no-op
$.package[?(@.name.value==='spaghetti-napi')].version 1 match → /package/106/version

End-to-end the updater rewrites exactly one line (L927), preserves Cargo's generated header, and leaves the line count identical.

Matching by name rather than array index is deliberate: [[package]] is sorted alphabetically, so an index would silently retarget a different crate as dependencies change.

index.js — no longer committed

napi build stamps the version into a per-platform guard in ~50 places; no updater can track that, and the file is pure generated output. History already carries a manual chore(napi): regenerate loader for 0.5.20 after native rebuild commit — this removes that chore permanently.

It is still published. files in package.json is an allowlist that overrides .gitignore, verified with npm pack --dry-run:

npm notice 5.0kB  index.d.ts
npm notice 25.1kB index.js      <-- present
npm notice 4.4MB  spaghetti.darwin-arm64.node

Every publish path builds before packing, and pnpm -r build builds the crate before the SDK, so CI and fresh clones generate it ahead of any consumer.

index.d.ts stays committed — it is the reviewable API surface and carries no version. Confirmed neither it nor parse-worker.js contains a version string, so the drift check keeps full teeth for real codegen rot.

This mirrors truffle, which already gitignores its napi loader for exactly this reason.

Verification

  • git diff --exit-code -- Cargo.lock crates/spaghetti-napi packages/sdk/src/workers/parse-worker.js → clean
  • npm pack --dry-run → loader present in tarball
  • GenericToml unit-tested against the committed config's jsonpath → 1 line changed
  • Rationale documented in RELEASING.md and the ci.yml comment, since JSON can't hold a comment and @.name.value invites a wrong "simplification"

After merge

release-please will regenerate #80 against the new config; its CI should then pass. That release is also the first real exercise of the retargeted npm OIDC trusted publishers.

🤖 Generated with Claude Code

release-please bumps `package.json` and `Cargo.toml` but cannot run a
build, so the two generated files that embed the version drifted the
moment it opened a release PR. `pnpm build` then regenerated them and
`Generated files are current` failed on the diff. Both files are in the
0.5.22 PR's failing diff, and version stamping landed in a6c88cf (#79),
which is *in* that release — so 0.5.22 is the first PR that could hit
this, and every future one would too.

Cargo.lock must stay committed, so release-please now bumps it via a
`toml` extra-file. The jsonpath matches on `@.name.value`, not `@.name`:
the updater parses TOML into position-annotated nodes ({start, end,
value}), so the intuitive form matches nothing and no-ops with only a
warning. Verified against the real lockfile by running release-please's
GenericToml updater directly — it rewrites exactly one line and leaves
Cargo's generated header intact. Matching by name rather than array
index matters because [[package]] is sorted alphabetically, so an index
would silently retarget another crate as dependencies change.

crates/spaghetti-napi/index.js is now gitignored instead. `napi build`
stamps the version into a per-platform guard in ~50 places, which no
updater can track, and the file is pure generated output — history
already carries a manual "regenerate loader for 0.5.20" chore commit.
It is still published: `files` in package.json is an allowlist that
overrides .gitignore, confirmed with `npm pack --dry-run` (index.js,
25.1kB, present in the tarball). index.d.ts stays committed — it is the
reviewable API surface and carries no version, so the drift check keeps
its teeth for real codegen rot.

This mirrors truffle, which already gitignores its napi loader for the
same reason.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jamesyong-42
jamesyong-42 merged commit 02452b0 into main Jul 28, 2026
6 checks passed
@jamesyong-42
jamesyong-42 deleted the fix/release-version-drift branch July 28, 2026 17:09
jamesyong-42 added a commit that referenced this pull request Jul 28, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant