From 08d1826dc7393e85b2f7525b266ccfe26311e073 Mon Sep 17 00:00:00 2001 From: kaigler Date: Sun, 25 Jan 2026 01:17:10 -0600 Subject: [PATCH 01/15] wip xstate: centralize status + review/logging --- apps/backend/runners/spec_runner.py | 3 + apps/backend/spec/pipeline/orchestrator.py | 39 +++++--- .../src/main/claude-profile-manager.ts | 3 +- apps/frontend/src/main/index.ts | 72 +++++++------- .../ipc-handlers/agent-events-handlers.ts | 93 ++++++++++--------- .../ipc-handlers/task/execution-handlers.ts | 51 +++++----- .../ipc-handlers/task/worktree-handlers.ts | 6 +- apps/frontend/src/main/task-state-machine.ts | 73 +++++++++++++++ 8 files changed, 219 insertions(+), 121 deletions(-) create mode 100644 apps/frontend/src/main/task-state-machine.ts diff --git a/apps/backend/runners/spec_runner.py b/apps/backend/runners/spec_runner.py index 53f64b357b..2e8204a771 100644 --- a/apps/backend/runners/spec_runner.py +++ b/apps/backend/runners/spec_runner.py @@ -318,6 +318,9 @@ 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/src/main/claude-profile-manager.ts b/apps/frontend/src/main/claude-profile-manager.ts index 5cd8cb0785..b6bf2e9b7f 100644 --- a/apps/frontend/src/main/claude-profile-manager.ts +++ b/apps/frontend/src/main/claude-profile-manager.ts @@ -44,6 +44,7 @@ import { } from './claude-profile/profile-scorer'; import { DEFAULT_CLAUDE_CONFIG_DIR, + getEmailFromConfigDir, generateProfileId as generateProfileIdImpl, createProfileDirectory as createProfileDirectoryImpl, isProfileAuthenticated as isProfileAuthenticatedImpl, @@ -111,8 +112,6 @@ export class ClaudeProfileManager { continue; } - // Import dynamically to avoid circular dependency issues - const { getEmailFromConfigDir } = require('./claude-profile/profile-utils'); const configEmail = getEmailFromConfigDir(profile.configDir); if (configEmail && profile.email !== configEmail) { diff --git a/apps/frontend/src/main/index.ts b/apps/frontend/src/main/index.ts index eebbcc7c7d..48c0f8dd14 100644 --- a/apps/frontend/src/main/index.ts +++ b/apps/frontend/src/main/index.ts @@ -303,7 +303,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; @@ -315,42 +316,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 + } } } } 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..573c809c08 100644 --- a/apps/frontend/src/main/ipc-handlers/agent-events-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/agent-events-handlers.ts @@ -28,6 +28,7 @@ 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"; /** * Validates status transitions to prevent invalid state changes. @@ -109,6 +110,7 @@ export function registerAgenteventsHandlers( agentManager: AgentManager, getMainWindow: () => BrowserWindow | null ): void { + const taskStateMachine = new TaskStateMachine(); // ============================================ // Agent Manager Events → Renderer // ============================================ @@ -247,6 +249,19 @@ 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, + }); + if (code === 0) { notificationService.notifyReviewNeeded(taskTitle, project.id, taskId); @@ -255,41 +270,47 @@ export function registerAgenteventsHandlers( // 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( + const decision = taskStateMachine.getStatusForProcessExit({ + taskId, + exitCode: code ?? 0, + task, + hasSubtasks, + allSubtasksDone, + requireReviewBeforeCoding, + }); + + if (isActiveStatus && decision.status) { + persistStatus(decision.status); + taskStateMachine.emitStatusChange( getMainWindow, - IPC_CHANNELS.TASK_STATUS_CHANGE, taskId, - "human_review" as TaskStatus, - projectId + decision.status, + projectId, + decision.reviewReason ); - } 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 + } else if (isActiveStatus && !decision.status) { console.warn( - `[Task ${taskId}] Process exited but no subtasks created yet - keeping current status (${task.status})` + `[Task ${taskId}] Process exited but status unchanged (current status: ${task.status})` ); } } else { notificationService.notifyTaskFailed(taskTitle, project.id, taskId); - persistStatus("human_review"); - // Include projectId for multi-project filtering (issue #723) - safeSendToRenderer( + const decision = taskStateMachine.getStatusForProcessExit({ + taskId, + exitCode: code ?? 0, + task, + hasSubtasks, + allSubtasksDone, + requireReviewBeforeCoding, + }); + const nextStatus = decision.status ?? "human_review"; + persistStatus(nextStatus); + taskStateMachine.emitStatusChange( getMainWindow, - IPC_CHANNELS.TASK_STATUS_CHANGE, taskId, - "human_review" as TaskStatus, - projectId + nextStatus, + projectId, + decision.reviewReason ); } } @@ -303,6 +324,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,27 +335,11 @@ 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]; + const newStatus = taskStateMachine.getStatusForExecutionProgress(progress); // 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 - ); + taskStateMachine.emitStatusChange(getMainWindow, 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, 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 3cd68bbd09..e9cdda6036 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,7 @@ import { import { findTaskWorktree } from '../../worktree-paths'; import { projectStore } from '../../project-store'; import { getIsolatedGitEnv } from '../../utils/git-isolation'; +import { TaskStateMachine } from '../../task-state-machine'; /** * Atomic file write to prevent TOCTOU race conditions. @@ -56,6 +57,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 +114,7 @@ export function registerTaskExecutionHandlers( agentManager: AgentManager, getMainWindow: () => BrowserWindow | null ): void { + const taskStateMachine = new TaskStateMachine(); /** * Start a task */ @@ -256,11 +265,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) { @@ -307,11 +313,8 @@ export function registerTaskExecutionHandlers( const ipcSentAt = Date.now(); const mainWindow = getMainWindow(); if (mainWindow) { - mainWindow.webContents.send( - IPC_CHANNELS.TASK_STATUS_CHANGE, - taskId, - 'backlog' - ); + logTaskStatusChange(taskId, 'backlog'); + taskStateMachine.emitStatusChange(getMainWindow, taskId, 'backlog'); } if (DEBUG) { @@ -394,11 +397,8 @@ export function registerTaskExecutionHandlers( // Notify UI immediately for instant feedback const mainWindow = getMainWindow(); if (mainWindow) { - mainWindow.webContents.send( - IPC_CHANNELS.TASK_STATUS_CHANGE, - taskId, - 'done' - ); + logTaskStatusChange(taskId, 'done'); + taskStateMachine.emitStatusChange(getMainWindow, taskId, 'done'); } // CRITICAL: Persist 'done' status to implementation_plan.json @@ -537,11 +537,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 @@ -817,11 +814,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'); } } @@ -1191,11 +1185,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/worktree-handlers.ts b/apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts index a9eed42637..816631fecf 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,7 @@ import { import { persistPlanStatus, updateTaskMetadataPrUrl } from './plan-file-utils'; import { getIsolatedGitEnv } from '../../utils/git-isolation'; import { killProcessGracefully } from '../../platform'; +import { TaskStateMachine } from '../../task-state-machine'; // Regex pattern for validating git branch names const GIT_BRANCH_REGEX = /^[a-zA-Z0-9][a-zA-Z0-9._/-]*[a-zA-Z0-9]$|^[a-zA-Z0-9]$/; @@ -1624,6 +1625,7 @@ export function registerWorktreeHandlers( pythonEnvManager: PythonEnvManager, getMainWindow: () => BrowserWindow | null ): void { + const taskStateMachine = new TaskStateMachine(); /** * Get the worktree status for a task * Per-spec architecture: Each spec has its own worktree at .auto-claude/worktrees/tasks/{spec-name}/ @@ -2296,7 +2298,7 @@ export function registerWorktreeHandlers( const mainWindow = getMainWindow(); if (mainWindow) { - mainWindow.webContents.send(IPC_CHANNELS.TASK_STATUS_CHANGE, taskId, newStatus); + taskStateMachine.emitStatusChange(getMainWindow, taskId, newStatus); } resolve({ @@ -2609,7 +2611,7 @@ export function registerWorktreeHandlers( if (!skipStatusChange) { const mainWindow = getMainWindow(); if (mainWindow) { - mainWindow.webContents.send(IPC_CHANNELS.TASK_STATUS_CHANGE, taskId, 'backlog'); + taskStateMachine.emitStatusChange(getMainWindow, 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..01e487241e --- /dev/null +++ b/apps/frontend/src/main/task-state-machine.ts @@ -0,0 +1,73 @@ +import type { BrowserWindow } from 'electron'; +import { IPC_CHANNELS } from '../shared/constants'; +import type { ExecutionProgress, ReviewReason, Task, TaskStatus } from '../shared/types'; +import { logger } from './app-logger'; +import { safeSendToRenderer } from './ipc-handlers/utils'; + +export type ProcessExitSnapshot = { + taskId: string; + exitCode: number; + task: Task | undefined; + hasSubtasks: boolean; + allSubtasksDone: boolean; + requireReviewBeforeCoding: boolean; +}; + +export class TaskStateMachine { + emitStatusChange( + getMainWindow: () => BrowserWindow | null, + taskId: string, + status: TaskStatus, + projectId?: string, + reviewReason?: ReviewReason + ): void { + const reason = reviewReason ?? 'none'; + logger.info(`[TASK_STATUS_CHANGE] taskId=${taskId} status=${status} reviewReason=${reason}`); + safeSendToRenderer(getMainWindow, IPC_CHANNELS.TASK_STATUS_CHANGE, taskId, status, projectId); + } + + 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' }; + } + + return {}; + } +} From ea4e1097dc3fdd351555ac61c3ab4abe689724d8 Mon Sep 17 00:00:00 2001 From: kaigler Date: Sun, 25 Jan 2026 01:28:46 -0600 Subject: [PATCH 02/15] xstate: avoid forcing review on clean exit --- apps/frontend/src/main/task-state-machine.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/frontend/src/main/task-state-machine.ts b/apps/frontend/src/main/task-state-machine.ts index 01e487241e..0875ebd7a3 100644 --- a/apps/frontend/src/main/task-state-machine.ts +++ b/apps/frontend/src/main/task-state-machine.ts @@ -68,6 +68,7 @@ export class TaskStateMachine { return { status: 'human_review', reviewReason: 'completed' }; } + // No review required and no completed subtasks -> keep current status return {}; } } From cf537089f806f481fba4fc08994855cd930f561f Mon Sep 17 00:00:00 2001 From: kaigler Date: Sun, 25 Jan 2026 01:56:59 -0600 Subject: [PATCH 03/15] xstate: propagate reviewReason + plan guardrails --- apps/frontend/src/main/task-state-machine.ts | 2 +- apps/frontend/src/preload/api/task-api.ts | 9 +++-- apps/frontend/src/renderer/hooks/useIpc.ts | 9 +++-- .../src/renderer/stores/task-store.ts | 37 +++++++++++++++++-- apps/frontend/src/shared/types/ipc.ts | 3 +- 5 files changed, 46 insertions(+), 14 deletions(-) diff --git a/apps/frontend/src/main/task-state-machine.ts b/apps/frontend/src/main/task-state-machine.ts index 0875ebd7a3..83661ca81f 100644 --- a/apps/frontend/src/main/task-state-machine.ts +++ b/apps/frontend/src/main/task-state-machine.ts @@ -23,7 +23,7 @@ export class TaskStateMachine { ): void { const reason = reviewReason ?? 'none'; logger.info(`[TASK_STATUS_CHANGE] taskId=${taskId} status=${status} reviewReason=${reason}`); - safeSendToRenderer(getMainWindow, IPC_CHANNELS.TASK_STATUS_CHANGE, taskId, status, projectId); + safeSendToRenderer(getMainWindow, IPC_CHANNELS.TASK_STATUS_CHANGE, taskId, status, projectId, reviewReason); } logExecutionProgress(taskId: string, progress: ExecutionProgress, projectId?: string): void { diff --git a/apps/frontend/src/preload/api/task-api.ts b/apps/frontend/src/preload/api/task-api.ts index f00cbea24a..138f9ccb48 100644 --- a/apps/frontend/src/preload/api/task-api.ts +++ b/apps/frontend/src/preload/api/task-api.ts @@ -70,7 +70,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; @@ -228,15 +228,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/hooks/useIpc.ts b/apps/frontend/src/renderer/hooks/useIpc.ts index 8bbeaeaaf9..815a7ee44d 100644 --- a/apps/frontend/src/renderer/hooks/useIpc.ts +++ b/apps/frontend/src/renderer/hooks/useIpc.ts @@ -14,6 +14,7 @@ import type { ImplementationPlan, TaskStatus, RoadmapGenerationStatus, Roadmap, */ interface BatchedUpdate { status?: TaskStatus; + reviewReason?: import('../../shared/types').ReviewReason; progress?: ExecutionProgress; plan?: ImplementationPlan; logs?: string[]; // Batched log lines @@ -24,7 +25,7 @@ interface BatchedUpdate { * Store action references type for batch flushing. */ interface StoreActions { - updateTaskStatus: (taskId: string, status: TaskStatus) => void; + updateTaskStatus: (taskId: string, status: TaskStatus, reviewReason?: import('../../shared/types').ReviewReason) => void; updateExecutionProgress: (taskId: string, progress: ExecutionProgress) => void; updateTaskFromPlan: (taskId: string, plan: ImplementationPlan) => void; batchAppendLogs: (taskId: string, logs: string[]) => void; @@ -66,7 +67,7 @@ function flushBatch(): void { totalUpdates++; } if (updates.status) { - actions.updateTaskStatus(taskId, updates.status); + actions.updateTaskStatus(taskId, updates.status, updates.reviewReason); totalUpdates++; } if (updates.progress) { @@ -198,10 +199,10 @@ export function useIpcListeners(): void { ); const cleanupStatus = window.electronAPI.onTaskStatusChange( - (taskId: string, status: TaskStatus, projectId?: string) => { + (taskId: string, status: TaskStatus, projectId?: string, reviewReason?: import('../../shared/types').ReviewReason) => { // Filter by project to prevent multi-project interference if (!isTaskForCurrentProject(projectId)) return; - queueUpdate(taskId, { status }); + queueUpdate(taskId, { status, reviewReason }); } ); diff --git a/apps/frontend/src/renderer/stores/task-store.ts b/apps/frontend/src/renderer/stores/task-store.ts index b82abacb01..15e12959f2 100644 --- a/apps/frontend/src/renderer/stores/task-store.ts +++ b/apps/frontend/src/renderer/stores/task-store.ts @@ -15,7 +15,7 @@ interface TaskState { setTasks: (tasks: Task[]) => void; addTask: (task: Task) => void; updateTask: (taskId: string, updates: Partial) => void; - updateTaskStatus: (taskId: string, status: TaskStatus) => void; + updateTaskStatus: (taskId: string, status: TaskStatus, reviewReason?: ReviewReason) => void; updateTaskFromPlan: (taskId: string, plan: ImplementationPlan) => void; updateExecutionProgress: (taskId: string, progress: Partial) => void; appendLog: (taskId: string, log: string) => void; @@ -93,8 +93,17 @@ function updateTaskAtIndex(tasks: Task[], index: number, updater: (task: Task) = * Returns true if valid, false if invalid/incomplete. */ function validatePlanData(plan: ImplementationPlan): boolean { + const allowEmptyPhases = + plan.planStatus === 'planning' || + plan.planStatus === 'in_progress' || + plan.status === 'planning' || + plan.status === 'in_progress'; + // Validate plan has phases array if (!plan.phases || !Array.isArray(plan.phases)) { + if (allowEmptyPhases) { + return true; + } console.warn('[validatePlanData] Invalid plan: missing or invalid phases array'); return false; } @@ -103,6 +112,9 @@ function validatePlanData(plan: ImplementationPlan): boolean { for (let i = 0; i < plan.phases.length; i++) { const phase = plan.phases[i]; if (!phase || !phase.subtasks || !Array.isArray(phase.subtasks)) { + if (allowEmptyPhases) { + continue; + } console.warn(`[validatePlanData] Invalid phase ${i}: missing or invalid subtasks array`); return false; } @@ -201,7 +213,7 @@ export const useTaskStore = create((set, get) => ({ }; }), - updateTaskStatus: (taskId, status) => { + updateTaskStatus: (taskId, status, reviewReason) => { // Capture old status before update const state = get(); const index = findTaskIndex(state.tasks, taskId); @@ -246,7 +258,11 @@ export const useTaskStore = create((set, get) => ({ newPhase: executionProgress?.phase }); - return { ...t, status, executionProgress, updatedAt: new Date() }; + const nextReviewReason = status === 'human_review' + ? (reviewReason ?? t.reviewReason) + : undefined; + + return { ...t, status, reviewReason: nextReviewReason, executionProgress, updatedAt: new Date() }; }) }; }); @@ -348,6 +364,8 @@ export const useTaskStore = create((set, get) => ({ // Note: ImplementationPlan type already defines status?: TaskStatus const planStatus = plan.status; const isExplicitHumanReview = planStatus === 'human_review'; + const isPlanReviewStage = plan.planStatus === 'review'; + const isActivePlanStatus = planStatus === 'planning' || planStatus === 'coding' || planStatus === 'in_progress'; // FIX (ACS-203): Add defensive check for terminal status transitions // Before allowing transition to 'done', 'human_review', or 'ai_review', verify: @@ -384,7 +402,9 @@ export const useTaskStore = create((set, get) => ({ // 3. NOT in a terminal task status (pr_created, done) - finalized workflow states // 4. Plan doesn't explicitly say human_review // 5. Would not create an invalid terminal transition (ACS-203) - if (!isInActivePhase && !isInTerminalPhase && !isInTerminalStatus && !isExplicitHumanReview) { + if (isActivePlanStatus) { + status = 'in_progress'; + } else if (!isInActivePhase && !isInTerminalPhase && !isInTerminalStatus && !isExplicitHumanReview) { if (allCompleted && hasSubtasks) { // FIX (Flip-Flop Bug): Don't downgrade from terminal statuses to ai_review // Once a task reaches human_review or done, it should stay there @@ -400,6 +420,12 @@ export const useTaskStore = create((set, get) => ({ } } + // If plan is explicitly in review and user requested review-before-coding, + // ensure reviewReason is plan_review (used by UI to show Approve Plan) + if (isPlanReviewStage && (isExplicitHumanReview || t.status === 'human_review') && t.metadata?.requireReviewBeforeCoding) { + reviewReason = 'plan_review'; + } + // FIX (ACS-203): Final validation - prevent invalid terminal status transitions // This catches cases where the logic above would set a terminal status // but the subtask state doesn't support it (e.g., empty subtasks array) @@ -1078,6 +1104,9 @@ export function isIncompleteHumanReview(task: Task): boolean { // JSON error tasks are intentionally in human_review with no subtasks - not incomplete if (task.reviewReason === 'errors') return false; + // Plan review tasks are intentionally in human_review before coding - not incomplete + if (task.reviewReason === 'plan_review') return false; + // If no subtasks defined, task hasn't been planned yet (shouldn't be in human_review) if (!task.subtasks || task.subtasks.length === 0) return true; diff --git a/apps/frontend/src/shared/types/ipc.ts b/apps/frontend/src/shared/types/ipc.ts index f6c1111ca8..0cb82f5041 100644 --- a/apps/frontend/src/shared/types/ipc.ts +++ b/apps/frontend/src/shared/types/ipc.ts @@ -28,6 +28,7 @@ import type { import type { Task, TaskStatus, + ReviewReason, TaskStartOptions, ImplementationPlan, ExecutionProgress, @@ -190,7 +191,7 @@ export interface ElectronAPI { onTaskProgress: (callback: (taskId: string, plan: ImplementationPlan) => void) => () => void; onTaskError: (callback: (taskId: string, error: string) => void) => () => void; onTaskLog: (callback: (taskId: string, log: string) => void) => () => void; - onTaskStatusChange: (callback: (taskId: string, status: TaskStatus) => void) => () => void; + onTaskStatusChange: (callback: (taskId: string, status: TaskStatus, reviewReason?: ReviewReason) => void) => () => void; onTaskExecutionProgress: (callback: (taskId: string, progress: ExecutionProgress) => void) => () => void; // Terminal operations From a221b01235ee02b8ffd512d395c96bb68d69c131 Mon Sep 17 00:00:00 2001 From: kaigler Date: Sun, 25 Jan 2026 02:05:19 -0600 Subject: [PATCH 04/15] fix: resolve project dir from spec location --- apps/backend/cli/main.py | 6 ++++++ apps/backend/cli/utils.py | 23 +++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/apps/backend/cli/main.py b/apps/backend/cli/main.py index dc1f6a9c32..68af2a04e8 100644 --- a/apps/backend/cli/main.py +++ b/apps/backend/cli/main.py @@ -33,6 +33,7 @@ DEFAULT_MODEL, find_spec, get_project_dir, + resolve_project_dir_for_spec, print_banner, setup_environment, ) @@ -321,6 +322,11 @@ 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. From b34dd2d7c8f02d3db9c0eaa9bd2e2589e6520efe Mon Sep 17 00:00:00 2001 From: kaigler Date: Sun, 25 Jan 2026 02:35:25 -0600 Subject: [PATCH 05/15] xstate: stabilize plan review status display --- .../ipc-handlers/agent-events-handlers.ts | 22 +++++++++++++------ .../src/renderer/components/TaskCard.tsx | 8 ++++++- .../src/renderer/stores/task-store.ts | 10 ++++++++- 3 files changed, 31 insertions(+), 9 deletions(-) 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 573c809c08..0a0987a4d2 100644 --- a/apps/frontend/src/main/ipc-handlers/agent-events-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/agent-events-handlers.ts @@ -278,17 +278,21 @@ export function registerAgenteventsHandlers( allSubtasksDone, requireReviewBeforeCoding, }); + const decisionWithFallback = + requireReviewBeforeCoding && !decision.status + ? { status: "human_review", reviewReason: "plan_review" } + : decision; - if (isActiveStatus && decision.status) { - persistStatus(decision.status); + if (decisionWithFallback.status) { + persistStatus(decisionWithFallback.status); taskStateMachine.emitStatusChange( getMainWindow, taskId, - decision.status, + decisionWithFallback.status, projectId, - decision.reviewReason + decisionWithFallback.reviewReason ); - } else if (isActiveStatus && !decision.status) { + } else if (isActiveStatus) { console.warn( `[Task ${taskId}] Process exited but status unchanged (current status: ${task.status})` ); @@ -303,14 +307,18 @@ export function registerAgenteventsHandlers( allSubtasksDone, requireReviewBeforeCoding, }); - const nextStatus = decision.status ?? "human_review"; + const decisionWithFallback = + requireReviewBeforeCoding && !decision.status + ? { status: "human_review", reviewReason: "plan_review" } + : decision; + const nextStatus = decisionWithFallback.status ?? "human_review"; persistStatus(nextStatus); taskStateMachine.emitStatusChange( getMainWindow, taskId, nextStatus, projectId, - decision.reviewReason + decisionWithFallback.reviewReason ); } } diff --git a/apps/frontend/src/renderer/components/TaskCard.tsx b/apps/frontend/src/renderer/components/TaskCard.tsx index 16e7ca1ce8..d51d072d00 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); diff --git a/apps/frontend/src/renderer/stores/task-store.ts b/apps/frontend/src/renderer/stores/task-store.ts index 15e12959f2..8abde228f3 100644 --- a/apps/frontend/src/renderer/stores/task-store.ts +++ b/apps/frontend/src/renderer/stores/task-store.ts @@ -402,7 +402,12 @@ export const useTaskStore = create((set, get) => ({ // 3. NOT in a terminal task status (pr_created, done) - finalized workflow states // 4. Plan doesn't explicitly say human_review // 5. Would not create an invalid terminal transition (ACS-203) - if (isActivePlanStatus) { + const shouldForceInProgress = + isActivePlanStatus && + !isExplicitHumanReview && + !isPlanReviewStage && + !(t.status === 'human_review' && reviewReason === 'plan_review'); + if (shouldForceInProgress) { status = 'in_progress'; } else if (!isInActivePhase && !isInTerminalPhase && !isInTerminalStatus && !isExplicitHumanReview) { if (allCompleted && hasSubtasks) { @@ -424,6 +429,9 @@ export const useTaskStore = create((set, get) => ({ // ensure reviewReason is plan_review (used by UI to show Approve Plan) if (isPlanReviewStage && (isExplicitHumanReview || t.status === 'human_review') && t.metadata?.requireReviewBeforeCoding) { reviewReason = 'plan_review'; + } else if (status === 'human_review' && reviewReason === 'plan_review' && allCompleted && hasSubtasks) { + // If coding finished and subtasks are complete, upgrade to completed review + reviewReason = 'completed'; } // FIX (ACS-203): Final validation - prevent invalid terminal status transitions From 3793974dad891793ce2bbc70aecb9577259cc8b7 Mon Sep 17 00:00:00 2001 From: kaigler Date: Sun, 25 Jan 2026 12:50:08 -0600 Subject: [PATCH 06/15] ui: show status/review/phase in task detail header --- .../src/renderer/components/task-detail/TaskDetailModal.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx b/apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx index 3a6ce51456..4d58e49be0 100644 --- a/apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx +++ b/apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx @@ -406,6 +406,9 @@ function TaskDetailModalContent({ open, task, onOpenChange, onSwitchToTerminals, )} +
+ status={task.status} reviewReason={task.reviewReason ?? 'none'} phase={task.executionProgress?.phase ?? 'none'} +
+ ) : task && task.status === 'human_review' && (
-
- status={task.status} reviewReason={task.reviewReason ?? 'none'} phase={task.executionProgress?.phase ?? 'none'} -
+ {window.DEBUG && ( +
+ status={task.status} reviewReason={task.reviewReason ?? 'none'} phase={task.executionProgress?.phase ?? 'none'} +
+ )}