diff --git a/apps/backend/cli/main.py b/apps/backend/cli/main.py index dc1f6a9c32..c471250f51 100644 --- a/apps/backend/cli/main.py +++ b/apps/backend/cli/main.py @@ -34,6 +34,7 @@ find_spec, get_project_dir, print_banner, + resolve_project_dir_for_spec, setup_environment, ) from .workspace_commands import ( @@ -321,6 +322,13 @@ def _run_cli() -> None: # Determine project directory project_dir = get_project_dir(args.project_dir) + if args.spec: + resolved_project_dir = resolve_project_dir_for_spec(project_dir, args.spec) + if resolved_project_dir != project_dir: + debug( + "run.py", f"Adjusted project directory for spec: {resolved_project_dir}" + ) + project_dir = resolved_project_dir debug("run.py", f"Using project directory: {project_dir}") # Get model from CLI arg or env var (None if not explicitly set) diff --git a/apps/backend/cli/utils.py b/apps/backend/cli/utils.py index f65b83c78f..b705100d77 100644 --- a/apps/backend/cli/utils.py +++ b/apps/backend/cli/utils.py @@ -262,6 +262,29 @@ def get_project_dir(provided_dir: Path | None) -> Path: return project_dir +def resolve_project_dir_for_spec(project_dir: Path, spec_identifier: str) -> Path: + """ + Resolve project dir if the spec exists under a child project folder. + + This handles cases where --project-dir is a parent (e.g., E:\\projects) + but the spec lives under a child repo (e.g., E:\\projects\\auto-claude). + """ + if find_spec(project_dir, spec_identifier): + return project_dir + + try: + for child in project_dir.iterdir(): + if not child.is_dir(): + continue + if find_spec(child, spec_identifier): + return child + except OSError: + # Non-fatal: fall back to original project_dir + pass + + return project_dir + + def find_specs_dir(project_dir: Path) -> Path: """ Find the specs directory for a project. diff --git a/apps/backend/runners/spec_runner.py b/apps/backend/runners/spec_runner.py index 53f64b357b..e933b82a76 100644 --- a/apps/backend/runners/spec_runner.py +++ b/apps/backend/runners/spec_runner.py @@ -318,6 +318,11 @@ def main(): if not review_state.is_approved(): debug_error("spec_runner", "Spec not approved - cannot start build") print() + if not args.interactive: + print_status( + "Spec requires human review. Build not started.", "warning" + ) + sys.exit(0) print_status("Build cannot start: spec not approved.", "error") print() print(f" {muted('To approve the spec, run:')}") diff --git a/apps/backend/spec/pipeline/orchestrator.py b/apps/backend/spec/pipeline/orchestrator.py index 08d6d5bddb..1749f0f43a 100644 --- a/apps/backend/spec/pipeline/orchestrator.py +++ b/apps/backend/spec/pipeline/orchestrator.py @@ -6,6 +6,7 @@ """ import json +import os from collections.abc import Callable from pathlib import Path @@ -283,17 +284,28 @@ def run_phase(name: str, phase_fn: Callable) -> phases.PhaseResult: ) return phase_fn() + skip_discovery = os.getenv("SKIP_DISCOVERY", "").lower() == "true" + # === PHASE 1: DISCOVERY === - result = await run_phase("discovery", phase_executor.phase_discovery) - results.append(result) - if not result.success: - print_status("Discovery failed", "error") - task_logger.end_phase( - LogPhase.PLANNING, success=False, message="Discovery failed" + if skip_discovery: + task_logger.log( + "Skipping discovery phase (flag enabled)", + LogEntryType.INFO, + LogPhase.PLANNING, ) - return False - # Store summary for subsequent phases (compaction) - await self._store_phase_summary("discovery") + print_status("Skipping discovery phase (flag enabled)", "info") + results.append(phases.PhaseResult("discovery", True, [], [], 0)) + else: + result = await run_phase("discovery", phase_executor.phase_discovery) + results.append(result) + if not result.success: + print_status("Discovery failed", "error") + task_logger.end_phase( + LogPhase.PLANNING, success=False, message="Discovery failed" + ) + return False + # Store summary for subsequent phases (compaction) + await self._store_phase_summary("discovery") # === PHASE 2: REQUIREMENTS GATHERING === result = await run_phase( @@ -409,7 +421,7 @@ def run_phase(name: str, phase_fn: Callable) -> phases.PhaseResult: ) # === HUMAN REVIEW CHECKPOINT === - return self._run_review_checkpoint(auto_approve) + return self._run_review_checkpoint(auto_approve, interactive) async def _create_linear_task_if_enabled(self) -> None: """Create a Linear task if Linear integration is enabled.""" @@ -613,11 +625,12 @@ def _print_completion_summary( ) ) - def _run_review_checkpoint(self, auto_approve: bool) -> bool: + def _run_review_checkpoint(self, auto_approve: bool, interactive: bool) -> bool: """Run the human review checkpoint. Args: auto_approve: Whether to auto-approve without human review + interactive: Whether to run in interactive mode Returns: True if approved, False otherwise @@ -625,6 +638,10 @@ def _run_review_checkpoint(self, auto_approve: bool) -> bool: print() print_section("HUMAN REVIEW CHECKPOINT", Icons.SEARCH) + if not auto_approve and not interactive: + print_status("Skipping interactive review (non-interactive mode)", "info") + return True + try: review_state = run_review_checkpoint( spec_dir=self.spec_dir, diff --git a/apps/frontend/package.json b/apps/frontend/package.json index 9860bd59c3..112d101d8c 100644 --- a/apps/frontend/package.json +++ b/apps/frontend/package.json @@ -98,6 +98,7 @@ "semver": "^7.7.3", "tailwind-merge": "^3.4.0", "uuid": "^13.0.0", + "xstate": "^5.17.0", "zod": "^4.2.1", "zustand": "^5.0.9" }, diff --git a/apps/frontend/src/__tests__/e2e/smoke.test.ts b/apps/frontend/src/__tests__/e2e/smoke.test.ts index 0864197c97..4500f09161 100644 --- a/apps/frontend/src/__tests__/e2e/smoke.test.ts +++ b/apps/frontend/src/__tests__/e2e/smoke.test.ts @@ -396,7 +396,7 @@ describe('E2E Smoke Tests', () => { statusHandler({}, 'task-001', 'in_progress'); } - expect(statusCallback).toHaveBeenCalledWith('task-001', 'in_progress', undefined); + expect(statusCallback).toHaveBeenCalledWith('task-001', 'in_progress', undefined, undefined); // Cleanup listeners cleanupProgress(); @@ -656,7 +656,8 @@ describe('E2E Smoke Tests', () => { index + 1, 'task-001', status, - undefined + undefined, + undefined // reviewReason ); }); diff --git a/apps/frontend/src/__tests__/integration/task-lifecycle.test.ts b/apps/frontend/src/__tests__/integration/task-lifecycle.test.ts index fffbed82d8..c253133d31 100644 --- a/apps/frontend/src/__tests__/integration/task-lifecycle.test.ts +++ b/apps/frontend/src/__tests__/integration/task-lifecycle.test.ts @@ -244,9 +244,9 @@ describe('Task Lifecycle Integration', () => { eventHandler({}, 'task-001', 'spec_complete'); } - // Verify callback was invoked with correct parameters (taskId, status, projectId) - // Note: projectId is optional and undefined when not provided - expect(callback).toHaveBeenCalledWith('task-001', 'spec_complete', undefined); + // Verify callback was invoked with correct parameters (taskId, status, projectId, reviewReason) + // Note: projectId and reviewReason are optional and undefined when not provided + expect(callback).toHaveBeenCalledWith('task-001', 'spec_complete', undefined, undefined); }); it('should emit task:progress event with updated plan during spec creation', async () => { diff --git a/apps/frontend/src/main/__tests__/ipc-handlers.test.ts b/apps/frontend/src/main/__tests__/ipc-handlers.test.ts index cd73005066..dcea18e86c 100644 --- a/apps/frontend/src/main/__tests__/ipc-handlers.test.ts +++ b/apps/frontend/src/main/__tests__/ipc-handlers.test.ts @@ -92,6 +92,20 @@ vi.mock("../cli-tool-manager", () => ({ getToolPathAsync: vi.fn((tool: string) => Promise.resolve(tool)), })); +// Mock task-state-utils to disable XState for these tests +// This ensures we test the fallback path behavior consistently +vi.mock("../task-state-utils", () => ({ + isXstateEnabled: vi.fn(() => false), + isDebugEnabled: vi.fn(() => false), + invalidateXstateCache: vi.fn(), + emitTaskStatusChange: vi.fn((getMainWindow, taskId, status, projectId, reviewReason) => { + const mainWindow = getMainWindow(); + if (mainWindow) { + mainWindow.webContents.send("task:statusChange", taskId, status, projectId, reviewReason); + } + }), +})); + // Mock modules before importing vi.mock("electron", () => { const mockIpcMain = new (class extends EventEmitter { @@ -689,7 +703,8 @@ describe("IPC Handlers", { timeout: 30000 }, () => { "task:statusChange", "task-1", "human_review", - expect.any(String) // projectId for multi-project filtering + expect.any(String), // projectId for multi-project filtering + expect.anything() // reviewReason (optional) ); }); }); diff --git a/apps/frontend/src/main/claude-profile-manager.ts b/apps/frontend/src/main/claude-profile-manager.ts index b2de28e86f..cbe9b18c44 100644 --- a/apps/frontend/src/main/claude-profile-manager.ts +++ b/apps/frontend/src/main/claude-profile-manager.ts @@ -45,6 +45,7 @@ import { } from './claude-profile/profile-scorer'; import { getCredentialsFromKeychain } from './claude-profile/credential-utils'; import { + DEFAULT_CLAUDE_CONFIG_DIR, CLAUDE_PROFILES_DIR, generateProfileId as generateProfileIdImpl, createProfileDirectory as createProfileDirectoryImpl, diff --git a/apps/frontend/src/main/index.ts b/apps/frontend/src/main/index.ts index d7c9f7a7c4..3a497b1e88 100644 --- a/apps/frontend/src/main/index.ts +++ b/apps/frontend/src/main/index.ts @@ -53,6 +53,7 @@ import { initSentryMain } from './sentry'; import { preWarmToolCache } from './cli-tool-manager'; import { initializeClaudeProfileManager, getClaudeProfileManager } from './claude-profile-manager'; import { isMacOS, isWindows } from './platform'; +import { cleanupTaskStateManager } from './task-state-manager'; import type { AppSettings, AuthFailureInfo } from '../shared/types'; // ───────────────────────────────────────────────────────────────────────────── @@ -304,7 +305,8 @@ app.whenReady().then(() => { // Validate and migrate autoBuildPath - must contain runners/spec_runner.py // Uses EAFP pattern (try/catch with accessSync) instead of existsSync to avoid TOCTOU race conditions - let validAutoBuildPath = settings.autoBuildPath; + const envAutoBuildPath = (process.env.AUTO_CLAUDE_BACKEND_PATH || '').trim() || undefined; + let validAutoBuildPath = envAutoBuildPath || settings.autoBuildPath; if (validAutoBuildPath) { const specRunnerPath = join(validAutoBuildPath, 'runners', 'spec_runner.py'); let specRunnerExists = false; @@ -316,42 +318,47 @@ app.whenReady().then(() => { } if (!specRunnerExists) { - // Migration: Try to fix stale paths from old project structure - // Old structure: /path/to/project/auto-claude - // New structure: /path/to/project/apps/backend - let migrated = false; - if (validAutoBuildPath.endsWith('/auto-claude') || validAutoBuildPath.endsWith('\\auto-claude')) { - const basePath = validAutoBuildPath.replace(/[/\\]auto-claude$/, ''); - const correctedPath = join(basePath, 'apps', 'backend'); - const correctedSpecRunnerPath = join(correctedPath, 'runners', 'spec_runner.py'); - - let correctedPathExists = false; - try { - accessSync(correctedSpecRunnerPath); - correctedPathExists = true; - } catch { - // Corrected path doesn't exist - } - - if (correctedPathExists) { - console.log('[main] Migrating autoBuildPath from old structure:', validAutoBuildPath, '->', correctedPath); - settings.autoBuildPath = correctedPath; - validAutoBuildPath = correctedPath; - migrated = true; - - // Save the corrected setting - we're the only process modifying settings at startup + if (envAutoBuildPath) { + console.warn('[main] AUTO_CLAUDE_BACKEND_PATH is invalid (missing runners/spec_runner.py), will use auto-detection:', validAutoBuildPath); + validAutoBuildPath = undefined; // Let auto-detection find the correct path + } else { + // Migration: Try to fix stale paths from old project structure + // Old structure: /path/to/project/auto-claude + // New structure: /path/to/project/apps/backend + let migrated = false; + if (validAutoBuildPath.endsWith('/auto-claude') || validAutoBuildPath.endsWith('\\auto-claude')) { + const basePath = validAutoBuildPath.replace(/[/\\]auto-claude$/, ''); + const correctedPath = join(basePath, 'apps', 'backend'); + const correctedSpecRunnerPath = join(correctedPath, 'runners', 'spec_runner.py'); + + let correctedPathExists = false; try { - writeFileSync(settingsPath, JSON.stringify(settings, null, 2), 'utf-8'); - console.log('[main] Successfully saved migrated autoBuildPath to settings'); - } catch (writeError) { - console.warn('[main] Failed to save migrated autoBuildPath:', writeError); + accessSync(correctedSpecRunnerPath); + correctedPathExists = true; + } catch { + // Corrected path doesn't exist + } + + if (correctedPathExists) { + console.log('[main] Migrating autoBuildPath from old structure:', validAutoBuildPath, '->', correctedPath); + settings.autoBuildPath = correctedPath; + validAutoBuildPath = correctedPath; + migrated = true; + + // Save the corrected setting - we're the only process modifying settings at startup + try { + writeFileSync(settingsPath, JSON.stringify(settings, null, 2), 'utf-8'); + console.log('[main] Successfully saved migrated autoBuildPath to settings'); + } catch (writeError) { + console.warn('[main] Failed to save migrated autoBuildPath:', writeError); + } } } - } - if (!migrated) { - console.warn('[main] Configured autoBuildPath is invalid (missing runners/spec_runner.py), will use auto-detection:', validAutoBuildPath); - validAutoBuildPath = undefined; // Let auto-detection find the correct path + if (!migrated) { + console.warn('[main] Configured autoBuildPath is invalid (missing runners/spec_runner.py), will use auto-detection:', validAutoBuildPath); + validAutoBuildPath = undefined; // Let auto-detection find the correct path + } } } } @@ -504,6 +511,10 @@ app.on('before-quit', async () => { usageMonitor.stop(); console.warn('[main] Usage monitor stopped'); + // Cleanup XState task state manager to stop all actors and prevent memory leaks + cleanupTaskStateManager(); + console.warn('[main] Task state manager cleaned up'); + // Kill all running agent processes if (agentManager) { await agentManager.killAll(); diff --git a/apps/frontend/src/main/ipc-handlers/agent-events-handlers.ts b/apps/frontend/src/main/ipc-handlers/agent-events-handlers.ts index c20fb79dad..e70d005044 100644 --- a/apps/frontend/src/main/ipc-handlers/agent-events-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/agent-events-handlers.ts @@ -16,6 +16,7 @@ import type { TaskStatus, Project, ImplementationPlan, + ReviewReason, } from "../../shared/types"; import { AgentManager } from "../agent"; import type { ProcessType, ExecutionProgressData } from "../agent"; @@ -28,6 +29,9 @@ import { findTaskWorktree } from "../worktree-paths"; import { findTaskAndProject } from "./task/shared"; import { safeSendToRenderer } from "./utils"; import { getClaudeProfileManager } from "../claude-profile-manager"; +import { TaskStateMachine } from "../task-state-machine"; +import { getTaskStateManager } from '../task-state-manager'; +import { isXstateEnabled } from '../task-state-utils'; /** * Validates status transitions to prevent invalid state changes. @@ -109,6 +113,7 @@ export function registerAgenteventsHandlers( agentManager: AgentManager, getMainWindow: () => BrowserWindow | null ): void { + const taskStateMachine = new TaskStateMachine(); // ============================================ // Agent Manager Events → Renderer // ============================================ @@ -247,50 +252,74 @@ export function registerAgenteventsHandlers( } }; + const hasSubtasks = task.subtasks && task.subtasks.length > 0; + const allSubtasksDone = hasSubtasks && task.subtasks.every((s) => s.status === "completed"); + const requireReviewBeforeCoding = task.metadata?.requireReviewBeforeCoding === true; + + taskStateMachine.logProcessExit({ + taskId, + exitCode: code ?? 0, + task, + hasSubtasks, + allSubtasksDone, + requireReviewBeforeCoding, + }); + + // Send notifications regardless of XState setting if (code === 0) { notificationService.notifyReviewNeeded(taskTitle, project.id, taskId); + } else { + notificationService.notifyTaskFailed(taskTitle, project.id, taskId); + } - // Fallback: Ensure status is updated even if COMPLETE phase event was missed - // This prevents tasks from getting stuck in ai_review status - // FIX (ACS-71): Only move to human_review if subtasks exist AND are all completed - // If no subtasks exist, the task is still in planning and shouldn't move to human_review - const isActiveStatus = task.status === "in_progress" || task.status === "ai_review"; - const hasSubtasks = task.subtasks && task.subtasks.length > 0; - const hasIncompleteSubtasks = - hasSubtasks && task.subtasks.some((s) => s.status !== "completed"); - - if (isActiveStatus && hasSubtasks && !hasIncompleteSubtasks) { - // All subtasks completed - safe to move to human_review - console.warn( - `[Task ${taskId}] Fallback: Moving to human_review (process exited successfully, all ${task.subtasks.length} subtasks completed)` - ); - persistStatus("human_review"); - // Include projectId for multi-project filtering (issue #723) - safeSendToRenderer( + // CRITICAL: XState and fallback paths must be MUTUALLY EXCLUSIVE + // Only one should handle status change and persistence to avoid duplicate IPC messages + if (isXstateEnabled()) { + // XState handles everything: status change, persistence, and IPC emission + const manager = getTaskStateManager(getMainWindow); + manager.handleProcessExit(task, project, code ?? 0, hasSubtasks, allSubtasksDone); + } else { + // Fallback: compute status and handle persistence + IPC emission + const decision = taskStateMachine.getStatusForProcessExit({ + taskId, + exitCode: code ?? 0, + task, + hasSubtasks, + allSubtasksDone, + requireReviewBeforeCoding, + }); + const decisionWithFallback: { status?: TaskStatus; reviewReason?: ReviewReason } = + requireReviewBeforeCoding && !decision.status + ? { status: "human_review", reviewReason: "plan_review" } + : decision; + + if (code === 0) { + const isActiveStatus = task.status === "in_progress" || task.status === "ai_review"; + if (decisionWithFallback.status) { + persistStatus(decisionWithFallback.status); + taskStateMachine.emitStatusChange( + getMainWindow, + taskId, + decisionWithFallback.status, + projectId, + decisionWithFallback.reviewReason + ); + } else if (isActiveStatus) { + console.warn( + `[Task ${taskId}] Process exited but status unchanged (current status: ${task.status})` + ); + } + } else { + const nextStatus: TaskStatus = decisionWithFallback.status ?? "human_review"; + persistStatus(nextStatus); + taskStateMachine.emitStatusChange( getMainWindow, - IPC_CHANNELS.TASK_STATUS_CHANGE, taskId, - "human_review" as TaskStatus, - projectId - ); - } else if (isActiveStatus && !hasSubtasks) { - // No subtasks yet - task is still in planning phase, don't change status - // This prevents the bug where tasks jump to human_review before planning completes - console.warn( - `[Task ${taskId}] Process exited but no subtasks created yet - keeping current status (${task.status})` + nextStatus, + projectId, + decisionWithFallback.reviewReason ); } - } else { - notificationService.notifyTaskFailed(taskTitle, project.id, taskId); - persistStatus("human_review"); - // Include projectId for multi-project filtering (issue #723) - safeSendToRenderer( - getMainWindow, - IPC_CHANNELS.TASK_STATUS_CHANGE, - taskId, - "human_review" as TaskStatus, - projectId - ); } } } catch (error) { @@ -303,6 +332,8 @@ export function registerAgenteventsHandlers( const { task, project } = findTaskAndProject(taskId); const taskProjectId = project?.id; + taskStateMachine.logExecutionProgress(taskId, progress, taskProjectId); + // Include projectId in execution progress event for multi-project filtering safeSendToRenderer( getMainWindow, @@ -312,58 +343,50 @@ export function registerAgenteventsHandlers( taskProjectId ); - const phaseToStatus: Record = { - idle: null, - planning: "in_progress", - coding: "in_progress", - qa_review: "ai_review", - qa_fixing: "ai_review", - complete: "human_review", - failed: "human_review", - }; - - const newStatus = phaseToStatus[progress.phase]; - // FIX (ACS-55, ACS-71): Validate status transition before sending/persisting - if (newStatus && validateStatusTransition(task, newStatus, progress.phase)) { - // Include projectId in status change event for multi-project filtering - safeSendToRenderer( - getMainWindow, - IPC_CHANNELS.TASK_STATUS_CHANGE, - taskId, - newStatus, - taskProjectId - ); - - // CRITICAL: Persist status to plan file(s) to prevent flip-flop on task list refresh - // When getTasks() is called, it reads status from the plan file. Without persisting, - // the status in the file might differ from the UI, causing inconsistent state. - // Uses shared utility with locking to prevent race conditions. - // IMPORTANT: We persist to BOTH main project AND worktree (if exists) to ensure - // consistency, since getTasks() prefers the worktree version. - if (task && project) { - try { - // Persist to main project plan file - const mainPlanPath = getPlanPath(project, task); - persistPlanStatusSync(mainPlanPath, newStatus, project.id); - - // Also persist to worktree plan file if it exists - // This ensures consistency since getTasks() prefers worktree version - const worktreePath = findTaskWorktree(project.path, task.specId); - if (worktreePath) { - const specsBaseDir = getSpecsDir(project.autoBuildPath); - const worktreePlanPath = path.join( - worktreePath, - specsBaseDir, - task.specId, - AUTO_BUILD_PATHS.IMPLEMENTATION_PLAN + // CRITICAL: XState and fallback paths must be MUTUALLY EXCLUSIVE + // Only one should handle status change and persistence to avoid duplicate IPC messages + if (task && project) { + if (isXstateEnabled()) { + // XState handles everything: status change, persistence, and IPC emission + const manager = getTaskStateManager(getMainWindow); + manager.handleExecutionProgress(task, project, progress); + } else { + // Fallback: compute status and handle persistence + IPC emission + const fallbackStatus = taskStateMachine.getStatusForExecutionProgress(progress); + if (fallbackStatus === "ai_review" || fallbackStatus === "human_review") { + if (validateStatusTransition(task, fallbackStatus, progress.phase)) { + const reviewReason = + fallbackStatus === "human_review" && progress.phase === "complete" + ? "completed" + : undefined; + taskStateMachine.emitStatusChange( + getMainWindow, + taskId, + fallbackStatus, + taskProjectId, + reviewReason ); - if (existsSync(worktreePlanPath)) { - persistPlanStatusSync(worktreePlanPath, newStatus, project.id); + } + try { + const mainPlanPath = getPlanPath(project, task); + persistPlanStatusSync(mainPlanPath, fallbackStatus, project.id); + + const worktreePath = findTaskWorktree(project.path, task.specId); + if (worktreePath) { + const specsBaseDir = getSpecsDir(project.autoBuildPath); + const worktreePlanPath = path.join( + worktreePath, + specsBaseDir, + task.specId, + AUTO_BUILD_PATHS.IMPLEMENTATION_PLAN + ); + if (existsSync(worktreePlanPath)) { + persistPlanStatusSync(worktreePlanPath, fallbackStatus, project.id); + } } + } catch (err) { + console.warn("[execution-progress] Could not persist fallback status:", err); } - } catch (err) { - // Ignore persistence errors - UI will still work, just might flip on refresh - console.warn("[execution-progress] Could not persist status:", err); } } } diff --git a/apps/frontend/src/main/ipc-handlers/task/crud-handlers.ts b/apps/frontend/src/main/ipc-handlers/task/crud-handlers.ts index bff3a2e676..f11b5e4d3d 100644 --- a/apps/frontend/src/main/ipc-handlers/task/crud-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/task/crud-handlers.ts @@ -1,4 +1,4 @@ -import { ipcMain, nativeImage } from 'electron'; +import { ipcMain, nativeImage, BrowserWindow } from 'electron'; import { IPC_CHANNELS, AUTO_BUILD_PATHS, getSpecsDir } from '../../../shared/constants'; import type { IPCResult, Task, TaskMetadata } from '../../../shared/types'; import path from 'path'; @@ -9,11 +9,15 @@ import { AgentManager } from '../../agent'; import { findTaskAndProject } from './shared'; import { findAllSpecPaths, isValidTaskId } from '../../utils/spec-path-helpers'; import { isPathWithinBase } from '../../worktree-paths'; +import { getTaskStateManager } from '../../task-state-manager'; /** * Register task CRUD (Create, Read, Update, Delete) handlers */ -export function registerTaskCRUDHandlers(agentManager: AgentManager): void { +export function registerTaskCRUDHandlers( + agentManager: AgentManager, + getMainWindow: () => BrowserWindow | null +): void { /** * List all tasks for a project * @param projectId - The project ID to fetch tasks for @@ -265,6 +269,9 @@ export function registerTaskCRUDHandlers(agentManager: AgentManager): void { } } + // Clean up XState actor for this task to prevent memory leaks + getTaskStateManager(getMainWindow).cleanupTask(taskId); + // Invalidate cache since a task was deleted projectStore.invalidateTasksCache(project.id); diff --git a/apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts b/apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts index 99369ffdf9..bb90e09645 100644 --- a/apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts @@ -18,6 +18,9 @@ import { import { findTaskWorktree } from '../../worktree-paths'; import { projectStore } from '../../project-store'; import { getIsolatedGitEnv, detectWorktreeBranch } from '../../utils/git-isolation'; +import { TaskStateMachine } from '../../task-state-machine'; +import { getTaskStateManager } from '../../task-state-manager'; +import { isXstateEnabled } from '../../task-state-utils'; /** * Atomic file write to prevent TOCTOU race conditions. @@ -56,6 +59,13 @@ function safeReadFileSync(filePath: string): string | null { } } +function logTaskStatusChange(taskId: string, status: TaskStatus, reviewReason?: string | null): void { + if (process.env.DEBUG === 'true') { + const reason = reviewReason || 'none'; + console.log(`[TASK_STATUS_CHANGE] taskId=${taskId} status=${status} reviewReason=${reason}`); + } +} + /** * Helper function to check subtask completion status */ @@ -106,6 +116,7 @@ export function registerTaskExecutionHandlers( agentManager: AgentManager, getMainWindow: () => BrowserWindow | null ): void { + const taskStateMachine = new TaskStateMachine(); /** * Start a task */ @@ -179,6 +190,17 @@ export function registerTaskExecutionHandlers( console.warn('[TASK_START] Found task:', task.specId, 'status:', task.status, 'subtasks:', task.subtasks.length); + const shouldResume = + task.status === 'human_review' && + task.reviewReason !== 'completed'; + + if (shouldResume || task.status === 'error') { + if (isXstateEnabled()) { + const taskStateManager = getTaskStateManager(getMainWindow); + taskStateManager.handleUserResumed(task, project); + } + } + // Start file watcher for this task const specsBaseDir = getSpecsDir(project.autoBuildPath); const specDir = path.join( @@ -256,11 +278,8 @@ export function registerTaskExecutionHandlers( // Notify status change IMMEDIATELY (don't wait for file write) // This provides instant UI feedback while file persistence happens in background const ipcSentAt = Date.now(); - mainWindow.webContents.send( - IPC_CHANNELS.TASK_STATUS_CHANGE, - taskId, - 'in_progress' - ); + logTaskStatusChange(taskId, 'in_progress'); + taskStateMachine.emitStatusChange(getMainWindow, taskId, 'in_progress'); const DEBUG = process.env.DEBUG === 'true'; if (DEBUG) { @@ -298,51 +317,24 @@ export function registerTaskExecutionHandlers( * Stop a task */ ipcMain.on(IPC_CHANNELS.TASK_STOP, (_, taskId: string) => { - const DEBUG = process.env.DEBUG === 'true'; - agentManager.killTask(taskId); fileWatcher.unwatch(taskId); - // Notify status change IMMEDIATELY for instant UI feedback - const ipcSentAt = Date.now(); - const mainWindow = getMainWindow(); - if (mainWindow) { - mainWindow.webContents.send( - IPC_CHANNELS.TASK_STATUS_CHANGE, - taskId, - 'backlog' - ); - } - - if (DEBUG) { - console.log(`[TASK_STOP] IPC sent immediately for task ${taskId}, deferring file persistence`); - } - - // Find task and project to update the plan file (async, non-blocking) const { task, project } = findTaskAndProject(taskId); - if (task && project) { - // Persist status to implementation_plan.json to prevent status flip-flop on refresh - // Uses shared utility for consistency with agent-events-handlers.ts - // NOTE: This is now async and non-blocking for better UI responsiveness - const planPath = getPlanPath(project, task); - setImmediate(async () => { - const persistStart = Date.now(); - try { - const persisted = await persistPlanStatus(planPath, 'backlog', project.id); - if (persisted) { - console.warn('[TASK_STOP] Updated plan status to backlog'); - } - if (DEBUG) { - const delay = persistStart - ipcSentAt; - const duration = Date.now() - persistStart; - console.log(`[TASK_STOP] File persistence: delayed ${delay}ms after IPC, completed in ${duration}ms`); - } - } catch (err) { + if (isXstateEnabled()) { + // XState handles status change and persistence + const manager = getTaskStateManager(getMainWindow); + manager.handleUserStopped(task, project); + } else { + // Fallback: emit status change and persist when XState disabled + taskStateMachine.emitStatusChange(getMainWindow, taskId, 'backlog'); + const planPath = getPlanPath(project, task); + // Fire-and-forget async persistence (non-blocking) + persistPlanStatus(planPath, 'backlog', project.id).catch(err => { console.error('[TASK_STOP] Failed to persist plan status:', err); - } - }); - // Note: File not found is expected for tasks without a plan file (persistPlanStatus handles ENOENT) + }); + } } }); @@ -393,27 +385,36 @@ export function registerTaskExecutionHandlers( } // Notify UI immediately for instant feedback + // Use XState or fallback to keep state in sync const mainWindow = getMainWindow(); if (mainWindow) { - mainWindow.webContents.send( - IPC_CHANNELS.TASK_STATUS_CHANGE, - taskId, - 'done' - ); + logTaskStatusChange(taskId, 'done'); + if (isXstateEnabled()) { + // Use XState manager to keep actor state in sync + const manager = getTaskStateManager(getMainWindow); + manager.handleManualStatus(task, project, 'done'); + } else { + // Fallback: emit status change directly + taskStateMachine.emitStatusChange(getMainWindow, taskId, 'done'); + } } // CRITICAL: Persist 'done' status to implementation_plan.json // Without this, the old status would be shown after page refresh since // getTasks() reads status from the plan file, not from the Zustand store. - const planPath = getPlanPath(project, task); - try { - const persisted = await persistPlanStatus(planPath, 'done', project.id); - if (persisted) { - console.warn('[TASK_REVIEW] Persisted approved status (done) to implementation_plan.json'); + // NOTE: When XState is enabled, TaskStateManager.handleManualStatus also persists, + // but we persist here as well for the fallback path + if (!isXstateEnabled()) { + const planPath = getPlanPath(project, task); + try { + const persisted = await persistPlanStatus(planPath, 'done', project.id); + if (persisted) { + console.warn('[TASK_REVIEW] Persisted approved status (done) to implementation_plan.json'); + } + } catch (err) { + console.error('[TASK_REVIEW] Failed to persist approved status:', err); + // Non-fatal: UI already updated, file persistence is best-effort } - } catch (err) { - console.error('[TASK_REVIEW] Failed to persist approved status:', err); - // Non-fatal: UI already updated, file persistence is best-effort } } else { // Reset and discard all changes from worktree merge in main @@ -539,11 +540,8 @@ export function registerTaskExecutionHandlers( // Notify UI immediately for instant feedback const mainWindow = getMainWindow(); if (mainWindow) { - mainWindow.webContents.send( - IPC_CHANNELS.TASK_STATUS_CHANGE, - taskId, - 'in_progress' - ); + logTaskStatusChange(taskId, 'in_progress'); + taskStateMachine.emitStatusChange(getMainWindow, taskId, 'in_progress'); } // CRITICAL: Persist 'in_progress' status to implementation_plan.json @@ -814,11 +812,8 @@ export function registerTaskExecutionHandlers( // Notify renderer about status change if (mainWindow) { - mainWindow.webContents.send( - IPC_CHANNELS.TASK_STATUS_CHANGE, - taskId, - 'in_progress' - ); + logTaskStatusChange(taskId, 'in_progress'); + taskStateMachine.emitStatusChange(getMainWindow, taskId, 'in_progress'); } } @@ -1188,11 +1183,8 @@ export function registerTaskExecutionHandlers( // Notify renderer of status change const mainWindow = getMainWindow(); if (mainWindow) { - mainWindow.webContents.send( - IPC_CHANNELS.TASK_STATUS_CHANGE, - taskId, - newStatus - ); + logTaskStatusChange(taskId, newStatus); + taskStateMachine.emitStatusChange(getMainWindow, taskId, newStatus); } return { diff --git a/apps/frontend/src/main/ipc-handlers/task/index.ts b/apps/frontend/src/main/ipc-handlers/task/index.ts index e387bf3018..51ee821a61 100644 --- a/apps/frontend/src/main/ipc-handlers/task/index.ts +++ b/apps/frontend/src/main/ipc-handlers/task/index.ts @@ -26,7 +26,7 @@ export function registerTaskHandlers( getMainWindow: () => BrowserWindow | null ): void { // Register CRUD handlers (create, read, update, delete) - registerTaskCRUDHandlers(agentManager); + registerTaskCRUDHandlers(agentManager, getMainWindow); // Register execution handlers (start, stop, review, status management, recovery) registerTaskExecutionHandlers(agentManager, getMainWindow); diff --git a/apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts b/apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts index 5e490e2f14..ea0b5dbfde 100644 --- a/apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts @@ -21,6 +21,8 @@ import { import { persistPlanStatus, updateTaskMetadataPrUrl } from './plan-file-utils'; import { getIsolatedGitEnv, detectWorktreeBranch, refreshGitIndex } from '../../utils/git-isolation'; import { killProcessGracefully } from '../../platform'; +import { getTaskStateManager } from '../../task-state-manager'; +import { isXstateEnabled } from '../../task-state-utils'; import { stripAnsiCodes } from '../../../shared/utils/ansi-sanitizer'; // Regex pattern for validating git branch names @@ -2296,9 +2298,20 @@ export function registerWorktreeHandlers( // Non-fatal: UI will still update, but status may not persist across refresh } + // Notify UI of status change - use XState or fallback const mainWindow = getMainWindow(); if (mainWindow) { - mainWindow.webContents.send(IPC_CHANNELS.TASK_STATUS_CHANGE, taskId, newStatus); + if (isXstateEnabled()) { + const taskStateManager = getTaskStateManager(getMainWindow); + taskStateManager.handleManualStatus( + task, + project, + newStatus as 'human_review' | 'done' + ); + } else { + // Fallback: emit status change directly via IPC + mainWindow.webContents.send(IPC_CHANNELS.TASK_STATUS_CHANGE, taskId, newStatus); + } } resolve({ @@ -2629,7 +2642,13 @@ export function registerWorktreeHandlers( if (!skipStatusChange) { const mainWindow = getMainWindow(); if (mainWindow) { - mainWindow.webContents.send(IPC_CHANNELS.TASK_STATUS_CHANGE, taskId, 'backlog'); + if (isXstateEnabled()) { + const taskStateManager = getTaskStateManager(getMainWindow); + taskStateManager.handleManualStatus(task, project, 'backlog'); + } else { + // Fallback: emit status change directly via IPC + mainWindow.webContents.send(IPC_CHANNELS.TASK_STATUS_CHANGE, taskId, 'backlog'); + } } } diff --git a/apps/frontend/src/main/task-state-machine.ts b/apps/frontend/src/main/task-state-machine.ts new file mode 100644 index 0000000000..1b375819be --- /dev/null +++ b/apps/frontend/src/main/task-state-machine.ts @@ -0,0 +1,75 @@ +import type { BrowserWindow } from 'electron'; +import type { ExecutionProgress, ReviewReason, Task, TaskStatus } from '../shared/types'; +import { logger } from './app-logger'; +import { emitTaskStatusChange } from './task-state-utils'; + +export type ProcessExitSnapshot = { + taskId: string; + exitCode: number; + task: Task | undefined; + hasSubtasks: boolean; + allSubtasksDone: boolean; + requireReviewBeforeCoding: boolean; +}; + +export class TaskStateMachine { + /** + * Emit a task status change via IPC. + * Delegates to shared utility to avoid code duplication with TaskStateManager. + */ + emitStatusChange( + getMainWindow: () => BrowserWindow | null, + taskId: string, + status: TaskStatus, + projectId?: string, + reviewReason?: ReviewReason + ): void { + emitTaskStatusChange(getMainWindow, taskId, status, projectId, reviewReason); + } + + logExecutionProgress(taskId: string, progress: ExecutionProgress, projectId?: string): void { + logger.info( + `[EXECUTION_PROGRESS] taskId=${taskId} phase=${progress.phase} phaseProgress=${progress.phaseProgress} overallProgress=${progress.overallProgress} projectId=${projectId ?? 'none'}` + ); + } + + logProcessExit(snapshot: ProcessExitSnapshot): void { + logger.info( + `[PROCESS_EXITED] taskId=${snapshot.taskId} code=${snapshot.exitCode} hasSubtasks=${snapshot.hasSubtasks} allSubtasksDone=${snapshot.allSubtasksDone} requireReviewBeforeCoding=${snapshot.requireReviewBeforeCoding}` + ); + } + + getStatusForExecutionProgress(progress: ExecutionProgress): TaskStatus | null { + const phaseToStatus: Record = { + idle: null, + planning: 'in_progress', + coding: 'in_progress', + qa_review: 'ai_review', + qa_fixing: 'ai_review', + complete: 'human_review', + failed: 'human_review' + }; + + return phaseToStatus[progress.phase] ?? null; + } + + getStatusForProcessExit(snapshot: ProcessExitSnapshot): { + status?: TaskStatus; + reviewReason?: ReviewReason; + } { + if (snapshot.exitCode !== 0) { + return { status: 'human_review', reviewReason: 'errors' }; + } + + if (snapshot.requireReviewBeforeCoding) { + return { status: 'human_review', reviewReason: 'plan_review' }; + } + + if (snapshot.hasSubtasks && snapshot.allSubtasksDone) { + return { status: 'human_review', reviewReason: 'completed' }; + } + + // No review required and no completed subtasks -> keep current status + return {}; + } +} diff --git a/apps/frontend/src/main/task-state-manager.ts b/apps/frontend/src/main/task-state-manager.ts new file mode 100644 index 0000000000..0801628af8 --- /dev/null +++ b/apps/frontend/src/main/task-state-manager.ts @@ -0,0 +1,418 @@ +import type { BrowserWindow } from 'electron'; +import path from 'path'; +import { existsSync } from 'fs'; +import { createActor } from 'xstate'; +import type { ActorRefFrom, SnapshotFrom } from 'xstate'; +import { taskMachine } from '../shared/state-machines/task-machine'; +import type { ExecutionProgress, Project, ReviewReason, Task, TaskStatus } from '../shared/types'; +import { AUTO_BUILD_PATHS, getSpecsDir } from '../shared/constants'; +import { logger } from './app-logger'; +import { findTaskWorktree } from './worktree-paths'; +import { getPlanPath, persistPlanStatusSync } from './ipc-handlers/task/plan-file-utils'; +import { emitTaskStatusChange } from './task-state-utils'; + +type TaskMachineActor = ActorRefFrom; +type TaskMachineSnapshot = SnapshotFrom; + +type LegacyStatus = { + status: TaskStatus; + reviewReason?: ReviewReason; +}; + +type TaskContext = { + task: Task; + project: Project; +}; + +export class TaskStateManager { + private readonly actors = new Map(); + private readonly taskContext = new Map(); + private readonly lastLegacy = new Map(); + + constructor(private readonly getMainWindow: () => BrowserWindow | null) {} + + /** + * Cleans up resources for a specific task. + * Call this when a task is deleted or no longer needs tracking. + */ + cleanupTask(taskId: string): void { + const actor = this.actors.get(taskId); + if (actor) { + actor.stop(); + this.actors.delete(taskId); + } + this.taskContext.delete(taskId); + this.lastLegacy.delete(taskId); + logger.info(`[TaskStateManager] Cleaned up task: ${taskId}`); + } + + /** + * Cleans up all resources. Call this on app shutdown. + */ + cleanup(): void { + for (const [taskId, actor] of this.actors) { + actor.stop(); + logger.info(`[TaskStateManager] Stopped actor for task: ${taskId}`); + } + this.actors.clear(); + this.taskContext.clear(); + this.lastLegacy.clear(); + logger.info('[TaskStateManager] All resources cleaned up'); + } + + handleExecutionProgress(task: Task, project: Project, progress: ExecutionProgress): void { + this.taskContext.set(task.id, { task, project }); + this.logExecutionProgress(task.id, progress, project.id); + + const actor = this.getActor(task); + const currentState = actor.getSnapshot().value; + const requireReviewBeforeCoding = task.metadata?.requireReviewBeforeCoding === true; + if ( + currentState === 'planning' && + progress.phase !== 'planning' && + progress.phase !== 'idle' + ) { + actor.send({ type: 'PLANNING_COMPLETE', requireReviewBeforeCoding }); + if (requireReviewBeforeCoding) { + return; + } + } + if ( + currentState === 'backlog' && + progress.phase !== 'planning' && + progress.phase !== 'idle' + ) { + actor.send({ type: 'PLANNING_STARTED' }); + actor.send({ type: 'PLANNING_COMPLETE', requireReviewBeforeCoding }); + if (requireReviewBeforeCoding) { + return; + } + } + this.syncActorToPhase(actor, progress.phase, requireReviewBeforeCoding); + const event = this.mapProgressToEvent(progress); + if (event) { + actor.send(event); + } + } + + private syncActorToPhase( + actor: TaskMachineActor, + phase: ExecutionProgress['phase'], + requireReviewBeforeCoding: boolean + ): void { + if (requireReviewBeforeCoding) { + return; + } + + const snapshot = actor.getSnapshot(); + const state = typeof snapshot.value === 'string' ? snapshot.value : null; + if (!state) { + return; + } + + if (state === 'human_review' || state === 'error' || state === 'done') { + return; + } + + const ensurePlanningComplete = () => { + if (state === 'backlog') { + actor.send({ type: 'PLANNING_STARTED' }); + } + if (state === 'backlog' || state === 'planning') { + actor.send({ type: 'PLANNING_COMPLETE', requireReviewBeforeCoding: false }); + } + }; + + switch (phase) { + case 'coding': + ensurePlanningComplete(); + if (state !== 'coding' && state !== 'qa_review' && state !== 'qa_fixing') { + actor.send({ type: 'CODING_STARTED' }); + } + break; + case 'qa_review': + if (state === 'qa_review' || state === 'qa_fixing') { + return; + } + ensurePlanningComplete(); + if (state !== 'coding') { + actor.send({ type: 'CODING_STARTED' }); + } + actor.send({ type: 'QA_STARTED' }); + break; + case 'qa_fixing': + if (state === 'qa_fixing') { + return; + } + ensurePlanningComplete(); + if (state !== 'coding') { + actor.send({ type: 'CODING_STARTED' }); + } + actor.send({ type: 'QA_STARTED' }); + actor.send({ type: 'QA_FAILED' }); + break; + case 'complete': + ensurePlanningComplete(); + if (state !== 'coding') { + actor.send({ type: 'CODING_STARTED' }); + } + actor.send({ type: 'QA_STARTED' }); + actor.send({ type: 'QA_PASSED' }); + break; + default: + break; + } + } + + handleProcessExit( + task: Task, + project: Project, + exitCode: number, + hasSubtasks: boolean, + allSubtasksDone: boolean + ): void { + this.taskContext.set(task.id, { task, project }); + const requireReviewBeforeCoding = task.metadata?.requireReviewBeforeCoding === true; + + this.logProcessExit({ + taskId: task.id, + exitCode, + hasSubtasks, + allSubtasksDone, + requireReviewBeforeCoding + }); + + const actor = this.getActor(task); + actor.send({ + type: 'PROCESS_EXITED', + exitCode, + hasSubtasks, + allSubtasksDone, + requireReviewBeforeCoding + }); + } + + handleUserStopped(task: Task, project: Project): void { + this.taskContext.set(task.id, { task, project }); + this.getActor(task).send({ type: 'USER_STOPPED' }); + } + + handleUserResumed(task: Task, project: Project): void { + this.taskContext.set(task.id, { task, project }); + this.getActor(task).send({ type: 'USER_RESUMED' }); + } + + handleManualStatus( + task: Task, + project: Project, + status: 'backlog' | 'human_review' | 'done', + reviewReason?: ReviewReason + ): void { + this.taskContext.set(task.id, { task, project }); + this.getActor(task).send({ type: 'MANUAL_SET_STATUS', status, reviewReason }); + } + + private getActor(task: Task): TaskMachineActor { + let actor = this.actors.get(task.id); + if (!actor) { + // Map task's persisted status to XState state for proper initialization + // This ensures actors start in the correct state after app restart + const initialState = this.mapTaskStatusToXstateState(task.status, task.reviewReason); + + actor = createActor(taskMachine, { + id: task.id, + // Initialize with the correct state based on task's persisted status + snapshot: initialState ? taskMachine.resolveState({ + value: initialState, + context: { reviewReason: task.reviewReason } + }) : undefined + }); + actor.subscribe((snapshot) => { + // XState v5: 'changed' is only present when state has actually changed + // For subscriptions, we receive updates on every transition attempt, so check if status changed + this.handleSnapshot(task.id, snapshot); + }); + actor.start(); + this.actors.set(task.id, actor); + } + return actor; + } + + /** + * Maps a TaskStatus to the corresponding XState state name. + * Used to initialize actors with the correct state after app restart. + */ + private mapTaskStatusToXstateState(status: TaskStatus, reviewReason?: ReviewReason): string | undefined { + switch (status) { + case 'backlog': + return 'backlog'; + case 'in_progress': + // Could be planning or coding - default to coding as it's more common + return 'coding'; + case 'ai_review': + return 'qa_review'; + case 'human_review': + // Check reviewReason to determine if it's plan review or regular human review + if (reviewReason === 'plan_review') { + return 'awaiting_plan_review'; + } + return 'human_review'; + case 'error': + return 'error'; + case 'done': + return 'done'; + default: + return undefined; + } + } + + private handleSnapshot(taskId: string, snapshot: TaskMachineSnapshot): void { + const legacy = this.mapSnapshotToLegacy(snapshot); + if (!legacy) return; + + const previous = this.lastLegacy.get(taskId); + if (previous && previous.status === legacy.status && previous.reviewReason === legacy.reviewReason) { + return; + } + + this.lastLegacy.set(taskId, legacy); + const context = this.taskContext.get(taskId); + const projectId = context?.project.id; + + if (!previous || previous.status !== legacy.status) { + if (context) { + this.persistStatus(context.project, context.task, legacy.status); + } + } + + this.emitStatusChange(taskId, legacy.status, projectId, legacy.reviewReason); + } + + private mapSnapshotToLegacy(snapshot: TaskMachineSnapshot): LegacyStatus | null { + const state = typeof snapshot.value === 'string' ? snapshot.value : null; + if (!state) { + logger.warn('[TaskStateManager] Unexpected xstate snapshot value:', snapshot.value); + return null; + } + + switch (state) { + case 'backlog': + return { status: 'backlog' }; + case 'planning': + case 'coding': + return { status: 'in_progress' }; + case 'awaiting_plan_review': + return { status: 'human_review', reviewReason: 'plan_review' }; + case 'qa_review': + case 'qa_fixing': + return { status: 'ai_review' }; + case 'human_review': + return { status: 'human_review', reviewReason: snapshot.context.reviewReason }; + case 'error': + return { status: 'error', reviewReason: 'errors' }; + case 'done': + return { status: 'done' }; + default: + logger.warn('[TaskStateManager] Unhandled xstate state:', state); + return null; + } + } + + private mapProgressToEvent(progress: ExecutionProgress) { + switch (progress.phase) { + case 'planning': + return { type: 'PLANNING_STARTED' as const }; + case 'coding': + return { type: 'CODING_STARTED' as const }; + case 'qa_review': + return { type: 'QA_STARTED' as const }; + case 'qa_fixing': + return { type: 'QA_FAILED' as const }; + case 'complete': + return { type: 'QA_PASSED' as const }; + default: + return null; + } + } + + private persistStatus(project: Project, task: Task, status: TaskStatus): void { + const mainPlanPath = getPlanPath(project, task); + const projectId = project.id; + const taskSpecId = task.specId; + const projectPath = project.path; + const autoBuildPath = project.autoBuildPath; + + const mainPersisted = persistPlanStatusSync(mainPlanPath, status, projectId); + if (mainPersisted) { + logger.info(`[TaskStateManager] Persisted status to main plan: ${status}`); + } + + const worktreePath = findTaskWorktree(projectPath, taskSpecId); + if (worktreePath) { + const specsBaseDir = getSpecsDir(autoBuildPath); + const worktreePlanPath = path.join( + worktreePath, + specsBaseDir, + taskSpecId, + AUTO_BUILD_PATHS.IMPLEMENTATION_PLAN + ); + if (existsSync(worktreePlanPath)) { + const worktreePersisted = persistPlanStatusSync(worktreePlanPath, status, projectId); + if (worktreePersisted) { + logger.info(`[TaskStateManager] Persisted status to worktree plan: ${status}`); + } + } + } + } + + /** + * Emit a task status change via IPC. + * Delegates to shared utility to avoid code duplication with TaskStateMachine. + */ + private emitStatusChange( + taskId: string, + status: TaskStatus, + projectId?: string, + reviewReason?: ReviewReason + ): void { + emitTaskStatusChange(this.getMainWindow, taskId, status, projectId, reviewReason); + } + + private logExecutionProgress(taskId: string, progress: ExecutionProgress, projectId?: string): void { + logger.info( + `[EXECUTION_PROGRESS] taskId=${taskId} phase=${progress.phase} phaseProgress=${progress.phaseProgress} overallProgress=${progress.overallProgress} projectId=${projectId ?? 'none'}` + ); + } + + private logProcessExit(snapshot: { + taskId: string; + exitCode: number; + hasSubtasks: boolean; + allSubtasksDone: boolean; + requireReviewBeforeCoding: boolean; + }): void { + logger.info( + `[PROCESS_EXITED] taskId=${snapshot.taskId} code=${snapshot.exitCode} hasSubtasks=${snapshot.hasSubtasks} allSubtasksDone=${snapshot.allSubtasksDone} requireReviewBeforeCoding=${snapshot.requireReviewBeforeCoding}` + ); + } +} + +let taskStateManager: TaskStateManager | null = null; + +export function getTaskStateManager(getMainWindow: () => BrowserWindow | null): TaskStateManager { + if (!taskStateManager) { + taskStateManager = new TaskStateManager(getMainWindow); + } + return taskStateManager; +} + +/** + * Cleans up and resets the singleton instance. + * Call this on app shutdown to prevent memory leaks. + */ +export function cleanupTaskStateManager(): void { + if (taskStateManager) { + taskStateManager.cleanup(); + taskStateManager = null; + } +} diff --git a/apps/frontend/src/main/task-state-utils.ts b/apps/frontend/src/main/task-state-utils.ts new file mode 100644 index 0000000000..30f9289bc7 --- /dev/null +++ b/apps/frontend/src/main/task-state-utils.ts @@ -0,0 +1,72 @@ +import type { BrowserWindow } from 'electron'; +import { DEFAULT_APP_SETTINGS, IPC_CHANNELS } from '../shared/constants'; +import type { AppSettings, ReviewReason, TaskStatus } from '../shared/types'; +import { readSettingsFile } from './settings-utils'; +import { safeSendToRenderer } from './ipc-handlers/utils'; +import { logger } from './app-logger'; + +// Cache for isXstateEnabled to avoid synchronous disk reads on every call +// This is important because isXstateEnabled() is called from hot paths like execution-progress events +let cachedXstateEnabled: boolean | null = null; +let xstateCacheTimestamp = 0; +const XSTATE_CACHE_TTL_MS = 5000; // 5 seconds + +/** + * Check if XState task machine is enabled. + * Caches the result for 5 seconds to avoid synchronous disk reads on every call. + */ +export function isXstateEnabled(): boolean { + const now = Date.now(); + if (cachedXstateEnabled !== null && now - xstateCacheTimestamp < XSTATE_CACHE_TTL_MS) { + return cachedXstateEnabled; + } + + const savedSettings = readSettingsFile(); + const settings = { ...DEFAULT_APP_SETTINGS, ...savedSettings } as AppSettings; + cachedXstateEnabled = settings.useXstateTaskMachine === true; + xstateCacheTimestamp = now; + return cachedXstateEnabled; +} + +/** + * Invalidate the XState enabled cache. + * Call this when settings are changed to ensure the new value is picked up immediately. + */ +export function invalidateXstateCache(): void { + cachedXstateEnabled = null; + xstateCacheTimestamp = 0; +} + +export function isDebugEnabled(): boolean { + return process.env.DEBUG === 'true'; +} + +/** + * Shared utility for emitting task status changes via IPC. + * Used by both TaskStateMachine (fallback) and TaskStateManager (XState). + * Extracts common emit logic to avoid code duplication. + * + * @param getMainWindow - Function to get the main BrowserWindow + * @param taskId - The task ID + * @param status - The new task status + * @param projectId - Optional project ID for multi-project filtering + * @param reviewReason - Optional review reason + */ +export function emitTaskStatusChange( + getMainWindow: () => BrowserWindow | null, + taskId: string, + status: TaskStatus, + projectId?: string, + reviewReason?: ReviewReason +): void { + if (isDebugEnabled()) { + const payload = { + taskId, + status, + reviewReason: reviewReason ?? 'none', + projectId: projectId ?? 'none' + }; + logger.info(`[TASK_STATUS_CHANGE] ${JSON.stringify(payload)}`); + } + safeSendToRenderer(getMainWindow, IPC_CHANNELS.TASK_STATUS_CHANGE, taskId, status, projectId, reviewReason); +} diff --git a/apps/frontend/src/preload/api/task-api.ts b/apps/frontend/src/preload/api/task-api.ts index 290fa7875c..27eea202ff 100644 --- a/apps/frontend/src/preload/api/task-api.ts +++ b/apps/frontend/src/preload/api/task-api.ts @@ -73,7 +73,7 @@ export interface TaskAPI { onTaskProgress: (callback: (taskId: string, plan: ImplementationPlan, projectId?: string) => void) => () => void; onTaskError: (callback: (taskId: string, error: string, projectId?: string) => void) => () => void; onTaskLog: (callback: (taskId: string, log: string, projectId?: string) => void) => () => void; - onTaskStatusChange: (callback: (taskId: string, status: TaskStatus, projectId?: string) => void) => () => void; + onTaskStatusChange: (callback: (taskId: string, status: TaskStatus, projectId?: string, reviewReason?: import('../../shared/types').ReviewReason) => void) => () => void; onTaskExecutionProgress: ( callback: (taskId: string, progress: import('../../shared/types').ExecutionProgress, projectId?: string) => void ) => () => void; @@ -235,15 +235,16 @@ export const createTaskAPI = (): TaskAPI => ({ }, onTaskStatusChange: ( - callback: (taskId: string, status: TaskStatus, projectId?: string) => void + callback: (taskId: string, status: TaskStatus, projectId?: string, reviewReason?: import('../../shared/types').ReviewReason) => void ): (() => void) => { const handler = ( _event: Electron.IpcRendererEvent, taskId: string, status: TaskStatus, - projectId?: string + projectId?: string, + reviewReason?: import('../../shared/types').ReviewReason ): void => { - callback(taskId, status, projectId); + callback(taskId, status, projectId, reviewReason); }; ipcRenderer.on(IPC_CHANNELS.TASK_STATUS_CHANGE, handler); return () => { diff --git a/apps/frontend/src/renderer/components/TaskCard.tsx b/apps/frontend/src/renderer/components/TaskCard.tsx index 16e7ca1ce8..26d968cda6 100644 --- a/apps/frontend/src/renderer/components/TaskCard.tsx +++ b/apps/frontend/src/renderer/components/TaskCard.tsx @@ -143,7 +143,13 @@ export const TaskCard = memo(function TaskCard({ const isRunning = task.status === 'in_progress'; const executionPhase = task.executionProgress?.phase; - const hasActiveExecution = executionPhase && executionPhase !== 'idle' && executionPhase !== 'complete' && executionPhase !== 'failed'; + const isPlanReview = task.status === 'human_review' && task.reviewReason === 'plan_review'; + const hasActiveExecution = + !isPlanReview && + executionPhase && + executionPhase !== 'idle' && + executionPhase !== 'complete' && + executionPhase !== 'failed'; // Check if task is in human_review but has no completed subtasks (crashed/incomplete) const isIncomplete = isIncompleteHumanReview(task); @@ -349,6 +355,8 @@ export const TaskCard = memo(function TaskCard({ return { label: t('reviewReason.qaIssues'), variant: 'warning' }; case 'plan_review': return { label: t('reviewReason.approvePlan'), variant: 'warning' }; + case 'stopped': + return { label: 'Stopped', variant: 'warning' }; default: return null; } diff --git a/apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx b/apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx index 3a6ce51456..d225a82f65 100644 --- a/apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx +++ b/apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx @@ -393,7 +393,8 @@ function TaskDetailModalContent({ open, task, onOpenChange, onSwitchToTerminals, > {task.reviewReason === 'completed' ? 'Completed' : task.reviewReason === 'errors' ? 'Has Errors' : - task.reviewReason === 'plan_review' ? 'Approve Plan' : 'QA Issues'} + task.reviewReason === 'plan_review' ? 'Approve Plan' : + task.reviewReason === 'stopped' ? 'Stopped' : 'QA Issues'} )} @@ -406,6 +407,11 @@ function TaskDetailModalContent({ open, task, onOpenChange, onSwitchToTerminals, )} + {window.DEBUG && ( +
+ status={task.status} reviewReason={task.reviewReason ?? 'none'} phase={task.executionProgress?.phase ?? 'none'} +
+ )}
+ ) : task && task.status === 'human_review' && (