-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(server) Avoid modifying path resolution for executables on MacOS #115
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(server) Avoid modifying path resolution for executables on MacOS #115
Conversation
|
Can you explain more about what is going on with spawn-rx? I'd rather actually just make it work correctly on all platforms |
|
@anaisbetts Hi Ani! Looking at For example with Volta-managed
The code comment suggests this was added for Windows PATH resolution. Would you mind sharing the original Windows issue this was solving? We might be able to handle that case specifically while preserving normal PATH behavior on Unix. Edit: rephrase to be more concise |
|
Hi @anaisbetts, hope you had a nice holiday break! Just wanted to follow up on this PR - would you have time to take another look when you get a chance? |
|
@jacksteamdev Update spawn-rx to v5.1.2 in this PR instead and this should be fixed |
|
Thanks @anaisbetts! It works a treat. |
jspahrsummers
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jacksteamdev and @anaisbetts!
fix(server) Avoid modifying path resolution for executables on MacOS
Motivation and Context
spawn-rxfollows symlinks when resolving executables, which breaks version manager shims on macOS. For example, when using Volta,bunincorrectly resolves tovolta-shiminstead of thebunexecutable. This PR works around the issue by bypassingspawn-rx's path resolution on non-Windows platforms.Closes #114
How Has This Been Tested?
bunon macOSBreaking Changes
None. This restores expected behavior on macOS while maintaining existing Windows functionality.
Types of changes
Checklist
Additional context
This could be a temporary fix while we investigate the underlying
spawn-rxpath resolution behavior. We may want to upstream a fix that handles Windows PATH resolution without affecting Unix shims.To Reproduce
Steps to reproduce the behavior:
bunornode.Expected behavior
The inspector should spawn the command as entered in the UI.
Logs
I'm getting this error:
Volta error: 'volta-shim' should not be called directly. Please use the existing shims provided by Volta (node, yarn, etc.) to run tools.Note that the command
bunis resolved incorrectly.Stdio transport: command=/Users/jacksteam/.volta/bin/volta-shim, args=src/index.ts, env={"HOME":"/Users/jacksteam","LOGNAME":"jacksteam","PATH":"/Users/jacksteam/.npm/_npx/5a9d879542beca3a/node_modules/.bin:/Users/jacksteam/Documents/Cline/MCP/zettelkasten-server/node_modules/.bin:/Users/jacksteam/Documents/Cline/MCP/node_modules/.bin:/Users/jacksteam/Documents/Cline/node_modules/.bin:/Users/jacksteam/Documents/node_modules/.bin:/Users/jacksteam/node_modules/.bin:/Users/node_modules/.bin:/node_modules/.bin:/Users/jacksteam/.volta/tools/image/node/20.16.0/lib/node_modules/npm/node_modules/@npmcli/run-script/lib/node-gyp-bin:/Users/jacksteam/.volta/tools/image/node/20.16.0/bin:/Users/jacksteam/perl5/bin:/Users/jacksteam/Library/pnpm:/Users/jacksteam/.volta/bin:/opt/homebrew/opt/openjdk/bin:/Users/jacksteam/.jenv/shims:/Users/jacksteam/.jenv/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/Library/Apple/usr/bin:/Users/jacksteam/Library/Application Support/Code/User/globalStorage/github.copilot-chat/debugCommand","SHELL":"/bin/zsh","TERM":"xterm-256color","USER":"jacksteam"}Edit: rephrase to be more clear and concise, add additional context