fix: improve setup.sh bootstrap detection and error handling#316
fix: improve setup.sh bootstrap detection and error handling#316marcusquinn merged 2 commits intomainfrom
Conversation
- Fix bootstrap detection for bash <(curl ...) by matching /dev/fd/* paths - Suppress chmod errors when paths don't exist in curl context - Add pipx support for beads_viewer install (handles externally-managed Python) - Improve Worktrunk shell integration error reporting - Suppress Playwright npx warning during browser install These fixes resolve issues seen when running setup via curl on a fresh Mac.
Summary of ChangesHello @marcusquinn, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Warning Rate limit exceeded
⌛ 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. WalkthroughUpdates to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Thu Feb 5 00:04:30 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
There was a problem hiding this comment.
Code Review
This pull request introduces several improvements to the setup.sh script, focusing on bootstrap detection, error handling, and installation logic for dependencies. The changes enhance robustness, especially for fresh installations on macOS.
My review identifies two areas for improvement:
- The logic for installing
beads_viewercontains duplicated code and a redundant command that could be refactored for clarity and correctness. - The method used to suppress a warning during Playwright installation introduces a bug where a successful installation could be reported as a failure.
I've provided detailed comments and code suggestions for both points. Overall, these are valuable fixes, and with these adjustments, the script will be even more reliable.
| # Use -y to auto-confirm npx install, suppress the "install without dependencies" warning | ||
| if npx -y playwright@latest install 2>&1 | grep -v "WARNING: It looks like you are running"; then | ||
| print_success "Playwright browsers installed" | ||
| else | ||
| print_warning "Playwright browser installation failed" | ||
| print_info "Run manually: npx playwright install" | ||
| print_info "Run manually: npx -y playwright@latest install" | ||
| fi |
There was a problem hiding this comment.
The use of grep -v in the if condition is problematic. grep -v exits with status 1 if it doesn't find any matching lines to filter. In this case, if the npx command is successful and does not produce the 'WARNING...' message, grep -v will exit with 1, causing the if condition to fail and incorrectly report that the installation failed.
To fix this, you should check the exit status of the npx command directly using the PIPESTATUS array after the command has run. The suggested change implements this fix.
| # Use -y to auto-confirm npx install, suppress the "install without dependencies" warning | |
| if npx -y playwright@latest install 2>&1 | grep -v "WARNING: It looks like you are running"; then | |
| print_success "Playwright browsers installed" | |
| else | |
| print_warning "Playwright browser installation failed" | |
| print_info "Run manually: npx playwright install" | |
| print_info "Run manually: npx -y playwright@latest install" | |
| fi | |
| # Use -y to auto-confirm npx install, suppress the "install without dependencies" warning | |
| npx -y playwright@latest install 2>&1 | grep -v "WARNING: It looks like you are running" | |
| if [[ ${PIPESTATUS[0]} -eq 0 ]]; then | |
| print_success "Playwright browsers installed" | |
| else | |
| print_warning "Playwright browser installation failed" | |
| print_info "Run manually: npx -y playwright@latest install" | |
| fi |
setup.sh
Outdated
| if command -v pipx &> /dev/null; then | ||
| read -r -p " Install beads_viewer (Python TUI with graph analytics)? (y/n): " install_viewer | ||
| if [[ "$install_viewer" == "y" ]]; then | ||
| print_info "Installing beads_viewer via pipx..." | ||
| if pipx install beads-viewer 2>/dev/null; then | ||
| print_success "beads_viewer installed (run: beads-viewer)" | ||
| ((installed_count++)) | ||
| else | ||
| print_warning "Failed to install beads_viewer" | ||
| print_info "Try manually: pipx install beads-viewer" | ||
| fi | ||
| fi | ||
| elif command -v pip3 &> /dev/null || command -v pip &> /dev/null; then | ||
| read -r -p " Install beads_viewer (Python TUI with graph analytics)? (y/n): " install_viewer | ||
| if [[ "$install_viewer" == "y" ]]; then | ||
| print_info "Installing beads_viewer..." | ||
| if pip3 install beads-viewer 2>/dev/null || pip install beads-viewer 2>/dev/null; then | ||
| # Try pipx first (handles externally-managed-environment), fall back to pip | ||
| if pipx install beads-viewer 2>/dev/null || pip3 install --user beads-viewer 2>/dev/null || pip install --user beads-viewer 2>/dev/null; then | ||
| print_success "beads_viewer installed" | ||
| ((installed_count++)) | ||
| else | ||
| print_warning "Failed to install beads_viewer" | ||
| print_info "On macOS, install pipx first: brew install pipx && pipx ensurepath" | ||
| fi | ||
| fi | ||
| fi |
There was a problem hiding this comment.
This block has some code duplication and a small logical flaw. The read prompt and the if [[ "$install_viewer" == "y" ]] block are repeated. More importantly, the elif branch at line 2639 is taken when pipx is not found, but the if condition at line 2644 still tries to use pipx, which is guaranteed to fail.
Consider refactoring to ask the user once, then decide which installation method to use. This will make the code cleaner and avoid the redundant pipx call.
# beads_viewer (Python) - use pipx for isolated install
if command -v pipx &> /dev/null || command -v pip3 &> /dev/null || command -v pip &> /dev/null; then
read -r -p " Install beads_viewer (Python TUI with graph analytics)? (y/n): " install_viewer
if [[ "$install_viewer" == "y" ]]; then
if command -v pipx &> /dev/null; then
print_info "Installing beads_viewer via pipx..."
if pipx install beads-viewer 2>/dev/null; then
print_success "beads_viewer installed (run: beads-viewer)"
((installed_count++))
else
print_warning "Failed to install beads_viewer"
print_info "Try manually: pipx install beads-viewer"
fi
else
print_info "Installing beads_viewer..."
if pip3 install --user beads-viewer 2>/dev/null || pip install --user beads-viewer 2>/dev/null; then
print_success "beads_viewer installed"
((installed_count++))
else
print_warning "Failed to install beads_viewer"
print_info "On macOS, install pipx first: brew install pipx && pipx ensurepath"
fi
fi
fi
fi- Refactor beads_viewer install to avoid duplicate prompts and redundant pipx call - Fix Playwright install check to use PIPESTATUS instead of grep exit code Co-authored-by: gemini-code-assist <gemini-code-assist@google.com>
|
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Thu Feb 5 00:08:39 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |



Summary
Fixes issues observed when running
bash <(curl -fsSL https://aidevops.sh/install)on a fresh Mac:bash <(curl ...)because it produces paths like/dev/fd/63. Added pattern matching for/dev/fd/*Test Plan
bash <(curl -fsSL https://aidevops.sh/install)on a fresh MacRelated Issues
Reported by user during fresh Mac setup - see conversation for full output.
Summary by CodeRabbit
Bug Fixes
Improvements