feat: setup npx shim and bun-compiled platform binaries - #15
Open
fdarian wants to merge 21 commits into
Open
Conversation
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughEstablishes automated release infrastructure by adding Changesets configuration for semantic versioning, implementing cross-platform binary builds for macOS and Linux across arm64 and x64 architectures, updating the CLI entrypoint to use a new Node runner, and configuring GitHub Actions workflows for continuous integration and multi-arch releases with Homebrew tap updates. ChangesRelease Process & Cross-Platform Distribution
Sequence DiagramsequenceDiagram
participant Developer
participant GitHub_PR
participant CI_Workflow
participant Release_Trigger
participant Build_Matrix
participant Artifact_Upload
participant Release_Creation
participant Homebrew_Tap
Developer->>GitHub_PR: Push changes to main
GitHub_PR->>CI_Workflow: Trigger on:push
CI_Workflow->>CI_Workflow: bun tsc --noEmit
CI_Workflow->>CI_Workflow: bun run test
CI_Workflow->>CI_Workflow: bun scripts/build-binary.ts
CI_Workflow-->>Developer: Report results
Developer->>Release_Trigger: Create release tag
Release_Trigger->>Build_Matrix: Run matrix job (4 targets)
Build_Matrix->>Build_Matrix: bun install --frozen-lockfile
Build_Matrix->>Build_Matrix: bun scripts/build-binary.ts --target
Build_Matrix->>Build_Matrix: tar -czf whap-os-arch.tar.gz
Build_Matrix->>Artifact_Upload: Upload per-platform tarball
Artifact_Upload->>Release_Creation: Gather all artifacts
Release_Creation->>Release_Creation: Calculate SHA256 checksums
Release_Creation->>Release_Creation: Create GitHub Release
Release_Creation->>Homebrew_Tap: Generate platform-aware formula
Homebrew_Tap->>Homebrew_Tap: Embed SHA256 + URLs per arch
Homebrew_Tap-->>Developer: Tap ready for brew install
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
fdarian
force-pushed
the
ff-90-setup-npx-and-deployable-binary
branch
from
May 16, 2026 02:22
d2d8bde to
c4af1ed
Compare
![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: 5
🧹 Nitpick comments (2)
.github/workflows/ci.yml (1)
15-17: ⚡ Quick winPin Bun to a deterministic version.
Line 17 uses
bun-version: latest, which makes CI behavior drift over time and can break reproducibility. This pattern also appears across other workflows (changeset.yml and release.yml). Pin to a specific version (e.g.,1.x.x) or usebun-version-filewith a.bun-versionor.tool-versionsfile.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 15 - 17, The workflow uses oven-sh/setup-bun@v2 with bun-version: latest which makes CI non-deterministic; update the CI step (symbol: oven-sh/setup-bun@v2 and key bun-version) to pin a specific Bun version (e.g., "1.x.x") or switch to bun-version-file and point to a .bun-version or .tool-versions file (create/update that file in the repo accordingly), and apply the same change to the other workflows referenced (changeset.yml and release.yml) so all uses of bun-version: latest are replaced with a deterministic pin or bun-version-file usage..github/workflows/changeset.yml (1)
19-21: ⚡ Quick winPin the Bun version for reproducibility.
Using
bun-version: latestcan lead to unexpected CI failures if Bun releases a breaking change. Pin to a specific version (e.g.,1.1.8) to ensure consistent, reproducible builds.📌 Proposed fix
- uses: oven-sh/setup-bun@v2 with: - bun-version: latest + bun-version: 1.1.8🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/changeset.yml around lines 19 - 21, Replace the non-deterministic bun-version setting in the GitHub Actions step that uses oven-sh/setup-bun@v2: change the bun-version key from "latest" to a pinned semantic version (for example "1.1.8") so CI runs use a stable, reproducible Bun release; update the bun-version value in the workflow step that references oven-sh/setup-bun@v2 accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 21-25: The CI job named "Typecheck" currently has
continue-on-error: true which allows TypeScript errors to pass; change this so
typechecking fails the pipeline by removing or setting continue-on-error to
false for that job (and ensure the run step uses bun tsc --noEmit as-is), or if
this is temporarily required, gate the non-blocking behavior behind a clearly
named environment variable (e.g., TYPECHECK_NON_BLOCKING) and add a
TODO/deadline comment in the workflow to revert the gate; update the workflow
entry for the "Typecheck" job accordingly so TypeScript errors are blocking by
default.
In @.github/workflows/release.yml:
- Around line 12-13: The build job currently sets repository write permission
via the "permissions: contents: write" entry; change this to least-privilege by
setting the build job's permissions to "contents: read" (or remove the explicit
write permission) so the build job only has read access to the repository;
update the permissions block in the release.yml build job to use "contents:
read" and ensure no other steps require write access.
In `@bin.mjs`:
- Around line 6-10: The shim currently always runs execFileSync('bun', [entry,
...process.argv.slice(2)]) inside the try/catch; change the control flow to
attempt a platform-native binary first and only fall back to Bun on failure:
before calling execFileSync('bun', ...), attempt to resolve and exec the
installed CLI binary (use the same args from process.argv.slice(2)) — e.g. try
execFileSync(resolvedBinaryPathOrName, [...process.argv.slice(2)], { stdio:
'inherit' }) and if that throws continue to the existing catch that then runs
execFileSync('bun', [entry, ...process.argv.slice(2)], { stdio: 'inherit' });
update the try/catch around execFileSync and use the existing entry,
execFileSync, resolve/dirname/fileURLToPath symbols to locate where to implement
this binary-first fallback.
In `@package.json`:
- Around line 7-19: The package.json currently only defines the "whap" bin entry
("whap": "./bin.mjs") and a "files" list but does not declare platform-specific
binary packages, so installers won't fetch native binaries; add an
optionalDependencies section in package.json that lists the platform binary
package names (e.g., the platform-specific whap-* packages your build publishes)
with appropriate version strings so package managers will attempt to fetch them
but not fail installs; ensure the entries are under "optionalDependencies" (not
"dependencies") and keep the existing "bin" mapping and "files" list intact so
the fallback Bun path in ./bin.mjs remains available if optional binaries are
not present.
In `@scripts/build-binary.ts`:
- Around line 12-13: Validate the incoming target string before splitting:
ensure the variable target contains a '-' and splits into exactly two non-empty
parts before deriving const os and const arch; if validation fails (e.g., arch
would be undefined) throw or log a clear error and exit. Update the code around
where target is used to compute os and arch (the target.split('-') logic) to
perform this check and handle malformed values with an explicit error message
that includes the bad target.
---
Nitpick comments:
In @.github/workflows/changeset.yml:
- Around line 19-21: Replace the non-deterministic bun-version setting in the
GitHub Actions step that uses oven-sh/setup-bun@v2: change the bun-version key
from "latest" to a pinned semantic version (for example "1.1.8") so CI runs use
a stable, reproducible Bun release; update the bun-version value in the workflow
step that references oven-sh/setup-bun@v2 accordingly.
In @.github/workflows/ci.yml:
- Around line 15-17: The workflow uses oven-sh/setup-bun@v2 with bun-version:
latest which makes CI non-deterministic; update the CI step (symbol:
oven-sh/setup-bun@v2 and key bun-version) to pin a specific Bun version (e.g.,
"1.x.x") or switch to bun-version-file and point to a .bun-version or
.tool-versions file (create/update that file in the repo accordingly), and apply
the same change to the other workflows referenced (changeset.yml and
release.yml) so all uses of bun-version: latest are replaced with a
deterministic pin or bun-version-file usage.
🪄 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: b13d5c19-b1ab-43fb-b498-97cb7b427048
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
.changeset/README.md.changeset/config.json.changeset/initial-cli-release.md.github/workflows/changeset.yml.github/workflows/ci.yml.github/workflows/release.ymlbin.mjspackage.jsonscripts/build-binary.ts
Comment on lines
+21
to
+25
| - name: Typecheck | ||
| # Pre-existing: webhooks.ts:323 calls serializeJsonWithEscapedUnicode | ||
| # without importing it. Clean up in a follow-up PR. | ||
| continue-on-error: true | ||
| run: bun tsc --noEmit |
Contributor
There was a problem hiding this comment.
Make typecheck blocking in CI.
Line 24 (continue-on-error: true) lets type errors pass the pipeline, so CI can go green while the codebase is type-broken. If this must be temporary, gate it behind a clearly scoped condition and deadline instead of making all PRs non-blocking.
Suggested change
- name: Typecheck
- # Pre-existing: webhooks.ts:323 calls serializeJsonWithEscapedUnicode
- # without importing it. Clean up in a follow-up PR.
- continue-on-error: true
run: bun tsc --noEmit📝 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
| - name: Typecheck | |
| # Pre-existing: webhooks.ts:323 calls serializeJsonWithEscapedUnicode | |
| # without importing it. Clean up in a follow-up PR. | |
| continue-on-error: true | |
| run: bun tsc --noEmit | |
| - name: Typecheck | |
| run: bun tsc --noEmit |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 21 - 25, The CI job named "Typecheck"
currently has continue-on-error: true which allows TypeScript errors to pass;
change this so typechecking fails the pipeline by removing or setting
continue-on-error to false for that job (and ensure the run step uses bun tsc
--noEmit as-is), or if this is temporarily required, gate the non-blocking
behavior behind a clearly named environment variable (e.g.,
TYPECHECK_NON_BLOCKING) and add a TODO/deadline comment in the workflow to
revert the gate; update the workflow entry for the "Typecheck" job accordingly
so TypeScript errors are blocking by default.
Comment on lines
+6
to
+10
| const entry = resolve(dirname(fileURLToPath(import.meta.url)), 'src/index.ts') | ||
|
|
||
| try { | ||
| execFileSync('bun', [entry, ...process.argv.slice(2)], { stdio: 'inherit' }) | ||
| } catch (e) { |
Contributor
There was a problem hiding this comment.
Implement binary-first resolution before Bun fallback.
The shim currently always executes bun src/index.ts, so the platform-binary path is never used. That means npx whap still requires Bun even when binary packages are available.
Proposed direction
import { execFileSync } from 'node:child_process'
+import { createRequire } from 'node:module'
+import { existsSync } from 'node:fs'
import { dirname, resolve } from 'node:path'
import { fileURLToPath } from 'node:url'
const entry = resolve(dirname(fileURLToPath(import.meta.url)), 'src/index.ts')
+const require = createRequire(import.meta.url)
+
+const platform = `${process.platform}-${process.arch}`
+const pkgByPlatform = {
+ 'darwin-arm64': 'whap-darwin-arm64',
+ 'darwin-x64': 'whap-darwin-x64',
+ 'linux-arm64': 'whap-linux-arm64',
+ 'linux-x64': 'whap-linux-x64',
+}
+
+let cmd = 'bun'
+let args = [entry, ...process.argv.slice(2)]
+
+const pkg = pkgByPlatform[platform]
+if (pkg) {
+ try {
+ const pkgJson = require.resolve(`${pkg}/package.json`)
+ const binPath = resolve(dirname(pkgJson), 'bin', `whap-${platform}`)
+ if (existsSync(binPath)) {
+ cmd = binPath
+ args = process.argv.slice(2)
+ }
+ } catch {}
+}
try {
- execFileSync('bun', [entry, ...process.argv.slice(2)], { stdio: 'inherit' })
+ execFileSync(cmd, args, { stdio: 'inherit' })
} catch (e) {📝 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
| const entry = resolve(dirname(fileURLToPath(import.meta.url)), 'src/index.ts') | |
| try { | |
| execFileSync('bun', [entry, ...process.argv.slice(2)], { stdio: 'inherit' }) | |
| } catch (e) { | |
| import { execFileSync } from 'node:child_process' | |
| import { createRequire } from 'node:module' | |
| import { existsSync } from 'node:fs' | |
| import { dirname, resolve } from 'node:path' | |
| import { fileURLToPath } from 'node:url' | |
| const entry = resolve(dirname(fileURLToPath(import.meta.url)), 'src/index.ts') | |
| const require = createRequire(import.meta.url) | |
| const platform = `${process.platform}-${process.arch}` | |
| const pkgByPlatform = { | |
| 'darwin-arm64': 'whap-darwin-arm64', | |
| 'darwin-x64': 'whap-darwin-x64', | |
| 'linux-arm64': 'whap-linux-arm64', | |
| 'linux-x64': 'whap-linux-x64', | |
| } | |
| let cmd = 'bun' | |
| let args = [entry, ...process.argv.slice(2)] | |
| const pkg = pkgByPlatform[platform] | |
| if (pkg) { | |
| try { | |
| const pkgJson = require.resolve(`${pkg}/package.json`) | |
| const binPath = resolve(dirname(pkgJson), 'bin', `whap-${platform}`) | |
| if (existsSync(binPath)) { | |
| cmd = binPath | |
| args = process.argv.slice(2) | |
| } | |
| } catch {} | |
| } | |
| try { | |
| execFileSync(cmd, args, { stdio: 'inherit' }) | |
| } catch (e) { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@bin.mjs` around lines 6 - 10, The shim currently always runs
execFileSync('bun', [entry, ...process.argv.slice(2)]) inside the try/catch;
change the control flow to attempt a platform-native binary first and only fall
back to Bun on failure: before calling execFileSync('bun', ...), attempt to
resolve and exec the installed CLI binary (use the same args from
process.argv.slice(2)) — e.g. try execFileSync(resolvedBinaryPathOrName,
[...process.argv.slice(2)], { stdio: 'inherit' }) and if that throws continue to
the existing catch that then runs execFileSync('bun', [entry,
...process.argv.slice(2)], { stdio: 'inherit' }); update the try/catch around
execFileSync and use the existing entry, execFileSync,
resolve/dirname/fileURLToPath symbols to locate where to implement this
binary-first fallback.
Comment on lines
+7
to
+19
| "whap": "./bin.mjs" | ||
| }, | ||
| "files": [ | ||
| "bin.mjs", | ||
| "src", | ||
| "!src/**/*.test.ts", | ||
| "!src/**/*.test.tsx", | ||
| "!src/**/*.spec.ts", | ||
| "!src/**/*.spec.tsx", | ||
| "templates", | ||
| "schema", | ||
| "whap.json.example" | ||
| ], |
Contributor
There was a problem hiding this comment.
Add platform binary packages as optionalDependencies.
Right now the package manifest does not declare platform binary packages, so installs won’t fetch any binaries. That forces the Bun fallback path for everyone.
Suggested manifest shape
{
"bin": {
"whap": "./bin.mjs"
},
+ "optionalDependencies": {
+ "whap-darwin-arm64": "0.2.1",
+ "whap-darwin-x64": "0.2.1",
+ "whap-linux-arm64": "0.2.1",
+ "whap-linux-x64": "0.2.1"
+ },
"files": [📝 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
| "whap": "./bin.mjs" | |
| }, | |
| "files": [ | |
| "bin.mjs", | |
| "src", | |
| "!src/**/*.test.ts", | |
| "!src/**/*.test.tsx", | |
| "!src/**/*.spec.ts", | |
| "!src/**/*.spec.tsx", | |
| "templates", | |
| "schema", | |
| "whap.json.example" | |
| ], | |
| "whap": "./bin.mjs" | |
| }, | |
| "optionalDependencies": { | |
| "whap-darwin-arm64": "0.2.1", | |
| "whap-darwin-x64": "0.2.1", | |
| "whap-linux-arm64": "0.2.1", | |
| "whap-linux-x64": "0.2.1" | |
| }, | |
| "files": [ | |
| "bin.mjs", | |
| "src", | |
| "!src/**/*.test.ts", | |
| "!src/**/*.test.tsx", | |
| "!src/**/*.spec.ts", | |
| "!src/**/*.spec.tsx", | |
| "templates", | |
| "schema", | |
| "whap.json.example" | |
| ], |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` around lines 7 - 19, The package.json currently only defines
the "whap" bin entry ("whap": "./bin.mjs") and a "files" list but does not
declare platform-specific binary packages, so installers won't fetch native
binaries; add an optionalDependencies section in package.json that lists the
platform binary package names (e.g., the platform-specific whap-* packages your
build publishes) with appropriate version strings so package managers will
attempt to fetch them but not fail installs; ensure the entries are under
"optionalDependencies" (not "dependencies") and keep the existing "bin" mapping
and "files" list intact so the fallback Bun path in ./bin.mjs remains available
if optional binaries are not present.
Comment on lines
+12
to
+13
| const os = target.split('-')[0] | ||
| const arch = target.split('-')[1] |
Contributor
There was a problem hiding this comment.
Validate target format before deriving os/arch.
If --target is malformed, arch becomes undefined and the script fails later with a less actionable error.
Suggested fix
-const os = target.split('-')[0]
-const arch = target.split('-')[1]
+const [os, arch] = target.split('-')
+if (!os || !arch) {
+ throw new Error(
+ `Invalid target "${target}". Expected format "<os>-<arch>", e.g. "linux-x64".`
+ )
+}📝 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
| const os = target.split('-')[0] | |
| const arch = target.split('-')[1] | |
| const [os, arch] = target.split('-') | |
| if (!os || !arch) { | |
| throw new Error( | |
| `Invalid target "${target}". Expected format "<os>-<arch>", e.g. "linux-x64".` | |
| ) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/build-binary.ts` around lines 12 - 13, Validate the incoming target
string before splitting: ensure the variable target contains a '-' and splits
into exactly two non-empty parts before deriving const os and const arch; if
validation fails (e.g., arch would be undefined) throw or log a clear error and
exit. Update the code around where target is used to compute os and arch (the
target.split('-') logic) to perform this check and handle malformed values with
an explicit error message that includes the bad target.
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
1 participant
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
Adds NPX shim and multi-platform bun-compiled binaries for distributable
whapCLI package.Changes
bun src/index.tswhen platform-specific binary package is not installedscripts/build-npm.ts): Compiles 4 binariesnpm/whap-{os}-{arch}/): Minimal package.json files for each platformscripts/sync-versions.ts): Keeps package versions aligned across platform packagesdist/andnpm/*/binadded to .gitignore--bytecodeflag removed due to top-level await errors in ink/yoga-layout dependenciesCommits on this branch
Summary by CodeRabbit
Release Notes
New Features
npxand Homebrew package manager for easier installationChores