diff --git a/apps/backend/core/dependency_validator.py b/apps/backend/core/dependency_validator.py index fdad64a759..015a4d907c 100644 --- a/apps/backend/core/dependency_validator.py +++ b/apps/backend/core/dependency_validator.py @@ -8,6 +8,8 @@ import sys from pathlib import Path +from core.platform import is_linux, is_windows + def validate_platform_dependencies() -> None: """ @@ -17,15 +19,23 @@ def validate_platform_dependencies() -> None: SystemExit: If required platform-specific dependencies are missing, 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": + # Check Windows-specific dependencies (all Python versions per ACS-306) + # pywin32 is required on all Python versions on Windows - MCP library unconditionally imports win32api + 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.""" @@ -76,3 +86,49 @@ 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 "" + ) + # Adjust step number based on whether activation step is included + install_step = ( + "2. Install secretstorage:\n" + if activation_prefix + else "Install secretstorage:\n" + ) + + 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}" + f"{install_step}" + " 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 diff --git a/apps/frontend/scripts/download-python.cjs b/apps/frontend/scripts/download-python.cjs index e626f3e753..6f18bf44ad 100644 --- a/apps/frontend/scripts/download-python.cjs +++ b/apps/frontend/scripts/download-python.cjs @@ -145,6 +145,16 @@ const CHECKSUMS = { 'linux-arm64': 'fb983ec85952513f5f013674fcbf4306b1a142c50fcfd914c2c3f00c61a874b0', }; +// Platform-specific critical packages that must be bundled +// pywin32 is platform-critical for Windows (ACS-306) - required by MCP library +// secretstorage is platform-critical for Linux (ACS-310) - required for OAuth token storage +// NOTE: python-env-manager.ts treats secretstorage as optional (falls back to .env) +// while this script validates it during build to ensure it's bundled +const PLATFORM_CRITICAL_PACKAGES = { + 'win32': ['pywintypes'], // Check for 'pywintypes' instead of 'pywin32' (pywin32 installs top-level modules) + 'linux': ['secretstorage'] // Linux OAuth token storage via Freedesktop.org Secret Service +}; + // Map Node.js platform names to electron-builder platform names function toElectronBuilderPlatform(nodePlatform) { const map = { @@ -707,20 +717,18 @@ async function downloadPython(targetPlatform, targetArch, options = {}) { // Verify critical packages exist (fixes GitHub issue #416) // 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) - // 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 + // NOTE: python-env-manager.ts treats secretstorage as optional (falls back to .env) + // while this script validates it during build to ensure it's bundled const criticalPackages = ['claude_agent_sdk', 'dotenv', 'pydantic_core'] - .concat(info.nodePlatform === 'win32' ? ['pywintypes'] : []); + .concat(PLATFORM_CRITICAL_PACKAGES[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); }); if (missingPackages.length > 0) { @@ -816,20 +824,18 @@ async function downloadPython(targetPlatform, targetArch, options = {}) { installPackages(pythonBin, requirementsPath, sitePackagesDir); // 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) - // 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 + // NOTE: python-env-manager.ts treats secretstorage as optional (falls back to .env) + // while this script validates it during build to ensure it's bundled const criticalPackages = ['claude_agent_sdk', 'dotenv', 'pydantic_core'] - .concat(info.nodePlatform === 'win32' ? ['pywintypes'] : []); + .concat(PLATFORM_CRITICAL_PACKAGES[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) { diff --git a/apps/frontend/src/__tests__/integration/terminal-copy-paste.test.ts b/apps/frontend/src/__tests__/integration/terminal-copy-paste.test.ts index ea4cec57d3..e8cb7faef6 100644 --- a/apps/frontend/src/__tests__/integration/terminal-copy-paste.test.ts +++ b/apps/frontend/src/__tests__/integration/terminal-copy-paste.test.ts @@ -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), diff --git a/apps/frontend/src/main/python-env-manager.ts b/apps/frontend/src/main/python-env-manager.ts index 510dbf566c..84c8e77cac 100644 --- a/apps/frontend/src/main/python-env-manager.ts +++ b/apps/frontend/src/main/python-env-manager.ts @@ -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; @@ -128,22 +128,35 @@ 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 = { + 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 = { + 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) { @@ -151,8 +164,14 @@ export class PythonEnvManager extends EventEmitter { `[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'); diff --git a/tests/test_dependency_validator.py b/tests/test_dependency_validator.py index b857794bfe..5b0661c419 100644 --- a/tests/test_dependency_validator.py +++ b/tests/test_dependency_validator.py @@ -18,8 +18,11 @@ # Add apps/backend directory to path for imports sys.path.insert(0, str(Path(__file__).parent.parent / "apps" / "backend")) -from core.dependency_validator import validate_platform_dependencies, _exit_with_pywin32_error - +from core.dependency_validator import ( + _exit_with_pywin32_error, + _warn_missing_secretstorage, + validate_platform_dependencies, +) # ============================================================================= # TESTS FOR validate_platform_dependencies @@ -38,23 +41,31 @@ def test_windows_python_312_with_pywin32_missing_exits(self): """ import builtins - with patch("sys.platform", "win32"), \ - patch("sys.version_info", (3, 12, 0)), \ - patch("core.dependency_validator._exit_with_pywin32_error") as mock_exit: - + with ( + patch("core.dependency_validator.is_windows", return_value=True), + patch("core.dependency_validator.is_linux", return_value=False), + patch("sys.version_info", (3, 12, 0)), + patch("core.dependency_validator._exit_with_pywin32_error") as mock_exit, + patch( + "core.dependency_validator._warn_missing_secretstorage" + ) as mock_warning, + ): # Mock pywintypes import to raise ImportError original_import = builtins.__import__ def mock_import(name, *args, **kwargs): if name == "pywintypes": raise ImportError("No module named 'pywintypes'") + if name == "secretstorage": + raise ImportError("No module named 'secretstorage'") return original_import(name, *args, **kwargs) with patch("builtins.__import__", side_effect=mock_import): validate_platform_dependencies() - # Should have called the error exit function + # Should have called the error exit function (not warning) mock_exit.assert_called_once() + mock_warning.assert_not_called() def test_windows_python_312_with_pywin32_installed_continues(self): """Windows + Python 3.12+ with pywin32 installed should continue.""" @@ -67,108 +78,347 @@ def selective_mock(name, *args, **kwargs): """Return mock for pywintypes, delegate everything else to original.""" if name == "pywintypes": return MagicMock() + if name == "secretstorage": + raise ImportError("No module named 'secretstorage'") return original_import(name, *args, **kwargs) - with patch("sys.platform", "win32"), \ - patch("sys.version_info", (3, 12, 0)), \ - patch("builtins.__import__", side_effect=selective_mock): + with ( + patch("core.dependency_validator.is_windows", return_value=True), + patch("core.dependency_validator.is_linux", return_value=False), + patch("sys.version_info", (3, 12, 0)), + patch( + "core.dependency_validator._warn_missing_secretstorage" + ) as mock_warning, + patch("builtins.__import__", side_effect=selective_mock), + ): # Should not raise SystemExit validate_platform_dependencies() + # Linux warning should not be called on Windows + mock_warning.assert_not_called() 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. - """ + """Windows + Python 3.11 should validate pywin32 (ACS-306).""" import builtins - with patch("sys.platform", "win32"), \ - patch("sys.version_info", (3, 11, 0)), \ - patch("core.dependency_validator._exit_with_pywin32_error") as mock_exit: + original_import = builtins.__import__ - 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) - def mock_import(name, *args, **kwargs): - if name == "pywintypes": - raise ImportError("No module named 'pywintypes'") - return original_import(name, *args, **kwargs) + with ( + patch("core.dependency_validator.is_windows", return_value=True), + patch("core.dependency_validator.is_linux", return_value=False), + patch("sys.version_info", (3, 11, 0)), + patch("core.dependency_validator._exit_with_pywin32_error") as mock_exit, + patch( + "core.dependency_validator._warn_missing_secretstorage" + ) as mock_warning, + patch("builtins.__import__", side_effect=mock_import), + ): + # Should call exit error function + validate_platform_dependencies() + mock_exit.assert_called_once() + # Linux warning should not be called on Windows + mock_warning.assert_not_called() - with patch("builtins.__import__", side_effect=mock_import): - validate_platform_dependencies() + def test_linux_skips_pywin32_validation(self): + """Linux should skip pywin32 validation but warn about secretstorage.""" + import builtins - # Should have called the error exit function (changed from skipping) - mock_exit.assert_called_once() + original_import = builtins.__import__ - def test_linux_skips_validation(self): - """Non-Windows platforms should skip pywin32 validation.""" - with patch("sys.platform", "linux"), \ - patch("sys.version_info", (3, 12, 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'") + def mock_import(name, *args, **kwargs): + if name == "secretstorage": + raise ImportError("No module named 'secretstorage'") + return original_import(name, *args, **kwargs) - # Should not raise SystemExit + with ( + patch("core.dependency_validator.is_windows", return_value=False), + patch("core.dependency_validator.is_linux", return_value=True), + patch("sys.version_info", (3, 12, 0)), + patch( + "core.dependency_validator._warn_missing_secretstorage" + ) as mock_warning, + patch("builtins.__import__", side_effect=mock_import), + ): + # Should not call pywin32 error, but should call secretstorage warning validate_platform_dependencies() - - def test_macos_skips_validation(self): - """macOS should skip pywin32 validation.""" - with patch("sys.platform", "darwin"), \ - patch("sys.version_info", (3, 12, 0)), \ - patch("builtins.__import__") as mock_import: + mock_warning.assert_called_once() + + def test_macos_skips_pywin32_validation(self): + """macOS should skip pywin32 validation and secretstorage warning.""" + with ( + patch("core.dependency_validator.is_windows", return_value=False), + patch("core.dependency_validator.is_linux", return_value=False), + patch("sys.version_info", (3, 12, 0)), + patch( + "core.dependency_validator._warn_missing_secretstorage" + ) as mock_warning, + patch("builtins.__import__") as mock_import, + ): # Even if pywintypes is not available, should not exit mock_import.side_effect = ImportError("No module named 'pywintypes'") # Should not raise SystemExit validate_platform_dependencies() + # Linux warning should not be called on macOS + mock_warning.assert_not_called() def test_windows_python_313_validates(self): """Windows + Python 3.13+ should validate pywin32.""" import builtins - with patch("sys.platform", "win32"), \ - patch("sys.version_info", (3, 13, 0)), \ - patch("core.dependency_validator._exit_with_pywin32_error") as mock_exit: - + with ( + patch("core.dependency_validator.is_windows", return_value=True), + patch("core.dependency_validator.is_linux", return_value=False), + patch("sys.version_info", (3, 13, 0)), + patch("core.dependency_validator._exit_with_pywin32_error") as mock_exit, + patch( + "core.dependency_validator._warn_missing_secretstorage" + ) as mock_warning, + ): original_import = builtins.__import__ def mock_import(name, *args, **kwargs): if name == "pywintypes": raise ImportError("No module named 'pywintypes'") + if name == "secretstorage": + raise ImportError("No module named 'secretstorage'") return original_import(name, *args, **kwargs) with patch("builtins.__import__", side_effect=mock_import): validate_platform_dependencies() - # Should have called the error exit function + # Should have called the error exit function (not warning) mock_exit.assert_called_once() + mock_warning.assert_not_called() def test_windows_python_310_validates_pywin32(self): + """Windows + Python 3.10 should validate pywin32 (ACS-306).""" + import builtins + + 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("core.dependency_validator.is_windows", return_value=True), + patch("core.dependency_validator.is_linux", return_value=False), + patch("sys.version_info", (3, 10, 0)), + patch("core.dependency_validator._exit_with_pywin32_error") as mock_exit, + patch( + "core.dependency_validator._warn_missing_secretstorage" + ) as mock_warning, + patch("builtins.__import__", side_effect=mock_import), + ): + # Should call exit error function + validate_platform_dependencies() + mock_exit.assert_called_once() + # Linux warning should not be called on Windows + mock_warning.assert_not_called() + + +# ============================================================================= +# TESTS FOR Linux secretstorage validation (ACS-310) +# ============================================================================= + + +class TestLinuxSecretstorageValidation: + """Tests for Linux secretstorage dependency validation (ACS-310).""" + + def test_linux_with_secretstorage_missing_warns(self): """ - Windows + Python 3.10 should validate pywin32 (ACS-306). + Linux without secretstorage should warn but not exit (ACS-310). - Previously, validation only happened on Python 3.12+, but the MCP - library requires pywin32 on all Python versions on Windows. + Unlike Windows pywin32 which is required, secretstorage is optional + and falls back to .env file storage. The warning informs users about + the security implications. """ import builtins - with patch("sys.platform", "win32"), \ - patch("sys.version_info", (3, 10, 0)), \ - patch("core.dependency_validator._exit_with_pywin32_error") as mock_exit: - + with ( + patch("core.dependency_validator.is_windows", return_value=False), + patch("core.dependency_validator.is_linux", return_value=True), + patch( + "core.dependency_validator._warn_missing_secretstorage" + ) as mock_warning, + ): original_import = builtins.__import__ def mock_import(name, *args, **kwargs): - if name == "pywintypes": - raise ImportError("No module named 'pywintypes'") + if name == "secretstorage": + raise ImportError("No module named 'secretstorage'") 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() + # Should have called the warning function + mock_warning.assert_called_once() + + def test_linux_with_secretstorage_installed_continues(self): + """Linux with secretstorage installed should continue without warning.""" + import builtins + + original_import = builtins.__import__ + + def selective_mock(name, *args, **kwargs): + """Return mock for secretstorage, delegate everything else to original.""" + if name == "secretstorage": + return MagicMock() + return original_import(name, *args, **kwargs) + + with ( + patch("core.dependency_validator.is_windows", return_value=False), + patch("core.dependency_validator.is_linux", return_value=True), + patch("builtins.__import__", side_effect=selective_mock), + patch( + "core.dependency_validator._warn_missing_secretstorage" + ) as mock_warning, + ): + # Should not call warning function when secretstorage is installed + validate_platform_dependencies() + mock_warning.assert_not_called() + + def test_windows_skips_secretstorage_validation(self): + """Windows should skip secretstorage validation.""" + import builtins + + original_import = builtins.__import__ + + def mock_import(name, *args, **kwargs): + # Allow pywintypes to succeed (Windows validation passes) + if name == "pywintypes": + return MagicMock() + # secretstorage import fails (but should be skipped on Windows) + if name == "secretstorage": + raise ImportError("No module named 'secretstorage'") + return original_import(name, *args, **kwargs) + + with ( + patch("core.dependency_validator.is_windows", return_value=True), + patch("core.dependency_validator.is_linux", return_value=False), + patch("sys.version_info", (3, 12, 0)), + patch( + "core.dependency_validator._warn_missing_secretstorage" + ) as mock_warning, + patch("builtins.__import__", side_effect=mock_import), + ): + # Should not call warning function + validate_platform_dependencies() + mock_warning.assert_not_called() + + def test_macos_skips_secretstorage_validation(self): + """macOS should skip secretstorage validation.""" + import builtins + + original_import = builtins.__import__ + + def mock_import(name, *args, **kwargs): + # All platform-specific imports fail (macOS has none required) + if name in ("pywintypes", "secretstorage"): + raise ImportError(f"No module named '{name}'") + return original_import(name, *args, **kwargs) + + with ( + patch("core.dependency_validator.is_linux", return_value=False), + patch("core.dependency_validator.is_windows", return_value=False), + patch( + "core.dependency_validator._warn_missing_secretstorage" + ) as mock_warning, + patch("builtins.__import__", side_effect=mock_import), + ): + # Should not call warning function + validate_platform_dependencies() + mock_warning.assert_not_called() + + +# ============================================================================= +# TESTS FOR _warn_missing_secretstorage (ACS-310) +# ============================================================================= + + +class TestExitWithSecretstorageWarning: + """Tests for _warn_missing_secretstorage function (ACS-310).""" + + def test_warning_message_contains_helpful_instructions(self, capsys): + """Warning message should include installation instructions.""" + _warn_missing_secretstorage() + + # Get stderr output + captured = capsys.readouterr() + message = captured.err + + # Verify helpful content + assert "secretstorage" in message.lower() + assert "pip install" in message.lower() + assert "linux" in message.lower() + assert "keyring" in message.lower() + + def test_warning_message_mentions_fallback_behavior(self, capsys): + """Warning should explain that app continues with .env fallback.""" + _warn_missing_secretstorage() + + captured = capsys.readouterr() + message = captured.err + + # Should mention the fallback behavior + assert ".env" in message + assert "continue" in message.lower() + + def test_warning_message_contains_venv_path(self, capsys, tmp_path): + """Warning message should include the virtual environment path when activate script exists.""" + # Create a temporary venv-like structure with activate script + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + activate_script = bin_dir / "activate" + activate_script.write_text("#!/bin/bash\n") + + with patch("sys.prefix", str(tmp_path)): + _warn_missing_secretstorage() + + captured = capsys.readouterr() + message = captured.err + + # Should reference the full venv bin/activate path since it exists + assert str(tmp_path) in message + assert "bin" in message + assert "activate" in message + + def test_warning_message_omits_activation_when_no_script(self, capsys, tmp_path): + """Warning message should omit activation instruction when activate script doesn't exist.""" + # Use tmp_path without creating bin/activate script + with patch("sys.prefix", str(tmp_path)): + _warn_missing_secretstorage() + + captured = capsys.readouterr() + message = captured.err + + # Should NOT include activation instruction since activate script doesn't exist + assert "Activate your virtual environment" not in message + # Verify no line contains "source" (the activation command hint) + # Using all() ensures we check every line, not just the message as a whole + assert all(line.find("source") == -1 for line in message.splitlines()) + # Should still have the install instructions + assert "Install secretstorage" in message + + def test_warning_does_not_exit(self, capsys): + """Warning function should write to stderr but not exit.""" + # This function should NOT call sys.exit + with patch("sys.exit") as mock_exit: + _warn_missing_secretstorage() + + # Should NOT have called sys.exit + mock_exit.assert_not_called() + + # But should have written to stderr + captured = capsys.readouterr() + assert len(captured.err) > 0 # ============================================================================= @@ -199,10 +449,11 @@ def test_exit_message_contains_helpful_instructions(self): def test_exit_message_contains_venv_path(self): """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("pathlib.Path.exists", return_value=True): - + with ( + patch("sys.exit") as mock_exit, + patch("sys.prefix", "/path/to/venv"), + patch("pathlib.Path.exists", return_value=True), + ): _exit_with_pywin32_error() # Get the message passed to sys.exit @@ -220,10 +471,11 @@ 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): - + 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 @@ -232,7 +484,9 @@ def test_exit_message_without_venv_activate(self): # 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 + 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 @@ -241,9 +495,10 @@ def test_exit_message_without_venv_activate(self): def test_exit_message_contains_python_executable(self): """Error message should include the current Python executable.""" - with patch("sys.exit") as mock_exit, \ - patch("sys.executable", "/usr/bin/python3.12"): - + with ( + patch("sys.exit") as mock_exit, + patch("sys.executable", "/usr/bin/python3.12"), + ): _exit_with_pywin32_error() # Get the message passed to sys.exit @@ -283,11 +538,13 @@ def tracking_import(name, *args, **kwargs): imported_modules.add(name) return original_import(name, *args, **kwargs) - # Use non-Windows to avoid import issues - with patch("builtins.__import__", side_effect=tracking_import), \ - patch("sys.platform", "linux"), \ - patch("sys.version_info", (3, 11, 0)): - + # Use non-Windows platform to avoid pywin32 import issues on Windows CI + with ( + patch("builtins.__import__", side_effect=tracking_import), + patch("core.dependency_validator.is_windows", return_value=False), + patch("core.dependency_validator.is_linux", return_value=True), + patch("sys.version_info", (3, 11, 0)), + ): validate_platform_dependencies() # Verify graphiti-related modules were NOT imported @@ -327,20 +584,26 @@ def test_cli_utils_lazy_import_of_graphiti_config(self): # Skip async functions and classes, find first regular function continue - assert first_function_lineno is not None, "Could not find first function in cli/utils.py" + assert first_function_lineno is not None, ( + "Could not find first function in cli/utils.py" + ) # Check module-level imports (before the first function) lines = utils_content.split("\n") module_level_imports = "\n".join(lines[:first_function_lineno]) - assert "from graphiti_config import" not in module_level_imports, \ + assert "from graphiti_config import" not in module_level_imports, ( "graphiti_config should not be imported at module level in cli/utils.py" + ) # Verify that graphiti_config IS imported inside validate_environment() validate_env_lineno = None validate_env_end_lineno = len(lines) # Initialize to end of file for node in tree.body: - if isinstance(node, ast.FunctionDef) and node.name == "validate_environment": + if ( + isinstance(node, ast.FunctionDef) + and node.name == "validate_environment" + ): validate_env_lineno = node.lineno # Find the end of the function (next top-level node or end of file) node_index = tree.body.index(node) @@ -349,12 +612,17 @@ def test_cli_utils_lazy_import_of_graphiti_config(self): validate_env_end_lineno = next_node.lineno break - assert validate_env_lineno is not None, "Could not find validate_environment function" + assert validate_env_lineno is not None, ( + "Could not find validate_environment function" + ) # Look for the import within the function's body - validate_env_block = "\n".join(lines[validate_env_lineno - 1:validate_env_end_lineno]) - assert "from graphiti_config import get_graphiti_status" in validate_env_block, \ - "graphiti_config should be imported inside validate_environment()" + validate_env_block = "\n".join( + lines[validate_env_lineno - 1 : validate_env_end_lineno] + ) + assert ( + "from graphiti_config import get_graphiti_status" in validate_env_block + ), "graphiti_config should be imported inside validate_environment()" def test_entry_points_validate_before_cli_imports(self): """ @@ -369,8 +637,9 @@ def test_entry_points_validate_before_cli_imports(self): run_content = run_py.read_text() # Verify validate_platform_dependencies is imported and called - assert "validate_platform_dependencies" in run_content, \ + assert "validate_platform_dependencies" in run_content, ( "run.py should import validate_platform_dependencies" + ) # Find the position of validation call and cli import validation_pos = run_content.find("validate_platform_dependencies()") @@ -378,24 +647,31 @@ def test_entry_points_validate_before_cli_imports(self): assert validation_pos > 0, "run.py should call validate_platform_dependencies" assert cli_import_pos > 0, "run.py should import cli.main" - assert validation_pos < cli_import_pos, \ + assert validation_pos < cli_import_pos, ( "run.py should validate dependencies BEFORE importing cli.main" + ) # Check spec_runner.py spec_runner_py = backend_dir / "runners" / "spec_runner.py" spec_runner_content = spec_runner_py.read_text() - assert "validate_platform_dependencies" in spec_runner_content, \ + assert "validate_platform_dependencies" in spec_runner_content, ( "spec_runner.py should import validate_platform_dependencies" + ) # Find positions - validation_pos_spec = spec_runner_content.find("validate_platform_dependencies()") + validation_pos_spec = spec_runner_content.find( + "validate_platform_dependencies()" + ) cli_utils_import_pos = spec_runner_content.find("from cli.utils import") - assert validation_pos_spec > 0, "spec_runner.py should call validate_platform_dependencies" + assert validation_pos_spec > 0, ( + "spec_runner.py should call validate_platform_dependencies" + ) assert cli_utils_import_pos > 0, "spec_runner.py should import cli.utils" - assert validation_pos_spec < cli_utils_import_pos, \ + assert validation_pos_spec < cli_utils_import_pos, ( "spec_runner.py should validate dependencies BEFORE importing cli.utils" + ) # ============================================================================= @@ -476,6 +752,7 @@ def test_get_project_dir_auto_detects_backend(self, temp_dir): # Change to backend directory import os + original_cwd = os.getcwd() try: os.chdir(backend_dir)