feat: add cli offline bundle script#8192
Conversation
sid-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno and
naman-bruno
as code owners
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds scripts to produce an offline-installable tarball for ChangesCLI Offline Bundle
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 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 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 |
There was a problem hiding this comment.
Pull request overview
Adds a new bundling utility under scripts/ to generate a standalone, offline-installable tarball for the Bruno CLI, intended for distribution scenarios.
Changes:
- Added a Bash wrapper (
bundle-cli.sh) to run the bundling flow consistently from the repo. - Added a Node.js script (
bundle-cli.js) that packs workspace packages, stages the CLI, installs dependencies, and packs a final distributable.tgzintodist/cli-bundle/.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| scripts/bundle-cli.sh | Thin shell entrypoint to run the CLI bundle script from a stable path. |
| scripts/bundle-cli.js | Implements the offline bundle creation flow (workspace packing, staging, install, final pack). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
scripts/bundle-cli.js (2)
66-66: 💤 Low valueInconsistent error logging.
This line uses
console.error(err)directly instead of theloghelper defined at line 19, creating inconsistent output formatting.For consistency, either use the
loghelper or useconsole.erroreverywhere.♻️ Suggested fix for consistency
} catch (err) { - console.error(err); - log(` skip ${name} (${err.message.split('\n')[0]})`); + log(` skip ${name} (${err.message.split('\n')[0]})`);🤖 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/bundle-cli.js` at line 66, The console.error(err) call is inconsistent with the project’s logging helper; replace that direct console call with the defined log helper (use the same helper used at line 19, e.g., log.error or the appropriate log(...) method) so error output formatting matches the rest of the script; update the call where console.error(err) appears and ensure the log invocation includes the error object or message for full context.
80-81: ⚡ Quick winUncaught errors are silently suppressed.
The
uncaughtExceptionandunhandledRejectionhandlers cleanup and exit without logging the actual error, making debugging failures difficult.Log the error before cleanup to aid troubleshooting.
📝 Proposed fix to log errors
-process.on('uncaughtException', () => { cleanup(); process.exit(1); }); -process.on('unhandledRejection', () => { cleanup(); process.exit(1); }); +process.on('uncaughtException', (err) => { console.error('Uncaught exception:', err); cleanup(); process.exit(1); }); +process.on('unhandledRejection', (err) => { console.error('Unhandled rejection:', err); cleanup(); process.exit(1); });🤖 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/bundle-cli.js` around lines 80 - 81, The current process.on handlers for 'uncaughtException' and 'unhandledRejection' call cleanup() and process.exit(1) without logging the error; update the handlers for process.on('uncaughtException', ...) and process.on('unhandledRejection', ...) to accept the error/reason parameter, call processLogger.error (or console.error) with a descriptive message and the error/reason object, then call cleanup() and exit; ensure both handlers include the error details to aid debugging.
🤖 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 `@scripts/bundle-cli.js`:
- Line 20: The helper run currently builds a shell string and calls execSync,
which risks command injection; change run to use a non-shell child API (e.g.,
child_process.spawnSync or child_process.execFile) and pass the command and its
arguments as an array instead of a single interpolated string, preserving cwd,
stdio/encoding behavior and returning the trimmed stdout; update every call site
that uses run to pass the executable and args separately (references: run and
its internal use of execSync) and ensure errors are propagated when the child
exits non-zero.
- Line 32: The call to fs.chmodSync(d, fs.statSync(s).mode) is not Windows-safe;
update the bundle logic in scripts/bundle-cli.js to skip or gracefully handle
chmod on Windows by checking process.platform !== 'win32' (or wrapping the
fs.chmodSync call in a try/catch that ignores ENOSYS/EPERM on Windows), using
the same fs.statSync(s).mode when applicable; reference the existing
fs.chmodSync and fs.statSync calls and variables d and s when making the change.
In `@scripts/bundle-cli.sh`:
- Around line 1-6: The Bash-only wrapper scripts/bundle-cli.sh won't run on
native Windows; fix by providing cross-platform ways to invoke bundle-cli.js:
add a package.json script entry (e.g., "bundle-cli": "node
scripts/bundle-cli.js") and update docs/README to instruct users to run node
scripts/bundle-cli.js or npm run bundle-cli as the supported entrypoint, and
optionally add small Windows wrappers (bundle-cli.cmd that runs node
"%~dp0\bundle-cli.js" and bundle-cli.ps1 that runs node
"$PSScriptRoot\bundle-cli.js") so users who call the script directly on Windows
get a working wrapper. Ensure references to scripts/bundle-cli.sh are replaced
or annotated to point to the cross-platform invocation.
---
Nitpick comments:
In `@scripts/bundle-cli.js`:
- Line 66: The console.error(err) call is inconsistent with the project’s
logging helper; replace that direct console call with the defined log helper
(use the same helper used at line 19, e.g., log.error or the appropriate
log(...) method) so error output formatting matches the rest of the script;
update the call where console.error(err) appears and ensure the log invocation
includes the error object or message for full context.
- Around line 80-81: The current process.on handlers for 'uncaughtException' and
'unhandledRejection' call cleanup() and process.exit(1) without logging the
error; update the handlers for process.on('uncaughtException', ...) and
process.on('unhandledRejection', ...) to accept the error/reason parameter, call
processLogger.error (or console.error) with a descriptive message and the
error/reason object, then call cleanup() and exit; ensure both handlers include
the error details to aid debugging.
🪄 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: fd86da94-90aa-47f0-95d1-13e3ca9a4af8
📒 Files selected for processing (2)
scripts/bundle-cli.jsscripts/bundle-cli.sh
| const TARBALLS = path.join(OUT_DIR, 'tarballs'); | ||
|
|
||
| const log = msg => process.stdout.write(`> ${msg}\n`); | ||
| const run = (cmd, cwd) => execSync(cmd, { cwd, stdio: 'pipe', encoding: 'utf8' }).trim(); |
There was a problem hiding this comment.
Command injection risk with string-based execSync.
The run helper passes shell command strings directly to execSync, which can be vulnerable if paths contain special characters (quotes, backticks, semicolons, etc.). Although the paths here are derived from __dirname and constants rather than direct user input, repository paths with unusual characters could still cause issues or security risks.
Use child_process.spawn or child_process.execFile with argument arrays instead of string interpolation.
🔒 Proposed fix using spawn with argument array
-const run = (cmd, cwd) => execSync(cmd, { cwd, stdio: 'pipe', encoding: 'utf8' }).trim();
+const { spawnSync } = require('child_process');
+const run = (cmd, args, cwd) => {
+ const result = spawnSync(cmd, args, { cwd, stdio: 'pipe', encoding: 'utf8' });
+ if (result.error) throw result.error;
+ if (result.status !== 0) throw new Error(`Command failed: ${cmd} ${args.join(' ')}`);
+ return result.stdout.trim();
+};Then update call sites:
- run(`npm pack --pack-destination "${TARBALLS}"`, dir);
+ run('npm', ['pack', '--pack-destination', TARBALLS], dir);-run(`npm pack --pack-destination "${OUT_DIR}"`, stage);
+run('npm', ['pack', '--pack-destination', OUT_DIR], stage);🧰 Tools
🪛 OpenGrep (1.22.0)
[ERROR] 20-20: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🤖 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/bundle-cli.js` at line 20, The helper run currently builds a shell
string and calls execSync, which risks command injection; change run to use a
non-shell child API (e.g., child_process.spawnSync or child_process.execFile)
and pass the command and its arguments as an array instead of a single
interpolated string, preserving cwd, stdio/encoding behavior and returning the
trimmed stdout; update every call site that uses run to pass the executable and
args separately (references: run and its internal use of execSync) and ensure
errors are propagated when the child exits non-zero.
| copyDir(s, d); | ||
| } else { | ||
| fs.copyFileSync(s, d); | ||
| fs.chmodSync(d, fs.statSync(s).mode); |
There was a problem hiding this comment.
File permission handling incompatible with Windows.
fs.chmodSync uses Unix-style permission bits that Windows doesn't support. Bruno is a cross-platform Electron app, and the coding guidelines require accounting for Windows not supporting Unix-style permission bits.
Wrap the chmodSync call in a platform check or handle permission errors gracefully.
🛠️ Proposed fix with platform check
- fs.copyFileSync(s, d);
- fs.chmodSync(d, fs.statSync(s).mode);
+ fs.copyFileSync(s, d);
+ if (process.platform !== 'win32') {
+ try {
+ fs.chmodSync(d, fs.statSync(s).mode);
+ } catch (err) {
+ // Ignore permission errors on platforms that don't support them
+ }
+ }As per coding guidelines: "File permissions (e.g., fs.chmod, fs.access) should account for Windows not supporting Unix-style permission bits."
🤖 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/bundle-cli.js` at line 32, The call to fs.chmodSync(d,
fs.statSync(s).mode) is not Windows-safe; update the bundle logic in
scripts/bundle-cli.js to skip or gracefully handle chmod on Windows by checking
process.platform !== 'win32' (or wrapping the fs.chmodSync call in a try/catch
that ignores ENOSYS/EPERM on Windows), using the same fs.statSync(s).mode when
applicable; reference the existing fs.chmodSync and fs.statSync calls and
variables d and s when making the change.
Description
Add a simple script to create a standalone cli tarball for distributions
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit