Skip to content

feat: Shell Customization Menu with Starship, Oh My Zsh, Prezto support#228

Merged
kshivang merged 25 commits intomasterfrom
dev
Jan 7, 2026
Merged

feat: Shell Customization Menu with Starship, Oh My Zsh, Prezto support#228
kshivang merged 25 commits intomasterfrom
dev

Conversation

@kshivang
Copy link
Copy Markdown
Owner

@kshivang kshivang commented Jan 7, 2026

Summary

  • Add comprehensive Shell submenu to context menu with support for:

    • Starship - Cross-shell prompt customization
    • Oh My Zsh - Zsh framework with themes and plugins
    • Prezto - Alternative Zsh framework
    • Shells - Zsh, Bash, Fish install and configuration
  • Mutual exclusivity - Installing one tool automatically uninstalls conflicting tools:

    • Starship uninstalls Oh My Zsh/Prezto when default shell is Zsh
    • Oh My Zsh/Prezto uninstall each other and Starship (if configured for Zsh)
  • Platform-aware install commands - Support for macOS (brew), Linux (apt/dnf/pacman), Windows (winget/WSL)

  • Install/Uninstall dialogs - All install and uninstall actions show confirmation dialog

  • Auto shell reload - Shell config automatically sourced after changes

Features

Tool Detection Install Config Options
Starship which starship curl script Presets, Shell setup, Edit config
Oh My Zsh ~/.oh-my-zsh dir Official script Themes, Plugins, Update
Prezto ~/.zprezto dir git clone Modules, Themes, Update
Zsh/Bash/Fish which Package manager Set default, Edit config

Test plan

  • Test Starship install/uninstall
  • Test Oh My Zsh install/uninstall
  • Test Prezto install/uninstall
  • Test mutual exclusivity (install one, verify others uninstalled)
  • Test shell setup commands auto-source config
  • Verify menus hidden when dependencies not met (Oh My Zsh/Prezto hidden without Zsh)

🤖 Generated with Claude Code

Add Starship shell prompt customization to context menu:
- New SHELL_CUSTOMIZATION tool category
- Starship definition with curl install script
- Detection paths including ~/.cargo/bin/starship
- New ShellCustomizationMenuProvider with:
  - Install + Learn More (when not installed)
  - Edit Config, Apply Preset, Shell Setup (when installed)
  - 7 preset themes (Nerd Font, Plain Text, Pastel Powerline, etc.)
  - Shell setup commands for Bash, Zsh, Fish
  - Documentation links

Menu structure:
  Shell > Starship > Configuration / Presets / Shell Setup / Help

🤖 Generated with [Claude Code](https://claude.com/claude-code)
- Add getOhMyZshInstallCommand() to AIAssistantLauncher
- Add Oh My Zsh definition to BUILTIN with SHELL_CUSTOMIZATION category
- Extend ShellCustomizationMenuProvider with Oh My Zsh detection and menu:
  - Detection: Check ~/.oh-my-zsh directory exists
  - Not installed: Install + Learn More submenu
  - Installed: Themes (popular presets), Plugins, Maintenance options
- Update status holders in TabbedTerminal and EmbeddableTerminal

🤖 Generated with [Claude Code](https://claude.com/claude-code)
- Starship: Removes binary via `rm $(which starship)`
- Oh My Zsh: Uses built-in `uninstall_oh_my_zsh` command

🤖 Generated with [Claude Code](https://claude.com/claude-code)
- Add detection for zsh and bash installation status
- Add platform-aware install commands (brew/apt/dnf/pacman)
- Zsh menu: version, set as default, edit .zshrc, reload config
- Bash menu: version, set as default, edit .bashrc/.bash_profile, reload
- Add "Shells" section separator in menu
- Update status holders to track zsh/bash installation

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Starship is typically installed in /usr/local/bin which requires
root permissions to modify.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Removes starship init lines from:
- ~/.bashrc
- ~/.zshrc
- ~/.config/fish/config.fish

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Adds `exec $SHELL` to restart the shell and clear the cached
prompt hooks that were trying to call the removed starship binary.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Both Starship and Oh My Zsh uninstall actions now use the same
dialog flow as install, allowing users to review the command
before execution.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Shell setup commands now automatically source the config file
after adding the starship init line, so users don't need to
manually reload.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
The onInstallRequest callback now strips "-uninstall" suffix to
find the base tool ID (e.g., "starship-uninstall" -> "starship")
so the dialog can show properly for uninstall actions.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Remove init lines from shell configs before deleting the binary,
so prompt hooks don't try to call missing starship during the
uninstall process.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
The `uninstall_oh_my_zsh` function may not be available if OMZ
isn't properly sourced. Use the script directly instead:
sh ~/.oh-my-zsh/tools/uninstall.sh

🤖 Generated with [Claude Code](https://claude.com/claude-code)
- Add detection for fish installation status
- Add platform-aware install command (brew/apt/dnf/pacman)
- Fish menu options: version, set as default, edit config.fish,
  reload config, web config (GUI)
- Update status holders to track fish installation

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Oh My Zsh requires Zsh to be installed, so the menu option is now
only shown when Zsh is detected on the system.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
- Add detection for Prezto (check ~/.zprezto directory)
- Only show when Zsh is installed (like Oh My Zsh)
- Menu options: modules, themes, update, reload, edit configs
- Install via git clone with automatic symlink setup
- Uninstall removes ~/.zprezto and related config files

🤖 Generated with [Claude Code](https://claude.com/claude-code)
When installing Starship, Oh My Zsh, or Prezto, conflicting tools
are automatically uninstalled first:

- Starship: Uninstalls Oh My Zsh/Prezto only when $SHELL is Zsh
- Oh My Zsh: Uninstalls Prezto + Starship (if configured for Zsh)
- Prezto: Uninstalls Oh My Zsh + Starship (if configured for Zsh)

Added helpers:
- getDefaultShell(): Detects user's default shell from $SHELL
- isStarshipConfiguredForZsh(): Checks ~/.zshrc for starship init
- buildInstallWithUninstall(): Combines uninstall + install commands
- Extracted uninstall commands as reusable methods

🤖 Generated with [Claude Code](https://claude.com/claude-code)
- Add Install + Learn More submenu pattern for Zsh, Bash, Fish
- Add Prezto, Zsh, Bash, Fish to AIAssistants registry
- Starship install now auto-adds init to shell config and restarts shell
- Starship uninstall properly removes init lines and restarts shell
- Set as Default Shell shows "Log out and log back in" message
- Fix default shell fallback: use /bin/sh (POSIX standard)
- Fix npm check for Gemini/Codex (was bypassing Node.js auto-install)

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@claude
Copy link
Copy Markdown

claude Bot commented Jan 7, 2026

Pull Request Review: Shell Customization Menu

Summary

This PR adds comprehensive shell customization support with Starship, Oh My Zsh, Prezto, and shell installations. Well-structured implementation following existing patterns.

Strengths

Architecture: Consistent pattern, proper separation of concerns, good documentation, reusable components

Feature Design: Well-implemented mutual exclusivity, platform awareness, dependency management, rich configuration options

User Experience: Install/Learn More pattern, auto shell reload, logical menu organization

CRITICAL Issues

1. Security: Sudo rm without confirmation

Problem: Line 136 uses sudo rm without adequate user confirmation
Recommendation: Add explicit confirmation messaging or use package manager uninstall

2. Dangerous Prezto Uninstall

Problem: Line 151 - rm -rf deletes ~/.zshrc which may have user customizations
Recommendation: Create backups before deletion or only remove Prezto-specific lines

3. Code Duplication

Problem: Lines 854-888 duplicate install command logic from AIAssistantLauncher
Recommendation: Call AIAssistantLauncher methods instead

Other Issues

  1. Platform Detection: System.getProperty called multiple times - should cache
  2. Error Handling: Commands assume success without verification
  3. Default Shell: Changed /bin/zsh to /bin/sh without documentation
  4. Race Condition: refreshStatus() suspend but getters called immediately

Testing Recommendations

  1. Test mutual exclusivity logic
  2. Test without Zsh installed
  3. Test uninstall operations
  4. Test error scenarios
  5. Test concurrent status checks

Security Review

Documentation Missing

  • CLAUDE.md not updated
  • No mention of /bin/zsh to /bin/sh change
  • No mutual exclusivity documentation

Overall Assessment

Code Quality: 8/10
Feature Completeness: 9/10
Security: 6/10 - Concerning sudo/file deletion
User Experience: 9/10

Recommendation: Request Changes - Security concerns (Issues #1 and #2) must be addressed before merge. Code duplication (#3) should also be cleaned up.

Great work overall! Main concerns are around safety of uninstall operations.

- Add multi-step onboarding wizard for new users
- Shell selection (Zsh, Bash, Fish) with current shell detection
- Shell customization (Starship, Oh My Zsh, Prezto) with conflict handling
- Version control tools (Git, GitHub CLI) installation
- AI assistants (Claude Code, Gemini CLI, Codex, OpenCode) installation
- GitHub authentication step after gh installation
- Embedded terminal for installation progress with password hint
- Relaunch/Dismiss buttons on completion
- Accessible from Help > Welcome Wizard menu
- Auto-shows on first launch, persists completion state
- Deduplicate Git/GitHub commands into GitUtils.kt
- Add Prezto support to window menu
- Remove debug println statements

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@claude
Copy link
Copy Markdown

claude Bot commented Jan 7, 2026

Code Review: Shell Customization Menu & Onboarding Wizard

This is a comprehensive PR that adds shell customization tools and a first-run onboarding wizard. The implementation is well-structured and follows project conventions.

Strengths

  1. Excellent Architecture - Clean separation of concerns, reusable GitUtils, proper Compose state management
  2. User Experience - Mutual exclusivity logic well-implemented, context-aware menus, embedded terminal feedback
  3. Platform Awareness - macOS/Linux/Windows detection, platform-specific install commands
  4. Settings Integration - onboardingCompleted flag persists properly

CRITICAL ISSUES

1. Race Condition in Process Timeouts (HIGH SEVERITY)
Location: ShellCustomizationMenuProvider.kt:41, OnboardingWizard.kt:456, GitUtils.kt:21-25
Problem: If waitFor() times out, calling process.exitValue() throws IllegalThreadStateException
Fix: Check completed before calling exitValue(), destroy process if timed out

2. Resource Leak: Unclosed ProcessBuilder Streams (MEDIUM)
All ProcessBuilder calls do not explicitly close input/output/error streams
Fix: Add try-finally blocks to close streams

3. Missing Error Handling in buildInstallCommand() (MEDIUM)
Location: OnboardingWizard.kt:495-712
713-line function with no error handling for invalid System properties or File I/O failures

MEDIUM ISSUES

  1. Hardcoded /proc/self/cmdline for Relaunch - Linux-only, will fail on macOS/Windows
  2. Potential Command Injection - sed -i commands should validate paths
  3. Blocking I/O on UI Thread - isCommandInstalled() blocks UI with 2-second timeout

MINOR ISSUES

  1. Inconsistent Default Shell - Changed from /bin/zsh to /bin/sh (Dash has limited features)
  2. Magic Numbers - Extract timeout values to constants
  3. Documentation - Add KDoc for public functions

SUMMARY

Blocking Issues (must fix before merge):

  1. Fix race condition in process.exitValue() after timeout
  2. Close ProcessBuilder streams to prevent resource leaks
  3. Add error handling to buildInstallCommand()

Recommended (follow-up PR):

  1. Security review of command string construction
  2. Platform-specific relaunch mechanism
  3. Add unit tests

Code Quality: 8/10 - Well-architected, but needs hardening around process management
Feature Completeness: 9/10 - Excellent UX, comprehensive tool support

Great work overall! Please address critical issues before merging.

- Add OnboardingWizard example to tabbed-example
- Create docs/onboarding.md with full API reference
- Update README features list and documentation links
- Example shows first-launch check and Help menu integration

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@claude
Copy link
Copy Markdown

claude Bot commented Jan 7, 2026

Comprehensive Code Review - PR #228

This is an impressive feature addition that brings shell customization capabilities and an onboarding wizard to BossTerm. Overall, the implementation is well-structured and thoughtfully designed. Here is my detailed feedback:


Strengths

Architecture & Design

  • Excellent separation of concerns: Clean split between detection, installation commands, and UI
  • Reusable GitUtils: Smart refactoring to share Git utilities between context menu and window menu
  • Platform-aware install commands: Good cross-platform support (macOS/Linux/Windows)
  • Mutual exclusivity logic: Well-implemented conflict resolution when installing competing tools

User Experience

  • Step indicator with progress tracking: Professional wizard flow
  • Status badges: Clear "Installed"/"Current" indicators help users understand what they already have
  • Embedded terminal for installation: Transparent installation process with live output
  • GitHub auth flow: Nice touch to include gh auth login step after installing GitHub CLI

Code Quality

  • Consistent styling: Good use of Material 3 design tokens (AccentColor, TextPrimary, etc.)
  • Comprehensive tool detection: Multi-strategy detection including fallback to common paths

⚠️ Issues & Recommendations

🔴 Critical Issues

1. Command Injection Vulnerability (ShellCustomizationMenuProvider.kt:550)

The uninstall command uses unvalidated $SHELL variable in shell execution. If $SHELL environment variable is malicious, executing exec $SHELL is dangerous.

Fix: Validate against a whitelist of known shells before execution.

2. Unclosed Process Resources (OnboardingWizard.kt:453, ShellCustomizationMenuProvider.kt:38)

Multiple instances of ProcessBuilder without closing streams. This causes resource leaks.

Fix: Use try-finally blocks or .use {} to ensure processes are destroyed and streams closed.

3. Destructive Prezto Uninstall (OnboardingWizard.kt:541, 759)

The command rm -rf ~/.zprezto ~/.zshrc ... deletes the user's .zshrc even if they have custom configs unrelated to Prezto.

Fix: Only delete Prezto-specific files and backup user configs before removal.

4. Hardcoded /proc Path (OnboardingWizard.kt:252-256)

Uses /proc/self/cmdline which does not exist on macOS - the restart feature will fail silently on Mac.

Fix: Check OS and use appropriate restart mechanism per platform.


🟡 Medium Priority Issues

5. Race Condition in Installation Detection (OnboardingWizard.kt:145-148)

Detection runs once at startup. If user installs tools in another terminal during wizard, installations will not be detected.

Fix: Add a refresh button or re-detect when navigating back to a step.

6. Sed Incompatibility (OnboardingWizard.kt:533-538)

The -i syntax differs between GNU sed (Linux) and BSD sed (macOS). The fallback pattern is brittle.

Fix: Use Perl for cross-platform compatibility or detect sed version first.

7. Blocking ProcessBuilder in Main Thread (ShellCustomizationMenuProvider.kt:38-46)

The isCommandInstalled function may be called from UI thread if statusOverride is null.

Fix: Enforce that refreshStatus() must be called before opening menu.

8. Inconsistent Error Handling

GitUtils.isGitRepo() and isCommandInstalled() return false on exception with no user feedback when detection fails unexpectedly.

Fix: Log warnings or show toast notifications for detection failures.

9. Prezto Installation Command Fails on Non-Zsh (OnboardingWizard.kt:614)

The setopt EXTENDED_GLOB is a Zsh builtin and will fail if run from bash.

Fix: Wrap the entire command in zsh -c execution.

10. Magic Number: 2-Second Timeout (Multiple files)

waitFor(2, TimeUnit.SECONDS) is repeated 15+ times throughout the codebase.

Fix: Extract to a constant like COMMAND_TIMEOUT_SECONDS = 2L.


🟢 Low Priority / Polish

11. Misleading "Keep Existing" Label

ShellCustomizationChoice.KEEP_EXISTING is shown even when no customization is detected.

Suggestion: Only show if installedTools.hasAnyShellCustomization == true.

12. Missing NPM Package Verification

AI assistant npm packages are hardcoded but never verified to exist on npm registry.

Suggestion: Add comment warning these need verification or test installation.

13. Settings Field Never Used

TerminalSettings.onboardingCompleted is set but never read anywhere except the wizard check.

Suggestion: Add logic to skip first-run dialogs based on this field.

14. Nvm PATH Not Persisted (OnboardingWizard.kt:682-684)

After installing nvm, the PATH changes are not persisted to shell config.

Fix: Append nvm initialization to .bashrc/.zshrc.

15. Starship Preset Command Wrong

The command starship preset -o ~/.config/starship.toml requires a preset name argument.

Fix: Add preset name like starship preset nerd-font-symbols -o ...


🔍 Security Concerns

  1. Shell injection via installCommand: Mitigated by controlled command building
  2. ⚠️ Unvalidated $SHELL execution: See Critical Issue feat: Terminal rendering improvements + 4 high-priority features (#2, #3, #4, #5) #1
  3. ⚠️ Sudo without password caching: User might need to enter password multiple times
  4. No arbitrary code execution: Install scripts are from trusted sources

🎯 Testing Recommendations

  1. Test uninstall flows: Verify Starship/Oh My Zsh uninstall does not break user's shell
  2. Test on fresh system: Ensure detection works when nothing is installed
  3. Test conflicting tools: Install Oh My Zsh, then Prezto - verify cleanup works
  4. Test macOS restart: The /proc/self/cmdline path will fail on macOS
  5. Test Windows paths: WSL detection might need work
  6. Test interrupted installations: What happens if user Ctrl+C during npm install?

📝 Documentation

  • ✅ Added docs/onboarding.md - comprehensive guide
  • ✅ Updated README.md with feature mention
  • ⚠️ Missing: How to disable onboarding wizard permanently
  • ⚠️ Missing: How to reset onboarding state for testing

🎉 Overall Assessment

Score: 8/10

This is a high-quality feature with excellent UX design and solid architecture. The main concerns are:

  1. Command injection risk with $SHELL
  2. Resource leaks in ProcessBuilder
  3. Destructive Prezto uninstall
  4. Platform-specific restart logic

Once the critical issues are addressed, this will be ready to merge. Great work! 🚀


Recommendation: Request changes for Critical Issues #1-4, then approve after fixes.

1. Race Condition Fix (HIGH):
   - Check waitFor() return before calling exitValue()
   - Destroy process if timeout occurs before accessing exit value
   - Affected: GitUtils.kt, OnboardingWizard.kt

2. Resource Leak Fix (MEDIUM):
   - Close inputStream, errorStream, outputStream in finally blocks
   - Use bufferedReader().use {} for auto-closing readers
   - Affected: All ProcessBuilder usage in GitUtils.kt, OnboardingWizard.kt

3. Error Handling Fix (MEDIUM):
   - Wrap buildInstallCommand() with try-catch
   - Handle null System.getProperty("os.name") gracefully
   - Return user-friendly error message on failure

🤖 Generated with [Claude Code](https://claude.com/claude-code)
- Linux: Use /proc/self/cmdline
- macOS: Use `ps -p PID -o command=`
- Windows: Use ProcessHandle.info() with quoted arguments

Extracted getRestartCommand() helper with proper error handling
and resource cleanup for the ps subprocess.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@claude
Copy link
Copy Markdown

claude Bot commented Jan 7, 2026

Code Review: Shell Customization Menu & Onboarding Wizard

This is a substantial and well-structured PR that adds comprehensive shell customization and onboarding capabilities. Overall the code quality is high, but there are several important issues to address.

Critical Issues

  1. Command Injection Vulnerabilities (HIGH SEVERITY) - ShellCustomizationMenuProvider.kt:136, OnboardingWizard.kt:566 - The uninstall command uses unsanitized which starship output in a subshell which could execute arbitrary commands if a malicious starship executable is in PATH. Use hardcoded paths or validate output.

  2. Incomplete Resource Cleanup (MEMORY LEAK) - Process streams are closed but process object is not destroyed if timeout occurs. Add explicit process.destroyForcibly() calls.

  3. sed Platform Compatibility - Using sed -i.bak without platform detection will fail on macOS. Consider using Kotlin file operations instead.

Bugs & Logic Issues

  1. Race Condition - Tool detection runs once on launch but never re-runs. Re-detect after installation step.

  2. Shell Change Behavior - The chsh command message contradicts the exec zsh -l at the end.

  3. Prezto Install - Uses setopt EXTENDED_GLOB which fails in non-Zsh shells. Wrap in zsh -c.

  4. Mutual Exclusivity Gap - Only uninstalls Oh My Zsh/Prezto when default shell is Zsh. Check conflicts regardless of target shell.

Performance

  1. Blocking Main Thread - refreshStatus() runs 6 sequential ProcessBuilder calls (12s max). Run in parallel with async.

  2. Repeated File System Checks - 48+ calls for 6 tools. Cache or parallelize.

Verdict

Conditional Approval - Do not merge until critical security issues and resource leaks are resolved. The onboarding wizard is a fantastic addition with excellent UX design, but command injection vulnerability and platform compatibility need immediate attention. Great work overall!

@claude
Copy link
Copy Markdown

claude Bot commented Jan 7, 2026

Code Review: Shell Customization Menu & Onboarding Wizard

This is a substantial and well-structured PR that adds comprehensive shell customization features and a first-time onboarding wizard. Overall, the code quality is high, but there are some important issues that should be addressed before merging.


🟢 Strengths

Architecture & Design

  • Clean separation of concerns: ShellCustomizationMenuProvider, OnboardingWizard, OnboardingSteps, and GitUtils are well-organized
  • Reusable utilities: The GitUtils object consolidates git/gh logic nicely
  • Mutual exclusivity logic: Smart conflict resolution between Starship/Oh My Zsh/Prezto
  • Platform-aware commands: Good macOS/Linux/Windows detection and package manager support

Code Quality

  • Comprehensive error handling: Try-catch blocks with fallbacks, process timeouts
  • Resource cleanup: Proper stream closure in finally blocks (GitUtils, OnboardingWizard)
  • Immutability: Data classes use immutable patterns
  • Type safety: Good use of sealed classes and enums

UX/Features

  • Professional UI: Step indicator, selection cards, progress indicators
  • Embedded terminal: Installation shown in real-time with user feedback
  • Detection logic: Multi-strategy tool detection (which, PATH checks, fallback paths)
  • Settings persistence: onboardingCompleted flag prevents re-showing

🟡 Issues to Address

1. Security: Command Injection Vulnerabilities ⚠️ HIGH PRIORITY

Problem

Multiple locations construct shell commands using string interpolation without proper escaping. While most commands use hardcoded strings (which is safe), the pattern is dangerous for future additions.

ShellCustomizationMenuProvider.kt:509-551: User commands directly in strings
OnboardingWizard.kt:618-633: Uninstall commands with sed operations
OnboardingWizard.kt:666-686: Curl-to-shell pattern

Recommendation

  1. Add input validation for any user-provided values before command construction
  2. Document that these commands should only use hardcoded, trusted strings
  3. Consider using ProcessBuilder instead of shell commands where feasible
  4. Avoid curl-to-shell patterns when possible (though Starship official installer is relatively safe)

2. Resource Management: Process Leaks ⚠️ MEDIUM PRIORITY

Problem

In OnboardingWizard.kt:453-500 (isCommandInstalled):

Early returns in the try block may skip the finally block stream cleanup. The function returns true/false before reaching the finally block in several code paths.

Recommendation

Refactor to use Kotlin use function for automatic resource management, or ensure all code paths reach cleanup.


3. Error Handling: Silent Failures ⚠️ MEDIUM PRIORITY

Problem

OnboardingWizard.kt:570-576 wraps all install command building in a try-catch that returns a generic error. While error messages are shown, there is no logging or telemetry. Users may see failures without context.

Recommendation

  • Add logging using a proper logging framework (SLF4J/Logback)
  • Consider showing a dialog with actionable error messages
  • Validate inputs before building complex command strings

4. Platform Support: Windows Compatibility ⚠️ LOW PRIORITY

Problem

Windows support is present but limited:

  • OnboardingWizard.kt:256-264: Uses cmd and bash interchangeably
  • OnboardingWizard.kt:757-759: PowerShell command may fail on older Windows versions
  • ShellCustomizationMenuProvider.kt:858-885: Messages say not supported instead of providing alternatives

Recommendation

  • Test on Windows to verify PowerShell/winget commands work
  • Consider detecting WSL and using Linux commands when available
  • Provide links to WSL setup instead of not supported messages

5. Code Duplication: Uninstall Logic 📋 REFACTORING

Problem

ShellCustomizationMenuProvider.kt and OnboardingWizard.kt have nearly identical uninstall command generation. Both define similar getStarshipUninstallCommand(), getOhMyZshUninstallCommand(), etc.

Recommendation

Extract to shared utility object to eliminate duplication and ensure consistency.


6. Type Safety: Magic Strings 📋 REFACTORING

Problem

AI assistant IDs are hardcoded strings scattered across files:

  • OnboardingWizard.kt:728-742: claude-code, gemini-cli, etc.
  • OnboardingSteps.kt:312-316: Same strings repeated

Recommendation

Define constants in AIAssistantDefinition.kt to eliminate magic strings and improve maintainability.


7. Testing Concerns 🧪

Missing Test Coverage

  • No unit tests for buildInstallCommand() - critical business logic
  • No tests for platform detection (macOS/Linux/Windows)
  • No tests for tool detection logic
  • No tests for mutual exclusivity rules

Recommendation

Add tests for:

  1. buildInstallCommand() with various selection combinations
  2. Platform detection edge cases
  3. Conflict resolution (Starship vs Oh My Zsh)
  4. Command injection prevention (if input validation added)

🔵 Minor Issues

Performance

  • OnboardingWizard.kt:432-446: detectInstalledTools() runs 12 sequential process checks on UI thread startup. Consider running checks in parallel using async/await or caching results.

UX

  • OnboardingWizard.kt:549: Password prompt message is helpful, but could add visual indicator
  • OnboardingSteps.kt:100-111: Welcome text could mention how to re-run wizard

Documentation

  • ShellCustomizationMenuProvider.kt:14-20: Good class-level docs, but missing parameter docs
  • GitUtils.kt: Missing examples in KDoc comments

📊 Statistics

  • Lines added: 3,737
  • Lines deleted: 108
  • Files changed: 19
  • New files: 5 (OnboardingWizard.kt, OnboardingSteps.kt, ShellCustomizationMenuProvider.kt, GitUtils.kt, docs/onboarding.md)

✅ Recommendation

REQUEST CHANGES - Address the security and resource management issues before merging. The codebase is well-structured and the feature is valuable, but the identified issues need attention:

Must Fix (Blocker):

  1. Document command injection safety or add validation
  2. Fix resource leaks in isCommandInstalled()

Should Fix (High Priority):

  1. Add error logging
  2. Test Windows compatibility
  3. Add unit tests for critical paths

Nice to Have:

  1. Extract duplicate uninstall logic
  2. Replace magic strings with constants
  3. Parallelize tool detection

Great work overall! The PR adds significant value with professional UX. Once the security and resource management issues are addressed, this will be a solid addition to BossTerm. 🚀

- Add "Open Folder..." context menu option that opens folder picker and
  runs cd + ls to navigate and show contents
- Simplify Window Menu > Tools to only show installed shell customization
  tools (Starship, Oh My Zsh, Prezto)
- Simplify context menu Shell submenu to only show installed tools +
  default shell (removed install options for all shells)
- Extract shared shell utilities to ShellCustomizationUtils.kt
- Add AIAssistantIds constants to replace magic strings
- Fix JFileChooser threading issue causing Compose reentry exception

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@claude
Copy link
Copy Markdown

claude Bot commented Jan 7, 2026

Pull Request Review: Shell Customization Menu

This is a comprehensive PR adding shell customization and onboarding features.

Total Changes: +3,770 additions, -115 deletions across 22 files


Strengths

  1. Excellent Code Organization - Well-structured with dedicated utility classes
  2. Comprehensive Mutual Exclusivity - Smart conflict detection between tools
  3. Platform-Aware Implementation - Cross-platform install commands
  4. Good UX Design - Clean wizard flow with embedded terminal
  5. Resource Management - Proper process cleanup with timeouts

HIGH PRIORITY Issues

1. Command Injection Vulnerability (SECURITY)

Location: GitUtils.kt:131-151, OnboardingWizard.kt:576

Issue: User-provided paths are interpolated into shell commands without sanitization. A directory like "; rm -rf /;" could execute arbitrary commands.

Recommendation: Add input sanitization or use ProcessBuilder with argument arrays.


2. Race Condition in Process Timeout (RELIABILITY)

Location: OnboardingWizard.kt:463, ShellCustomizationUtils.kt:43

Issue: Code may call exitValue() after timeout without checking if process completed.

Recommendation: Check waitFor() return value before accessing exitValue().


3. Missing Null Check on System.getProperty (RELIABILITY)

Location: OnboardingWizard.kt:585, ShellCustomizationMenuProvider.kt:592

Issue: Some paths assume System.getProperty("os.name") never returns null.

Recommendation: Use null-safe operators everywhere.


MEDIUM PRIORITY Issues

  1. Inconsistent Error Handling - No logging in buildInstallCommand
  2. Magic Strings - Use AIAssistantIds constants consistently
  3. Sudo Failures - No user feedback when sudo -v fails
  4. Resource Leaks - Process streams may not close in all paths

LOW PRIORITY Issues

  1. Shell Fallback - Falls back to Bash menu for unknown shells
  2. Code Duplication - Similar structure in menu builders
  3. Documentation - Missing examples for complex functions

Testing Recommendations

Unit Tests Needed:

  • Command injection with malicious paths
  • Process timeout behavior
  • Platform detection
  • Conflict resolution

Manual Testing:

  • Install tools with conflicts present
  • Test on Linux, macOS, Windows
  • Test paths with spaces/special chars

Code Quality Metrics

Metric Score Notes
Organization 9/10 Excellent separation
Error Handling 6/10 Some edge cases missing
Security 5/10 Command injection risks
Documentation 7/10 Good but needs examples
Performance 8/10 Efficient caching

Final Verdict

Status: ⚠️ Approve with Changes Required

Excellent feature implementation with strong architecture. Must fix 3 high-priority security/reliability issues before merge.

Estimated Fix Time: 2-3 hours for critical issues

Recommendation:

  1. Fix high-priority issues immediately
  2. Create follow-up tickets for medium/low priority
  3. Add test coverage in subsequent PR

Reviewed by: Claude Code (Sonnet 4.5)
Date: 2026-01-07

When $SHELL points to a non-existent shell (e.g., fish not installed),
the terminal now falls back to /bin/bash, then /bin/sh instead of
crashing with "Unable to find full path" error.

Added ShellCustomizationUtils.getValidShell() that:
1. Checks if $SHELL exists and is executable
2. Falls back to /bin/bash if available
3. Ultimate fallback to /bin/sh (POSIX, always available)

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@claude
Copy link
Copy Markdown

claude Bot commented Jan 7, 2026

Code Review for PR #228: Shell Customization Menu

This PR adds comprehensive shell customization features. Overall excellent architecture and UX, but requires security fixes before merge.

✅ Strengths

  • Clean architecture with dedicated utility classes
  • Reusable menu provider pattern
  • Cross-platform support (macOS, Linux, Windows)
  • Smart mutual exclusivity logic

🔴 Critical Issues (Must Fix Before Merge)

1. Command Injection Vulnerabilities ⚠️

Location: OnboardingWizard.kt:651-659, ShellCustomizationUtils.kt

Commands concatenate user-controlled values (/bin/bash, cwd) without sanitization.

Fix: Validate /bin/bash against whitelist of known shells, canonicalize paths.

2. Process Resource Leaks

Location: OnboardingWizard.kt:456-503

ProcessBuilder cleanup incomplete if destroyForcibly() fails.

Fix: Add explicit destroy() before destroyForcibly() with waitFor().

3. Unsafe File Operations

Location: ShellCustomizationMenuProvider.kt:46

readText() can throw on permission errors - no error handling.

Fix: Wrap in try-catch.

🟡 High Priority

  1. Race condition in installation status check (Main.kt:104-110)
  2. Hardcoded 2-second timeout too short (GitUtils.kt:22)
  3. sed -i portability issues (Linux vs macOS)
  4. Missing error handling in restart flow
  5. No unit tests for critical utilities

🔒 Security Concerns

  • Path traversal risk in git commands with cwd
  • No checksum verification for remote install scripts

Overall Assessment

Code Quality: ⭐⭐⭐⭐☆ (4/5)
Architecture: ⭐⭐⭐⭐⭐ (5/5)
Security: ⭐⭐⭐☆☆ (3/5) - needs fixes
Test Coverage: ⭐⭐☆☆☆ (2/5)

Verdict: Excellent feature with solid architecture, but requires security fixes (command injection, file operation safety, process cleanup) before merge.

cc @kshivang - Great work! Please address critical security issues above. 🚀

@claude
Copy link
Copy Markdown

claude Bot commented Jan 7, 2026

Code Review: Shell Customization Menu and Onboarding Wizard

This is an impressive feature addition that brings shell customization and a first-time setup wizard to BossTerm. Here is my comprehensive review:

Strengths

  1. Comprehensive Shell Support - Excellent coverage of popular shells (Zsh, Bash, Fish) and customization tools (Starship, Oh My Zsh, Prezto). Smart mutual exclusivity logic prevents conflicts between competing frameworks.

  2. Well-Structured Architecture - Clean separation of concerns with ShellCustomizationUtils for shared utilities. Proper use of Kotlin sealed classes for wizard steps.

  3. User Experience - Onboarding wizard provides a polished first-run experience. Context menu integration makes tools easily discoverable. Clear status indicators.

  4. Code Quality - Consistent error handling with process timeouts. Proper resource cleanup in isCommandInstalled(). Good use of Compose patterns.

Critical Issues

1. Security: Command Injection Vulnerabilities (HIGH PRIORITY)

Location: ShellCustomizationMenuProvider.kt lines 249-264, 324-393

The context menu items directly interpolate shell commands without proper escaping. The EDITOR variable and theme names could be exploited if controlled by an attacker.

Fix: Use explicit quoting or avoid environment variable expansion in user-facing commands. Validate all input that goes into shell commands.

2. Unsafe sed Usage (MEDIUM PRIORITY)

Location: ShellCustomizationMenuProvider.kt lines 324-350

The Oh My Zsh theme switcher uses sed with theme names that are not validated. Special characters in theme names could break sed syntax or inject commands.

Fix: Validate theme names against a whitelist or use a safer update mechanism.

3. macOS sed Compatibility Issues (MEDIUM PRIORITY)

Location: ShellCustomizationUtils.kt lines 103-105

The code uses GNU sed syntax with fallback to BSD sed, but this creates backup files and the fallback pattern is fragile.

Fix: Detect sed version upfront or use a platform-agnostic approach.

4. Potential Deadlock in OnboardingWizard (MEDIUM PRIORITY)

Location: OnboardingWizard.kt lines 534-586

If the install command hangs or prompts for input indefinitely, the wizard gets stuck with no timeout or escape mechanism.

Fix: Add a timeout or Cancel button to the Installing step.

Priority Fixes (Before Merge)

  1. Fix command injection in context menu (lines 249-264, 324-393)
  2. Add timeout/cancellation to onboarding installer
  3. Improve error messages in buildInstallCommand()
  4. Test macOS sed compatibility

Overall Assessment

Code Quality: 7/10
Security: 5/10 (security concerns need addressing)
User Experience: 9/10
Architecture: 8/10

This is a well-designed feature with excellent UX, but the security concerns (command injection, unsafe sed usage) need to be addressed before merging to master. The onboarding wizard is polished and the context menu integration is intuitive.

Recommendation: Request changes for security fixes, then approve after review.

Great work overall! The feature is nearly production-ready with just a few critical security improvements needed.

When selecting a shell in the Welcome Wizard, now runs chsh to set
it as the default shell in addition to installing it.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@claude
Copy link
Copy Markdown

claude Bot commented Jan 7, 2026

Code Review: Shell Customization Menu & Onboarding Wizard

This is an impressive feature addition! The implementation is well-structured with proper separation of concerns.

✅ Strengths

Architecture: Excellent separation with ShellCustomizationUtils, ShellCustomizationMenuProvider, and GitUtils. Smart extraction of shared logic.

Code Quality: Consistent patterns, proper resource cleanup in ProcessBuilder calls, defensive programming with 2s timeouts.

UX: Great onboarding wizard, smart mutual exclusivity handling, context-aware menus.

⚠️ Critical Security Issues

1. Command Injection in GitUtils.kt (lines 132-136, 145-150)

The cwd parameter is interpolated directly without escaping:

"git -C \"$cwd\" $cmd\n"  // VULNERABLE

If cwd contains shell metacharacters (semicolons, pipes, backticks), they execute. Same in ghCommand().

Fix: Escape cwd or validate it contains no special characters.

2. Command Injection in OnboardingWizard.kt (line 258-260)

restartCommand interpolated into bash -c without validation.

Fix: Verify getRestartCommand() source is safe.

3. sed Command Risk (currently safe)

Theme changes use hardcoded strings (good). Never allow user-selected theme names without escaping.

🐛 Potential Bugs

  1. Race condition: Tool status not refreshed after installation during session
  2. Shell validation: canExecute() doesn't verify it's actually a shell
  3. Sed inconsistency: Creates .bak files on Linux

📊 Performance

Sequential ProcessBuilder calls spawn 10+ processes. Consider parallelizing with coroutines.

🧪 Test Coverage

Missing tests for: command injection, cross-platform behavior, concurrent installations, failed installs.

📋 Recommendations

MUST FIX (Blocking):

  1. Fix command injection in GitUtils.gitCommand() and ghCommand()
  2. Verify getRestartCommand() safety

SHOULD FIX:
3. Add path validation
4. Document security assumptions
5. Add error handling

NICE TO HAVE:
6. Parallelize detection
7. Extract constants
8. Add security tests

🎯 Assessment

  • Code Quality: ⭐⭐⭐⭐ (4/5)
  • Security: ⚠️ 2/5 (Critical issues)
  • Performance: ⭐⭐⭐⭐ (4/5)
  • Documentation: ⭐⭐⭐ (3/5)
  • Test Coverage: ⭐⭐ (2/5)

Recommendation: Request Changes - Fix security issues before merge.

Great work overall! Security issues are fixable with minor refactoring.


Reviewed by Claude Code (Sonnet 4.5)

Shows "🔐 Administrator password required for installation..." message
before prompting for sudo password in the Welcome Wizard.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@claude
Copy link
Copy Markdown

claude Bot commented Jan 7, 2026

Code Review: Shell Customization Menu & Onboarding Wizard

This is an impressive PR that adds significant value to BossTerm! The implementation is well-structured and follows the project's established patterns. Here's my detailed feedback:


Strengths

Architecture & Code Quality

  1. Excellent code organization - Clean separation of concerns with dedicated utilities and menu providers
  2. Consistent with existing patterns - Follows established architecture from AI Assistant and VCS features
  3. Platform-aware commands - Good handling of macOS/Linux/Windows differences
  4. Error handling - Process timeouts and resource cleanup properly implemented

Features

  1. Mutual exclusivity logic - Smart handling of conflicting tools (Starship, Oh My Zsh, Prezto)
  2. Comprehensive tool support - Starship presets, Oh My Zsh themes, Prezto modules
  3. Shell detection - getValidShell() with proper fallbacks is a good safety mechanism
  4. Onboarding flow - Well-designed wizard with clear steps and progress indicators

🔴 Critical Issues

1. Command Injection Vulnerabilities

Location: ShellCustomizationMenuProvider.kt lines 249-256, 323-350

Problem: User-controlled paths passed directly to shell commands without sanitization. If config file paths contain malicious content, could execute arbitrary commands.

Solution: Escape shell metacharacters or use safer parameter expansion with proper quoting

2. Destructive Operations Without Confirmation

Location: ShellCustomizationUtils.kt lines 114-124, 131-132

Problem: Uninstall commands can delete user config files without warning (e.g., line 495 deletes ~/.zshrc and other rc files)

Solution: Back up files before deletion and show explicit confirmation dialogs

3. sed -i Platform Inconsistency

Location: ShellCustomizationMenuProvider.kt lines 324-350

Problem: sed -i syntax differs between Linux (GNU sed) and macOS (BSD sed). Line 324 will fail on macOS.

Solution: Use the pattern from uninstall commands consistently: sed -i.bak ... 2>/dev/null || sed -i '' ... 2>/dev/null

4. ProcessBuilder Timeout Inconsistency

Problem: Inconsistent timeout handling across files (2 vs 3 seconds, some missing timeouts)

Solution: Standardize timeouts and ensure all ProcessBuilder calls have them to prevent hangs


⚠️ Important Issues

5. Missing Input Validation for chsh

Location: OnboardingWizard.kt line 614

Problem: chsh executed without verifying shell exists in /etc/shells. On many systems this will fail, leaving system in bad state.

Solution: Add validation: SHELL_PATH=$(which $shellCmd) && grep -q "$SHELL_PATH" /etc/shells && sudo chsh -s "$SHELL_PATH" $USER

6. Race Condition in Application Restart

Location: OnboardingWizard.kt lines 253-266

Problem: Fixed delays (0.5-1 second) may not be enough for clean shutdown

Solution: Use longer delay (2-3 seconds) or let OS handle restart via launcher script

7. Gradle Properties Changes

Location: gradle.properties (+4 lines)

Request: Please show what was changed in gradle.properties for review (not visible in diff)


💡 Suggestions for Improvement

  1. Error Messaging - Add user-friendly error messages when operations fail
  2. Logging - Add structured logging for shell customization operations
  3. NPM Install Optimization - Check for nvm first, use curl -fsSL, add error handling
  4. UI Polish - Show individual tool progress, add Learn More links, offer Retry/Skip on errors
  5. Test Coverage - Add automated tests for mutual exclusivity logic

🔍 Minor Issues

  1. Typo in comment OnboardingWizard.kt:455
  2. Unused import in TabbedTerminal.kt:38
  3. Magic numbers - DpSize(650.dp, 550.dp) should be constants
  4. Inconsistent naming - shellStatusHolder vs vcsStatusHolder

🎯 Security Checklist

Before merging, verify:

  • All sed -i commands handle macOS/Linux differences
  • All shell variable expansions are properly quoted
  • No user input passed to shell without validation
  • Destructive operations have confirmation dialogs
  • All ProcessBuilder calls have timeouts
  • File paths validated before operations

📊 Summary

Category Count
Critical 4
Important 3
Suggestions 9
Minor 4

Overall Assessment: This is high-quality work with excellent architecture. The critical security issues (command injection, destructive operations) must be addressed before merging. Once fixed, this will be a fantastic addition to BossTerm!

Recommended Action:

  1. Fix critical security issues (feat: Terminal rendering improvements + 4 high-priority features (#2, #3, #4, #5) #1-4)
  2. Address important issues (🌏 Implement IME (Input Method Editor) support for CJK languages #5-7)
  3. Consider suggestions for next iteration
  4. Add test coverage for mutual exclusivity logic

Great work on this feature! The onboarding wizard will significantly improve the first-time user experience. 🎉


Files Requiring Changes:

  • compose-ui/.../shell/ShellCustomizationMenuProvider.kt (command injection, sed -i)
  • compose-ui/.../shell/ShellCustomizationUtils.kt (destructive operations, backups)
  • compose-ui/.../onboarding/OnboardingWizard.kt (chsh validation, restart timing)
  • gradle.properties (please show changes)

@kshivang kshivang merged commit 4327bed into master Jan 7, 2026
7 checks passed
@claude
Copy link
Copy Markdown

claude Bot commented Jan 7, 2026

PR Review: Shell Customization Menu

Summary

Comprehensive shell customization support with approximately 3,700 lines across 22 files. Well-structured implementation.

Strengths

  • Excellent architecture: ShellCustomizationUtils provides reusable utilities for both context menu and wizard
  • Good defensive programming: 2-second timeouts on ProcessBuilder, proper resource cleanup
  • Mutual exclusivity logic for conflicting tools (Starship/Oh My Zsh/Prezto)
  • Progressive enhancement: menus adapt based on installation status

Critical Issues

1. CRITICAL: Command Injection Vulnerability

Location: ShellCustomizationUtils.kt lines 97-133

Line 100 uses unsanitized shell interpolation. If command returns malicious path from tampered PATH, could execute arbitrary commands.

Recommendation: Validate paths with regex pattern before using in shell commands.

2. Race Condition in Status Checking

Location: EmbeddableTerminal.kt:419-427, TabbedTerminal.kt:757-766

Status values are read immediately but refreshStatus() is async, causing stale data.

Recommendation: Wait for refreshStatus() completion in coroutine scope.

3. Performance Issue

Location: ShellCustomizationMenuProvider.kt:55-62

refreshStatus() runs 6+ which commands sequentially with 2s timeout each (potentially 12+ seconds total).

Recommendation: Parallelize checks using async/await pattern.

Medium Priority Issues

  • Shell detection uses fragile pattern matching (line 155)
  • sed creates .bak files never cleaned up (lines 103-105)
  • /bin/sh fallback may not exist on all systems (lines 23-41)
  • Missing error recovery in onboarding wizard

Minor Issues

  • Inconsistent naming: ohmyzshInstalled vs preztoInstalled
  • Magic timeout values (extract to constant)
  • Documentation should cover security considerations

Positive Patterns

  • Reusable utility design
  • Graceful degradation
  • Consistent tool category system
  • Settings persistence

Recommendation

Conditional Approval - Fix command injection vulnerability before merge. Other issues are medium/low priority.

Priority fixes:

  1. Command injection (CRITICAL)
  2. Race condition (MEDIUM)
  3. Performance optimization (MEDIUM)

Overall well-architected feature. Great work on comprehensive shell customization support!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant