Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions apps/frontend/src/__tests__/integration/subprocess-spawn.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down
21 changes: 21 additions & 0 deletions apps/frontend/src/main/agent/agent-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@

// Listen for auto-swap restart events
this.on('auto-swap-restart-task', (taskId: string, newProfileId: string) => {
console.log('[AgentManager] Received auto-swap-restart-task event:', { taskId, newProfileId });

Check warning on line 48 in apps/frontend/src/main/agent/agent-manager.ts

View workflow job for this annotation

GitHub Actions / test-frontend

Unexpected console statement. Only these console methods are allowed: warn, error
const success = this.restartTask(taskId, newProfileId);
console.log('[AgentManager] Task restart result:', success ? 'SUCCESS' : 'FAILED');

Check warning on line 50 in apps/frontend/src/main/agent/agent-manager.ts

View workflow job for this annotation

GitHub Actions / test-frontend

Unexpected console statement. Only these console methods are allowed: warn, error
});

// Listen for task completion to clean up context (prevent memory leak)
Expand Down Expand Up @@ -110,6 +110,13 @@
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) {
Expand Down Expand Up @@ -196,6 +203,13 @@
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) {
Expand Down Expand Up @@ -250,6 +264,13 @@
projectPath: string,
specId: string
): Promise<void> {
// 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) {
Expand Down Expand Up @@ -391,12 +412,12 @@
* @param newProfileId - Optional new profile ID to apply (from auto-swap)
*/
restartTask(taskId: string, newProfileId?: string): boolean {
console.log('[AgentManager] restartTask called for:', taskId, 'with newProfileId:', newProfileId);

Check warning on line 415 in apps/frontend/src/main/agent/agent-manager.ts

View workflow job for this annotation

GitHub Actions / test-frontend

Unexpected console statement. Only these console methods are allowed: warn, error

const context = this.taskExecutionContext.get(taskId);
if (!context) {
console.error('[AgentManager] No context for task:', taskId);
console.log('[AgentManager] Available task contexts:', Array.from(this.taskExecutionContext.keys()));

Check warning on line 420 in apps/frontend/src/main/agent/agent-manager.ts

View workflow job for this annotation

GitHub Actions / test-frontend

Unexpected console statement. Only these console methods are allowed: warn, error
return false;
}

Expand Down
129 changes: 129 additions & 0 deletions apps/frontend/src/main/agent/agent-process.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof import('fs')>();
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;
Expand Down Expand Up @@ -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');
});
});
});
32 changes: 32 additions & 0 deletions apps/frontend/src/main/agent/agent-process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
23 changes: 6 additions & 17 deletions apps/frontend/src/main/agent/agent-queue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -67,23 +69,10 @@ export class AgentQueueManager {
projectId: string,
eventType: 'ideation-error' | 'roadmap-error'
): Promise<boolean> {
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;
}
Expand Down
Loading