fix: Improve bootstrap.sh cross-platform compatibility

[evansenter]

Summary

• Skip LaunchAgents installation on non-macOS systems (LaunchAgents are macOS-only)
• Auto-initialize git submodules when btop themes are missing instead of just printing a message

Test plan

• Test ./bootstrap.sh on macOS - should behave as before
• Test ./bootstrap.sh on Linux - should skip LaunchAgents installation gracefully
• Test with uninitialized submodules - should auto-initialize them

🤖 Generated with Claude Code

[claude]

PR Review: Cross-Platform Compatibility Improvements

✅ Strengths

1. Solid Cross-Platform Fix
The macOS-only check for LaunchAgents (bootstrap.sh:80) is implemented correctly using uname to detect Darwin (macOS), returns early with success code (0) on non-macOS systems, and uses a simple and effective approach.

2. Improved User Experience
Auto-initializing submodules (bootstrap.sh:127-128) is a significant UX improvement that eliminates manual steps from installation process and is more intuitive than just printing an error message.

3. Clean, Minimal Changes
The PR maintains focus with only essential changes (+5/-3 lines), no scope creep.

---

🔍 Considerations & Suggestions

1. Error Handling for Submodule Initialization

The auto-initialization at bootstrap.sh:128 could fail silently if network is unavailable, git submodule URLs are inaccessible, or there are insufficient permissions.

Suggestion: Add error handling to check if submodule initialization succeeds, and gracefully fall back with a warning message if it fails.

2. Potential Logic Issue After Submodule Init

After initializing submodules, the code immediately proceeds to symlink themes. If initialization fails or vendor_themes still doesn't exist, the for loop at bootstrap.sh:134 will fail with an error.

Suggestion: Add validation after initialization to verify the themes directory exists before attempting to symlink.

3. Testing Considerations

The test plan mentions manual testing, but there's no automated test coverage. Consider adding shellcheck for shell script linting, bats for unit tests, or a CI workflow to test on both macOS and Linux.

---

🎯 Code Quality

✅ Follows existing code style and conventions
✅ Proper use of bash conditionals and operators
✅ Comments explain the why (LaunchAgents are macOS-only)
✅ No security concerns introduced
✅ No performance issues

---

📋 Recommendation

Approve with minor suggestions. The core changes are sound and improve cross-platform compatibility. The error handling suggestions are nice-to-haves that would make the script more robust, but not blockers for merging. The most important suggestion is adding submodule initialization error handling since it's the most likely failure scenario users will encounter.

[claude]

PR Review: Improve bootstrap.sh cross-platform compatibility

Overall Assessment

Good improvements! These changes make the bootstrap script more robust and user-friendly. The macOS platform check and automatic submodule initialization are both sensible enhancements.

Code Quality & Best Practices

Strengths:

• Clean, readable implementation
• Follows existing script conventions
• Proper error handling with fallback messages
• Non-breaking changes for existing macOS workflows

Suggestions:

1. Consider adding verbose output at bootstrap.sh:80 - Currently the script silently skips LaunchAgents on non-macOS systems. An echo statement would make it clear that LaunchAgents are being intentionally skipped.

Potential Issues

Minor Issue - Working Directory Assumption:
In install_btop_themes() at line 128, if the repository has network issues, git submodule update --init could fail. The current error handling is good (returns 0 and shows a warning). Current behavior is appropriate - themes are non-critical and shouldn't block bootstrap.

Security Considerations

No security concerns identified:

• No user input is used unsanitized
• Platform detection uses safe built-in command
• Git operations are scoped to known directory

Performance Considerations

Performance impact is minimal:

• Early return for non-macOS systems is efficient
• Submodule initialization only runs when needed
• The double directory check could be optimized but current approach is more defensive and clearer

Test Coverage

The PR description includes a good test plan. Additional test scenarios to consider:

1. macOS with initialized submodules
2. macOS without submodules
3. Linux without submodules
4. Missing: Test with git submodule initialization failure (network issues, permissions)
5. Missing: Test on FreeBSD or other Unix systems

Recommendation: Test on a non-macOS system with no network access to verify failure handling works gracefully.

Additional Suggestions

1. Documentation Update: The CLAUDE.md file mentions installation but doesn't document the cross-platform behavior. Consider adding a note about platform support.
2. Error Message Consistency: Line 129 says Warning while line 137 says Skipping. Consider using consistent prefixes.

Summary

This is a solid PR that improves cross-platform compatibility without breaking existing functionality. The changes are minimal, focused, and well-implemented.

Recommendation: Approve with minor suggestions

The code is ready to merge as-is. The suggestions above are optional improvements that could be addressed in a follow-up PR if desired.

Reviewed by: Claude Sonnet 4.5 via Claude Code

[claude]

Code Review

Great work on improving cross-platform compatibility! This PR makes meaningful improvements to support Linux systems while maintaining macOS functionality.

✅ Strengths

1. Clean Platform Detection: The uname check in install_launch_agents() (bootstrap.sh:80) is the standard approach
2. Automatic Submodule Initialization: The auto-init logic (bootstrap.sh:130-136) is a nice UX improvement
3. Proper Error Handling: Good defensive programming with the double-check after initialization (bootstrap.sh:138-142)
4. Documentation Updates: CLAUDE.md accurately reflects the new cross-platform support

🐛 Potential Issues

Critical: Git Detection Omission
The install_btop_themes() function now calls git (line 131) but doesn't verify git is installed. If git is unavailable, users will see a cryptic error. Recommend adding a git availability check before attempting submodule operations.

Minor: Inconsistent Error Messages
Lines 81, 132, and 140 use slightly different message styles. Consider standardizing.

Enhancement: Verbose Output
When submodule initialization succeeds, there's no confirmation message. Consider adding success feedback.

🔒 Security Considerations

No security concerns. Changes don't introduce new user input handling, don't execute untrusted code, and maintain existing file permission patterns.

⚡ Performance Considerations

Submodule initialization adds network I/O on first run, but this is acceptable as it's a one-time operation.

🧪 Test Coverage

Gap: No Automated Tests
Consider adding basic smoke tests using GitHub Actions with platform matrix testing (macOS and Ubuntu).

📋 Summary

Overall: ✅ Approve with minor suggestions

Priority fixes before merge:

• Add git availability check in install_btop_themes()

Nice-to-haves for follow-up:

• Standardize error message format
• Add success confirmation for submodule init
• Consider adding basic CI tests

Review by Claude Code per CLAUDE.md conventions

[claude]

Code Review Summary

This PR successfully improves cross-platform compatibility for the bootstrap script. The changes are well-implemented with good error handling. Here's my detailed feedback:

✅ Strengths

1. Proper Platform Detection (bootstrap.sh:80)

   • Correctly uses uname to detect macOS before installing LaunchAgents
   • Clean early return pattern avoids unnecessary code execution on Linux

2. Robust Error Handling (bootstrap.sh:130-146)

   • Checks for git availability before attempting submodule operations
   • Graceful degradation with helpful error messages
   • Verification step after initialization prevents downstream failures

3. Documentation Updates (CLAUDE.md:7,60-66)

   • Accurately reflects the new cross-platform support
   • Updated line number references keep documentation in sync with code

4. Backward Compatibility

   • No breaking changes to existing workflows
   • macOS behavior remains unchanged

🔍 Code Quality Observations

Good Practices:

• Consistent error message format with actionable guidance
• Proper use of return 0 for graceful exits
• Clean separation of concerns (OS check, git check, verification)

Minor Considerations:

1. Submodule Initialization Scope (bootstrap.sh:135)

   • git submodule update --init initializes ALL submodules, not just btop-catppuccin
   • For a repository with only one submodule, this is fine
   • If more submodules are added later, consider: git submodule update --init vendor/btop-catppuccin
   • Impact: Low - current implementation is acceptable

2. Error Message Consistency (bootstrap.sh:81)

   • Most messages use format: Skipping X (reason)
   • LaunchAgents uses: Skipping LaunchAgents (macOS-only)
   • Consider: Skipping LaunchAgents (not on macOS) for consistency with other messages
   • Impact: Very low - stylistic preference

🔒 Security

• ✅ No security concerns identified
• Platform detection uses standard system utilities
• No user input processing in changed code
• Submodule initialization uses existing git configuration

⚡ Performance

• ✅ No performance concerns
• Early returns prevent unnecessary work
• Submodule initialization only runs when needed

🧪 Test Coverage Recommendations

The test plan in the PR description is comprehensive. Suggested test scenarios:

1. ✅ macOS with initialized submodules (existing behavior)
2. ✅ macOS with uninitialized submodules (new auto-init)
3. ✅ Linux with initialized submodules (new skip logic)
4. ✅ Linux with uninitialized submodules (combined new features)
5. 🆕 Environment without git installed (edge case)
6. 🆕 Submodule initialization failure scenario (network issues, permissions)

📝 Suggestions (Optional)

1. Consider more specific submodule initialization:

   git -C "$dotfiles_dir" submodule update --init vendor/btop-catppuccin

   This is future-proofing if you add more submodules later.

2. Error message alignment:

   echo "Skipping LaunchAgents (not on macOS)"

✅ Overall Assessment

Recommendation: Approve and Merge

This is a solid PR that achieves its stated goals with proper error handling and documentation. The code is clean, maintainable, and follows the existing patterns in the codebase. The suggestions above are minor refinements, not blockers.

Great work on improving the Linux compatibility! 🎉