Conversation
The npm `bun` package ships its bins as `bin/bunx.exe` on every platform. Package-manager shims that exec the target by path (pnpm, cmd-shim) pass that path as argv[0], so on Linux/macOS argv[0] ends with `bunx.exe` and the posix `is_bun_x` check (which only matched the `bunx` suffix) returned false. Bun then fell through to AutoCommand and printed `Script not found "codemod@latest"` instead of running bunx. Match `bunx.exe` on every platform, and restore the `bunx-debug` match for debug builds (dropped in b67739b when the check was collapsed to `"bunx" ++ exe_suffix`). Fixes #14596
|
Warning Review limit reached
Next review available in: 9 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
There was a problem hiding this comment.
The Rust change in is_bun_x looks correct — it's called from both which() and the fast-path guard in start(), so both dispatch routes are covered, and the restored bunx-debug / bunx-debug.exe suffixes match what install_completions_command.rs actually creates. Only the test assertions need fixing (see inline).
Extended reasoning...
Verified the predicate change is exercised on every path that reads argv0 for bunx detection (the !is_bun_x(argv0) fast-path guard and which()), and cross-checked the bunx-debug link names against install_completions_command.rs:20,123,128. The inline finding on the test's stdout/exit-code assertions is the only issue; once that's addressed this should be good.
|
Reproduced with: cp $(which bun) /tmp/bunx.exe && /tmp/bunx.exe codemod@latest --version
# error: Script not found "codemod@latest"Test verified locally: fails on main (shows full CI: build 80763 passed all test lanes including |
There was a problem hiding this comment.
LGTM — the earlier feedback is fully addressed in 657d25f.
What was reviewed:
is_bun_xis a strict widening (addsbunx.exeon posix +bunx-debug/bunx-debug.exeunderIS_DEBUG); nothing that matched before stops matching, andEnvironment::IS_DEBUGconst-folds away in release.- Confirmed
install_completions_command.rsdoes createbunx-debug/bunx-debug.exelinks in debug builds, so the restored suffix is load-bearing. - Test now asserts stderr/stdout/exit-code separately and mirrors the sibling "should output usage if no arguments are passed" test; the posix symlink keeps
argv[0]ending inbunx.exewhile avoiding the ~1GB debug-binary copy.
Extended reasoning...
Overview
Two-file change fixing #14596: is_bun_x(argv0) in src/runtime/cli/mod.rs now matches the bunx.exe suffix on every platform (previously only on Windows), and additionally matches bunx-debug / bunx-debug.exe when IS_DEBUG is set. A regression test in test/cli/install/bunx.test.ts spawns a bunx.exe-named symlink/copy with --help and asserts the bunx usage banner (stderr, exit 1) rather than the full bun help (stdout, exit 0).
Security risks
None. This is a byte-suffix comparison on argv[0] deciding which subcommand to dispatch. The predicate is strictly widened; the pre-existing ends_with semantics (no path-separator boundary) are unchanged, so no new false-positive class is introduced beyond what bunx / bunx.exe on Windows already had. No untrusted input, no allocation, no I/O.
Level of scrutiny
Low. The Rust change is 8 lines that collapse a #[cfg] split into a single platform-agnostic check plus a debug-only fallback. bun_core::Environment is a re-export of bun_core::env (lib.rs:1091), so Environment::IS_DEBUG is the same compile-time const used by install_completions_command.rs, and the bunx-debug arm compiles out of release builds. I verified install_completions_command.rs:19-20,122-130 really does emit bunx-debug / bunx-debug.exe under IS_DEBUG, so the restored match is not speculative.
Other factors
This PR has already been through two review rounds from me (the stdout/exit-0 bug and the out+err concatenation nit); 657d25f adopts exactly the shape suggested — separate err/out/exited assertions matching the sibling test ~35 lines up — and additionally swaps the posix copyFileSync for symlinkSync so the test no longer copies a ~1GB debug binary (spawning the symlink still yields argv[0] ending in bunx.exe, which is what the fix keys on). The Windows path retains copyFileSync, consistent with the neighbouring "symlinked bunx" test in the same file. All prior inline threads are resolved and the comment-cop note was addressed by shortening the comment to one line.
There was a problem hiding this comment.
LGTM — the is_bun_x widening is a strict superset of the old match and all earlier review feedback has been addressed. The one remaining inline nit (extension-less bunx-debug variant on a Windows debug build) is test-only and latent — no Windows-debug lane runs in default CI — so it doesn't block.
Extended reasoning...
Overview
Two files: src/runtime/cli/mod.rs collapses the platform-split is_bun_x predicate into a single body that matches bunx / bunx.exe (and, under IS_DEBUG, bunx-debug / bunx-debug.exe) on every platform; test/cli/install/bunx.test.ts adds an .each test that symlinks/copies the bun binary to each of those names and asserts --help produces the bunx usage banner on stderr with exit 1 rather than the full bun help on stdout with exit 0.
Security risks
None. This is a suffix comparison on argv[0] that only widens the set of names classified as bunx. The old posix branch already accepted any argv0 ending in bunx, so accepting bunx.exe too introduces no new false-positive class. No parsing of untrusted input, no filesystem/network access.
Level of scrutiny
Low-to-moderate. The runtime change is 7 lines in the CLI dispatch predicate; both call sites (start()'s fast-path guard and which()) were traced and behave correctly with the widened match — the fast-path is guarded by !is_bun_x(argv0) so it is now correctly skipped, and which() returns Tag::BunxCommand before the entrypoint heuristic. The IS_DEBUG-gated bunx-debug arm compiles out of release builds. The test now mirrors the sibling "should output usage if no arguments are passed" test's assertion shape (stderr / empty stdout / exit 1), addressing both of my earlier review rounds.
Other factors
All three prior review threads (stdout/stderr mix-up, comment-cop paragraph, weak out+err assertions) are resolved and the current test asserts stderr/stdout/exitCode separately. The only new finding this run is a nit: on a Windows debug build the extension-less "bunx-debug" matrix entry would spawn an unexecutable extension-less PE. That's latent (no Windows-debug lane in the default CI matrix), test-only, and a one-token gate to fix — it does not affect the correctness of the shipped runtime change, so I'm not holding approval on it.
There was a problem hiding this comment.
LGTM — all three prior review rounds addressed; the fix is a 7-line unification of the posix/Windows is_bun_x suffix check.
What was reviewed:
is_bun_xnow matchesbunx/bunx.exeon all platforms andbunx-debug/bunx-debug.exeunderIS_DEBUG; verified againstinstall_completions_command.rswhich creates exactly those names.- The two
is_bun_x(argv0)call sites instart()(fast-path guard) andwhich()both pick up the widened match consistently. - Test now asserts stderr/stdout/exit-code separately, symlinks on posix instead of copying the debug binary, and gates the extension-less
bunx-debugvariant off Windows.
Extended reasoning...
Overview
The PR widens is_bun_x(argv0) in src/runtime/cli/mod.rs to match the bunx.exe suffix on posix (previously only bunx), and restores the bunx-debug / bunx-debug.exe matches for debug builds. The npm bun package ships bin/bunx.exe on all platforms; pnpm's cmd-shim wrappers exec that path directly, so on posix argv[0] ends in bunx.exe and the old suffix check missed it, falling through to AutoCommand and producing error: Script not found "<pkg>". The change collapses the previous #[cfg(windows)] split into a single platform-agnostic block plus an IS_DEBUG-gated debug-name branch (compile-time const, so dead-code-eliminated in release).
A parameterized test in test/cli/install/bunx.test.ts creates the binary under each argv[0] variant (symlink on posix, copy on Windows), spawns --help, and asserts the bunx usage banner on stderr with exit code 1 vs. the full CLI help on stdout with exit code 0.
Security risks
None. This is argv[0] classification for CLI mode selection; no auth, crypto, filesystem, or network behaviour changes. The suffix check is no more permissive than the pre-existing Windows branch (which already matched both suffixes), and the pre-existing pattern of bare ends_with (which also matches e.g. foobunx) is unchanged in shape.
Level of scrutiny
Low-to-medium. The Rust change is 7 lines in a single leaf predicate with two callers, both in the same file and both read during this review. The debug-name claim was verified against install_completions_command.rs:19-23,122-131, which creates bunx-debug on posix and bunx-debug.exe/bunx-debug.cmd on Windows. bun_core::Environment::IS_DEBUG is a compile-time const, so the extra checks add nothing to release builds.
Other factors
Three prior review rounds on this PR were all resolved by follow-up commits: (1) test asserted on stdout/exit 0 when bunx help goes to stderr/exit 1 — fixed; (2) test concatenated out+err and dropped the exit-code assertion — now asserts each stream and exited === 1 separately, and switched from copying a ~1 GB debug binary to a symlink on posix; (3) the extension-less bunx-debug matrix entry would fail on a Windows-debug build — now gated on !isWindows. The current test mirrors the sibling "should output usage if no arguments are passed" test's assertion shape, uses it.concurrent.each, await using on the subprocess, and the file's shared setup() helper. bunExe() returns process.execPath (absolute), so the symlink target resolves regardless of cwd.
What does this PR do?
Addresses the posix
error: Script not found "codemod@latest"symptom reported in #14596.is_bun_xis the predicate that decides whetherbunwas invoked asbunx. On posix it only matched the literal suffixbunx:The npm
bunpackage ships its bins asbin/bunx.exeon every platform so that a single tarball works on Windows too. npm on Unix creates a symlinknode_modules/.bin/bunx -> ../bun/bin/bunx.exe, and running that symlink givesargv[0]ending inbunx, which matched. But pnpm (and other cmd-shim-style linkers) write a shell wrapper instead:so
argv[0]is the real path ending inbunx.exe. On posix that failed the suffix check, Bun fell through toAutoCommand, and the user saw:Repro
Fix
Match both
bunxandbunx.exeon every platform (the Windows branch already matched both; the posix branch is now brought in line). Also restores thebunx-debug/bunx-debug.exematch for debug builds, whichinstall_completions_commandcreates as the bunx link name underIS_DEBUG; it was dropped in b67739b when the check was collapsed to"bunx" ++ exe_suffix.Note on #14596 scope
This PR does not address the Windows
Cannot find module '.bin/jscodeshift'failure reported in the later comments on #14596. In that case bunx detection already succeeded (the package was installed to the bunx temp cache); the failure is in transitive.binresolution inside the installed package, which is a separate issue. #10739 and #15708 were closed as duplicates of #14596 for that symptom, so leaving #14596 open for now.How did you verify your code works?
New parameterized test in
test/cli/install/bunx.test.tscreates the bun binary under each ofbunx.exe(all builds) andbunx-debug/bunx-debug.exe(debug builds only), spawns--help, and asserts the bunx usage banner on stderr with exit code 1 rather than the full bun help on stdout with exit code 0. Fails on main, passes with this change.no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/install/bunx.test.ts