Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions packages/core/src/services/shellExecutionService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const mockGetShellConfiguration = vi.hoisted(() =>
shell: 'bash',
}),
);
const mockIsRunningInMSYS2 = vi.hoisted(() => vi.fn().mockReturnValue(false));

// Top-level Mocks
vi.mock('@lydell/node-pty', () => ({
Expand Down Expand Up @@ -77,6 +78,7 @@ vi.mock('../utils/terminalSerializer.js', () => ({
}));
vi.mock('../utils/shell-utils.js', () => ({
getShellConfiguration: mockGetShellConfiguration,
isRunningInMSYS2: mockIsRunningInMSYS2,
}));
vi.mock('../utils/systemEncoding.js', () => ({
getCachedEncodingForBuffer: vi.fn().mockReturnValue('utf-8'),
Expand Down Expand Up @@ -1162,6 +1164,7 @@ describe('ShellExecutionService execution method selection', () => {

beforeEach(() => {
vi.clearAllMocks();
mockIsRunningInMSYS2.mockReturnValue(false);
onOutputEventMock = vi.fn();

// Mock for pty
Expand Down Expand Up @@ -1263,4 +1266,30 @@ describe('ShellExecutionService execution method selection', () => {
expect(mockCpSpawn).toHaveBeenCalled();
expect(result.executionMethod).toBe('child_process');
});

it('should use child_process when running in MSYS2 even if shouldUseNodePty is true', async () => {
// Simulate MSYS2 environment
mockIsRunningInMSYS2.mockReturnValue(true);

const abortController = new AbortController();
const handle = await ShellExecutionService.execute(
'test command',
'/test/dir',
onOutputEventMock,
abortController.signal,
true, // shouldUseNodePty is true, but MSYS2 should force child_process
shellExecutionConfig,
);

// Simulate exit to allow promise to resolve
mockChildProcess.emit('exit', 0, null);
const result = await handle.result;

// In MSYS2, should skip PTY and use child_process directly
expect(mockIsRunningInMSYS2).toHaveBeenCalled();
expect(mockGetPty).not.toHaveBeenCalled();
expect(mockPtySpawn).not.toHaveBeenCalled();
expect(mockCpSpawn).toHaveBeenCalled();
expect(result.executionMethod).toBe('child_process');
});
});
9 changes: 7 additions & 2 deletions packages/core/src/services/shellExecutionService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ import os from 'node:os';
import type { IPty } from '@lydell/node-pty';
import { getCachedEncodingForBuffer } from '../utils/systemEncoding.js';
import { isBinary } from '../utils/textUtils.js';
import { getShellConfiguration } from '../utils/shell-utils.js';
import {
getShellConfiguration,
isRunningInMSYS2,
} from '../utils/shell-utils.js';
import pkg from '@xterm/headless';
import {
serializeTerminalToObject,
Expand Down Expand Up @@ -341,7 +344,9 @@ export class ShellExecutionService {
shouldUseNodePty: boolean,
shellExecutionConfig: ShellExecutionConfig,
): Promise<ShellExecutionHandle> {
if (shouldUseNodePty) {
// MSYS2 bash is incompatible with Windows ConPTY (causes crashes)
// Fallback to child_process in MSYS2 environments
if (shouldUseNodePty && !isRunningInMSYS2()) {
const ptyInfo = await getPty();
if (ptyInfo) {
try {
Expand Down
212 changes: 171 additions & 41 deletions packages/core/src/utils/shell-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import {
getShellConfiguration,
isCommandAllowed,
isCommandNeedsPermission,
isRunningInMSYS2,
isRunningInGitBash,
stripShellWrapper,
} from './shell-utils.js';
import type { Config } from '../config/config.js';
Expand Down Expand Up @@ -624,10 +626,10 @@ describe('getShellConfiguration', () => {
});

describe('Git Bash / MSYS2 / MinTTY detection', () => {
it('should return bash configuration when MSYSTEM starts with MINGW', () => {
it('should return bash configuration when in Git Bash (MINGW64 without MSYSTEM_PREFIX)', () => {
process.env['MSYSTEM'] = 'MINGW64';
delete process.env['MSYSTEM_PREFIX'];
const config = getShellConfiguration();
// executable should be bash.exe path (either 'bash' or full path like 'C:\...\bash.exe')
expect(
config.executable.endsWith('bash.exe') ||
config.executable === 'bash',
Expand All @@ -636,8 +638,9 @@ describe('getShellConfiguration', () => {
expect(config.shell).toBe('bash');
});

it('should return bash configuration when MSYSTEM starts with MSYS', () => {
process.env['MSYSTEM'] = 'MSYS';
it('should return bash configuration when in Git Bash (MINGW32 without MSYSTEM_PREFIX)', () => {
process.env['MSYSTEM'] = 'MINGW32';
delete process.env['MSYSTEM_PREFIX'];
const config = getShellConfiguration();
expect(
config.executable.endsWith('bash.exe') ||
Expand All @@ -647,40 +650,44 @@ describe('getShellConfiguration', () => {
expect(config.shell).toBe('bash');
});

it('should return bash configuration when TERM includes msys', () => {
delete process.env['MSYSTEM'];
process.env['TERM'] = 'xterm-256color-msys';
it('should return cmd.exe when in MSYS2 MINGW64 (with MSYSTEM_PREFIX)', () => {
process.env['MSYSTEM'] = 'MINGW64';
process.env['MSYSTEM_PREFIX'] = '/mingw64';
delete process.env['ComSpec'];
const config = getShellConfiguration();
expect(
config.executable.endsWith('bash.exe') ||
config.executable === 'bash',
).toBe(true);
expect(config.argsPrefix).toEqual(['-c']);
expect(config.shell).toBe('bash');
expect(config.executable).toBe('cmd.exe');
expect(config.argsPrefix).toEqual(['/d', '/s', '/c']);
expect(config.shell).toBe('cmd');
});

it('should return bash configuration when TERM includes cygwin', () => {
delete process.env['MSYSTEM'];
process.env['TERM'] = 'xterm-256color-cygwin';
it('should return cmd.exe when in MSYS2 UCRT64', () => {
process.env['MSYSTEM'] = 'UCRT64';
process.env['MSYSTEM_PREFIX'] = '/ucrt64';
delete process.env['ComSpec'];
const config = getShellConfiguration();
expect(
config.executable.endsWith('bash.exe') ||
config.executable === 'bash',
).toBe(true);
expect(config.argsPrefix).toEqual(['-c']);
expect(config.shell).toBe('bash');
expect(config.executable).toBe('cmd.exe');
expect(config.argsPrefix).toEqual(['/d', '/s', '/c']);
expect(config.shell).toBe('cmd');
});

it('should prioritize MSYSTEM over TERM for Git Bash detection', () => {
process.env['MSYSTEM'] = 'MINGW64';
process.env['TERM'] = 'xterm';
it('should return cmd.exe when in MSYS2 CLANG64', () => {
process.env['MSYSTEM'] = 'CLANG64';
process.env['MSYSTEM_PREFIX'] = '/clang64';
delete process.env['ComSpec'];
const config = getShellConfiguration();
expect(
config.executable.endsWith('bash.exe') ||
config.executable === 'bash',
).toBe(true);
expect(config.argsPrefix).toEqual(['-c']);
expect(config.shell).toBe('bash');
expect(config.executable).toBe('cmd.exe');
expect(config.argsPrefix).toEqual(['/d', '/s', '/c']);
expect(config.shell).toBe('cmd');
});

it('should return cmd.exe when in MSYS2 MSYS', () => {
process.env['MSYSTEM'] = 'MSYS';
process.env['MSYSTEM_PREFIX'] = '/usr';
delete process.env['ComSpec'];
const config = getShellConfiguration();
expect(config.executable).toBe('cmd.exe');
expect(config.argsPrefix).toEqual(['/d', '/s', '/c']);
expect(config.shell).toBe('cmd');
});

it('should return cmd.exe when MSYSTEM and TERM do not indicate Git Bash', () => {
Expand All @@ -692,17 +699,140 @@ describe('getShellConfiguration', () => {
expect(config.argsPrefix).toEqual(['/d', '/s', '/c']);
expect(config.shell).toBe('cmd');
});
});
});
});

it('should return bash when MSYSTEM is MINGW32', () => {
process.env['MSYSTEM'] = 'MINGW32';
const config = getShellConfiguration();
expect(
config.executable.endsWith('bash.exe') ||
config.executable === 'bash',
).toBe(true);
expect(config.argsPrefix).toEqual(['-c']);
expect(config.shell).toBe('bash');
});
describe('isRunningInMSYS2', () => {
const originalEnv = { ...process.env };

beforeEach(() => {
process.env = { ...originalEnv };
});

afterEach(() => {
process.env = originalEnv;
});

describe('MSYS2 environments', () => {
it('should return true when MSYSTEM_PREFIX is /usr (MSYS)', () => {
process.env['MSYSTEM_PREFIX'] = '/usr';
expect(isRunningInMSYS2()).toBe(true);
});

it('should return true when MSYSTEM_PREFIX is /mingw64', () => {
process.env['MSYSTEM_PREFIX'] = '/mingw64';
expect(isRunningInMSYS2()).toBe(true);
});

it('should return true when MSYSTEM_PREFIX is /mingw32', () => {
process.env['MSYSTEM_PREFIX'] = '/mingw32';
expect(isRunningInMSYS2()).toBe(true);
});

it('should return true when MSYSTEM_PREFIX is /ucrt64', () => {
process.env['MSYSTEM_PREFIX'] = '/ucrt64';
expect(isRunningInMSYS2()).toBe(true);
});

it('should return true when MSYSTEM_PREFIX is /clang64', () => {
process.env['MSYSTEM_PREFIX'] = '/clang64';
expect(isRunningInMSYS2()).toBe(true);
});
});

describe('non-MSYS2 environments', () => {
it('should return false when MSYSTEM_PREFIX is undefined', () => {
delete process.env['MSYSTEM_PREFIX'];
expect(isRunningInMSYS2()).toBe(false);
});

it('should return false when MSYSTEM_PREFIX is unknown', () => {
process.env['MSYSTEM_PREFIX'] = '/unknown';
expect(isRunningInMSYS2()).toBe(false);
});

it('should return false when MSYSTEM_PREFIX is empty', () => {
process.env['MSYSTEM_PREFIX'] = '';
expect(isRunningInMSYS2()).toBe(false);
});

it('should return false in Git Bash (MINGW64 without MSYSTEM_PREFIX)', () => {
process.env['MSYSTEM'] = 'MINGW64';
delete process.env['MSYSTEM_PREFIX'];
expect(isRunningInMSYS2()).toBe(false);
});
});
});

describe('isRunningInGitBash', () => {
const originalEnv = { ...process.env };

beforeEach(() => {
process.env = { ...originalEnv };
});

afterEach(() => {
process.env = originalEnv;
});

describe('Git Bash environments', () => {
it('should return true when MSYSTEM is MINGW64', () => {
process.env['MSYSTEM'] = 'MINGW64';
expect(isRunningInGitBash()).toBe(true);
});

it('should return true when MSYSTEM is MINGW32', () => {
process.env['MSYSTEM'] = 'MINGW32';
expect(isRunningInGitBash()).toBe(true);
});

it('should return true when MSYSTEM starts with MINGW', () => {
process.env['MSYSTEM'] = 'MINGW123';
expect(isRunningInGitBash()).toBe(true);
});

it('should return true when MSYSTEM is MSYS (legacy behavior)', () => {
process.env['MSYSTEM'] = 'MSYS';
expect(isRunningInGitBash()).toBe(true);
});

it('should return true when TERM includes msys', () => {
delete process.env['MSYSTEM'];
process.env['TERM'] = 'xterm-256color-msys';
expect(isRunningInGitBash()).toBe(true);
});

it('should return true when TERM includes cygwin', () => {
delete process.env['MSYSTEM'];
process.env['TERM'] = 'xterm-256color-cygwin';
expect(isRunningInGitBash()).toBe(true);
});
});

describe('non-Git Bash environments', () => {
it('should return false when MSYSTEM is UCRT64', () => {
process.env['MSYSTEM'] = 'UCRT64';
delete process.env['TERM'];
expect(isRunningInGitBash()).toBe(false);
});

it('should return false when MSYSTEM is CLANG64', () => {
process.env['MSYSTEM'] = 'CLANG64';
delete process.env['TERM'];
expect(isRunningInGitBash()).toBe(false);
});

it('should return false when MSYSTEM is UNKNOWN and TERM is xterm', () => {
process.env['MSYSTEM'] = 'UNKNOWN';
process.env['TERM'] = 'xterm';
expect(isRunningInGitBash()).toBe(false);
});

it('should return false when MSYSTEM and TERM are undefined', () => {
delete process.env['MSYSTEM'];
delete process.env['TERM'];
expect(isRunningInGitBash()).toBe(false);
});
});
});
Expand Down
Loading
Loading