Build envio CLI as NAPI native module instead of binary#1011
Closed
Build envio CLI as NAPI native module instead of binary#1011
Conversation
Replace the standalone Rust binary distribution with a napi-rs native module (.node file) loaded directly by Node.js. This eliminates the need for a separate binary and allows the CLI to be invoked as a JS bin entry point (`envio` command via npm/pnpm). Key changes: - Add napi/napi-derive deps behind optional `napi` feature flag - Add src/napi.rs with `run(argv)` function wrapping the executor - Add core.js/core.d.ts ESM loader that resolves platform .node file - Add bin/envio.js entry point to envio package - Update build scripts to ship envio.node instead of bin/envio binary - Update CI workflows to build cdylib with --features napi - Update verify-artifact to expect new file structure https://claude.ai/code/session_011KGFQXXYDMBAGr3oQSSKRh
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughConverts envio distribution from standalone Rust CLI binaries to a Node.js N‑API native module. CI, build, packaging, tests, and dev tooling now produce and consume platform-specific Changes
Sequence DiagramsequenceDiagram
participant CI as Build System
participant Cargo as Cargo (Rust)
participant Pack as Packager
participant NPM as npm/pnpm
participant App as Application
participant Loader as Node.js Loader
participant Native as Native Module (envio.node)
participant CLI as Rust CLI Logic
rect rgba(100,150,255,0.5)
Note over CI,Cargo: Old flow — binary distribution
CI->>Cargo: cargo build --release --bins
Cargo-->>CI: bin/envio (executable)
CI->>Pack: package CLI binary
Pack->>NPM: publish with bin/envio
App->>NPM: install
App->>App: execute ./bin/envio
end
rect rgba(150,200,100,0.5)
Note over CI,Native: New flow — N‑API native module
CI->>Cargo: cargo build --release --lib --features napi --target
Cargo-->>CI: libenvio.so / libenvio.dylib
CI->>CI: rename -> envio.node
CI->>Pack: package envio.node + core.js/core.d.ts/bin
Pack->>NPM: publish platform package / optional deps
App->>NPM: install
App->>Loader: import 'envio' / require
Loader->>Native: load envio.node
Native->>CLI: invoke run(argv) via N‑API
CLI-->>Native: result
Native-->>App: exported run executes
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…iles
- Remove win32/windows mapping from core.js (no Windows platform package exists)
- Remove unused `main` field from platform package.json (core.js uses explicit path)
- Fix stale JSDoc in build-platform-package.ts ("binary" → "native module")
- Add *.node to .gitignore to prevent accidental commits of dev builds
https://claude.ai/code/session_011KGFQXXYDMBAGr3oQSSKRh
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build_and_verify.yml:
- Line 40: The workflow currently builds the Linux binary using the musl target
("x86_64-unknown-linux-musl") but that artifact is published as the generic
"envio-linux-x64" package, breaking glibc systems; update the build step that
runs "cargo build --release --lib --features napi --target
x86_64-unknown-linux-musl" to target "x86_64-unknown-linux-gnu" instead (and
remove any musl tool/install steps), or alternatively keep the musl build and
change the published package name to "envio-linux-x64-musl" in the publish
workflow so consumers know it's musl-linked; ensure references to
"envio-linux-x64" and the build target string are updated consistently (also
consider build-platforms.yml's use of aarch64-unknown-linux-gnu as the canonical
glibc variant).
In `@packages/envio/core.js`:
- Around line 16-20: PLATFORM_MAP incorrectly signals Windows support; either
remove the 'win32' => 'windows' entry from PLATFORM_MAP so the code and
user-facing support message remain consistent for unsupported Windows, or (if
Windows should be supported) add Windows to the published build targets and
update the support message to include windows-x64/windows-arm64 and ensure
packaging/release scripts consume PLATFORM_MAP['win32'] -> 'windows'; modify the
PLATFORM_MAP constant and the user-facing support-text so both reflect the same
supported platforms.
- Around line 41-48: The current broad catch blocks around the two require()
attempts hide real native load errors; update both try/catch blocks that load
`${pkgName}/envio.node` and `./envio.node` to only swallow errors where err.code
=== 'MODULE_NOT_FOUND' (or the specific Module not found indicator) and rethrow
any other errors (e.g., ERR_DLOPEN_FAILED) so ABI/dlopen failures surface; keep
the fallback behavior only for missing-module cases and ensure you reference the
same require calls when implementing the conditional rethrow.
ℹ️ Review info
Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c03b2984-3871-4453-8313-7e567ed7eadc
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
.github/actions/prepare-envio-artifacts/action.yml.github/workflows/build-platforms.yml.github/workflows/build_and_verify.yml.github/workflows/publish.yml.pnpmfile.cjspackages/build-envio/build-platform-package.tspackages/build-envio/src/build-artifact.tspackages/build-envio/src/verify-artifact.tspackages/cli/Cargo.tomlpackages/cli/src/lib.rspackages/cli/src/napi.rspackages/e2e-tests/src/config.tspackages/envio/bin/envio.jspackages/envio/core.d.tspackages/envio/core.jspackages/envio/package.json
💤 Files with no reviewable changes (1)
- .github/actions/prepare-envio-artifacts/action.yml
Comment on lines
+41
to
+48
| try { | ||
| return require(`${pkgName}/envio.node`); | ||
| } catch (_) {} | ||
|
|
||
| // Fallback: local build (development) | ||
| try { | ||
| return require("./envio.node"); | ||
| } catch (_) {} |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the actual code in the repository
cd packages/envio && wc -l core.jsRepository: enviodev/hyperindex
Length of output: 73
🏁 Script executed:
# Read the relevant section of the file
head -60 packages/envio/core.js | tail -30Repository: enviodev/hyperindex
Length of output: 815
🌐 Web query:
Node.js native addon error codes MODULE_NOT_FOUND ERR_DLOPEN_FAILED ABI version mismatch
💡 Result:
What these errors usually mean
MODULE_NOT_FOUND(when requiring a native addon) often means Node can’t resolve the addon’s compiled.nodebinary or the binary exists but fails to load, in which case you’ll often see a nestedERR_DLOPEN_FAILED.ERR_DLOPEN_FAILEDmeans a call toprocess.dlopen()failed (i.e., the OS loader couldn’t load the native binary). Common causes include missing shared libs, wrong CPU/OS, or ABI mismatch. [1]- ABI /
NODE_MODULE_VERSIONmismatch means the addon was compiled against a different Node/Electron C++ module ABI than the runtime you’re using. Node tracks this asprocess.versions.modulesand will refuse to load mismatched binaries. [2]
Quick diagnosis checklist
- Confirm the runtime ABI you’re actually running
node -p "process.versions.modules"(this is the ABI / “node_abi” value). [2]
- If the error mentions
NODE_MODULE_VERSION XvsY: rebuild/reinstall- Typical fix: delete and reinstall, then rebuild native deps:
rm -rf node_modules package-lock.json && npm cinpm rebuild
- The error message itself commonly recommends
npm rebuild/ reinstall for this situation. [3]
- Typical fix: delete and reinstall, then rebuild native deps:
- If this is an Electron app: rebuild for Electron, not system Node
- Use
@electron/rebuildto rebuild native modules against Electron’s ABI. [4][5]
- Use
- Check platform/arch/libc mismatches (common on Alpine/musl)
- Prebuilt binaries are often selected by
(platform, arch, libc, node_abi). If any differs, you may getERR_DLOPEN_FAILEDor no matching binary. [6][7]
- Prebuilt binaries are often selected by
- Watch for “wrong ABI reported” Node builds from OS package managers
- Some distro Node builds can report an unexpected ABI (breaking prebuild selection). Fix: install Node from official Node.js distributions instead of the OS package manager. [7][8]
If you paste your exact error block
Include:
- the full stack trace (especially the line mentioning
NODE_MODULE_VERSION ... requires ...) node -v- whether this is Node or Electron
- OS + CPU (
uname -a,node -p "process.platform+' '+process.arch")
…and I can tell you which of the above is the most likely root cause and the minimal fix.
Sources:
[1] Node.js error docs: ERR_DLOPEN_FAILED definition. [2] Node docs: process.versions.modules is the ABI and mismatches are refused. [3] Electron docs show the common NODE_MODULE_VERSION mismatch error and suggest rebuild/reinstall. [4][5] Electron + @electron/rebuild usage for ABI-aligned rebuilds. [6] node-pre-gyp explains node_abi / process.versions.modules and how binaries are selected. [7] Datadog docs discuss ABI-reporting issues and libc/musl causing ERR_DLOPEN_FAILED. [8] Ubuntu bug report on wrong ABI being reported.
🌐 Web query:
Node.js require error handling best practices native modules optional dependencies
💡 Result:
Best practices: handling require() failures for optional deps (esp. native modules)
1) Mark truly-optional packages as optionalDependencies
If a dependency may fail to install (common for native addons), put it in optionalDependencies so npm won’t fail the whole install on build/download errors—but your code must handle its absence at runtime. [1]
2) Use try/catch, but only swallow “missing module” errors
require() throws an error with code === 'MODULE_NOT_FOUND' when a module can’t be resolved. [2] However, the same error code can also occur for nested missing imports inside the optional package. So:
- Only ignore the error if it refers to the top-level optional package you attempted to load.
- Otherwise, rethrow, because it’s a real bug (broken install, missing transitive dep, bad binary, etc.).
function optionalRequire(name) {
try {
return require(name);
} catch (err) {
// Only treat as optional if *this* module wasn't found
if (err && err.code === 'MODULE_NOT_FOUND' && err.message?.includes(`'${name}'`)) {
return null;
}
throw err;
}
}
const fast = optionalRequire('my-native-addon');
module.exports = fast ?? require('./pure-js-fallback');3) Prefer “probe then load” when you need extra certainty
You can probe resolution with require.resolve(), which also throws MODULE_NOT_FOUND if the module can’t be found. [2] This can help you separate “not installed” from “installed but throws during initialization”:
function safeLoad(name) {
try {
require.resolve(name); // existence / resolvability probe
} catch (e) {
if (e.code === 'MODULE_NOT_FOUND') return null;
throw e;
}
// If it exists, let real errors surface:
return require(name);
}4) For native addons: use Node-API (N-API) to reduce load/install breakage
If you ship a native module, using Node-API (formerly N-API) is the recommended path for ABI stability across Node.js versions, reducing “compiled against a different Node version” style failures. [3][4]
Sources
- npm docs:
optionalDependenciesbehavior and exampletry/catchpattern. [1] - Node.js docs:
require()/require.resolve()throwingMODULE_NOT_FOUND. [2] - Node.js guide: ABI stability + rationale/guarantees for N-API. [3]
- Node.js docs: Node-API is ABI-stable and intended to insulate addons from engine changes. [4]
Don't swallow non-missing-module native load errors.
Lines 41-48 currently suppress all errors with broad catch (_) {} blocks, including real addon load failures (ABI mismatches, dlopen failures, missing symbols). This masks diagnostic information and makes debugging much harder.
In Node.js, require() throws MODULE_NOT_FOUND when a module can't be resolved, but throws ERR_DLOPEN_FAILED for OS loader failures and other errors when loading the binary itself. Only the former should be caught during fallback resolution; real errors need to surface.
Suggested targeted error handling
+function isModuleNotFound(err) {
+ return err && (err.code === "MODULE_NOT_FOUND" || err.code === "ERR_MODULE_NOT_FOUND");
+}
+
// Try platform package first (installed via optionalDependencies)
try {
return require(`${pkgName}/envio.node`);
- } catch (_) {}
+ } catch (err) {
+ if (!isModuleNotFound(err)) throw err;
+ }
// Fallback: local build (development)
try {
return require("./envio.node");
- } catch (_) {}
+ } catch (err) {
+ if (!isModuleNotFound(err)) throw err;
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/envio/core.js` around lines 41 - 48, The current broad catch blocks
around the two require() attempts hide real native load errors; update both
try/catch blocks that load `${pkgName}/envio.node` and `./envio.node` to only
swallow errors where err.code === 'MODULE_NOT_FOUND' (or the specific Module not
found indicator) and rethrow any other errors (e.g., ERR_DLOPEN_FAILED) so
ABI/dlopen failures surface; keep the fallback behavior only for missing-module
cases and ensure you reference the same require calls when implementing the
conditional rethrow.
… to gnu - Upgrade napi 2→3.8.3 and napi-derive 2→3.5.2 (latest stable) - Fix get_envio_version() for napi: set ENVIO_PKG_DIR from JS wrapper so Rust can find the envio package.json (current_exe() returns Node.js) - Add current_dir() fallback for dev-mode packages/envio discovery - Switch linux-x64 CI build from musl to gnu (consistent with other targets, avoids musl+glibc cdylib issues) - Remove unnecessary needs: build-linux-package from build-envio-package (they share no artifacts, can now run in parallel) https://claude.ai/code/session_011KGFQXXYDMBAGr3oQSSKRh
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/envio/bin/envio.js (1)
13-13: Consider whether unhandled errors need explicit handling.If
run()throws (e.g., NAPI error from Rust), the process will exit with a stack trace. This is standard CLI behavior, but if cleaner error output is desired for user-facing errors, you could wrap in try-catch:try { run(process.argv.slice(2)); } catch (err) { console.error(err.message); process.exitCode = 1; }This is optional since the current approach is valid for CLI tools and the Rust layer likely handles error formatting.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/envio/bin/envio.js` at line 13, Wrap the top-level invocation of run(process.argv.slice(2)) in a try-catch to handle any synchronous exceptions from run (e.g., NAPI/Rust errors); catch the error, log a concise user-facing message (using err.message) via console.error and set process.exitCode = 1 so the CLI exits cleanly instead of printing a full stack trace. This change should be made around the run(...) call in envio.js to provide cleaner output for user-facing errors while preserving current behavior for unexpected failures.packages/cli/src/config_parsing/system_config.rs (1)
426-446: Please add regression tests for dual-start-path discovery.Line [431] and Line [432] introduce a two-anchor search (
current_exethencurrent_dir). Add focused tests for precedence, fallback, and failure messaging to keep this path-resolution behavior stable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/config_parsing/system_config.rs` around lines 426 - 446, Add unit tests for the dual-anchor discovery implemented via start_paths (uses env::current_exe() then env::current_dir()) to cover precedence, fallback, and failure cases: (1) create temporary directories simulating an executable path with packages/envio and a different CWD also with packages/envio and assert the function returns the path under the current_exe anchor (i.e., start_paths[0] wins); (2) simulate current_exe failing (make it None or return a path without packages/envio) while current_dir contains packages/envio and assert the function returns the current_dir-based path; and (3) simulate both anchors not containing packages/envio and assert the function returns an Err whose message contains "could not find packages/envio above executable or working directory". Reference the search logic using start_paths, env::current_exe, env::current_dir, and the candidate = d.join("packages/envio") to locate where to call and what to assert.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build_and_verify.yml:
- Around line 174-181: Jobs that run the packaged CLI via "pnpm envio codegen"
(e.g., the scenarios-test/template-tests/e2e-test workflows and the fuel_test
job shown) must wait for the Linux package build; add an explicit job dependency
on the build-linux-package job (or the job that produces
npm-pkg-envio-linux-x64) to each of these jobs by including it in their "needs"
array so they don't start before the linux artifact is available.
In `@packages/cli/Cargo.toml`:
- Around line 12-13: The crate exposes #[napi] exports in
packages/cli/src/napi.rs but Cargo.toml lacks build-dependencies and a build
script, so N-API codegen will fail; add napi-build (napi-build = { version =
"...", optional = false } or appropriate version) under [build-dependencies] in
Cargo.toml and create a build.rs at packages/cli/build.rs that calls
napi_build::setup() so the #[napi] macros in src/napi.rs run at build time and
produce the required bindings.
---
Nitpick comments:
In `@packages/cli/src/config_parsing/system_config.rs`:
- Around line 426-446: Add unit tests for the dual-anchor discovery implemented
via start_paths (uses env::current_exe() then env::current_dir()) to cover
precedence, fallback, and failure cases: (1) create temporary directories
simulating an executable path with packages/envio and a different CWD also with
packages/envio and assert the function returns the path under the current_exe
anchor (i.e., start_paths[0] wins); (2) simulate current_exe failing (make it
None or return a path without packages/envio) while current_dir contains
packages/envio and assert the function returns the current_dir-based path; and
(3) simulate both anchors not containing packages/envio and assert the function
returns an Err whose message contains "could not find packages/envio above
executable or working directory". Reference the search logic using start_paths,
env::current_exe, env::current_dir, and the candidate = d.join("packages/envio")
to locate where to call and what to assert.
In `@packages/envio/bin/envio.js`:
- Line 13: Wrap the top-level invocation of run(process.argv.slice(2)) in a
try-catch to handle any synchronous exceptions from run (e.g., NAPI/Rust
errors); catch the error, log a concise user-facing message (using err.message)
via console.error and set process.exitCode = 1 so the CLI exits cleanly instead
of printing a full stack trace. This change should be made around the run(...)
call in envio.js to provide cleaner output for user-facing errors while
preserving current behavior for unexpected failures.
ℹ️ Review info
Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f966bd5b-48e1-496e-9333-5c85da5f9884
⛔ Files ignored due to path filters (3)
Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yamlscenarios/test_codegen/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
.github/workflows/build_and_verify.ymlpackages/cli/Cargo.tomlpackages/cli/src/config_parsing/system_config.rspackages/envio/bin/envio.js
Comment on lines
+12
to
+13
| napi = { version = "3", features = ["tokio_rt"], optional = true } | ||
| napi-derive = { version = "3", optional = true } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verification for napi-build wiring in packages/cli
echo "=== Checking for build.rs files ==="
fd -HI '^build.rs$' .
echo -e "\n=== Checking Cargo.toml and build.rs for napi-build references ==="
rg -n 'napi[-_]build|napi_build::setup' packages/cli/Cargo.toml packages/cli/build.rs 2>/dev/null
echo -e "\n=== Checking for #[napi] attributes in napi.rs ==="
rg -n '#\[napi\]' packages/cli/src/napi.rs 2>/dev/null
echo -e "\n=== Full Cargo.toml [build-dependencies] section ==="
rg -A 10 '\[build-dependencies\]' packages/cli/Cargo.toml 2>/dev/null
echo -e "\n=== Full packages/cli/Cargo.toml ==="
wc -l packages/cli/Cargo.toml
cat -n packages/cli/Cargo.tomlRepository: enviodev/hyperindex
Length of output: 3187
Add N-API build wiring to compile NAPI exports.
Lines 12-13 add napi/napi-derive, and line 59 enables the feature. However, packages/cli/src/napi.rs contains #[napi] exports at line 5 that require build-time codegen. Missing napi-build in [build-dependencies] and a build.rs file will cause addon compilation to fail.
Add to Cargo.toml:
Required changes
+[build-dependencies]
+napi-build = "3"Create packages/cli/build.rs:
fn main() {
napi_build::setup();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| napi = { version = "3", features = ["tokio_rt"], optional = true } | |
| napi-derive = { version = "3", optional = true } | |
| napi = { version = "3", features = ["tokio_rt"], optional = true } | |
| napi-derive = { version = "3", optional = true } | |
| [build-dependencies] | |
| napi-build = "3" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/Cargo.toml` around lines 12 - 13, The crate exposes #[napi]
exports in packages/cli/src/napi.rs but Cargo.toml lacks build-dependencies and
a build script, so N-API codegen will fail; add napi-build (napi-build = {
version = "...", optional = false } or appropriate version) under
[build-dependencies] in Cargo.toml and create a build.rs at
packages/cli/build.rs that calls napi_build::setup() so the #[napi] macros in
src/napi.rs run at build time and produce the required bindings.
…4 artifact The prepare-envio-artifacts action downloads both npm-pkg-envio and npm-pkg-envio-linux-x64, but test jobs only declared needs on build-envio-package. Add build-linux-package to the needs list. https://claude.ai/code/session_011KGFQXXYDMBAGr3oQSSKRh
…al import - Add napi-build build-dependency and build.rs calling napi_build::setup() for platform-specific linker flags (macOS: -undefined dynamic_lookup, Linux: -Wl,-z,nodelete) - Fix bin/envio.js: use relative import ../core.js instead of self-referential "envio/core.js" which fails in workspace dev mode - Set ENVIO_PKG_DIR using dirname of bin/ (the envio package root) - Add build:native script for local dev (cargo build + copy .node) - Clean up core.js: remove extra imports, keep standard napi loading pattern https://claude.ai/code/session_011KGFQXXYDMBAGr3oQSSKRh
Remove the two-artifact build (build-linux-package + build-envio-package) in favor of a single build-native job that uploads just the .so file. Test jobs download it into packages/envio/envio.node where core.js picks it up via the local-file fallback. - Merge build-linux-package + build-envio-package into build-native - Simplify prepare-envio-artifacts: download .so → rename to envio.node - Remove .pnpmfile.cjs redirect (no longer needed for CI) https://claude.ai/code/session_011KGFQXXYDMBAGr3oQSSKRh
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 6: The "build:native" npm script maps non-darwin platforms to ".so"
causing unclear failures on Windows; update the inline node check used in the
"build:native" script to explicitly handle process.platform === 'darwin' ->
'dylib', process.platform === 'win32' -> 'dll', and otherwise either use 'so' or
immediately throw a clear error for unsupported platforms so the script fails
fast with a descriptive message; locate the "build:native" entry and change the
node -e platform branch accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 11feee35-ccf2-4b36-848b-bc0989ca0133
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
package.jsonpackages/cli/Cargo.tomlpackages/cli/build.rspackages/envio/bin/envio.jspackages/envio/core.js
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/cli/Cargo.toml
- packages/envio/core.js
- packages/envio/bin/envio.js
| "private": true, | ||
| "type": "module", | ||
| "scripts": { | ||
| "build:native": "cargo build --lib --features napi && node -e \"const p=process.platform==='darwin'?'dylib':'so'; require('fs').copyFileSync('target/debug/libenvio.'+p, 'packages/envio/envio.node')\"", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Current build:native script ==="
rg -n '"build:native"' package.json -C1
echo
echo "=== Native loader platform support in core.js ==="
rg -n 'Unsupported platform|PLATFORM_MAP|darwin|linux|win32|platform' packages/envio/core.js -C2 | head -100Repository: enviodev/hyperindex
Length of output: 1517
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check for other platform handling in build/native code ==="
rg -n 'process.platform|platform|darwin|linux|win32' package.json -C1
echo
echo "=== Verify packages/envio directory structure ==="
ls -la packages/envio/ 2>/dev/null || echo "Directory not found"
echo
echo "=== Check if there are native build configs ==="
fd -n 'Cargo.toml|build\.rs|napi\.rs|\.cargo' packages/envio 2>/dev/null || echo "No native build files found in packages/envio"Repository: enviodev/hyperindex
Length of output: 1409
Fail fast on unsupported platforms in build:native.
Line 6 currently maps every non-darwin OS to .so. On win32, this fails later with a generic file-copy error instead of a clear unsupported-platform error. Make platform handling explicit.
Suggested patch
- "build:native": "cargo build --lib --features napi && node -e \"const p=process.platform==='darwin'?'dylib':'so'; require('fs').copyFileSync('target/debug/libenvio.'+p, 'packages/envio/envio.node')\"",
+ "build:native": "cargo build --lib --features napi && node -e \"const os=process.platform; const ext=os==='darwin'?'dylib':os==='linux'?'so':null; if(!ext){ throw new Error('Unsupported platform for build:native: '+os); } require('fs').copyFileSync('target/debug/libenvio.'+ext, 'packages/envio/envio.node')\"",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "build:native": "cargo build --lib --features napi && node -e \"const p=process.platform==='darwin'?'dylib':'so'; require('fs').copyFileSync('target/debug/libenvio.'+p, 'packages/envio/envio.node')\"", | |
| "build:native": "cargo build --lib --features napi && node -e \"const os=process.platform; const ext=os==='darwin'?'dylib':os==='linux'?'so':null; if(!ext){ throw new Error('Unsupported platform for build:native: '+os); } require('fs').copyFileSync('target/debug/libenvio.'+ext, 'packages/envio/envio.node')\"", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 6, The "build:native" npm script maps non-darwin
platforms to ".so" causing unclear failures on Windows; update the inline node
check used in the "build:native" script to explicitly handle process.platform
=== 'darwin' -> 'dylib', process.platform === 'win32' -> 'dll', and otherwise
either use 'so' or immediately throw a clear error for unsupported platforms so
the script fails fast with a descriptive message; locate the "build:native"
entry and change the node -e platform branch accordingly.
The napi-only build doesn't produce a cargo binary, so resolveEnvio() fell through to `pnpm envio` which fails when cwd is outside the workspace (template tests run in /tmp). Use absolute node path instead. Add .claude/skills/ci/ for investigating CI failures via GitHub API. https://claude.ai/code/session_011KGFQXXYDMBAGr3oQSSKRh
When running via napi, current_exe() returns the Node binary and cwd may be outside the workspace (e.g. template tests in /tmp/). The existing walk-up strategy fails to find packages/envio in both cases. Fix: when ENVIO_PKG_DIR is set (by bin/envio.js), use it directly as the dev-mode envio path. This is the same directory the napi wrapper already uses for module loading. Also update ci skill to use raw GitHub API commands directly. https://claude.ai/code/session_011KGFQXXYDMBAGr3oQSSKRh
core.js now has a third fallback: if no platform package or local envio.node is found, it locates the workspace Cargo.toml, runs `cargo build --lib --features napi`, and copies the cdylib as envio.node. Subsequent runs skip the build if the cdylib exists. This means `pnpm envio dev` just works on a fresh clone without any manual cargo build step. https://claude.ai/code/session_011KGFQXXYDMBAGr3oQSSKRh
Instead of only building when the cdylib is missing, always run cargo build (which is a no-op at ~0.5s when nothing changed). This gives the same auto-rebuild behavior as cargo run would, while still exercising the production napi code path. https://claude.ai/code/session_011KGFQXXYDMBAGr3oQSSKRh
When installed via `pnpm link` or `file:` dependency, __dirname points into node_modules/.pnpm/... which is a symlink. Use realpathSync to resolve back to the real repo path so the workspace root walk-up finds Cargo.toml correctly. https://claude.ai/code/session_011KGFQXXYDMBAGr3oQSSKRh
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
2 participants
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.
Summary
This PR refactors the envio CLI build system to compile as a NAPI native module (
envio.node) instead of a standalone binary. This enables the CLI to be invoked from JavaScript/Node.js while maintaining the same Rust implementation.Key Changes
New NAPI binding (
packages/cli/src/napi.rs): Exposes arun(argv: Vec<String>)function that executes the CLI with given arguments, wrapping clap parsing and tokio runtime initialization.JavaScript wrapper (
packages/envio/core.js): Loads the platform-specific native module with fallback logic:envio-{os}-{arch})./envio.node)CLI entry point (
packages/envio/bin/envio.js): Executable script that imports and calls the nativerun()function, allowingpnpm envioto work as before.Build configuration updates:
Cargo.toml: Addednapifeature flag and cdylib crate type for native module compilationcargo build --binstocargo build --lib --features napienvio.nodeinstead ofbin/envioCI/CD updates:
bin/directory needed)pnpm envioinstead of direct binary pathsPackage configuration:
packages/envio/package.json: Addedbinfield pointing tobin/envio.jsenvio.nodeas main entry pointImplementation Details
tokio::runtime::Builderto execute async CLI commands from synchronous JavaScript contextprocess.platformandprocess.archwith mapping to Rust target triplescargo buildstill works locally, with the module loaded from./envio.nodehttps://claude.ai/code/session_011KGFQXXYDMBAGr3oQSSKRh
Summary by CodeRabbit
New Features
Chores