From e8ef97a251691c5e3050fd85db06bcf24b95ca36 Mon Sep 17 00:00:00 2001 From: StillKnotKnown Date: Wed, 14 Jan 2026 13:33:07 +0200 Subject: [PATCH 1/5] fix(agent): ensure Python env is ready before spawning tasks (ACS-254) Fixes race condition where task creation fails with exit code 127 when Python environment initialization hasn't completed. The issue occurred because AgentManager.startSpecCreation() and startTaskExecution() spawned Python processes without ensuring pythonEnvManager.isEnvReady() was true. This caused getPythonPath() to fall back to findPythonCommand() which could return an invalid path during the async initialization window. Changes: - Add pythonEnvManager import to agent-manager.ts - Add ensurePythonEnvReady() private method (mirrors agent-queue.ts pattern) - Call ensurePythonEnvReady() in startSpecCreation() before spawning - Call ensurePythonEnvReady() in startTaskExecution() before spawning The fix ensures that if a task is started before Python venv is ready, the task will wait for initialization to complete rather than failing with "command not found" (exit code 127). Refs: https://linear.app/stillknotknown/issue/ACS-254 --- apps/frontend/src/main/agent/agent-manager.ts | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/apps/frontend/src/main/agent/agent-manager.ts b/apps/frontend/src/main/agent/agent-manager.ts index 400ba21cdb..82a5188b33 100644 --- a/apps/frontend/src/main/agent/agent-manager.ts +++ b/apps/frontend/src/main/agent/agent-manager.ts @@ -6,6 +6,7 @@ import { AgentEvents } from './agent-events'; import { AgentProcessManager } from './agent-process'; import { AgentQueueManager } from './agent-queue'; import { getClaudeProfileManager, initializeClaudeProfileManager } from '../claude-profile-manager'; +import { pythonEnvManager } from '../python-env-manager'; import { SpecCreationMetadata, TaskExecutionOptions, @@ -84,6 +85,37 @@ export class AgentManager extends EventEmitter { this.processManager.configure(pythonPath, autoBuildSourcePath); } + /** + * Ensure Python environment is ready before spawning processes. + * This prevents race condition where tasks start before venv initialization completes. + * + * Pattern copied from AgentQueueManager.ensurePythonEnvReady() for consistency. + * + * @param taskId - The task ID for error event emission + * @returns true if environment is ready, false if initialization failed (error already emitted) + */ + private async ensurePythonEnvReady(taskId: string): Promise { + const autoBuildSource = this.processManager.getAutoBuildSourcePath(); + + if (!pythonEnvManager.isEnvReady()) { + console.log('[AgentManager] Python environment not ready, waiting for initialization...'); + if (autoBuildSource) { + const status = await pythonEnvManager.initialize(autoBuildSource); + if (!status.ready) { + console.error('[AgentManager] Python environment initialization failed:', status.error); + this.emit('error', taskId, `Python environment not ready: ${status.error || 'initialization failed'}`); + return false; + } + console.log('[AgentManager] Python environment now ready'); + } else { + console.error('[AgentManager] Cannot initialize Python - auto-build source not found'); + this.emit('error', taskId, 'Python environment not ready: auto-build source not found'); + return false; + } + } + return true; + } + /** * Start spec creation process */ @@ -110,6 +142,12 @@ export class AgentManager extends EventEmitter { return; } + // Ensure Python environment is ready before spawning process (prevents exit code 127 race condition) + const pythonReady = await this.ensurePythonEnvReady(taskId); + if (!pythonReady) { + return; // Error already emitted by ensurePythonEnvReady + } + const autoBuildSource = this.processManager.getAutoBuildSourcePath(); if (!autoBuildSource) { @@ -196,6 +234,12 @@ export class AgentManager extends EventEmitter { return; } + // Ensure Python environment is ready before spawning process (prevents exit code 127 race condition) + const pythonReady = await this.ensurePythonEnvReady(taskId); + if (!pythonReady) { + return; // Error already emitted by ensurePythonEnvReady + } + const autoBuildSource = this.processManager.getAutoBuildSourcePath(); if (!autoBuildSource) { From d0b24ba4e7a40d6672b7de39801934947d35e63f Mon Sep 17 00:00:00 2001 From: StillKnotKnown Date: Wed, 14 Jan 2026 13:54:54 +0200 Subject: [PATCH 2/5] refactor(agent): extract shared ensurePythonEnvReady to AgentProcessManager Address PR review feedback about code duplication between AgentManager and AgentQueueManager.ensurePythonEnvReady(). Changes: - Add AgentProcessManager.ensurePythonEnvReady() as shared method - Remove duplicated private method from AgentManager - Update AgentManager to use processManager.ensurePythonEnvReady() - Simplify AgentQueueManager.ensurePythonEnvReady() to delegate to shared method - Add unit tests for ensurePythonEnvReady covering all scenarios The shared method returns { ready: boolean; error?: string } to allow callers to handle error emission in their own way (AgentManager emits 'error' event, AgentQueueManager emits specific event types). Test coverage added for: - Python environment already ready (no initialization needed) - Python environment not ready (initializes successfully) - autoBuildSource not found (returns error) - Python initialization fails with error message - Python initialization fails without error message Reduces code duplication by ~55 lines while maintaining same behavior. Refs: https://linear.app/stillknotknown/issue/ACS-254 --- apps/frontend/src/main/agent/agent-manager.ts | 46 ++------ .../src/main/agent/agent-process.test.ts | 110 ++++++++++++++++++ apps/frontend/src/main/agent/agent-process.ts | 32 +++++ apps/frontend/src/main/agent/agent-queue.ts | 23 +--- 4 files changed, 156 insertions(+), 55 deletions(-) diff --git a/apps/frontend/src/main/agent/agent-manager.ts b/apps/frontend/src/main/agent/agent-manager.ts index 82a5188b33..0436335b68 100644 --- a/apps/frontend/src/main/agent/agent-manager.ts +++ b/apps/frontend/src/main/agent/agent-manager.ts @@ -6,7 +6,6 @@ import { AgentEvents } from './agent-events'; import { AgentProcessManager } from './agent-process'; import { AgentQueueManager } from './agent-queue'; import { getClaudeProfileManager, initializeClaudeProfileManager } from '../claude-profile-manager'; -import { pythonEnvManager } from '../python-env-manager'; import { SpecCreationMetadata, TaskExecutionOptions, @@ -85,37 +84,6 @@ export class AgentManager extends EventEmitter { this.processManager.configure(pythonPath, autoBuildSourcePath); } - /** - * Ensure Python environment is ready before spawning processes. - * This prevents race condition where tasks start before venv initialization completes. - * - * Pattern copied from AgentQueueManager.ensurePythonEnvReady() for consistency. - * - * @param taskId - The task ID for error event emission - * @returns true if environment is ready, false if initialization failed (error already emitted) - */ - private async ensurePythonEnvReady(taskId: string): Promise { - const autoBuildSource = this.processManager.getAutoBuildSourcePath(); - - if (!pythonEnvManager.isEnvReady()) { - console.log('[AgentManager] Python environment not ready, waiting for initialization...'); - if (autoBuildSource) { - const status = await pythonEnvManager.initialize(autoBuildSource); - if (!status.ready) { - console.error('[AgentManager] Python environment initialization failed:', status.error); - this.emit('error', taskId, `Python environment not ready: ${status.error || 'initialization failed'}`); - return false; - } - console.log('[AgentManager] Python environment now ready'); - } else { - console.error('[AgentManager] Cannot initialize Python - auto-build source not found'); - this.emit('error', taskId, 'Python environment not ready: auto-build source not found'); - return false; - } - } - return true; - } - /** * Start spec creation process */ @@ -143,9 +111,10 @@ export class AgentManager extends EventEmitter { } // Ensure Python environment is ready before spawning process (prevents exit code 127 race condition) - const pythonReady = await this.ensurePythonEnvReady(taskId); - if (!pythonReady) { - return; // Error already emitted by ensurePythonEnvReady + 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(); @@ -235,9 +204,10 @@ export class AgentManager extends EventEmitter { } // Ensure Python environment is ready before spawning process (prevents exit code 127 race condition) - const pythonReady = await this.ensurePythonEnvReady(taskId); - if (!pythonReady) { - return; // Error already emitted by ensurePythonEnvReady + 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(); diff --git a/apps/frontend/src/main/agent/agent-process.test.ts b/apps/frontend/src/main/agent/agent-process.test.ts index c06b8f6824..f01bf92320 100644 --- a/apps/frontend/src/main/agent/agent-process.test.ts +++ b/apps/frontend/src/main/agent/agent-process.test.ts @@ -95,6 +95,15 @@ 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 })) + }, + getConfiguredPythonPath: vi.fn(() => 'python3') +})); + vi.mock('electron', () => ({ app: { getAppPath: vi.fn(() => '/fake/app/path') @@ -107,6 +116,7 @@ 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 +501,104 @@ 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'); + }); + }); }); 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; } From a217f869abc67a2684c9910c026a3ae3efe5d53f Mon Sep 17 00:00:00 2001 From: StillKnotKnown Date: Wed, 14 Jan 2026 14:02:45 +0200 Subject: [PATCH 3/5] refactor(agent): add Python env check to startQAProcess for consistency Address CodeRabbit review suggestion to add ensurePythonEnvReady check to startQAProcess, providing consistent protection against the race condition for all Python process spawning methods. Now all three process-spawning methods in AgentManager have the check: - startSpecCreation - startTaskExecution - startQAProcess (newly added) This prevents edge-case failures where QA might be triggered before Python environment initialization completes. Refs: https://linear.app/stillknotknown/issue/ACS-254 --- apps/frontend/src/main/agent/agent-manager.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/apps/frontend/src/main/agent/agent-manager.ts b/apps/frontend/src/main/agent/agent-manager.ts index 0436335b68..7ce8790954 100644 --- a/apps/frontend/src/main/agent/agent-manager.ts +++ b/apps/frontend/src/main/agent/agent-manager.ts @@ -264,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) { From abe02d1d8fac5cae9c217bb74270d6e00474b096 Mon Sep 17 00:00:00 2001 From: StillKnotKnown Date: Wed, 14 Jan 2026 14:10:56 +0200 Subject: [PATCH 4/5] test: fix pythonEnvManager mock to include getPythonEnv method The test mock was missing the getPythonEnv() method that spawnProcess() calls, causing 14 test failures. Added getPythonEnv mock returning empty object to match production usage. Signed-off-by: StillKnotKnown --- .../src/main/agent/agent-process.test.ts | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/apps/frontend/src/main/agent/agent-process.test.ts b/apps/frontend/src/main/agent/agent-process.test.ts index f01bf92320..b6897acd95 100644 --- a/apps/frontend/src/main/agent/agent-process.test.ts +++ b/apps/frontend/src/main/agent/agent-process.test.ts @@ -99,7 +99,8 @@ vi.mock('../python-detector', () => ({ vi.mock('../python-env-manager', () => ({ pythonEnvManager: { isEnvReady: vi.fn(() => true), - initialize: vi.fn(() => Promise.resolve({ ready: true })) + initialize: vi.fn(() => Promise.resolve({ ready: true })), + getPythonEnv: vi.fn(() => ({})) }, getConfiguredPythonPath: vi.fn(() => 'python3') })); @@ -110,6 +111,23 @@ vi.mock('electron', () => ({ } })); +// 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'; @@ -599,6 +617,7 @@ describe('AgentProcessManager - API Profile Env Injection (Story 2.3)', () => { expect(result.ready).toBe(false); expect(result.error).toBe('initialization failed'); + expect(pythonEnvManager.initialize).toHaveBeenCalledWith('/fake/auto-build'); }); }); }); From 6131f71333d7ec979835ad02ee7eea9fcfa565c7 Mon Sep 17 00:00:00 2001 From: StillKnotKnown Date: Wed, 14 Jan 2026 14:24:30 +0200 Subject: [PATCH 5/5] test: add python-env-manager mock for integration tests Integration tests were timing out because python-env-manager wasn't mocked. The ensurePythonEnvReady changes added calls to pythonEnvManager.isEnvReady() and getPythonEnv() which weren't mocked in the integration test suite. Fixes 13 timeout failures in subprocess-spawn.test.ts Signed-off-by: StillKnotKnown --- .../src/__tests__/integration/subprocess-spawn.test.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) 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');