Skip to content

Conversation

sforsethi
Copy link
Contributor

  • Prioritize Homebrew node over NVM/system node for build reliability
  • Add early exit when Homebrew node is found and working
  • Disable auto-switching for fnm/NVM in CI/Xcode environments
  • Add comprehensive error handling and fallback logic
  • Fix issue Xcode doesn't always find node with changes on latest main #246 where NVM was overriding Homebrew node selection

Tested: Successfully switches from v24.2.0 to v24.3.0 during builds

- Prioritize Homebrew node over NVM/system node for build reliability
- Add early exit when Homebrew node is found and working
- Disable auto-switching for fnm/NVM in CI/Xcode environments
- Add comprehensive error handling and fallback logic
- Fix issue amantus-ai#246 where NVM was overriding Homebrew node selection

Tested: Successfully switches from v24.2.0 to v24.3.0 during builds
cursor[bot]

This comment was marked as outdated.

@steipete
Copy link
Collaborator

steipete commented Jul 6, 2025

Holy moly this looks complicated. By simply adding the various paths in order, wouldn't we achieve the same result with 10% code? Also the reviews here seem valid?

@sforsethi
Copy link
Contributor Author

Holy moly this looks complicated. By simply adding the various paths in order, wouldn't we achieve the same result with 10% code? Also the reviews here seem valid?

Yeah, let me clean it up a bit

@steipete
Copy link
Collaborator

steipete commented Jul 7, 2025

@sforsethi I'll do a release later today, do you have time to work on this? Would love to get that in.

cursor[bot]

This comment was marked as outdated.

@sforsethi
Copy link
Contributor Author

@steipete Sure, I have some time right now I'll try to fix it, but tbh fully just vibe coding at the point, not familiar with the code

- Remove complex early-exit logic and CI environment handling
- Maintain priority order: Homebrew → Volta → fnm → NVM
- Keep core functionality while eliminating 85% of code complexity
- Address manager feedback about over-engineering
cursor[bot]

This comment was marked as outdated.

- Add missing NVM_DIR export before sourcing nvm.sh
- Restore PATH priority after fnm/NVM modify it
- Remove unnecessary comments
- Maintain Homebrew → Volta → fnm → NVM priority order
cursor[bot]

This comment was marked as outdated.

- Eliminate duplicate PATH entries from multiple exports
- Remove unused ORIGINAL_PATH variable
- Use single PATH export with priority order
- Reduce from 23 to 17 lines while maintaining functionality
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

- Use return instead of exit to avoid terminating parent shell
- Set priority paths before loading fnm/NVM to preserve their modifications
- Maintain proper priority order while respecting manager PATH changes
cursor[bot]

This comment was marked as outdated.

- Restore simple approach with clear Homebrew prioritization
- Remove over-engineered error handling and validation
- Maintain compatibility with zsh build system
- Fix PATH priority by restoring Homebrew paths after version managers load
- Reduce from 156 to 24 lines while solving original issue amantus-ai#246
cursor[bot]

This comment was marked as outdated.

- Add error suppression to NVM loading for consistency with fnm
- Remove duplicate PATH exports to eliminate redundant entries
- Maintain Homebrew priority with single PATH assignment
@steipete steipete merged commit 48ea889 into amantus-ai:main Jul 7, 2025
5 of 6 checks passed
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.

3 participants