Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 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
55 changes: 52 additions & 3 deletions apps/backend/core/dependency_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import sys
from pathlib import Path

from core.platform import is_linux, is_windows


def validate_platform_dependencies() -> None:
"""
Expand All @@ -18,14 +20,21 @@ def validate_platform_dependencies() -> None:
with helpful installation instructions.
"""
# Check Windows-specific dependencies
# pywin32 is required on all Python versions on Windows (ACS-306)
# The MCP library unconditionally imports win32api on Windows
if sys.platform == "win32":
if is_windows() and sys.version_info >= (3, 12):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

This change appears to introduce a regression related to pywin32 validation on Windows. The previous implementation (from ACS-306) validated this dependency on all Windows Python versions, as it was deemed critical for the MCP library. This new logic restricts the check to only Python 3.12+, which could lead to runtime errors for users on older Python versions on Windows.

The removed tests (test_windows_python_311_validates_pywin32, test_windows_python_310_validates_pywin32) confirm this was the intended behavior before. If pywin32 is still required for all versions, this condition should be reverted.

Suggested change
if is_windows() and sys.version_info >= (3, 12):
if is_windows():

try:
import pywintypes # noqa: F401
except ImportError:
_exit_with_pywin32_error()

# Check Linux-specific dependencies (ACS-310)
# Note: secretstorage is optional for app functionality (falls back to .env),
# but we validate it to ensure proper OAuth token storage via keyring
if is_linux():
try:
import secretstorage # noqa: F401
except ImportError:
_warn_missing_secretstorage()


def _exit_with_pywin32_error() -> None:
"""Exit with helpful error message for missing pywin32."""
Expand Down Expand Up @@ -76,3 +85,43 @@ def _exit_with_pywin32_error() -> None:
"\n"
f"Current Python: {sys.executable}\n"
)


def _warn_missing_secretstorage() -> None:
"""Emit warning message for missing secretstorage.

Note: This is a warning, not a hard error - the app will fall back to .env
file storage for OAuth tokens. We warn users to ensure they understand the
security implications.
"""
# Use sys.prefix to detect the virtual environment path
venv_activate = Path(sys.prefix) / "bin" / "activate"
# Only include activation instruction if venv script actually exists
activation_prefix = (
f"1. Activate your virtual environment:\n source {venv_activate}\n\n"
if venv_activate.exists()
else ""
)

sys.stderr.write(
"Warning: Linux dependency 'secretstorage' is not installed.\n"
"\n"
"Auto Claude can use secretstorage for secure OAuth token storage via\n"
"the system keyring (gnome-keyring, kwallet, etc.). Without it, tokens\n"
"will be stored in plaintext in your .env file.\n"
"\n"
"To enable keyring integration:\n"
f"{activation_prefix}"
"2. Install secretstorage:\n"
" pip install 'secretstorage>=3.3.3'\n"
"\n"
" Or reinstall all dependencies:\n"
" pip install -r requirements.txt\n"
"\n"
"Note: The app will continue to work, but OAuth tokens will be stored\n"
"in your .env file instead of the system keyring.\n"
"\n"
f"Current Python: {sys.executable}\n"
)
sys.stderr.flush()
# Continue execution - this is a warning, not a blocking error
26 changes: 16 additions & 10 deletions apps/frontend/scripts/download-python.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -710,17 +710,20 @@ async function downloadPython(targetPlatform, targetArch, options = {}) {
// 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)
// 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
// secretstorage is platform-critical for Linux (ACS-310) - required for OAuth token storage
const platformCriticalPackages = {
'win32': ['pywintypes'], // Check for 'pywintypes' instead of 'pywin32' (pywin32 installs top-level modules)
'linux': ['secretstorage'] // Linux OAuth token storage via Freedesktop.org Secret Service
};
const criticalPackages = ['claude_agent_sdk', 'dotenv', 'pydantic_core']
.concat(info.nodePlatform === 'win32' ? ['pywintypes'] : []);
.concat(platformCriticalPackages[info.nodePlatform] || []);
const missingPackages = criticalPackages.filter(pkg => {
const pkgPath = path.join(sitePackagesDir, pkg);
const initFile = path.join(pkgPath, '__init__.py');
const initPath = path.join(pkgPath, '__init__.py');
// 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);
return !(fs.existsSync(pkgPath) && fs.existsSync(initPath)) && !fs.existsSync(moduleFile);
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This block of logic for determining and validating critical packages is duplicated later in the file (around line 825). This duplication makes the code harder to maintain, as any changes need to be applied in two places (as was done in this PR).

To improve maintainability, consider refactoring this logic into a reusable helper function. This function could take sitePackagesDir and nodePlatform as arguments and return the list of missing packages.


if (missingPackages.length > 0) {
Expand Down Expand Up @@ -819,17 +822,20 @@ async function downloadPython(targetPlatform, targetArch, options = {}) {
// 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)
// 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
// secretstorage is platform-critical for Linux (ACS-310) - required for OAuth token storage
const platformCriticalPackages = {
'win32': ['pywintypes'], // Check for 'pywintypes' instead of 'pywin32' (pywin32 installs top-level modules)
'linux': ['secretstorage'] // Linux OAuth token storage via Freedesktop.org Secret Service
};
const criticalPackages = ['claude_agent_sdk', 'dotenv', 'pydantic_core']
.concat(info.nodePlatform === 'win32' ? ['pywintypes'] : []);
.concat(platformCriticalPackages[info.nodePlatform] || []);
const postInstallMissing = criticalPackages.filter(pkg => {
const pkgPath = path.join(sitePackagesDir, pkg);
const initFile = path.join(pkgPath, '__init__.py');
const initPath = path.join(pkgPath, '__init__.py');
// 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);
return !(fs.existsSync(pkgPath) && fs.existsSync(initPath)) && !fs.existsSync(moduleFile);
});

if (postInstallMissing.length > 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,13 @@ describe('Terminal copy/paste integration', () => {
};
});

// Mock requestAnimationFrame for xterm.js integration tests
global.requestAnimationFrame = vi.fn((callback: FrameRequestCallback) => {
// Synchronously execute the callback to avoid timing issues in tests
callback.call(window, 0);
return 0;
}) as unknown as Mock;

// Mock navigator.clipboard
mockClipboard = {
writeText: vi.fn().mockResolvedValue(undefined),
Expand Down
45 changes: 32 additions & 13 deletions apps/frontend/src/main/python-env-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import path from 'path';
import { EventEmitter } from 'events';
import { app } from 'electron';
import { findPythonCommand, getBundledPythonPath } from './python-detector';
import { isWindows } from './platform';
import { isLinux, isWindows } from './platform';

export interface PythonEnvStatus {
ready: boolean;
Expand Down Expand Up @@ -128,31 +128,50 @@ export class PythonEnvManager extends EventEmitter {
// 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)
// 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
// For traditional packages: directory + __init__.py
// For single-file modules (like pywintypes.py): just the .py file
const missingPackages = criticalPackages.filter((pkg) => {
const platformCriticalPackages: Record<string, string[]> = {
win32: ['pywintypes'] // Check for 'pywintypes' instead of 'pywin32' (pywin32 installs top-level modules)
};
// secretstorage is optional for Linux (ACS-310) - nice to have for keyring integration
// but app falls back to .env file storage if missing, so don't block bundled packages
const platformOptionalPackages: Record<string, string[]> = {
linux: ['secretstorage'] // Linux OAuth token storage via Freedesktop.org Secret Service
};

const criticalPackages = [
'claude_agent_sdk',
'dotenv',
'pydantic_core',
...(isWindows() ? platformCriticalPackages.win32 : [])
];
const optionalPackages = isLinux() ? platformOptionalPackages.linux : [];

// Check each package exists with valid structure (directory + __init__.py or single-file module)
const packageExists = (pkg: string): boolean => {
const pkgPath = path.join(sitePackagesPath, pkg);
const initPath = path.join(pkgPath, '__init__.py');
// 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);
});
return (existsSync(pkgPath) && existsSync(initPath)) || existsSync(moduleFile);
};

const missingPackages = criticalPackages.filter((pkg) => !packageExists(pkg));
const missingOptional = optionalPackages.filter((pkg) => !packageExists(pkg));

// Log missing packages for debugging
for (const pkg of missingPackages) {
console.log(
`[PythonEnvManager] Missing critical package: ${pkg} at ${path.join(sitePackagesPath, pkg)}`
);
}
// Log warnings for missing optional packages (non-blocking)
for (const pkg of missingOptional) {
console.warn(
`[PythonEnvManager] Optional package missing: ${pkg} at ${path.join(sitePackagesPath, pkg)}`
);
}

// All packages must exist - don't rely solely on marker file
// All critical packages must exist - don't rely solely on marker file
if (missingPackages.length === 0) {
// Also check marker for logging purposes
const markerPath = path.join(sitePackagesPath, '.bundled');
Expand Down
Loading
Loading