electron-rare/Aperant
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(linux): ensure secretstorage is bundled in Linux binary (ACS-310) (#1211)

Browse Source 
* fix(linux): add secretstorage to platform-critical packages (ACS-310)

Linux binary installations were missing the `secretstorage` package,
causing OAuth token storage via Freedesktop.org Secret Service to fail
silently. The build script cache was created before secretstorage was
added to requirements.txt (Jan 16, 2026), so the package was not being
validated during bundling.

Changes:
- Add platform-aware critical packages validation in download-python.cjs
- Add platform-aware critical packages validation in python-env-manager.ts
- Add Linux secretstorage warning in dependency_validator.py
- Add comprehensive tests for Linux secretstorage validation

The fix follows the same pattern as the Windows pywin32 fix (ACS-306):
- Platform-specific packages are only validated on their target platform
- Linux: secretstorage (OAuth token storage via keyring)
- Windows: pywintypes (MCP library dependency)
- macOS: No platform-specific packages

The Linux validation emits a warning (not blocking error) since the app
gracefully falls back to .env file storage when secretstorage is unavailable.

Refs: ACS-310

* fix(tests): mock pywintypes import in Windows/macOS secretstorage tests

The tests test_windows_skips_secretstorage_validation and
test_macos_skips_secretstorage_validation were failing on Windows CI
because they didn't mock the pywintypes import. When running on
actual Windows in CI, the pywin32 validation runs first and exits
because pywin32 isn't installed in the test environment.

The fix mocks pywintypes to succeed so we can properly test that
secretstorage validation is skipped on non-Linux platforms.

* fix(linux): address CodeRabbit review comments for ACS-310

Changes made based on CodeRabbit AI review:

Backend (dependency_validator.py):
- Rename _exit_with_secretstorage_warning to _warn_missing_secretstorage
  to accurately reflect that it doesn't exit (it only emits a warning)
- Add sys.stderr.flush() to ensure warning is immediately visible

Frontend (download-python.cjs):
- Fix critical __init__.py validation logic - packages are now only considered
  valid if directory+__init__.py exists OR single-file module exists

Frontend (python-env-manager.ts):
- Use platform abstraction (isWindows(), isLinux()) instead of process.platform
- Fix critical packages validation to match download-python.cjs logic

Tests (test_dependency_validator.py):
- Update function name to _warn_missing_secretstorage
- Add is_windows patches to Linux tests to prevent pywin32 validation
  from running on Windows CI

Refs: ACS-310

* fix(tests): mock requestAnimationFrame for xterm integration tests

* style: fix ruff formatting in test_dependency_validator.py

* fix: address CodeRabbit review comments for ACS-310

- Fix venv activation warning to only suggest sourcing activate script
  when it actually exists (not for system Python)
- Move secretstorage from critical to optional packages in frontend
  runtime check to avoid forcing venv creation on Linux (build script
  still validates it as critical)
- Update test_windows_skips_secretstorage_validation to properly
  exercise Windows path by mocking is_windows
- Add test for activation instruction omission when script doesn't exist

Refs: ACS-310

* fix: address Auto Claude PR review feedback (CRITICAL+HIGH+MEDIUM+LOW issues)

CRITICAL: Remove Python 3.12+ version check from Windows pywin32 validation
- pywin32 is required on ALL Python versions on Windows per ACS-306
- MCP library unconditionally imports win32api, not just on Python 3.12+
- Changed from `if is_windows() and sys.version_info >= (3, 12):` to `if is_windows():`

HIGH: Update tests to validate pywin32 on Python 3.10 and 3.11 on Windows
- Replaced `test_windows_python_311_skips_validation` with `test_windows_python_311_validates_pywin32`
- Replaced `test_windows_python_310_skips_validation` with `test_windows_python_310_validates_pywin32`
- Both tests now verify that validation RUNS on these Python versions

MEDIUM: Extract duplicated platformCriticalPackages to single constant
- Created PLATFORM_CRITICAL_PACKAGES constant at module scope in download-python.cjs
- Both validation locations now reference the same constant
- Added clarifying comment about intentional difference from python-env-manager.ts

LOW: Fix step numbering inconsistency in warning message
- When venv activate script doesn't exist, "Install secretstorage:" is shown (no step number)
- When activate script exists, "1. Activate... 2. Install..." is shown
- Matches the pattern used in _exit_with_pywin32_error()

LOW: Update comments to clarify build vs runtime package classification
- download-python.cjs treats secretstorage as critical (must be bundled)
- python-env-manager.ts treats secretstorage as optional (logs warning, doesn't block)
- Comments now explain this intentional difference

Refs: ACS-310, ACS-306

* fix(tests): mock is_windows/is_linux directly in graphiti import test

Fix Windows CI test failure by properly mocking platform detection functions
instead of just sys.platform. The test_validate_platform_dependencies_does_not_import_graphiti
test now patches core.dependency_validator.is_windows/is_linux to avoid pywin32
import issues on Windows CI.

Refs: ACS-310, ACS-253

* fix(tests): fix flawed assertion logic in activation omission test

Replace the faulty 'or' assertion with a proper check using all() to verify
that no line contains the 'source' substring when activation script doesn't exist.
The previous logic 'assert "source" not in message or "source" not in message.split("\n")'
was logically incorrect and could produce false positives.

Addressed CodeRabbit review comment.

Refs: ACS-310

* test: add assertion to verify warning not called when secretstorage installed

Update test_linux_with_secretstorage_installed_continues to patch and assert
that _warn_missing_secretstorage is NOT called when secretstorage is installed.
This ensures the test properly verifies that no warning is emitted in the
success case, matching the pattern used in other tests in this class.

Addressed CodeRabbit refactor suggestion.

Refs: ACS-310

---------

Co-authored-by: StillKnotKnown <stillknotknown@users.noreply.github.com>
This commit is contained in:
StillKnotKnown
2026-01-16 23:34:59 +02:00
committed by GitHub
parent ba7358afd2
commit 48bd4a9c07
5 changed files with 487 additions and 122 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/backend/core/dependency_validator.py
+60 -4
View File
@@ -8,6 +8,8 @@ Validates platform-specific dependencies are installed before running agents.
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
apps/frontend/scripts/download-python.cjs
+20 -14
View File
@@ -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) {
apps/frontend/src/__tests__/integration/terminal-copy-paste.test.ts
+7
View File
@@ -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),
apps/frontend/src/main/python-env-manager.ts
+31 -12
View File
@@ -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'] : []);
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
};
// 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 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');
tests/test_dependency_validator.py
+369 -92
View File
@@ -18,8 +18,11 @@ import pytest
# 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 @@ class TestValidatePlatformDependencies:
"""
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 @@ class TestValidatePlatformDependencies:
"""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("builtins.__import__", side_effect=mock_import):
validate_platform_dependencies()
# Should have called the error exit function (changed from skipping)
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()
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'")
# Should not raise SystemExit
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:
def test_linux_skips_pywin32_validation(self):
"""Linux should skip pywin32 validation but warn about secretstorage."""
import builtins
original_import = builtins.__import__
def mock_import(name, *args, **kwargs):
if name == "secretstorage":
raise ImportError("No module named 'secretstorage'")
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("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()
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).
"""Windows + Python 3.10 should validate pywin32 (ACS-306)."""
import builtins
Previously, validation only happened on Python 3.12+, but the MCP
library requires pywin32 on all Python versions on Windows.
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):
"""
Linux without secretstorage should warn but not exit (ACS-310).
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 @@ class TestExitWithPywin32Error:
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 @@ class TestExitWithPywin32Error:
"""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 @@ class TestExitWithPywin32Error:
# 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 @@ class TestExitWithPywin32Error:
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 @@ class TestImportOrderPreventsEarlyFailure:
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 @@ class TestImportOrderPreventsEarlyFailure:
# 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 @@ class TestImportOrderPreventsEarlyFailure:
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 @@ class TestImportOrderPreventsEarlyFailure:
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 @@ class TestImportOrderPreventsEarlyFailure:
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 @@ class TestCliUtilsGetProjectDir:
# Change to backend directory
import os
original_cwd = os.getcwd()
try:
os.chdir(backend_dir)