fix(windows): resolve pywin32 DLL loading failure on Python 3.8+ (#1244)

* fix(windows): resolve pywin32 DLL loading failure on Python 3.8+ Python 3.8+ changed DLL search behavior - os.add_dll_directory() is now required for DLL resolution. PYTHONPATH alone doesn't work because: 1. .pth files are NOT processed when PYTHONPATH is set 2. pywin32_bootstrap.py (which calls os.add_dll_directory) never runs 3. pywintypes312.dll cannot be found without explicit DLL path setup This fix adds a PYTHONSTARTUP bootstrap script that: - Calls os.add_dll_directory() for pywin32_system32 before any imports - Uses site.addsitedir() to properly process .pth files - Adds pywin32_system32 to PATH as fallback Changes: - python-env-manager.ts: Add PYTHONSTARTUP and PATH configuration - download-python.cjs: Generate bootstrap script during build - Added comprehensive tests for the fix Fixes #810, #861 May also fix #943, #656, #630, #853 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(windows): address PR review feedback for pywin32 DLL loading Changes based on PR #1005 review comments: 1. Add .pyd extension check to sysloader filter (coderabbitai) - More precise filtering to avoid matching unrelated files 2. Add comments documenting DLL duplication trade-off (coderabbitai) - Explains why DLLs are copied to 3 locations (~2MB extra) - Documents the reliability vs bundle size trade-off 3. Add sync comments between bootstrap script locations (gemini, coderabbitai) - download-python.cjs and python-env-manager.ts now reference each other - Ensures future maintainers know to keep them synchronized 4. Add warning log when PYTHONSTARTUP creation fails (coderabbitai) - Surfaces the failure so users know pywin32 fix may not work - Mentions PATH fallback limitation on Python 3.8+ 5. Improve tests with Vitest best practices (coderabbitai) - Use vi.stubEnv instead of manual process.env mutation - Better PYTHONSTARTUP assertion logic - Integration test now uses actual generated content instead of duplicate Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: address TOCTOU race conditions in bootstrap script creation Replace existsSync + writeFileSync pattern with atomic 'wx' flag approach as recommended by Node.js official documentation to prevent file system race conditions. Changes: - python-env-manager.ts: Use writeFileSync with { flag: 'wx' } and handle EEXIST error silently (file already exists is expected) - download-python.cjs: Same pattern for __init__.py creation - Updated tests to verify new atomic write behavior The 'wx' flag atomically fails if the file exists (EEXIST), eliminating the window between existsSync check and writeFileSync where another process could create/modify the file. Reference: https://nodejs.org/api/fs.html#file-system-flags Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(windows): use platform abstraction and atomic write for consistency Address PR review findings: 1. Platform abstraction (python-env-manager.ts): - Replace direct `process.platform === 'win32'` checks with `isWindows()` - Replace hardcoded `;` path separator with `getPathDelimiter()` - Follows project guidelines in CLAUDE.md for cross-platform code 2. Atomic write (download-python.cjs): - Add `{ flag: 'wx' }` to writeFileSync for bootstrap script creation - Prevents TOCTOU race condition, consistent with other atomic writes - Handle EEXIST gracefully when file already exists Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(windows): remove dead PYTHONSTARTUP code, fix PATH case sensitivity This commit addresses verified findings from PR review: 1. Remove PYTHONSTARTUP dead code - PYTHONSTARTUP only runs in interactive Python mode (REPL), NOT when running scripts (python script.py). All Python invocations in Auto Claude pass scripts as arguments, so PYTHONSTARTUP never executes. - The DLL copying in fixPywin32() is what actually makes pywin32 work. - Removed ensurePywin32StartupScript() method from python-env-manager.ts - Removed bootstrap script creation from download-python.cjs 2. Fix PATH case sensitivity issue on Windows - On Windows, env vars are case-insensitive but Node.js preserves case. - If both 'PATH' and 'Path' exist, Node.js lexicographically sorts and uses first match, causing fragile behavior. - Now normalizes to single uppercase 'PATH' key. - See: https://github.com/nodejs/node/issues/9157 3. Add directory existence check for win32Dir - Prevents crash if pywin32 is partially installed. References: - Python PYTHONSTARTUP docs: https://docs.python.org/3/using/cmdline.html - Node.js env case sensitivity: https://github.com/nodejs/node/issues/9157 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(test): use manual env mocking for cross-platform PATH test vi.stubEnv('Path') adds a new variable but doesn't remove existing PATH on Linux/macOS CI runners. Use manual delete/set pattern to properly simulate Windows environment where only 'Path' exists. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> Co-authored-by: Andy <119136210+AndyMik90@users.noreply.github.com>
2026-01-17 19:17:08 +01:00
parent 14fbc2ebb8
commit cb786cac4c
3 changed files with 373 additions and 5 deletions
@@ -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);

@@ -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<typeof import('fs')>();
+  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;
+        }
+      }
+    });
+  });
+});
@@ -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 <prefix>" 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<string, string> {
    // 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<string, string> = {};
+    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 } : {}),
    };
  }