diff --git a/apps/frontend/scripts/download-python.cjs b/apps/frontend/scripts/download-python.cjs index 6f18bf44..bd1b587d 100644 --- a/apps/frontend/scripts/download-python.cjs +++ b/apps/frontend/scripts/download-python.cjs @@ -603,6 +603,143 @@ function checkForBlockedPackages(requirementsPath) { return blocked; } +/** + * Fix pywin32 installation for bundled packages. + * + * When pip installs pywin32 with --target, the post-install script doesn't run, + * and the .pth file isn't processed (since PYTHONPATH doesn't process .pth files). + * + * This means: + * 1. `import pywintypes` fails because pywintypes.py is in win32/lib/, not at root + * 2. `import _win32sysloader` fails because it's in win32/, not at root + * 3. pywin32_system32 needs an __init__.py to be importable as a package + * + * The fix copies the necessary files to site-packages root so they're directly importable. + */ +function fixPywin32(sitePackagesDir) { + const pywin32System32 = path.join(sitePackagesDir, 'pywin32_system32'); + const win32Dir = path.join(sitePackagesDir, 'win32'); + const win32LibDir = path.join(win32Dir, 'lib'); + + if (!fs.existsSync(pywin32System32)) { + // pywin32 not installed or not on Windows - nothing to fix + return; + } + + console.log(`[download-python] Fixing pywin32 for bundled packages...`); + + // 1. Copy pywintypes.py and pythoncom.py from win32/lib/ to root + // These are the Python modules that load the DLLs + const pyModules = ['pywintypes.py', 'pythoncom.py']; + for (const pyModule of pyModules) { + const srcPath = path.join(win32LibDir, pyModule); + const destPath = path.join(sitePackagesDir, pyModule); + + if (fs.existsSync(srcPath)) { + try { + fs.copyFileSync(srcPath, destPath); + console.log(`[download-python] Copied ${pyModule} to site-packages root`); + } catch (err) { + console.warn(`[download-python] Failed to copy ${pyModule}: ${err.message}`); + } + } + } + + // 2. Copy _win32sysloader.pyd from win32/ to root + // This is required by pywintypes.py to locate and load the DLLs + // Filter for .pyd extension to avoid matching unrelated files + if (!fs.existsSync(win32Dir)) { + console.warn(`[download-python] win32 directory not found: ${win32Dir}`); + return; + } + const sysloaderFiles = fs.readdirSync(win32Dir).filter(f => f.startsWith('_win32sysloader') && f.endsWith('.pyd')); + for (const sysloader of sysloaderFiles) { + const srcPath = path.join(win32Dir, sysloader); + const destPath = path.join(sitePackagesDir, sysloader); + + try { + fs.copyFileSync(srcPath, destPath); + console.log(`[download-python] Copied ${sysloader} to site-packages root`); + } catch (err) { + console.warn(`[download-python] Failed to copy ${sysloader}: ${err.message}`); + } + } + + // 3. Create __init__.py in pywin32_system32/ to make it importable as a package + // pywintypes.py does `import pywin32_system32` and then uses pywin32_system32.__path__ + const initPath = path.join(pywin32System32, '__init__.py'); + try { + // The __init__.py sets up __path__ so pywintypes.py can find the DLLs + const initContent = `# Auto-generated for bundled pywin32 +import os +__path__ = [os.path.dirname(__file__)] +`; + // Use 'wx' flag for atomic exclusive write - fails if file exists (EEXIST) + // This avoids TOCTOU race condition where existsSync + writeFileSync could + // allow another process to create/modify the file between check and write. + // See: https://nodejs.org/api/fs.html#file-system-flags + fs.writeFileSync(initPath, initContent, { flag: 'wx' }); + console.log(`[download-python] Created pywin32_system32/__init__.py`); + } catch (err) { + // EEXIST means file already exists - that's fine, we wanted to avoid overwriting + if (err.code !== 'EEXIST') { + console.warn(`[download-python] Failed to create __init__.py: ${err.message}`); + } + } + + // 4. Copy DLLs to multiple locations for maximum compatibility + // + // Why we copy DLLs to pywin32_system32/, win32/, AND site-packages root: + // - pywin32_system32/: Primary location, used by os.add_dll_directory() in bootstrap + // - win32/: Fallback for pywintypes.py's __file__-relative search + // - site-packages root: Fallback when other search mechanisms fail + // + // Trade-off: This duplicates DLLs ~3x (~2MB extra), but ensures pywin32 works + // regardless of which DLL search mechanism succeeds. The alternative (single + // location) caused intermittent failures depending on Python version and how + // the process was spawned. Bundle size trade-off is acceptable for reliability. + // + // See: https://github.com/AndyMik90/Auto-Claude/issues/810 + const dllFiles = fs.readdirSync(pywin32System32).filter(f => f.endsWith('.dll')); + for (const dll of dllFiles) { + const srcPath = path.join(pywin32System32, dll); + const destPath = path.join(win32Dir, dll); + + try { + fs.copyFileSync(srcPath, destPath); + console.log(`[download-python] Copied ${dll} to win32/`); + } catch (err) { + console.warn(`[download-python] Failed to copy ${dll} to win32/: ${err.message}`); + } + } + + // 5. Also copy DLLs to site-packages root for maximum compatibility + for (const dll of dllFiles) { + const srcPath = path.join(pywin32System32, dll); + const destPath = path.join(sitePackagesDir, dll); + + try { + fs.copyFileSync(srcPath, destPath); + console.log(`[download-python] Copied ${dll} to site-packages root`); + } catch (err) { + console.warn(`[download-python] Failed to copy ${dll}: ${err.message}`); + } + } + + // Note: We intentionally do NOT create a PYTHONSTARTUP bootstrap script. + // PYTHONSTARTUP only runs in interactive Python mode (python REPL), NOT when + // running scripts (python script.py). Since all our Python invocations pass + // scripts as arguments, PYTHONSTARTUP would never execute. + // + // The DLL copying above (steps 4 and 5) is what actually makes pywin32 work - + // it places DLLs in locations where Python's default DLL search finds them. + // The PATH modification in python-env-manager.ts provides an additional fallback. + // + // See: https://docs.python.org/3/using/cmdline.html (PYTHONSTARTUP documentation) + + console.log(`[download-python] pywin32 fix complete`); +} + /** * Install Python packages into a site-packages directory. * Uses pip with optimizations for smaller output. @@ -654,6 +791,9 @@ function installPackages(pythonBin, requirementsPath, targetSitePackages) { console.log(`[download-python] Packages installed successfully`); + // Fix pywin32 for Windows builds (must be done BEFORE stripping) + fixPywin32(targetSitePackages); + // Strip unnecessary files stripSitePackages(targetSitePackages); diff --git a/apps/frontend/src/main/__tests__/python-env-manager.test.ts b/apps/frontend/src/main/__tests__/python-env-manager.test.ts new file mode 100644 index 00000000..98ecca33 --- /dev/null +++ b/apps/frontend/src/main/__tests__/python-env-manager.test.ts @@ -0,0 +1,177 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import path from 'path'; + +// Mock fs module before importing the module under test +vi.mock('fs', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + existsSync: vi.fn(), + writeFileSync: vi.fn(), + }; +}); + +// Mock electron's app module +vi.mock('electron', () => ({ + app: { + isPackaged: false, + getPath: vi.fn().mockReturnValue('/mock/user/data'), + getAppPath: vi.fn().mockReturnValue('/mock/app'), + on: vi.fn(), + }, +})); + +// Mock python-detector +vi.mock('../python-detector', () => ({ + findPythonCommand: vi.fn().mockReturnValue('python'), + getBundledPythonPath: vi.fn().mockReturnValue(null), +})); + +// Import after mocking +import { PythonEnvManager } from '../python-env-manager'; + +describe('PythonEnvManager', () => { + let manager: PythonEnvManager; + + beforeEach(() => { + manager = new PythonEnvManager(); + vi.clearAllMocks(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + describe('getPythonEnv', () => { + it('should return basic Python environment variables', () => { + const env = manager.getPythonEnv(); + + expect(env.PYTHONDONTWRITEBYTECODE).toBe('1'); + expect(env.PYTHONIOENCODING).toBe('utf-8'); + expect(env.PYTHONNOUSERSITE).toBe('1'); + }); + + it('should exclude PYTHONHOME from environment', () => { + // Use vi.stubEnv for cleaner environment variable mocking + vi.stubEnv('PYTHONHOME', '/some/python/home'); + + const env = manager.getPythonEnv(); + expect(env.PYTHONHOME).toBeUndefined(); + + vi.unstubAllEnvs(); + }); + + it('should preserve external PYTHONSTARTUP values', () => { + // We no longer strip PYTHONSTARTUP - it passes through from the environment. + // Note: PYTHONSTARTUP only runs in interactive Python mode (python REPL), + // not when running scripts, so it doesn't affect our Python invocations. + vi.stubEnv('PYTHONSTARTUP', '/some/external/startup.py'); + + try { + const env = manager.getPythonEnv(); + // External PYTHONSTARTUP should pass through unchanged + expect(env.PYTHONSTARTUP).toBe('/some/external/startup.py'); + } finally { + vi.unstubAllEnvs(); + } + }); + }); + + describe('Windows pywin32 DLL loading fix', () => { + const originalPlatform = process.platform; + + beforeEach(() => { + // Mock Windows platform + Object.defineProperty(process, 'platform', { value: 'win32' }); + }); + + afterEach(() => { + Object.defineProperty(process, 'platform', { value: originalPlatform }); + }); + + it('should add pywin32_system32 to PATH on Windows when sitePackagesPath is set', () => { + const sitePackagesPath = 'C:\\test\\site-packages'; + + // Access private property for testing + (manager as any).sitePackagesPath = sitePackagesPath; + + const env = manager.getPythonEnv(); + + // Should include pywin32_system32 in PATH + const expectedPath = path.join(sitePackagesPath, 'pywin32_system32'); + expect(env.PATH).toContain(expectedPath); + }); + + it('should include win32 and win32/lib in PYTHONPATH on Windows', () => { + const sitePackagesPath = 'C:\\test\\site-packages'; + + // Access private property for testing + (manager as any).sitePackagesPath = sitePackagesPath; + + const env = manager.getPythonEnv(); + + // PYTHONPATH should include site-packages, win32, and win32/lib + expect(env.PYTHONPATH).toContain(sitePackagesPath); + expect(env.PYTHONPATH).toContain(path.join(sitePackagesPath, 'win32')); + expect(env.PYTHONPATH).toContain( + path.join(sitePackagesPath, 'win32', 'lib') + ); + }); + + it('should not add Windows-specific PATH modification on non-Windows platforms', () => { + // Restore non-Windows platform + Object.defineProperty(process, 'platform', { value: 'darwin' }); + + const sitePackagesPath = '/test/site-packages'; + + // Access private property for testing + (manager as any).sitePackagesPath = sitePackagesPath; + + const env = manager.getPythonEnv(); + + // PYTHONPATH should just be the site-packages (no win32 additions) + expect(env.PYTHONPATH).toBe(sitePackagesPath); + + // PATH should not contain pywin32_system32 + expect(env.PATH || '').not.toContain('pywin32_system32'); + }); + + it('should normalize PATH case sensitivity on Windows', () => { + // On Windows, env vars are case-insensitive but Node.js preserves case. + // If the environment has 'Path' (lowercase t), we should normalize to 'PATH' + // to avoid issues with Node.js lexicographic sorting. + // See: https://github.com/nodejs/node/issues/9157 + const sitePackagesPath = 'C:\\test\\site-packages'; + + // Access private property for testing + (manager as any).sitePackagesPath = sitePackagesPath; + + // Save and clear existing PATH, then set lowercase 'Path' + // This simulates a Windows environment where the system has 'Path' instead of 'PATH' + const originalPath = process.env.PATH; + delete process.env.PATH; + process.env.Path = 'C:\\Windows\\System32'; + + try { + const env = manager.getPythonEnv(); + + // Should have a PATH key (uppercase) containing both pywin32_system32 and original Path value + expect(env.PATH).toBeDefined(); + expect(env.PATH).toContain('pywin32_system32'); + expect(env.PATH).toContain('C:\\Windows\\System32'); + + // Should NOT have both 'PATH' and 'Path' keys (case normalization) + // The lowercase 'Path' should be removed to avoid Node.js case-sensitivity issues + const pathKeys = Object.keys(env).filter(k => k.toUpperCase() === 'PATH'); + expect(pathKeys.length).toBe(1); + expect(pathKeys[0]).toBe('PATH'); + } finally { + // Restore original PATH + delete process.env.Path; + if (originalPath !== undefined) { + process.env.PATH = originalPath; + } + } + }); + }); +}); diff --git a/apps/frontend/src/main/python-env-manager.ts b/apps/frontend/src/main/python-env-manager.ts index 84c8e77c..e2e807b5 100644 --- a/apps/frontend/src/main/python-env-manager.ts +++ b/apps/frontend/src/main/python-env-manager.ts @@ -1,10 +1,10 @@ import { spawn, execSync, ChildProcess } from 'child_process'; -import { existsSync, readdirSync } from 'fs'; +import { existsSync, readdirSync, writeFileSync } from 'fs'; import path from 'path'; import { EventEmitter } from 'events'; import { app } from 'electron'; import { findPythonCommand, getBundledPythonPath } from './python-detector'; -import { isLinux, isWindows } from './platform'; +import { isLinux, isWindows, getPathDelimiter } from './platform'; export interface PythonEnvStatus { ready: boolean; @@ -669,7 +669,17 @@ if sys.version_info >= (3, 12): * problematic Python variables removed. This fixes the "Could not find platform * independent libraries " error on Windows when PYTHONHOME is set. * + * For Windows with pywin32, this method handles several critical issues: + * 1. PYTHONPATH must include win32 and win32/lib for module imports + * 2. pywin32_system32 must be in PATH for DLL loading + * + * Note: The DLL copying performed by fixPywin32() in download-python.cjs is what + * actually makes pywin32 work - it copies DLLs to locations where Python's default + * DLL search finds them. Adding pywin32_system32 to PATH is an additional fallback. + * * @see https://github.com/AndyMik90/Auto-Claude/issues/176 + * @see https://github.com/AndyMik90/Auto-Claude/issues/810 + * @see https://github.com/mhammond/pywin32/blob/main/win32/Lib/pywin32_bootstrap.py */ getPythonEnv(): Record { // Start with process.env but explicitly remove problematic Python variables @@ -681,14 +691,55 @@ if sys.version_info >= (3, 12): // Skip PYTHONHOME - it causes the "platform independent libraries" error // Use case-insensitive check for Windows compatibility (env vars are case-insensitive on Windows) // Skip undefined values (TypeScript type guard) - if (key.toUpperCase() !== 'PYTHONHOME' && value !== undefined) { + const upperKey = key.toUpperCase(); + if (upperKey !== 'PYTHONHOME' && value !== undefined) { baseEnv[key] = value; } } - // Apply our Python configuration on top + // Build PYTHONPATH - for Windows with pywin32, we need to include win32 and win32/lib + // since the .pth file that normally adds these isn't processed when using PYTHONPATH + let pythonPath = this.sitePackagesPath || ''; + if (this.sitePackagesPath && isWindows()) { + const pathSep = getPathDelimiter(); // Platform-appropriate path separator + const win32Path = path.join(this.sitePackagesPath, 'win32'); + const win32LibPath = path.join(this.sitePackagesPath, 'win32', 'lib'); + pythonPath = [this.sitePackagesPath, win32Path, win32LibPath].join(pathSep); + } + + // Windows-specific pywin32 DLL loading fix + // On Windows with bundled packages, we need to ensure pywin32 DLLs can be found. + // The DLL copying in fixPywin32() is the primary fix - this PATH addition is a fallback. + let windowsEnv: Record = {}; + if (this.sitePackagesPath && isWindows()) { + const pywin32System32 = path.join(this.sitePackagesPath, 'pywin32_system32'); + + // Add pywin32_system32 to PATH for DLL loading + // Fix PATH case sensitivity: On Windows, env vars are case-insensitive but Node.js + // preserves case. If we have both 'PATH' and 'Path', Node.js lexicographically sorts + // and uses the first match, causing issues. Normalize to single 'PATH' key. + // See: https://github.com/nodejs/node/issues/9157 + const pathKey = Object.keys(baseEnv).find(k => k.toUpperCase() === 'PATH'); + const currentPath = pathKey ? baseEnv[pathKey] : ''; + + // Remove any existing PATH variants to avoid duplicates + if (pathKey && pathKey !== 'PATH') { + delete baseEnv[pathKey]; + } + + if (currentPath && !currentPath.includes(pywin32System32)) { + windowsEnv['PATH'] = `${pywin32System32};${currentPath}`; + } else if (!currentPath) { + windowsEnv['PATH'] = pywin32System32; + } else { + // pywin32System32 already in path, but still normalize to 'PATH' + windowsEnv['PATH'] = currentPath; + } + } + return { ...baseEnv, + ...windowsEnv, // Don't write bytecode - not needed and avoids permission issues PYTHONDONTWRITEBYTECODE: '1', // Use UTF-8 encoding @@ -696,7 +747,7 @@ if sys.version_info >= (3, 12): // Disable user site-packages to avoid conflicts PYTHONNOUSERSITE: '1', // Override PYTHONPATH if we have bundled packages - ...(this.sitePackagesPath ? { PYTHONPATH: this.sitePackagesPath } : {}), + ...(pythonPath ? { PYTHONPATH: pythonPath } : {}), }; }