diff --git a/apps/frontend/src/__tests__/integration/subprocess-spawn.test.ts b/apps/frontend/src/__tests__/integration/subprocess-spawn.test.ts index cf03c5136e..e73f60ea54 100644 --- a/apps/frontend/src/__tests__/integration/subprocess-spawn.test.ts +++ b/apps/frontend/src/__tests__/integration/subprocess-spawn.test.ts @@ -58,6 +58,16 @@ vi.mock('../../main/python-detector', async (importOriginal) => { }; }); +// Mock python-env-manager for ensurePythonEnvReady (ACS-254) +vi.mock('../../main/python-env-manager', () => ({ + pythonEnvManager: { + isEnvReady: vi.fn(() => true), + initialize: vi.fn(() => Promise.resolve({ ready: true })), + getPythonEnv: vi.fn(() => ({})) + }, + getConfiguredPythonPath: vi.fn(() => DETECTED_PYTHON_CMD) +})); + // Auto-claude source path (for getAutoBuildSourcePath to find) const AUTO_CLAUDE_SOURCE = path.join(TEST_DIR, 'auto-claude-source'); diff --git a/apps/frontend/src/main/agent/agent-manager.ts b/apps/frontend/src/main/agent/agent-manager.ts index 400ba21cdb..7ce8790954 100644 --- a/apps/frontend/src/main/agent/agent-manager.ts +++ b/apps/frontend/src/main/agent/agent-manager.ts @@ -110,6 +110,13 @@ export class AgentManager extends EventEmitter { return; } + // Ensure Python environment is ready before spawning process (prevents exit code 127 race condition) + const pythonStatus = await this.processManager.ensurePythonEnvReady('AgentManager'); + if (!pythonStatus.ready) { + this.emit('error', taskId, `Python environment not ready: ${pythonStatus.error || 'initialization failed'}`); + return; + } + const autoBuildSource = this.processManager.getAutoBuildSourcePath(); if (!autoBuildSource) { @@ -196,6 +203,13 @@ export class AgentManager extends EventEmitter { return; } + // Ensure Python environment is ready before spawning process (prevents exit code 127 race condition) + const pythonStatus = await this.processManager.ensurePythonEnvReady('AgentManager'); + if (!pythonStatus.ready) { + this.emit('error', taskId, `Python environment not ready: ${pythonStatus.error || 'initialization failed'}`); + return; + } + const autoBuildSource = this.processManager.getAutoBuildSourcePath(); if (!autoBuildSource) { @@ -250,6 +264,13 @@ export class AgentManager extends EventEmitter { projectPath: string, specId: string ): Promise { + // Ensure Python environment is ready before spawning process (prevents exit code 127 race condition) + const pythonStatus = await this.processManager.ensurePythonEnvReady('AgentManager'); + if (!pythonStatus.ready) { + this.emit('error', taskId, `Python environment not ready: ${pythonStatus.error || 'initialization failed'}`); + return; + } + const autoBuildSource = this.processManager.getAutoBuildSourcePath(); if (!autoBuildSource) { diff --git a/apps/frontend/src/main/agent/agent-process.test.ts b/apps/frontend/src/main/agent/agent-process.test.ts index c06b8f6824..b6897acd95 100644 --- a/apps/frontend/src/main/agent/agent-process.test.ts +++ b/apps/frontend/src/main/agent/agent-process.test.ts @@ -95,18 +95,46 @@ vi.mock('../python-detector', () => ({ parsePythonCommand: vi.fn(() => ['python', []]) })); +// Mock python-env-manager for ensurePythonEnvReady tests +vi.mock('../python-env-manager', () => ({ + pythonEnvManager: { + isEnvReady: vi.fn(() => true), + initialize: vi.fn(() => Promise.resolve({ ready: true })), + getPythonEnv: vi.fn(() => ({})) + }, + getConfiguredPythonPath: vi.fn(() => 'python3') +})); + vi.mock('electron', () => ({ app: { getAppPath: vi.fn(() => '/fake/app/path') } })); +// Mock fs.existsSync for getAutoBuildSourcePath path validation +vi.mock('fs', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + existsSync: vi.fn((path: string) => { + // Return true for the fake auto-build path and its expected files + if (path === '/fake/auto-build' || + path === '/fake/auto-build/runners' || + path === '/fake/auto-build/runners/spec_runner.py') { + return true; + } + return false; + }) + }; +}); + // Import AFTER all mocks are set up import { AgentProcessManager } from './agent-process'; import { AgentState } from './agent-state'; import { AgentEvents } from './agent-events'; import * as profileService from '../services/profile'; import * as rateLimitDetector from '../rate-limit-detector'; +import { pythonEnvManager } from '../python-env-manager'; describe('AgentProcessManager - API Profile Env Injection (Story 2.3)', () => { let processManager: AgentProcessManager; @@ -491,4 +519,105 @@ describe('AgentProcessManager - API Profile Env Injection (Story 2.3)', () => { expect(envArg.ANTHROPIC_BASE_URL).toBe(''); }); }); + + describe('ensurePythonEnvReady - Python Environment Readiness (ACS-254)', () => { + let testProcessManager: AgentProcessManager; + + beforeEach(() => { + // Reset all mocks + vi.clearAllMocks(); + spawnCalls.length = 0; + + // Create fresh process manager for these tests + state = new AgentState(); + events = new AgentEvents(); + emitter = new EventEmitter(); + testProcessManager = new AgentProcessManager(state, events, emitter); + }); + + it('should return ready: true when Python environment is already ready', async () => { + vi.mocked(pythonEnvManager.isEnvReady).mockReturnValue(true); + + // Configure with valid autoBuildSource + testProcessManager.configure(undefined, '/fake/auto-build'); + + const result = await testProcessManager.ensurePythonEnvReady('TestContext'); + + expect(result.ready).toBe(true); + expect(result.error).toBeUndefined(); + expect(pythonEnvManager.initialize).not.toHaveBeenCalled(); + }); + + it('should initialize Python environment when not ready', async () => { + vi.mocked(pythonEnvManager.isEnvReady).mockReturnValue(false); + vi.mocked(pythonEnvManager.initialize).mockResolvedValue({ + ready: true, + pythonPath: '/fake/python', + sitePackagesPath: '/fake/site-packages', + venvExists: true, + depsInstalled: true, + usingBundledPackages: false + }); + + testProcessManager.configure(undefined, '/fake/auto-build'); + + const result = await testProcessManager.ensurePythonEnvReady('TestContext'); + + expect(result.ready).toBe(true); + expect(result.error).toBeUndefined(); + expect(pythonEnvManager.initialize).toHaveBeenCalledWith('/fake/auto-build'); + }); + + it('should return error when autoBuildSource is not found', async () => { + vi.mocked(pythonEnvManager.isEnvReady).mockReturnValue(false); + + // Don't configure - autoBuildSource will be null + const result = await testProcessManager.ensurePythonEnvReady('TestContext'); + + expect(result.ready).toBe(false); + expect(result.error).toBe('auto-build source not found'); + expect(pythonEnvManager.initialize).not.toHaveBeenCalled(); + }); + + it('should return error when Python initialization fails', async () => { + vi.mocked(pythonEnvManager.isEnvReady).mockReturnValue(false); + vi.mocked(pythonEnvManager.initialize).mockResolvedValue({ + ready: false, + pythonPath: null, + sitePackagesPath: null, + venvExists: false, + depsInstalled: false, + usingBundledPackages: false, + error: 'Failed to create venv: permission denied' + }); + + testProcessManager.configure(undefined, '/fake/auto-build'); + + const result = await testProcessManager.ensurePythonEnvReady('TestContext'); + + expect(result.ready).toBe(false); + expect(result.error).toBe('Failed to create venv: permission denied'); + }); + + it('should return error when Python initialization fails without message', async () => { + vi.mocked(pythonEnvManager.isEnvReady).mockReturnValue(false); + vi.mocked(pythonEnvManager.initialize).mockResolvedValue({ + ready: false, + pythonPath: null, + sitePackagesPath: null, + venvExists: false, + depsInstalled: false, + usingBundledPackages: false + // No error field + }); + + testProcessManager.configure(undefined, '/fake/auto-build'); + + const result = await testProcessManager.ensurePythonEnvReady('TestContext'); + + expect(result.ready).toBe(false); + expect(result.error).toBe('initialization failed'); + expect(pythonEnvManager.initialize).toHaveBeenCalledWith('/fake/auto-build'); + }); + }); }); diff --git a/apps/frontend/src/main/agent/agent-process.ts b/apps/frontend/src/main/agent/agent-process.ts index 6291912a4e..b94488322d 100644 --- a/apps/frontend/src/main/agent/agent-process.ts +++ b/apps/frontend/src/main/agent/agent-process.ts @@ -301,6 +301,38 @@ export class AgentProcessManager { return null; } + /** + * Ensure Python environment is ready before spawning processes. + * This is a shared method used by AgentManager and AgentQueueManager + * to prevent race conditions where tasks start before venv initialization completes. + * + * @param context - Context identifier for logging (e.g., 'AgentManager', 'AgentQueue') + * @returns Object with ready status and optional error message + */ + async ensurePythonEnvReady(context: string): Promise<{ ready: boolean; error?: string }> { + if (pythonEnvManager.isEnvReady()) { + return { ready: true }; + } + + console.log(`[${context}] Python environment not ready, waiting for initialization...`); + + const autoBuildSource = this.getAutoBuildSourcePath(); + if (!autoBuildSource) { + const error = 'auto-build source not found'; + console.error(`[${context}] Cannot initialize Python - ${error}`); + return { ready: false, error }; + } + + const status = await pythonEnvManager.initialize(autoBuildSource); + if (!status.ready) { + console.error(`[${context}] Python environment initialization failed:`, status.error); + return { ready: false, error: status.error || 'initialization failed' }; + } + + console.log(`[${context}] Python environment now ready`); + return { ready: true }; + } + /** * Get project-specific environment variables based on project settings */ diff --git a/apps/frontend/src/main/agent/agent-queue.ts b/apps/frontend/src/main/agent/agent-queue.ts index 6f4220ec58..279ecc2b70 100644 --- a/apps/frontend/src/main/agent/agent-queue.ts +++ b/apps/frontend/src/main/agent/agent-queue.ts @@ -59,6 +59,8 @@ export class AgentQueueManager { * Prevents the race condition where generation starts before dependencies are installed, * which would cause it to fall back to system Python and fail with ModuleNotFoundError. * + * Delegates to AgentProcessManager.ensurePythonEnvReady() for the actual initialization. + * * @param projectId - The project ID for error event emission * @param eventType - The error event type to emit on failure * @returns true if environment is ready, false if initialization failed (error already emitted) @@ -67,23 +69,10 @@ export class AgentQueueManager { projectId: string, eventType: 'ideation-error' | 'roadmap-error' ): Promise { - const autoBuildSource = this.processManager.getAutoBuildSourcePath(); - - if (!pythonEnvManager.isEnvReady()) { - debugLog('[Agent Queue] Python environment not ready, waiting for initialization...'); - if (autoBuildSource) { - const status = await pythonEnvManager.initialize(autoBuildSource); - if (!status.ready) { - debugError('[Agent Queue] Python environment initialization failed:', status.error); - this.emitter.emit(eventType, projectId, `Python environment not ready: ${status.error || 'initialization failed'}`); - return false; - } - debugLog('[Agent Queue] Python environment now ready'); - } else { - debugError('[Agent Queue] Cannot initialize Python - auto-build source not found'); - this.emitter.emit(eventType, projectId, 'Python environment not ready: auto-build source not found'); - return false; - } + const status = await this.processManager.ensurePythonEnvReady('AgentQueue'); + if (!status.ready) { + this.emitter.emit(eventType, projectId, `Python environment not ready: ${status.error || 'initialization failed'}`); + return false; } return true; }