-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fix: make bash tool respect $SHELL #3494
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
Conversation
|
@istarkov @rekram1-node I feel like we should update the system prompt or tool description to tell the LLM what shell will be used. Especially for windows. The tool is called bash after all. |
Probably here
const shellName = (process.env.SHELL || process.env.COMSPEC || (process.platform === 'win32' ? 'cmd.exe' : 'sh'))
.split(/[\\/]/).pop();
...
Platform: ${process.platform}, Shell: ${shellName}But I would not touch this until real windows users have reproducible issues, as probably Platform is enough |
|
Yeah I need to setup a windows environment to test this more and I need to analyze how other tools do it. the tool name being bash yeah it is confusing for the llm almost certainly I think we can addresss that separately tho I want to do more investigation before just changing prompts |
|
I have feeling that at the end it will be running bash in some container env. For reasons like prevent any operation out of project folder etc. |
|
@istarkov for those who use things like nunshell or fish, I wonder if this change could actually make their experience worse We could just also try a check if they have bash installed and use that, but i suppose using the users shell offers some benefits and then it does come back to tool descriptions & names This looks good, I will merge soon just wanna think more about how we can best serve everyone |
|
Given the tool is called bash, almost all unix users have bash, and a lot of windows users have git bash, and the prompts are all about bash, should we just do shell: "bash" if bash is installed? I feel like that's the safest bet here |
|
I have few reasons why current approach is better:
macOS /bin/bash (3.2) 2007 (18+ years old) ;-) |
|
i tend to agree, im just cautious, lemme test something with fish and nushell |
772b621 to
eb855e1
Compare
|
I suggest it respect SHELL on Windows as well to enable compatibility with Git Bash / MINGW. As it is on Windows, it uses cmd.exe even if you launch from bash. SHELL won't typically be set in cmd/powershell sessions. Or alternatively, make it configurable. |
|
/oc use $SHELL if it is set, otherwise use |
|
Perfect! I've successfully implemented the requested changes:
The change simplifies the shell configuration to a single line:
This approach works consistently across all platforms (Windows, macOS, Linux) and respects the user's preferred shell when available. |
Co-authored-by: rekram1-node <[email protected]>
|
Will the LLM know which shell the bash tool will use? |
|
nope, but i looked at some other tools and im gonna pass over this and change some things. I wonder if we should rename the tool to something else tho (thats a later thing tho) |
|
This seems like a downgrade for non bash shell users |
|
yeah im doing a pass to fix this I didn't want it to be released already lol |
|
@Ninja3047 i was thinking something like this, seems to be similar to how the other coding agents do it: |
|
Windows should probably default to something more bash compatible since the agent will likely be very confused. Claude code for example expects you to have git bash and will not work with cmd.exe so checking default git bash install locations might be a better default. |
|
Yeah good point, tho I noticed the other tools let u use powershell and stuff so maybe we should just rename the tool or something. This is a good fix for now tho Ill make it do that |
|
@Ninja3047 IMO I don't see any reason to restrict the user not to be able to use whatever shell the user has available. @rekram1-node I agree that renaming the tool might be a good idea but in any case I think the agent should be informed beforehand what shell the tool is gonna invoke in the end. I'm talking as a fish user myself. Nevermind this makes no sense :D - Aiden's proposed fix is good. |
|
Yes but that would require a new prompt and/or tool at least |
|
I think we do a follow up for that... Im just gonna do this for now: Preserves old behavior if u use random shells |
|
But I did like what you said about git bash, this does some stuff but maybe we need to search install locations too to be safe |

For spawn:
Fixes #3479