Publish prebuilt CLI binaries#259
Merged
Merged
Conversation
- add Bun-built platform binaries - remove install-time runtime deps - update release and tests for bin entry
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 99460b8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughPublishes prebuilt platform-specific CLI binaries with a Node selector entrypoint, generates version constants at build time, adds Bun-based binary build scripts, replaces an external package-sort dependency with a local utility, updates pnpm workspace output and init bootstrap, updates templates/tests to use local artifacts, and adds Bun setup to CI workflows. ChangesCLI Binary Distribution Infrastructure
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/release.yml:
- Around line 159-162: The workflow currently pins Bun via the "Setup Bun" step
using oven-sh/setup-bun@v2 with bun-version: 1.3.11; update the bun-version
input to the latest release (e.g., 1.3.13) or add a comment/documentation in the
workflow explaining why bun-version: 1.3.11 must remain pinned for compatibility
with your build/test steps so reviewers know the pin is intentional.
In `@packages/cli/package.json`:
- Around line 43-45: The script write-cli-version.mjs writes package-versions.ts
with fs.writeFileSync without ensuring the parent folder exists, causing ENOENT
when src/generated is missing; fix by creating the output directory before
writing—call fs.mkdirSync(path.dirname(outputPath), { recursive: true }) (or
equivalent) just before the existing fs.writeFileSync(...) in
write-cli-version.mjs so the src/generated directory is created when needed.
In `@packages/cli/scripts/build-binaries.mjs`:
- Around line 20-25: The script currently parses PROOFKIT_BINARY_TARGETS into
selectedTargets (const selectedTargets = new Set(...)) but proceeds to build
even if that set is empty; update the code right after selectedTargets is
created (and similarly after the target-filtering logic around the loop between
lines ~35-72) to detect when selectedTargets.size === 0 and immediately log a
clear error including the provided PROOFKIT_BINARY_TARGETS value and available
valid targets, then exit non‑zero (e.g., process.exit(1)) so the build fails
fast when no valid binary targets were specified.
In `@packages/cli/scripts/write-cli-version.mjs`:
- Around line 12-26: The write-to-file can fail with ENOENT because the target
directory for outputPath isn't created; before calling writeFileSync(outputPath,
...), ensure the directory exists by creating the directory returned from
path.dirname(outputPath) (using fs.mkdirSync or fs.promises.mkdir with {
recursive: true } or your project's ensureDir helper), so that writeFileSync in
the script (packages/cli/scripts/write-cli-version.mjs) always has an existing
src/generated folder to write the generated package-versions.ts file into.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3ea25848-c22f-4bde-9778-69176805a3de
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
.changeset/ninety-pets-argue.md.github/workflows/release.ymlpackages/cli/bin/proofkit.cjspackages/cli/package.jsonpackages/cli/scripts/build-binaries.mjspackages/cli/scripts/build-current-binary.mjspackages/cli/scripts/write-cli-version.mjspackages/cli/src/consts.tspackages/cli/src/core/executeInitPlan.tspackages/cli/src/index.tspackages/cli/src/utils/addPackageDependency.tspackages/cli/src/utils/getProofKitVersion.tspackages/cli/src/utils/sortPackageJson.tspackages/cli/template/nextjs-shadcn/src/app/navigation.tsxpackages/cli/template/nextjs-shadcn/src/app/proofkit-route.tspackages/cli/template/nextjs-shadcn/src/components/AppShell/internal/HeaderNavLink.tsxpackages/cli/tests/browser-apps.smoke.test.tspackages/cli/tests/cli.test.tspackages/cli/tests/init-non-interactive-failures.test.tspackages/cli/tests/init-scaffold-contract.test.tspackages/cli/tests/webviewer-apps.test.tspackages/cli/tsdown.config.ts
💤 Files with no reviewable changes (1)
- packages/cli/tsdown.config.ts
- honor `PROOFKIT_DISABLE_BUNDLED_BINARY` - switch `jsonc-parser` imports to ESM path - create CLI version dir before write
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/cli/bin/proofkit.cjs`:
- Around line 23-40: The run() wrapper currently throws on spawn errors and
turns signal-terminated children into exit code 1; instead, when spawnSync
returns result.error, do not throw—return/propagate a failure value so the JS
fallback can run (update callers that expect a boolean or use its return). Also,
if result.signal is set, re-emit that signal against the current process (e.g.,
process.kill(process.pid, result.signal)) to preserve signal semantics;
otherwise if typeof result.status === "number" call process.exit(result.status).
Update the logic used around spawnSync/result.error and the handling of
result.signal and result.status in run() (and the duplicate block at the other
occurrence).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4387c6b6-d9d5-4e1f-9319-f08365f67a7b
📒 Files selected for processing (8)
.gitignorepackages/cli/bin/proofkit.cjspackages/cli/package.jsonpackages/cli/scripts/write-cli-version.mjspackages/cli/src/core/doctor.tspackages/cli/src/generators/fmdapi.tspackages/cli/src/utils/projectFiles.tspackages/cli/tests/init-scaffold-contract.test.ts
✅ Files skipped from review due to trivial changes (2)
- packages/cli/src/utils/projectFiles.ts
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/cli/tests/init-scaffold-contract.test.ts
- packages/cli/package.json
Comment on lines
+23
to
+40
| function run(command, args) { | ||
| const result = spawnSync(command, args, { | ||
| stdio: "inherit", | ||
| env: { | ||
| ...process.env, | ||
| PROOFKIT_PKG_ROOT: path.resolve(__dirname, ".."), | ||
| }, | ||
| }); | ||
|
|
||
| if (result.error) { | ||
| throw result.error; | ||
| } | ||
|
|
||
| if (typeof result.status === "number") { | ||
| process.exit(result.status); | ||
| } | ||
|
|
||
| process.exit(1); |
There was a problem hiding this comment.
Preserve fallback on spawn errors and propagate child signals.
run() currently throws on spawn errors, so a present-but-unexecutable bundled binary prevents reaching the JS fallback. Also, signal-terminated child processes are converted to exit code 1 instead of preserving signal semantics.
Suggested patch
function run(command, args) {
const result = spawnSync(command, args, {
stdio: "inherit",
env: {
...process.env,
PROOFKIT_PKG_ROOT: path.resolve(__dirname, ".."),
},
});
if (result.error) {
throw result.error;
}
if (typeof result.status === "number") {
process.exit(result.status);
}
+
+ if (result.signal) {
+ process.kill(process.pid, result.signal);
+ return;
+ }
process.exit(1);
}
if (process.env.PROOFKIT_DISABLE_BUNDLED_BINARY !== "1") {
const binaryName = BINARIES[process.platform]?.[process.arch];
if (binaryName) {
const binaryPath = path.join(__dirname, binaryName);
if (existsSync(binaryPath)) {
- run(binaryPath, process.argv.slice(2));
+ try {
+ run(binaryPath, process.argv.slice(2));
+ } catch (error) {
+ if (!["EACCES", "ENOENT", "ENOEXEC"].includes(error?.code)) {
+ throw error;
+ }
+ }
}
}
}Also applies to: 47-49
🤖 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 `@packages/cli/bin/proofkit.cjs` around lines 23 - 40, The run() wrapper
currently throws on spawn errors and turns signal-terminated children into exit
code 1; instead, when spawnSync returns result.error, do not
throw—return/propagate a failure value so the JS fallback can run (update
callers that expect a boolean or use its return). Also, if result.signal is set,
re-emit that signal against the current process (e.g., process.kill(process.pid,
result.signal)) to preserve signal semantics; otherwise if typeof result.status
=== "number" call process.exit(result.status). Update the logic used around
spawnSync/result.error and the handling of result.signal and result.status in
run() (and the duplicate block at the other occurrence).
- add Bun 1.3.11 to continuous and tagged release jobs - keep release installs aligned for prebuilt binary publishing
@proofkit/better-auth
@proofkit/cli
create-proofkit
@proofkit/fmdapi
@proofkit/fmodata
@proofkit/typegen
@proofkit/webviewer
commit: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/release.yml:
- Around line 82-86: The workflow currently uses the mutable action reference
"uses: oven-sh/setup-bun@v2" in the "Setup Bun" step; replace that mutable tag
with the action's immutable commit SHA (e.g.,
oven-sh/setup-bun@<full-commit-sha>) in both occurrences of the "Setup Bun" step
so the release job pins to a specific commit instead of `@v2`.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 194cde64-18d6-4e60-9bfb-bdf455f56b32
📒 Files selected for processing (2)
.github/workflows/continuous-release.yml.github/workflows/release.yml
Comment on lines
+82
to
+86
| - name: Setup Bun | ||
| uses: oven-sh/setup-bun@v2 | ||
| with: | ||
| bun-version: 1.3.11 | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that Bun setup action refs are pinned to 40-char commit SHAs.
rg -nP --glob '.github/workflows/*.yml' 'uses:\s*oven-sh/setup-bun@(?![0-9a-f]{40}\b).+'Repository: proofsh/proofkit
Length of output: 209
🏁 Script executed:
# Find workflow files in the repository
fd -e yml -e yaml . .github/workflows/ 2>/dev/null || echo "Directory may not exist, checking alternatives"
# Also check if .github directory exists
ls -la .github/ 2>/dev/null || echo ".github directory not found"
# List all files in .github if it exists
find .github -type f 2>/dev/null | head -20Repository: proofsh/proofkit
Length of output: 590
🏁 Script executed:
# Check the content of release.yml to see lines around 82-86 and 164-168
wc -l .github/workflows/release.yml
# Read lines 75-95 to see context around line 82-86
echo "=== Lines 75-95 (context for 82-86) ==="
sed -n '75,95p' .github/workflows/release.yml
# Read lines 155-175 to see context around line 164-168
echo ""
echo "=== Lines 155-175 (context for 164-168) ==="
sed -n '155,175p' .github/workflows/release.yml
# Search for all oven-sh/setup-bun references
echo ""
echo "=== All oven-sh/setup-bun references ==="
rg -n 'oven-sh/setup-bun' .github/workflows/release.ymlRepository: proofsh/proofkit
Length of output: 1314
🏁 Script executed:
# Search for any existing commit SHAs in the workflow file that might give us a pattern
rg -n 'uses:.*@[0-9a-f]{40}' .github/workflows/release.yml
# Check if there are any other GitHub actions pinned to commit SHAs in this file
rg -n 'uses:.*@[0-9a-f]{8,40}' .github/workflows/release.yml
# Look at other actions in the file to see the pinning pattern used
rg -n 'uses:' .github/workflows/release.yml | head -20Repository: proofsh/proofkit
Length of output: 637
🏁 Script executed:
# Let me check if there's a way to find the commit SHA for oven-sh/setup-bun@v2
# First, let's see if there's any documentation or other files that might reference it
rg -i 'oven-sh/setup-bun' . --type md --type txt 2>/dev/null | head -10
# Check if there are any comments or notes about commit SHAs in the repo
rg -i 'commit.*sha|pin.*commit' .github/workflows/ 2>/dev/null | head -10Repository: proofsh/proofkit
Length of output: 42
Pin oven-sh/setup-bun to a full commit SHA in both jobs.
Using @v2 is mutable and weakens release-pipeline supply-chain guarantees. Pin these references to immutable commit SHAs at lines 83 and 165.
Example change
- uses: oven-sh/setup-bun@v2
+ uses: oven-sh/setup-bun@<full_commit_sha_for_v2>🤖 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/release.yml around lines 82 - 86, The workflow currently
uses the mutable action reference "uses: oven-sh/setup-bun@v2" in the "Setup
Bun" step; replace that mutable tag with the action's immutable commit SHA
(e.g., oven-sh/setup-bun@<full-commit-sha>) in both occurrences of the "Setup
Bun" step so the release job pins to a specific commit instead of `@v2`.
- write new pnpm-workspace trust policy - self-update pnpm before install - adjust init tests
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/cli/src/core/planInit.ts (1)
24-34: ⚡ Quick winExtract policy literals into named constants
Line 32 introduces a magic number (
43200). Pull policy values into named constants so intent is clearer and future updates are safer.Suggested refactor
+const PNPM_TRUST_POLICY = "no-downgrade"; +const PNPM_TRUST_POLICY_IGNORE_AFTER = 43_200; +const PNPM_BLOCK_EXOTIC_SUBDEPS = true; + -function createPnpmWorkspaceFileContent() { +function createPnpmWorkspaceFileContent(): string { return [ @@ - "trustPolicy: no-downgrade", + `trustPolicy: ${PNPM_TRUST_POLICY}`, @@ - "trustPolicyIgnoreAfter: 43200", + `trustPolicyIgnoreAfter: ${PNPM_TRUST_POLICY_IGNORE_AFTER}`, @@ - "blockExoticSubdeps: true", + `blockExoticSubdeps: ${PNPM_BLOCK_EXOTIC_SUBDEPS}`,As per coding guidelines, "Use meaningful variable names instead of magic numbers - extract constants with descriptive names".
🤖 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 `@packages/cli/src/core/planInit.ts` around lines 24 - 34, The trustPolicyIgnoreAfter literal (43200) in createPnpmWorkspaceFileContent should be extracted to a well-named constant (e.g., TRUST_POLICY_IGNORE_AFTER_SECONDS or DEFAULT_TRUST_POLICY_IGNORE_AFTER_HOURS) declared at the top of the module; replace the inline magic number in the trustPolicyIgnoreAfter line with that constant name so the intent is clear and future changes are centralized. Ensure the constant name reflects the unit (seconds/hours) and update any related comments or tests if present.
🤖 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 `@packages/cli/src/core/planInit.ts`:
- Line 26: The generated workspace comment string contains typos and awkward
phrasing; update the string that currently reads "\"# This setting defines where
in the repo your apps/packages that need installed dependancies exist. This of
this as a list of paths to your package.json files. \"" to a clear, user-facing
sentence such as "\"# This setting defines where in the repo your apps/packages
that need installed dependencies exist. Think of this as a list of paths to your
package.json files.\"", fixing "dependancies" → "dependencies" and "This of
this" → "Think of this".
---
Nitpick comments:
In `@packages/cli/src/core/planInit.ts`:
- Around line 24-34: The trustPolicyIgnoreAfter literal (43200) in
createPnpmWorkspaceFileContent should be extracted to a well-named constant
(e.g., TRUST_POLICY_IGNORE_AFTER_SECONDS or
DEFAULT_TRUST_POLICY_IGNORE_AFTER_HOURS) declared at the top of the module;
replace the inline magic number in the trustPolicyIgnoreAfter line with that
constant name so the intent is clear and future changes are centralized. Ensure
the constant name reflects the unit (seconds/hours) and update any related
comments or tests if present.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7e02c07a-6f67-4eaf-9696-0ced05add7da
📒 Files selected for processing (5)
packages/cli/src/core/executeInitPlan.tspackages/cli/src/core/planInit.tspackages/cli/tests/executor.test.tspackages/cli/tests/init-scaffold-contract.test.tspackages/cli/tests/planner.test.ts
✅ Files skipped from review due to trivial changes (1)
- packages/cli/tests/planner.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/cli/tests/init-scaffold-contract.test.ts
- packages/cli/src/core/executeInitPlan.ts
| return [ | ||
| "allowBuilds:", | ||
| ...Object.entries(policy).map(([packageName, allowed]) => ` ${JSON.stringify(packageName)}: ${allowed}`), | ||
| "# This setting defines where in the repo your apps/packages that need installed dependancies exist. This of this as a list of paths to your package.json files. ", |
There was a problem hiding this comment.
Fix typo/grammar in generated workspace comment
Line 26 has user-facing wording issues (dependancies, This of this). Since this is scaffolded into new projects, it’s worth cleaning up.
Suggested patch
- "# This setting defines where in the repo your apps/packages that need installed dependancies exist. This of this as a list of paths to your package.json files. ",
+ "# This setting defines where in the repo your apps/packages with installable dependencies exist. Think of this as a list of paths to your package.json 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
| "# This setting defines where in the repo your apps/packages that need installed dependancies exist. This of this as a list of paths to your package.json files. ", | |
| "# This setting defines where in the repo your apps/packages with installable dependencies exist. Think of this as a list of paths to your package.json files.", |
🤖 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 `@packages/cli/src/core/planInit.ts` at line 26, The generated workspace
comment string contains typos and awkward phrasing; update the string that
currently reads "\"# This setting defines where in the repo your apps/packages
that need installed dependancies exist. This of this as a list of paths to your
package.json files. \"" to a clear, user-facing sentence such as "\"# This
setting defines where in the repo your apps/packages that need installed
dependencies exist. Think of this as a list of paths to your package.json
files.\"", fixing "dependancies" → "dependencies" and "This of this" → "Think of
this".
- fail fast on invalid PROOFKIT_BINARY_TARGETS - skip empty builds and update Bun version in release workflow
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
proofkitentrypoint to a CJS launcher with native-binary fallback@proofkit/cliTesting
pnpm run cinot runSummary by CodeRabbit
New Features
Improvements
Tests