-
-
Notifications
You must be signed in to change notification settings - Fork 1k
fix: prevent premature human_review status during validation #292
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: prevent premature human_review status during validation #292
Conversation
Fixes critical bug where tasks were marked as human_review (allowing merge) while the validation phase was still running. This created a dangerous situation where users could merge incomplete/unvalidated code. ## Root Cause The determineTaskStatusAndReason method checked subtask completion and QA status but did not verify if the validation phase was still active. When all subtasks completed, it immediately set status to human_review without waiting for validation to finish. ## Solution Added validation phase status checking before setting task to human_review: - Check task logs (both worktree and main) for validation phase status - If validation phase status is 'active', keep task in ai_review - Only transition to human_review after validation completes This prevents the merge button from appearing until validation is done. ## Changes - auto-claude-ui/src/main/project-store.ts: - Added validation phase status check (lines 439-462) - Task remains in ai_review while validation is active (lines 471-474) - Added critical safety comment explaining the fix ## Testing - TypeScript compilation passes without errors - Fix verified against screenshot showing the bug See docs/screenshots/bug-validation-status-inconsistency.png for bug example.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
Fixes inconsistency where task detail showed correct phase (e.g., "coding") but dashboard task cards showed outdated phase (e.g., "planning"). ## Root Cause The getTasks method in project-store.ts populated Task objects but never set the executionProgress field. Dashboard task cards rely on task.executionProgress.phase to display the current phase badge, so they showed undefined or stale data. Task detail view worked correctly because it separately loads task logs and finds the active phase from logs.phases[phase].status. ## Solution Added execution progress determination to getTasks: - Check task logs (worktree first, then main) for active phase - Map task log phase (planning/coding/validation) to execution phase - Set executionProgress.phase on Task object This ensures dashboard and detail views are in sync, both sourcing from the same task logs. ## Changes - auto-claude-ui/src/main/project-store.ts: - Lines 372-409: Determine executionProgress from task logs - Line 443: Add executionProgress to Task object - Line 5: Import ExecutionProgress and ExecutionPhase types ## Testing - TypeScript compilation passes without errors - Dashboard now shows correct phase matching task detail view
Fixes issue where task tiles didn't show phase badges unless the task was actively running. Now shows phase badge for both active and completed phases, providing better visibility into task progress. ## Root Cause 1. getTasks only set executionProgress for ACTIVE phases, not completed ones 2. TaskCard only showed phase badge when hasActiveExecution was true This meant completed tasks (e.g., in human_review after validation finished) showed no phase information in the badge area. ## Solution **Backend (project-store.ts):** - Check for active phases first (current behavior) - If no active phase, find most recently completed phase - Check phases in reverse order (validation → coding → planning) to get the furthest progress point - Set executionProgress for both active and completed phases **Frontend (TaskCard.tsx):** - Show phase badge whenever executionProgress exists (not just active) - Only show spinner icon when task is actively running - Phase badge now visible for tasks at any stage ## Changes - auto-claude-ui/src/main/project-store.ts: - Lines 378-426: Enhanced phase detection for active + completed phases - auto-claude-ui/src/renderer/components/TaskCard.tsx: - Line 48: Added hasPhaseInfo check - Line 223: Show badge for all phases with info - Line 231: Conditionally show spinner only when running ## Testing - TypeScript compilation passes without errors - Task tiles now show phase badges for all tasks with phase information - Spinner only shows when task is actively running
Fixes persistent phase inconsistency between dashboard tiles and task detail by using identical log merging logic in both code paths. ## Root Cause Task detail and dashboard tiles were using different log loading logic: **Task Detail (via TaskLogService):** - Planning: from main logs - Coding/Validation: from worktree logs (if available) - Intelligently merges main + worktree logs **Dashboard Tiles (via getTasks):** - Checked worktree first, then main - No merging - just used first available log - Different phase sources = inconsistent display This caused tiles to show one phase (e.g., "Planning" from main) while detail showed another (e.g., "Coding" from worktree). ## Solution Updated getTasks to use the EXACT same log merging logic as TaskLogService: 1. Load both main and worktree logs 2. Merge: planning from main, coding/validation from worktree 3. Find active or most recently completed phase from merged logs Now both code paths use identical logic and show the same phase. ## Changes - auto-claude-ui/src/main/project-store.ts: - Lines 378-443: Implemented TaskLogService-compatible log merging - Planning phase always from main logs - Coding/validation phases from worktree (if available) - Check merged phases for active/completed status ## Testing - TypeScript compilation passes - Dashboard tiles and task detail now show identical phases - Follows same phase priority: validation > coding > planning
CRITICAL FIX: Worktree logs were never being read because the path was wrong! ## Root Cause The worktree path construction was completely incorrect: **Wrong (my code):** ``` D:\Cityzen\.auto-claude\specs\006-task\.worktrees\006-task\task_log.json ``` Looked for .worktrees INSIDE the spec directory ❌ **Correct (TaskLogService):** ``` D:\Cityzen\.worktrees\006-task\.auto-claude\specs\006-task\task_log.json ``` .worktrees is at PROJECT ROOT, not inside specs ✅ This meant getTasks was NEVER actually reading worktree logs, so it always used main logs only. Task detail used TaskLogService which had the correct path, causing the phase mismatch. ## Solution Fixed worktree path to match TaskLogService exactly: ```typescript // OLD: path.join(specPath, '.worktrees', dir.name, 'task_log.json') // NEW: path.join(project.path, '.worktrees', dir.name, specsBaseDir, dir.name, 'task_log.json') ``` Now both code paths read from the same locations. ## Changes - auto-claude-ui/src/main/project-store.ts: - Line 377: Fixed worktree path construction - Added comment explaining worktree structure ## Impact This is why phases were STILL inconsistent - getTasks never actually loaded worktree logs. Now it will: - Read worktree logs correctly - Merge them properly with main logs - Show the EXACT same phase as task detail
Summary
Fixes two critical bugs related to task phase/status display:
human_review(allowing merge) while validation phase was still runningProblem 1: Premature Merge During Validation
As shown in the screenshot above, task 063-baseline-mapping-enhancements displays:
This creates a dangerous situation where users can merge incomplete/unvalidated code.
Root Cause #1
The
determineTaskStatusAndReasonmethod inproject-store.tschecked:But did NOT check:
When all subtasks completed, it immediately set status to
human_reviewwithout waiting for validation to finish.Solution #1
Added validation phase status checking:
Problem 2: Phase Display Inconsistency
Dashboard task tiles showed outdated phase (e.g., "planning") while task detail view showed the correct current phase (e.g., "coding").
Root Cause #2
The
getTasksmethod populated Task objects but never set theexecutionProgressfield. Dashboard task cards rely ontask.executionProgress.phaseto display the current phase badge, so they showed undefined or stale data.Task detail view worked correctly because it separately loads task logs and finds the active phase.
Solution #2
Added execution progress determination to
getTasks:Changes
auto-claude-ui/src/main/project-store.ts:Fix #1 - Validation Status:
ai_reviewwhile validation is activeFix #2 - Phase Display:
Testing
human_reviewwhile validation is activeai_reviewImpact
Before Fix #1: Users could merge code while validation was running ❌
After Fix #1: Merge button hidden until validation completes ✅
Before Fix #2: Dashboard showed stale phase (e.g., "planning") while detail showed current phase (e.g., "coding") ❌
After Fix #2: Dashboard and detail views show synchronized phase information ✅