From 69b6299677411761acb5db9fb7ffd64b15d6076b Mon Sep 17 00:00:00 2001 From: Pedram Amini Date: Tue, 3 Feb 2026 14:27:03 -0600 Subject: [PATCH] fix(windows): handle full paths in known exe command detection Fixes an issue in PR #288's Windows shell fix where full paths like 'C:\Program Files\Git\bin\git' weren't recognized as known commands. Changes: - Extract command basename using regex for both Unix and Windows separators - Change from array to Set for O(1) lookup performance - Add additional common commands: npx, pnpm, pip, pip3 - Export needsWindowsShell for testability - Add comprehensive test suite for needsWindowsShell function --- .../main/utils/needsWindowsShell.test.ts | 84 +++++++++++++++++++ src/main/utils/execFile.ts | 9 +- 2 files changed, 90 insertions(+), 3 deletions(-) create mode 100644 src/__tests__/main/utils/needsWindowsShell.test.ts diff --git a/src/__tests__/main/utils/needsWindowsShell.test.ts b/src/__tests__/main/utils/needsWindowsShell.test.ts new file mode 100644 index 00000000..30764560 --- /dev/null +++ b/src/__tests__/main/utils/needsWindowsShell.test.ts @@ -0,0 +1,84 @@ +/** + * Tests for needsWindowsShell function in src/main/utils/execFile.ts + * + * This function determines whether a command needs shell execution on Windows. + * Separated into its own test file to avoid module mocking conflicts. + */ + +import { describe, it, expect } from 'vitest'; +import { needsWindowsShell } from '../../../main/utils/execFile'; + +describe('needsWindowsShell', () => { + describe('batch files', () => { + it('should return true for .cmd files', () => { + expect(needsWindowsShell('setup.cmd')).toBe(true); + expect(needsWindowsShell('C:\\path\\to\\setup.CMD')).toBe(true); + }); + + it('should return true for .bat files', () => { + expect(needsWindowsShell('install.bat')).toBe(true); + expect(needsWindowsShell('C:\\path\\to\\INSTALL.BAT')).toBe(true); + }); + }); + + describe('executables', () => { + it('should return false for .exe files', () => { + expect(needsWindowsShell('program.exe')).toBe(false); + expect(needsWindowsShell('C:\\path\\to\\program.EXE')).toBe(false); + }); + + it('should return false for .com files', () => { + expect(needsWindowsShell('command.com')).toBe(false); + expect(needsWindowsShell('C:\\path\\to\\COMMAND.COM')).toBe(false); + }); + }); + + describe('known commands with .exe variants', () => { + it('should return false for git', () => { + expect(needsWindowsShell('git')).toBe(false); + expect(needsWindowsShell('GIT')).toBe(false); + }); + + it('should return false for git with full path', () => { + expect(needsWindowsShell('C:\\Program Files\\Git\\bin\\git')).toBe(false); + expect(needsWindowsShell('/usr/bin/git')).toBe(false); + }); + + it('should return false for node', () => { + expect(needsWindowsShell('node')).toBe(false); + expect(needsWindowsShell('C:\\nodejs\\node')).toBe(false); + }); + + it('should return false for npm/npx/yarn/pnpm', () => { + expect(needsWindowsShell('npm')).toBe(false); + expect(needsWindowsShell('npx')).toBe(false); + expect(needsWindowsShell('yarn')).toBe(false); + expect(needsWindowsShell('pnpm')).toBe(false); + }); + + it('should return false for python/python3', () => { + expect(needsWindowsShell('python')).toBe(false); + expect(needsWindowsShell('python3')).toBe(false); + }); + + it('should return false for pip/pip3', () => { + expect(needsWindowsShell('pip')).toBe(false); + expect(needsWindowsShell('pip3')).toBe(false); + }); + }); + + describe('unknown commands without extension', () => { + it('should return true for unknown commands (need PATHEXT resolution)', () => { + expect(needsWindowsShell('mycustomtool')).toBe(true); + expect(needsWindowsShell('somecommand')).toBe(true); + }); + }); + + describe('commands with other extensions', () => { + it('should return false for commands with unknown extensions', () => { + // These have an extension, so no PATHEXT resolution needed + expect(needsWindowsShell('script.ps1')).toBe(false); + expect(needsWindowsShell('tool.msi')).toBe(false); + }); + }); +}); diff --git a/src/main/utils/execFile.ts b/src/main/utils/execFile.ts index 0683ad91..97ad4c17 100644 --- a/src/main/utils/execFile.ts +++ b/src/main/utils/execFile.ts @@ -30,7 +30,7 @@ export interface ExecResult { * to prevent percent-sign escaping issues in arguments * - Executables (.exe, .com) can run directly */ -function needsWindowsShell(command: string): boolean { +export function needsWindowsShell(command: string): boolean { const lowerCommand = command.toLowerCase(); // Batch files always need shell @@ -45,8 +45,11 @@ function needsWindowsShell(command: string): boolean { // Commands without extension: skip shell for known commands that have .exe variants // This prevents issues like % being interpreted as environment variables on Windows - const knownExeCommands = ['git', 'node', 'npm', 'yarn', 'python', 'python3']; - if (knownExeCommands.includes(lowerCommand)) { + // Extract basename to handle full paths like 'C:\Program Files\Git\bin\git' + // Use regex to handle both Unix (/) and Windows (\) path separators + const knownExeCommands = new Set(['git', 'node', 'npm', 'npx', 'yarn', 'pnpm', 'python', 'python3', 'pip', 'pip3']); + const commandBaseName = lowerCommand.split(/[\\/]/).pop() || lowerCommand; + if (knownExeCommands.has(commandBaseName)) { return false; }