intake(lote-A): import upstream PR #1833#5
Open
nsalvacao wants to merge 8 commits intobaseline/develop-sync-2026-02-17from
Open
intake(lote-A): import upstream PR #1833#5nsalvacao wants to merge 8 commits intobaseline/develop-sync-2026-02-17from
nsalvacao wants to merge 8 commits intobaseline/develop-sync-2026-02-17from
Conversation
When an agent process exits with incomplete/stuck subtasks, the task gets
stuck in the Kanban UI — it spins forever, can't be stopped, and can't be
dragged back to planning.
This fix addresses 5 specific state synchronization gaps between the backend
process lifecycle and the frontend XState state machine:
1. Reset execution progress on terminal state transitions (task-store.ts)
- When tasks reach terminal states (human_review, error, done, pr_created),
execution progress is now reset to idle
- Prevents stuck tasks from showing stale progress indicators in UI
2. Propagate final phase updates even after XState settles (agent-events-handlers.ts)
- Final 'complete' or 'failed' phase updates are now sent to renderer
- Previously these were silently dropped, causing UI to never show completion
3. Add fallback exit handler to force state transition (agent-events-handlers.ts)
- If task remains in_progress 500ms after process exit, force to human_review
- Safety net for when XState fails to properly handle PROCESS_EXITED event
4. Disable dragging for in_progress tasks (SortableTaskCard.tsx)
- Prevents users from dragging tasks that are currently running or stuck
- Adds disabled flag to useSortable hook when status is 'in_progress'
5. Allow stop button for stuck tasks (TaskCard.tsx)
- Users can now force-stop stuck tasks using the stop button
- Removed the isStuck check that prevented stopping stuck tasks
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The 500ms fallback safety net was calling `taskStateManager.forceTransition()` which doesn't exist, causing TypeScript compilation errors. Fixed to: - Use `handleUiEvent()` with `USER_STOPPED` event (proper XState transition) - Look up both task and project fresh (avoid stale closure references) - Add null check for `checkProject` before proceeding This ensures the fallback actually works when XState fails to transition tasks out of in_progress after process exit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add named constant STUCK_TASK_FALLBACK_TIMEOUT_MS for magic number - Use XState getCurrentState() instead of cached task status to avoid stale cache issues - Check XState active states directly (planning, coding, qa_review, qa_fixing) - Improve logging to show actual XState state name - Add inline comments explaining the stale cache avoidance strategy This resolves all 3 blocking issues from the Auto Claude review: 1. CRITICAL: forceTransition() method - fixed by using handleUiEvent() 2. MEDIUM: Stale closure - fixed by checking XState directly 3. MEDIUM: Magic number - fixed by using named constant Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…onstants Resolves NEW-001 and CMT-001 from the follow-up review: - Extract XSTATE_ACTIVE_STATES to shared constant in task-state-utils.ts - Export XSTATE_ACTIVE_STATES from state-machines index - Replace hardcoded hasPlan: true with dynamic plan file check - Pattern matches TASK_STOP handler (execution-handlers.ts lines 299-310) - Tasks stuck in 'planning' state now correctly route to backlog, not human_review - Improved logging shows hasPlan value for debugging This ensures the fallback safety net routes tasks to the correct Kanban column based on whether a plan file exists with subtasks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Extract hasPlan logic into shared hasPlanWithSubtasks() utility in plan-file-utils.ts to eliminate duplication and centralize plan validation logic - Make setTimeout callback async to avoid blocking readFileSync in event loop - Replace hardcoded terminal status check with cleaner .includes() pattern - Add status gate for final phase updates to prevent UI flicker when failed phase arrives after task has transitioned to human_review - Import and use hasPlanWithSubtasks in agent-events-handlers.ts All review comments addressed: - Gemini: Critical/Medium - forceTransition method, magic number (already fixed in previous commit) - Gemini: Medium - hardcoded status list (now uses .includes) - CodeRabbit: Trivial - extract XSTATE_ACTIVE_STATES (already fixed in previous commit) - CodeRabbit: Minor - derive hasPlan from file check (now uses shared utility) - CodeRabbit: Minor - prevent UI flicker from final phase updates (now gates on status) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fixed all 3 findings from the latest auto-claude review: 1. [NCR-NEW-001] MEDIUM: Fallback safety net timeout race condition - Store setTimeout timer reference in a Map keyed by taskId - Export cancelFallbackTimer() function to clear pending timers - Call cancelFallbackTimer() at start of TASK_START handler - Prevents stale timer from incorrectly stopping newly restarted tasks - Clean up timer reference after it fires 2. [CMT-NEW-001] LOW: Duplicate hasPlan detection logic - Replace inline hasPlan logic in execution-handlers.ts TASK_STOP - Use shared hasPlanWithSubtasks() utility from plan-file-utils.ts - Eliminates code duplication and ensures consistent behavior 3. [CMT-001] LOW: Misleading async keyword - Remove async from setTimeout callback in agent-events-handlers.ts - No await expressions exist in the callback - All operations (getCurrentState, hasPlanWithSubtasks) are synchronous All changes maintain backward compatibility and fix the race condition where restarting a task within 500ms could be incorrectly stopped by the fallback timer from the previous process exit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The fallback safety net timer was only cancelled in the TASK_START handler, but not in the TASK_UPDATE_STATUS auto-start path or TASK_RECOVER_STUCK auto-restart path. This meant a stale timer could incorrectly stop a newly restarted task if it was restarted within the 500ms window via drag-to-in-progress or recovery auto-restart. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ake/lote-A-pr-1833
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Upstream intake PR for isolated review.
intake/lote-A-pr-1833baseline/develop-sync-2026-02-17Checklist: