Uh oh!
There was an error while loading. Please reload this page.
chore: add build:bin and install:bin scripts - #42
Conversation
wyattjoh
commented
Mar 20, 2026
This might be superseded by the #31 which adds this as well |
📝 WalkthroughWalkthroughThis change adds support for distributing a standalone binary. Two npm scripts are added to 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment Tip CodeRabbit can use oxc to improve the quality of JavaScript and TypeScript code reviews.Add a configuration file to your project to customize how CodeRabbit runs oxc. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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`:
- Around line 11-17: Add tests covering the new binary install flow: create
tests that invoke the "build:bin" and "install:bin" scripts (or call
scripts/install-bin.sh) to verify the produced binary is placed at the expected
target path and that PATH manipulation behavior (e.g., symlink creation, PATH
export, or failure modes) behaves correctly; add these tests under the existing
test suite (using the same test runner invoked by "test": "bun test") and
include cleanup steps to remove artifacts. Specifically, add a test file (e.g.,
tests/install-bin.test.*) that runs the build step (or a mocked build), runs
scripts/install-bin.sh, asserts the binary exists at the intended location and
has executable permissions, and asserts any PATH-related side effects (or mocks
environment updates) and error handling; ensure tests are deterministic by using
a temporary directory and restoring the PATH/env afterward.
In `@scripts/install-bin.sh`:
- Around line 27-35: path_snippet_present currently returns true if any of
several different shell RC files contains ".local/bin" or the MARKER, which can
skip adding the PATH snippet for the user's actual shell; update it to detect
the user's active shell (use $SHELL or fallback to $0) and only inspect that
shell's relevant RC file(s) (e.g., for bash: .bashrc/.bash_profile, for zsh:
.zshrc, for fish: config.fish), then return true only if the MARKER or
".local/bin" is present in the detected shell's RC file(s); reference the
function path_snippet_present and constant MARKER when making the change so
other callers still work as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 26330c3d-fb42-45b1-8540-2da398eff05f
📒 Files selected for processing (2)
package.jsonscripts/install-bin.sh
| "build:bin": "bun build ./src/cli.ts --compile --outfile ./dist/clerk --external @napi-rs/keyring", | ||
| "dev": "bun run ./src/cli.ts", | ||
| "test": "bun test", | ||
| "lint": "oxlint src/", | ||
| "format": "oxfmt --write src/", | ||
| "format:check": "oxfmt --check src/", | ||
| "install:bin": "bash scripts/install-bin.sh", |
There was a problem hiding this comment.
Missing test coverage for new install path (build:bin / install:bin).
Please add tests for the new binary-install flow (at minimum: verifies binary placement and PATH config behavior).
As per coding guidelines, "If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 11 - 17, Add tests covering the new binary install
flow: create tests that invoke the "build:bin" and "install:bin" scripts (or
call scripts/install-bin.sh) to verify the produced binary is placed at the
expected target path and that PATH manipulation behavior (e.g., symlink
creation, PATH export, or failure modes) behaves correctly; add these tests
under the existing test suite (using the same test runner invoked by "test":
"bun test") and include cleanup steps to remove artifacts. Specifically, add a
test file (e.g., tests/install-bin.test.*) that runs the build step (or a mocked
build), runs scripts/install-bin.sh, asserts the binary exists at the intended
location and has executable permissions, and asserts any PATH-related side
effects (or mocks environment updates) and error handling; ensure tests are
deterministic by using a temporary directory and restoring the PATH/env
afterward.
| path_snippet_present() { | ||
| local f | ||
| for f in "${HOME}/.config/fish/config.fish" "${HOME}/.zshrc" "${HOME}/.bash_profile" "${HOME}/.bashrc" "${HOME}/.profile"; do | ||
| [[ -f "${f}" ]] || continue | ||
| grep -qF "${MARKER}" "${f}" && return 0 | ||
| grep -qF ".local/bin" "${f}" && return 0 | ||
| done | ||
| return 1 | ||
| } |
There was a problem hiding this comment.
Cross-shell PATH detection can skip required shell config update (runtime install break).
At Line 73, path_snippet_present returns true if any rc file has .local/bin (Lines 29-33), so the script may skip writing the snippet for the user’s actual shell rc file. Result: clerk may not be on PATH in new sessions for that shell.
Proposed fix
-path_snippet_present() {- local f- for f in "${HOME}/.config/fish/config.fish" "${HOME}/.zshrc" "${HOME}/.bash_profile" "${HOME}/.bashrc" "${HOME}/.profile"; do- [[ -f "${f}" ]] || continue- grep -qF "${MARKER}" "${f}" && return 0- grep -qF ".local/bin" "${f}" && return 0- done- return 1-}+path_snippet_present() {+ local f="$1"+ [[ -f "${f}" ]] || return 1+ grep -qF "${MARKER}" "${f}" && return 0+ grep -qF ".local/bin" "${f}" && return 0+ return 1+}
@@
-added_path_to_rc=false-rc=""-if path_snippet_present; then+added_path_to_rc=false+rc="$(pick_shell_config)"+if path_snippet_present "${rc}"; then
:
else
- rc="$(pick_shell_config)"
append_path_snippet "${rc}"
added_path_to_rc=true
fiAlso applies to: 73-79
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/install-bin.sh` around lines 27 - 35, path_snippet_present currently
returns true if any of several different shell RC files contains ".local/bin" or
the MARKER, which can skip adding the PATH snippet for the user's actual shell;
update it to detect the user's active shell (use $SHELL or fallback to $0) and
only inspect that shell's relevant RC file(s) (e.g., for bash:
.bashrc/.bash_profile, for zsh: .zshrc, for fish: config.fish), then return true
only if the MARKER or ".local/bin" is present in the detected shell's RC
file(s); reference the function path_snippet_present and constant MARKER when
making the change so other callers still work as before.
jfoshee
commented
Mar 20, 2026
jfoshee
commented
Mar 20, 2026
Closing this one as superseded by @wyattjoh's work which adds a |
to simplify local testing / installation
so you can pull down the repo and just run:
and it copies the
clerkbinary to~/.local/bin.It also updates the user's PATH without asking for their permission...
Summary by CodeRabbit