You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This PR adds support for detecting Git Bash/MSYS2/MinTTY environments on Windows and ensures that shell commands are executed using bash instead of cmd.exe or PowerShell when running in these environments.
Screenshots / Video Demo
Before fix:
After fix:
Dive Deeper
The getShellConfiguration() function always returned cmd.exe or PowerShell configuration on Windows, regardless of whether the user was running in Git Bash, MSYS2, or MinTTY environments. Users in these bash-based terminals expect shell commands to execute using bash, not Windows shells.
Key changes:
Added detection logic in getShellConfiguration() that checks MSYSTEM and TERM environment variables
Returns bash configuration when Git Bash/MSYS2/MinTTY/Cygwin is detected
Falls back to cmd.exe or PowerShell for regular Windows terminals
This PR adds support for detecting Git Bash/MSYS2/MinTTY environments on Windows and ensures shell commands execute using bash instead of cmd.exe or PowerShell in these environments. The implementation is clean, well-tested, and addresses a genuine usability issue for Windows developers using bash-based terminals.
🔍 General Feedback
The implementation follows existing code patterns and conventions in the codebase
Test coverage is comprehensive, covering all major detection scenarios
The detection logic is straightforward and maintainable
Good use of environment variables (MSYSTEM, TERM) that are standard in these environments
The change is minimal and focused, reducing risk of unintended side effects
🎯 Specific Feedback
🟢 Medium
File: packages/core/src/utils/shell-utils.ts:54-60 - Consider extracting the Git Bash detection logic into a separate helper function (e.g., isGitBashEnvironment()) for better testability and code organization. This would also make the getShellConfiguration() function more readable.
File: packages/core/src/utils/shell-utils.ts:58 - The TERM check uses || '' default but MSYSTEM doesn't. For consistency and to avoid potential issues if MSYSTEM is undefined, consider: const msystem = process.env['MSYSTEM'] || '';
🔵 Low
File: packages/core/src/utils/shell-utils.ts:54-57 - Add a brief comment explaining what MSYSTEM and TERM environment variables are for future maintainers who may not be familiar with Git Bash/MSYS2 internals. Example:
// MSYSTEM is set by Git Bash/MSYS2 (e.g., 'MINGW64', 'MSYS')// TERM may contain 'msys' or 'cygwin' in bash-based terminals
File: packages/core/src/utils/shell-utils.test.ts:620 - The test "should return cmd.exe when MSYSTEM and TERM do not indicate Git Bash" uses MSYSTEM = 'UNKNOWN'. Consider adding a comment that this is a hypothetical value since in practice MSYSTEM would either be unset or have known values like MINGW64/MSYS.
File: packages/core/src/utils/shell-utils.test.ts:563-567 - The afterEach hook restores process.env, but consider using a more robust approach like vi.restoreAllMocks() or explicitly restoring only the modified env variables to avoid potential test pollution.
✅ Highlights
Excellent test coverage with 9 new test cases covering all detection scenarios:
MINGW64/MINGW32 detection via MSYSTEM
MSYS detection via MSYSTEM
TERM-based detection for msys and cygwin
Priority handling (MSYSTEM over TERM)
Proper fallback to cmd.exe when no bash indicators present
The implementation correctly handles the edge case where both MSYSTEM and TERM are present
All existing tests continue to pass, demonstrating backward compatibility
The fix directly addresses the user-facing issue shown in the PR screenshots
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TLDR
This PR adds support for detecting Git Bash/MSYS2/MinTTY environments on Windows and ensures that shell commands are executed using bash instead of cmd.exe or PowerShell when running in these environments.
Screenshots / Video Demo
Before fix:


After fix:
Dive Deeper
The
getShellConfiguration()function always returned cmd.exe or PowerShell configuration on Windows, regardless of whether the user was running in Git Bash, MSYS2, or MinTTY environments. Users in these bash-based terminals expect shell commands to execute using bash, not Windows shells.Key changes:
getShellConfiguration()that checks MSYSTEM and TERM environment variablesReviewer Test Plan
1、In Git Bash / MSYS2 terminal on Windows:
Testing Matrix
Linked issues / bugs
#2537