chore: cross-platform native builds + release readiness fixes - #41
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdded multi‑arch native build and per‑platform npm packaging/publishing, introduced Vitest integration config and smoke test, removed a backend factory export, added platform npm manifests/READMEs and LICENSEs, adjusted .gitignore, and updated CI workflows for build/test/publish. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as "GitHub Actions"
participant Runner as "Runner"
participant Toolchain as "Rust toolchain / cross-compiler"
participant Build as "Cargo / npm build"
participant Artifacts as "Action Artifacts"
participant NPM as "npm registry"
GH->>Runner: start build matrix job
Runner->>Toolchain: install toolchain / cross-compiler
Runner->>Build: compile native target(s)
Build->>Artifacts: upload platform artifact
GH->>Artifacts: download artifacts
GH->>Build: run `npx napi artifacts` → produce `npm/*/` dirs
GH->>NPM: publish each `npm/*/` package
NPM-->>GH: publish confirmations
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
280772a to
d1a90f0
Compare
Enable all 5 NAPI platform targets (linux-x64, linux-arm64, darwin-x64, darwin-arm64, win32-x64-msvc) in CI. Previously only linux-x64 was built, causing install failures on macOS/Windows/ARM64. - Scaffold npm/ platform packages via napi create-npm-dirs - Add optionalDependencies to cachekit-core-ts package.json - Update publish job to publish platform packages before main - Add aarch64-linux cross-compilation via gcc-aarch64-linux-gnu - Exclude integration tests from default test run (separate config) - Remove dead cachekitio() factory from cachekitio.ts
optionalDependencies pointing to unpublished platform packages breaks pnpm install in CI. The NAPI-RS pattern is to inject optionalDependencies at publish time via `napi prepublish -t npm`, not commit them to source. Also fix shell quoting in publish workflow.
Self-hosted cachekit-lean blocks sudo (no-new-privileges container flag), so aarch64 cross-compiler install fails. Move to ubuntu-latest. Global RUSTUP_HOME=/tmp/rustup breaks Windows (no /tmp/ on Windows). Scope RUSTUP_HOME/CARGO_HOME to self-hosted runner only via GITHUB_ENV.
d1a90f0 to
0a3ff2b
Compare
- CJS build now outputs to dist/cjs/ with {"type":"commonjs"} marker
instead of clobbering ESM in dist/. Previously both tsc passes wrote
to the same directory — CJS overwrote ESM, breaking both formats.
- Copy LICENSE to both publishable packages (npm publish requires it)
- Add smoke-test CI job that validates ESM import() and CJS require()
against the built dist/ output — catches format issues before release
- Update exports map: require points to dist/cjs/index.js
- Use pnpm publish instead of npm publish in release-please workflow
to resolve workspace:* protocol (npm ships it literally, breaking
every consumer's install)
- Add smoke-test result check to ci-success gate (was in needs: but
not in the conditional — failures passed silently)
- Add top-level permissions: { contents: read } to ci.yml and
build-native.yml (PR-triggered workflows inherited default perms)
|
@coderabbitai review please |
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/cachekit-core-ts/npm/darwin-x64/README.md (1)
1-3: Consider enriching the platform package README.Consistent with other platform packages, adding installation context would improve discoverability.
📝 Example enhancement
# `@cachekit-io/cachekit-core-ts-darwin-x64` This is the **x86_64-apple-darwin** binary for `@cachekit-io/cachekit-core-ts` + +This package is typically installed automatically as an optional dependency. See the main [`@cachekit-io/cachekit-core-ts`](https://www.npmjs.com/package/@cachekit-io/cachekit-core-ts) package for documentation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cachekit-core-ts/npm/darwin-x64/README.md` around lines 1 - 3, Update the package README for `@cachekit-io/cachekit-core-ts-darwin-x64` to match other platform packages by adding installation and usage context: include a short installation snippet showing how to install this platform-specific package (npm/yarn/pnpm), a minimal usage example demonstrating how to import or load the native binary from `@cachekit-io/cachekit-core-ts`, and a compatibility note indicating the target platform (x86_64-apple-darwin) and Node.js or runtime requirements; ensure the README still includes the existing package title and one-line description but enriched with these installation, usage, and compatibility sections.packages/cachekit-core-ts/npm/linux-x64-gnu/README.md (1)
1-3: Consider enriching the platform package README.While functional, this README could be more helpful by adding installation context, a link to main documentation, or a note that users typically don't install this directly (it's resolved via
optionalDependencies).📝 Example enhancement
# `@cachekit-io/cachekit-core-ts-linux-x64-gnu` This is the **x86_64-unknown-linux-gnu** binary for `@cachekit-io/cachekit-core-ts` + +This package is typically installed automatically as an optional dependency. See the main [`@cachekit-io/cachekit-core-ts`](https://www.npmjs.com/package/@cachekit-io/cachekit-core-ts) package for documentation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cachekit-core-ts/npm/linux-x64-gnu/README.md` around lines 1 - 3, Update the README for the platform package `@cachekit-io/cachekit-core-ts-linux-x64-gnu` to add brief installation/context guidance: state that this is the x86_64-unknown-linux-gnu native binary for `@cachekit-io/cachekit-core-ts`, note that end-users normally do not install this package directly because it is pulled via optionalDependencies, add a short instruction or example for how it is resolved (npm/yarn install behavior), and include a link to the main project documentation or repo README for configuration and usage details.packages/cachekit-core-ts/npm/win32-x64-msvc/README.md (1)
1-3: Consider enriching the platform package README.Same suggestion as the Linux x64 package - adding installation context or a link to main docs would improve user experience.
📝 Example enhancement
# `@cachekit-io/cachekit-core-ts-win32-x64-msvc` This is the **x86_64-pc-windows-msvc** binary for `@cachekit-io/cachekit-core-ts` + +This package is typically installed automatically as an optional dependency. See the main [`@cachekit-io/cachekit-core-ts`](https://www.npmjs.com/package/@cachekit-io/cachekit-core-ts) package for documentation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cachekit-core-ts/npm/win32-x64-msvc/README.md` around lines 1 - 3, Update the README for the `@cachekit-io/cachekit-core-ts-win32-x64-msvc` package to include brief installation/consumption instructions (npm/yarn install command and example import/require snippet), a short note about the platform/ABI (x86_64-pc-windows-msvc) and a prominent link to the main package documentation or repo; look for the header and package name in this README and add a Usage/Installation and Docs section to mirror the enhancement done for the Linux x64 README.packages/cachekit-core-ts/npm/linux-arm64-gnu/README.md (1)
1-3: Consider enriching the platform package README.Consistent with other platform packages, adding installation context would be helpful.
📝 Example enhancement
# `@cachekit-io/cachekit-core-ts-linux-arm64-gnu` This is the **aarch64-unknown-linux-gnu** binary for `@cachekit-io/cachekit-core-ts` + +This package is typically installed automatically as an optional dependency. See the main [`@cachekit-io/cachekit-core-ts`](https://www.npmjs.com/package/@cachekit-io/cachekit-core-ts) package for documentation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cachekit-core-ts/npm/linux-arm64-gnu/README.md` around lines 1 - 3, The README for the package `@cachekit-io/cachekit-core-ts-linux-arm64-gnu` is minimal; update it to match other platform package READMEs by adding installation and usage context: state the target platform (aarch64-unknown-linux-gnu), show the npm install command or tarball name to download, include basic usage/require/import snippet for consuming the binary, mention compatibility/ABI notes and include checksum or verification info and a pointer to the main `@cachekit-io/cachekit-core-ts` README for more details; edit the README.md in this package to contain those sections so users can quickly install and verify the binary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cachekit/package.json`:
- Line 27: The build script currently uses an echo with single quotes which
breaks on Windows; replace the echo fragment in the "build" npm script so it
writes a JSON file using Node (e.g., run a node -e that requires fs and writes
{"type":"commonjs"} to dist/cjs/package.json) instead of shell echo, updating
the "build" script entry to call that Node write operation so the package.json
is valid cross-platform.
---
Nitpick comments:
In `@packages/cachekit-core-ts/npm/darwin-x64/README.md`:
- Around line 1-3: Update the package README for
`@cachekit-io/cachekit-core-ts-darwin-x64` to match other platform packages by
adding installation and usage context: include a short installation snippet
showing how to install this platform-specific package (npm/yarn/pnpm), a minimal
usage example demonstrating how to import or load the native binary from
`@cachekit-io/cachekit-core-ts`, and a compatibility note indicating the target
platform (x86_64-apple-darwin) and Node.js or runtime requirements; ensure the
README still includes the existing package title and one-line description but
enriched with these installation, usage, and compatibility sections.
In `@packages/cachekit-core-ts/npm/linux-arm64-gnu/README.md`:
- Around line 1-3: The README for the package
`@cachekit-io/cachekit-core-ts-linux-arm64-gnu` is minimal; update it to match
other platform package READMEs by adding installation and usage context: state
the target platform (aarch64-unknown-linux-gnu), show the npm install command or
tarball name to download, include basic usage/require/import snippet for
consuming the binary, mention compatibility/ABI notes and include checksum or
verification info and a pointer to the main `@cachekit-io/cachekit-core-ts` README
for more details; edit the README.md in this package to contain those sections
so users can quickly install and verify the binary.
In `@packages/cachekit-core-ts/npm/linux-x64-gnu/README.md`:
- Around line 1-3: Update the README for the platform package
`@cachekit-io/cachekit-core-ts-linux-x64-gnu` to add brief installation/context
guidance: state that this is the x86_64-unknown-linux-gnu native binary for
`@cachekit-io/cachekit-core-ts`, note that end-users normally do not install
this package directly because it is pulled via optionalDependencies, add a short
instruction or example for how it is resolved (npm/yarn install behavior), and
include a link to the main project documentation or repo README for
configuration and usage details.
In `@packages/cachekit-core-ts/npm/win32-x64-msvc/README.md`:
- Around line 1-3: Update the README for the
`@cachekit-io/cachekit-core-ts-win32-x64-msvc` package to include brief
installation/consumption instructions (npm/yarn install command and example
import/require snippet), a short note about the platform/ABI
(x86_64-pc-windows-msvc) and a prominent link to the main package documentation
or repo; look for the header and package name in this README and add a
Usage/Installation and Docs section to mirror the enhancement done for the Linux
x64 README.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0b15922f-0c59-4210-80d1-4bd89ba3f0d5
📒 Files selected for processing (22)
.github/workflows/build-native.yml.github/workflows/ci.yml.github/workflows/release-please.yml.gitignorepackages/cachekit-core-ts/LICENSEpackages/cachekit-core-ts/npm/darwin-arm64/README.mdpackages/cachekit-core-ts/npm/darwin-arm64/package.jsonpackages/cachekit-core-ts/npm/darwin-x64/README.mdpackages/cachekit-core-ts/npm/darwin-x64/package.jsonpackages/cachekit-core-ts/npm/linux-arm64-gnu/README.mdpackages/cachekit-core-ts/npm/linux-arm64-gnu/package.jsonpackages/cachekit-core-ts/npm/linux-x64-gnu/README.mdpackages/cachekit-core-ts/npm/linux-x64-gnu/package.jsonpackages/cachekit-core-ts/npm/win32-x64-msvc/README.mdpackages/cachekit-core-ts/npm/win32-x64-msvc/package.jsonpackages/cachekit/LICENSEpackages/cachekit/package.jsonpackages/cachekit/src/backends/cachekitio.test.tspackages/cachekit/src/backends/cachekitio.tspackages/cachekit/tsconfig.cjs.jsonpackages/cachekit/vitest.config.tspackages/cachekit/vitest.integration.config.ts
💤 Files with no reviewable changes (1)
- packages/cachekit/src/backends/cachekitio.ts
Shell echo with single quotes doesn't work on Windows cmd.exe. Use node fs.writeFileSync for cross-platform portability.
Summary
Cannot find native bindingon installnpm/directories with platform-specific packages andoptionalDependenciesin cachekit-core-tsoptionalDependenciesresolution workspnpm testvia separate vitest config, preventing false failures when Docker is partially availablecachekitio()factory from cachekitio.tsTest plan
pnpm build— cleanpnpm lint— cleanpnpm type-check— cleanpnpm test— 31/31 files, 430+ tests pass (integration excluded)Summary by CodeRabbit
New Features
Chores
Tests
Documentation
Refactor