fix(windows): ensure pywin32 is bundled in Windows binary (ACS-306) (#1197)
* fix(spec): resolve circular import between spec.pipeline and core.client Implement two-part fix to break circular import dependency: 1. Add __getattr__ lazy imports in spec/__init__.py to defer imports of SpecOrchestrator and get_specs_dir until accessed. 2. Move create_client import inside run_agent() method in spec/pipeline/agent_runner.py to avoid top-level import. The circular import chain: spec.pipeline.agent_runner -> core.client -> agents.tools_pkg -> spec.validate_pkg.auto_fix -> spec.pipeline By deferring both the pipeline imports in spec/__init__.py AND the create_client import in agent_runner.py, the circular dependency is broken. The __getattr__ implementation caches imports in globals() for efficiency. Refs: ACS-302 * fix(windows): ensure pywin32 is bundled in Windows binary (ACS-306) Fixes ModuleNotFoundError: No module named 'win32api' when the MCP library attempts to import Windows-specific utilities in packaged apps. Root cause: The build script's critical packages validation did not include pywin32 for Windows, causing cached bundles without pywin32 to be accepted during Windows binary builds. Changes: - Add pywin32 to critical packages validation on Windows (download-python.cjs, python-env-manager.ts) - Remove Python version constraint from pywin32 dependency The MCP library unconditionally imports win32api on Windows regardless of Python version - Validate pywin32 on all Python versions on Windows in dependency_validator.py (was 3.12+ only) - Update error message to mention MCP library requirement - Update tests to reflect new behavior Refs: ACS-306 --------- Co-authored-by: StillKnotKnown <stillknotknown@users.noreply.github.com>
This commit is contained in:
@@ -18,7 +18,9 @@ def validate_platform_dependencies() -> None:
|
||||
with helpful installation instructions.
|
||||
"""
|
||||
# Check Windows-specific dependencies
|
||||
if sys.platform == "win32" and sys.version_info >= (3, 12):
|
||||
# pywin32 is required on all Python versions on Windows (ACS-306)
|
||||
# The MCP library unconditionally imports win32api on Windows
|
||||
if sys.platform == "win32":
|
||||
try:
|
||||
import pywintypes # noqa: F401
|
||||
except ImportError:
|
||||
@@ -29,13 +31,19 @@ def _exit_with_pywin32_error() -> None:
|
||||
"""Exit with helpful error message for missing pywin32."""
|
||||
# Use sys.prefix to detect the virtual environment path
|
||||
# This works for venv and poetry environments
|
||||
venv_activate = Path(sys.prefix) / "Scripts" / "activate"
|
||||
# Check for common Windows activation scripts (activate, activate.bat, Activate.ps1)
|
||||
scripts_dir = Path(sys.prefix) / "Scripts"
|
||||
activation_candidates = [
|
||||
scripts_dir / "activate",
|
||||
scripts_dir / "activate.bat",
|
||||
scripts_dir / "Activate.ps1",
|
||||
]
|
||||
venv_activate = next((p for p in activation_candidates if p.exists()), None)
|
||||
|
||||
sys.exit(
|
||||
"Error: Required Windows dependency 'pywin32' is not installed.\n"
|
||||
"\n"
|
||||
"Auto Claude requires pywin32 on Windows for LadybugDB/Graphiti memory integration.\n"
|
||||
"\n"
|
||||
# Build activation step only if activate script exists
|
||||
activation_step = ""
|
||||
if venv_activate:
|
||||
activation_step = (
|
||||
"To fix this:\n"
|
||||
"1. Activate your virtual environment:\n"
|
||||
f" {venv_activate}\n"
|
||||
@@ -45,6 +53,26 @@ def _exit_with_pywin32_error() -> None:
|
||||
"\n"
|
||||
" Or reinstall all dependencies:\n"
|
||||
" pip install -r requirements.txt\n"
|
||||
)
|
||||
else:
|
||||
# For system Python or environments without activate script
|
||||
activation_step = (
|
||||
"To fix this:\n"
|
||||
"Install pywin32:\n"
|
||||
" pip install pywin32>=306\n"
|
||||
"\n"
|
||||
" Or reinstall all dependencies:\n"
|
||||
" pip install -r requirements.txt\n"
|
||||
)
|
||||
|
||||
sys.exit(
|
||||
"Error: Required Windows dependency 'pywin32' is not installed.\n"
|
||||
"\n"
|
||||
"Auto Claude requires pywin32 on Windows for:\n"
|
||||
" - MCP library (win32api, win32con, win32job modules)\n"
|
||||
" - LadybugDB/Graphiti memory integration\n"
|
||||
"\n"
|
||||
f"{activation_step}"
|
||||
"\n"
|
||||
f"Current Python: {sys.executable}\n"
|
||||
)
|
||||
|
||||
@@ -17,7 +17,8 @@ graphiti-core>=0.5.0; python_version >= "3.12"
|
||||
|
||||
# Windows-specific dependency for LadybugDB/Graphiti
|
||||
# pywin32 provides Windows system bindings required by real_ladybug
|
||||
pywin32>=306; sys_platform == "win32" and python_version >= "3.12"
|
||||
# Required on all Python versions on Windows (ACS-306) - MCP library unconditionally imports win32api
|
||||
pywin32>=306; sys_platform == "win32"
|
||||
|
||||
# Google AI (optional - for Gemini LLM and embeddings)
|
||||
google-generativeai>=0.8.0
|
||||
|
||||
@@ -12,7 +12,6 @@ from ui.capabilities import configure_safe_encoding
|
||||
|
||||
configure_safe_encoding()
|
||||
|
||||
from core.client import create_client
|
||||
from debug import debug, debug_detailed, debug_error, debug_section, debug_success
|
||||
from security.tool_input_validator import get_safe_tool_input
|
||||
from task_logger import (
|
||||
@@ -21,6 +20,10 @@ from task_logger import (
|
||||
TaskLogger,
|
||||
)
|
||||
|
||||
# Lazy import create_client to avoid circular import with core.client
|
||||
# The import chain: spec.pipeline -> agent_runner -> core.client -> agents.tools_pkg -> spec.validate_pkg
|
||||
# By deferring the import, we break the circular dependency.
|
||||
|
||||
|
||||
class AgentRunner:
|
||||
"""Manages agent execution with logging and error handling."""
|
||||
@@ -116,6 +119,9 @@ class AgentRunner:
|
||||
"Creating Claude SDK client...",
|
||||
thinking_budget=thinking_budget,
|
||||
)
|
||||
# Lazy import to avoid circular import with core.client
|
||||
from core.client import create_client
|
||||
|
||||
client = create_client(
|
||||
self.project_dir,
|
||||
self.spec_dir,
|
||||
|
||||
@@ -709,12 +709,18 @@ async function downloadPython(targetPlatform, targetArch, options = {}) {
|
||||
// Without this check, corrupted caches with missing packages would be accepted
|
||||
// Note: Same list exists in python-env-manager.ts - keep them in sync
|
||||
// This validation assumes traditional Python packages with __init__.py (not PEP 420 namespace packages)
|
||||
const criticalPackages = ['claude_agent_sdk', 'dotenv', 'pydantic_core'];
|
||||
// pywin32 is platform-critical for Windows (ACS-306) - required by MCP library
|
||||
// Note: We check for 'pywintypes' instead of 'pywin32' because pywin32 installs
|
||||
// top-level modules (pywintypes, win32api, win32con, win32com) without a pywin32/__init__.py
|
||||
const criticalPackages = ['claude_agent_sdk', 'dotenv', 'pydantic_core']
|
||||
.concat(info.nodePlatform === 'win32' ? ['pywintypes'] : []);
|
||||
const missingPackages = criticalPackages.filter(pkg => {
|
||||
const pkgPath = path.join(sitePackagesDir, pkg);
|
||||
// Check both directory and __init__.py for more robust validation
|
||||
const initFile = path.join(pkgPath, '__init__.py');
|
||||
return !fs.existsSync(pkgPath) || !fs.existsSync(initFile);
|
||||
// For single-file modules (like pywintypes.py), check for the file directly
|
||||
const moduleFile = path.join(sitePackagesDir, pkg + '.py');
|
||||
// Package is valid if directory+__init__.py exists OR single-file module exists
|
||||
return !fs.existsSync(initFile) && !fs.existsSync(moduleFile);
|
||||
});
|
||||
|
||||
if (missingPackages.length > 0) {
|
||||
@@ -812,11 +818,18 @@ async function downloadPython(targetPlatform, targetArch, options = {}) {
|
||||
// Verify critical packages were installed before creating marker (fixes #416)
|
||||
// Note: Same list exists in python-env-manager.ts - keep them in sync
|
||||
// This validation assumes traditional Python packages with __init__.py (not PEP 420 namespace packages)
|
||||
const criticalPackages = ['claude_agent_sdk', 'dotenv', 'pydantic_core'];
|
||||
// pywin32 is platform-critical for Windows (ACS-306) - required by MCP library
|
||||
// Note: We check for 'pywintypes' instead of 'pywin32' because pywin32 installs
|
||||
// top-level modules (pywintypes, win32api, win32con, win32com) without a pywin32/__init__.py
|
||||
const criticalPackages = ['claude_agent_sdk', 'dotenv', 'pydantic_core']
|
||||
.concat(info.nodePlatform === 'win32' ? ['pywintypes'] : []);
|
||||
const postInstallMissing = criticalPackages.filter(pkg => {
|
||||
const pkgPath = path.join(sitePackagesDir, pkg);
|
||||
const initFile = path.join(pkgPath, '__init__.py');
|
||||
return !fs.existsSync(pkgPath) || !fs.existsSync(initFile);
|
||||
// For single-file modules (like pywintypes.py), check for the file directly
|
||||
const moduleFile = path.join(sitePackagesDir, pkg + '.py');
|
||||
// Package is valid if directory+__init__.py exists OR single-file module exists
|
||||
return !fs.existsSync(initFile) && !fs.existsSync(moduleFile);
|
||||
});
|
||||
|
||||
if (postInstallMissing.length > 0) {
|
||||
|
||||
@@ -4,6 +4,7 @@ import path from 'path';
|
||||
import { EventEmitter } from 'events';
|
||||
import { app } from 'electron';
|
||||
import { findPythonCommand, getBundledPythonPath } from './python-detector';
|
||||
import { isWindows } from './platform';
|
||||
|
||||
export interface PythonEnvStatus {
|
||||
ready: boolean;
|
||||
@@ -67,7 +68,7 @@ export class PythonEnvManager extends EventEmitter {
|
||||
if (!venvPath) return null;
|
||||
|
||||
const venvPython =
|
||||
process.platform === 'win32'
|
||||
isWindows()
|
||||
? path.join(venvPath, 'Scripts', 'python.exe')
|
||||
: path.join(venvPath, 'bin', 'python');
|
||||
|
||||
@@ -126,14 +127,22 @@ export class PythonEnvManager extends EventEmitter {
|
||||
// This fixes GitHub issue #416 where marker exists but packages are missing
|
||||
// Note: Same list exists in download-python.cjs - keep them in sync
|
||||
// This validation assumes traditional Python packages with __init__.py (not PEP 420 namespace packages)
|
||||
const criticalPackages = ['claude_agent_sdk', 'dotenv', 'pydantic_core'];
|
||||
// pywin32 is platform-critical for Windows (ACS-306) - required by MCP library
|
||||
// Note: We check for 'pywintypes' instead of 'pywin32' because pywin32 installs
|
||||
// top-level modules (pywintypes, win32api, win32con, win32com) without a pywin32/__init__.py
|
||||
const criticalPackages = ['claude_agent_sdk', 'dotenv', 'pydantic_core']
|
||||
.concat(isWindows() ? ['pywintypes'] : []);
|
||||
|
||||
// Check each package exists with valid structure (directory + __init__.py)
|
||||
// Check each package exists with valid structure
|
||||
// For traditional packages: directory + __init__.py
|
||||
// For single-file modules (like pywintypes.py): just the .py file
|
||||
const missingPackages = criticalPackages.filter((pkg) => {
|
||||
const pkgPath = path.join(sitePackagesPath, pkg);
|
||||
const initPath = path.join(pkgPath, '__init__.py');
|
||||
// Package is valid if directory and __init__.py both exist
|
||||
return !existsSync(pkgPath) || !existsSync(initPath);
|
||||
// For single-file modules (like pywintypes.py), check for the file directly
|
||||
const moduleFile = path.join(sitePackagesPath, `${pkg}.py`);
|
||||
// Package is valid if directory+__init__.py exists OR single-file module exists
|
||||
return !existsSync(initPath) && !existsSync(moduleFile);
|
||||
});
|
||||
|
||||
// Log missing packages for debugging
|
||||
@@ -551,7 +560,7 @@ if sys.version_info >= (3, 12):
|
||||
// For venv, site-packages is inside the venv
|
||||
const venvBase = this.getVenvBasePath();
|
||||
if (venvBase) {
|
||||
if (process.platform === 'win32') {
|
||||
if (isWindows()) {
|
||||
// Windows venv structure: Lib/site-packages (no python version subfolder)
|
||||
this.sitePackagesPath = path.join(venvBase, 'Lib', 'site-packages');
|
||||
} else {
|
||||
|
||||
@@ -4,10 +4,9 @@ Tests for dependency_validator module.
|
||||
|
||||
Tests cover:
|
||||
- Platform-specific dependency validation
|
||||
- pywin32 validation on Windows Python 3.12+
|
||||
- pywin32 validation on Windows (all Python versions, ACS-306)
|
||||
- Helpful error messages for missing dependencies
|
||||
- No validation on non-Windows platforms
|
||||
- No validation on Python < 3.12
|
||||
"""
|
||||
|
||||
import sys
|
||||
@@ -76,17 +75,32 @@ class TestValidatePlatformDependencies:
|
||||
# Should not raise SystemExit
|
||||
validate_platform_dependencies()
|
||||
|
||||
def test_windows_python_311_skips_validation(self):
|
||||
"""Windows + Python < 3.12 should skip pywin32 validation."""
|
||||
def test_windows_python_311_validates_pywin32(self):
|
||||
"""
|
||||
Windows + Python 3.11 should validate pywin32 (ACS-306).
|
||||
|
||||
Previously, validation only happened on Python 3.12+, but the MCP
|
||||
library requires pywin32 on all Python versions on Windows.
|
||||
"""
|
||||
import builtins
|
||||
|
||||
with patch("sys.platform", "win32"), \
|
||||
patch("sys.version_info", (3, 11, 0)), \
|
||||
patch("builtins.__import__") as mock_import:
|
||||
# Even if pywintypes is not available, should not exit
|
||||
mock_import.side_effect = ImportError("No module named 'pywintypes'")
|
||||
patch("core.dependency_validator._exit_with_pywin32_error") as mock_exit:
|
||||
|
||||
# Should not raise SystemExit
|
||||
original_import = builtins.__import__
|
||||
|
||||
def mock_import(name, *args, **kwargs):
|
||||
if name == "pywintypes":
|
||||
raise ImportError("No module named 'pywintypes'")
|
||||
return original_import(name, *args, **kwargs)
|
||||
|
||||
with patch("builtins.__import__", side_effect=mock_import):
|
||||
validate_platform_dependencies()
|
||||
|
||||
# Should have called the error exit function (changed from skipping)
|
||||
mock_exit.assert_called_once()
|
||||
|
||||
def test_linux_skips_validation(self):
|
||||
"""Non-Windows platforms should skip pywin32 validation."""
|
||||
with patch("sys.platform", "linux"), \
|
||||
@@ -130,16 +144,32 @@ class TestValidatePlatformDependencies:
|
||||
# Should have called the error exit function
|
||||
mock_exit.assert_called_once()
|
||||
|
||||
def test_windows_python_310_skips_validation(self):
|
||||
"""Windows + Python 3.10 should skip pywin32 validation."""
|
||||
def test_windows_python_310_validates_pywin32(self):
|
||||
"""
|
||||
Windows + Python 3.10 should validate pywin32 (ACS-306).
|
||||
|
||||
Previously, validation only happened on Python 3.12+, but the MCP
|
||||
library requires pywin32 on all Python versions on Windows.
|
||||
"""
|
||||
import builtins
|
||||
|
||||
with patch("sys.platform", "win32"), \
|
||||
patch("sys.version_info", (3, 10, 0)), \
|
||||
patch("builtins.__import__") as mock_import:
|
||||
mock_import.side_effect = ImportError("No module named 'pywintypes'")
|
||||
patch("core.dependency_validator._exit_with_pywin32_error") as mock_exit:
|
||||
|
||||
# Should not raise SystemExit
|
||||
original_import = builtins.__import__
|
||||
|
||||
def mock_import(name, *args, **kwargs):
|
||||
if name == "pywintypes":
|
||||
raise ImportError("No module named 'pywintypes'")
|
||||
return original_import(name, *args, **kwargs)
|
||||
|
||||
with patch("builtins.__import__", side_effect=mock_import):
|
||||
validate_platform_dependencies()
|
||||
|
||||
# Should have called the error exit function (changed from skipping)
|
||||
mock_exit.assert_called_once()
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# TESTS FOR _exit_with_pywin32_error
|
||||
@@ -150,7 +180,7 @@ class TestExitWithPywin32Error:
|
||||
"""Tests for _exit_with_pywin32_error function."""
|
||||
|
||||
def test_exit_message_contains_helpful_instructions(self):
|
||||
"""Error message should include installation instructions."""
|
||||
"""Error message should include installation instructions and mention MCP library."""
|
||||
with patch("sys.exit") as mock_exit:
|
||||
_exit_with_pywin32_error()
|
||||
|
||||
@@ -163,11 +193,15 @@ class TestExitWithPywin32Error:
|
||||
assert "pip install" in message.lower()
|
||||
assert "windows" in message.lower()
|
||||
assert "python" in message.lower()
|
||||
# Should mention MCP library (ACS-306)
|
||||
assert "mcp" in message.lower()
|
||||
|
||||
def test_exit_message_contains_venv_path(self):
|
||||
"""Error message should include the virtual environment path."""
|
||||
"""Error message should include the virtual environment path when activate script exists."""
|
||||
# Mock existsSync to return True for the activate script path
|
||||
with patch("sys.exit") as mock_exit, \
|
||||
patch("sys.prefix", "/path/to/venv"):
|
||||
patch("sys.prefix", "/path/to/venv"), \
|
||||
patch("pathlib.Path.exists", return_value=True):
|
||||
|
||||
_exit_with_pywin32_error()
|
||||
|
||||
@@ -182,6 +216,29 @@ class TestExitWithPywin32Error:
|
||||
assert expected_path in message or "/path/to/venv" in message
|
||||
assert "Scripts" in message
|
||||
|
||||
def test_exit_message_without_venv_activate(self):
|
||||
"""Error message should not include venv path when activate script doesn't exist."""
|
||||
# Mock existsSync to return False (simulate system Python or missing activate)
|
||||
# Also mock Path.exists to make the test deterministic
|
||||
with patch("sys.exit") as mock_exit, \
|
||||
patch("sys.prefix", "/usr"), \
|
||||
patch("pathlib.Path.exists", return_value=False):
|
||||
|
||||
_exit_with_pywin32_error()
|
||||
|
||||
# Get the message passed to sys.exit
|
||||
call_args = mock_exit.call_args[0][0]
|
||||
message = str(call_args)
|
||||
|
||||
# Should NOT reference Scripts/activate when it doesn't exist
|
||||
# Note: "Scripts" may appear in sys.executable path, so check specifically for activate references
|
||||
assert "Scripts/activate" not in message and "Scripts\\activate" not in message
|
||||
# Also check that "1. Activate your virtual environment" step is not present
|
||||
assert "Activate your virtual environment" not in message
|
||||
# Should still show installation instructions
|
||||
assert "pip install" in message
|
||||
assert "pywin32" in message
|
||||
|
||||
def test_exit_message_contains_python_executable(self):
|
||||
"""Error message should include the current Python executable."""
|
||||
with patch("sys.exit") as mock_exit, \
|
||||
|
||||
Reference in New Issue
Block a user