Fix False Stuck Detection During Planning Phase (#1236)
* auto-claude: subtask-1-1 - Add 'planning' phase to stuck detection skip logic Add 'planning' phase to the stuck detection skip logic in TaskCard.tsx. Previously only 'complete' and 'failed' phases were skipped. Now 'planning' is also skipped since process tracking is async and may show false negatives during initial startup. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * auto-claude: subtask-1-2 - Add debug logging to stuck detection for better di Add debug logging when stuck check is skipped due to planning phase or terminal phases (complete/failed). This helps diagnose false-positive stuck detection issues by logging when and why the check was bypassed. The logging uses the existing window.DEBUG flag pattern to avoid noise in production while enabling diagnostics when needed. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(ui): add 'planning' phase to useTaskDetail.ts stuck detection and extract constant - Add 'planning' phase check at lines 113 and 126 in useTaskDetail.ts to match TaskCard.tsx behavior, preventing false stuck indicators during initial task startup - Extract STUCK_CHECK_SKIP_PHASES constant and shouldSkipStuckCheck() helper in TaskCard.tsx to reduce code duplication Fixes PR review findings: ed766093f258 (HIGH), 91a0a4fcd67b (LOW) * fix(ui): don't set hasCheckedRunning for planning phase Fixes regression where stuck detection was disabled after planning→coding transition because hasCheckedRunning remained true from planning phase. Now 'planning' phase only clears isStuck without setting hasCheckedRunning, allowing proper stuck detection when task transitions to 'coding' phase. Fixes NEW-001 from PR review. * fix(ui): move stuck check constants outside TaskCard component Move STUCK_CHECK_SKIP_PHASES constant and shouldSkipStuckCheck function outside the TaskCard component to avoid recreation on every render. Addresses PR review finding NEW-003 (code quality). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(ui): reset hasCheckedRunning when task stops in planning phase Move the !isActiveTask check to run FIRST before any phase checks. This ensures hasCheckedRunning is always reset when a task becomes inactive, even if it stops while in 'planning' phase. Previously, the planning phase check returned early, preventing the !isActiveTask reset from running, which caused stale hasCheckedRunning state and skipped stuck checks on task restart. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(test): use callback(0) instead of callback.call(window, 0) Fix unhandled ReferenceError in terminal-copy-paste.test.ts where window was not defined when requestAnimationFrame callback executed asynchronously. The callback just needs the timestamp parameter. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> Co-authored-by: Test User <test@example.com>
This commit is contained in:
Andy
@@ -80,8 +80,8 @@ describe('Terminal copy/paste integration', () => {
|
||||
// Mock requestAnimationFrame for xterm.js integration tests
|
||||
global.requestAnimationFrame = vi.fn((callback: FrameRequestCallback) => {
|
||||
// Synchronously execute the callback to avoid timing issues in tests
|
||||
// Use globalThis instead of window for cross-platform compatibility
|
||||
callback.call(globalThis, 0);
|
||||
// Just pass timestamp directly - this context isn't used by RAF callbacks
|
||||
callback(0);
|
||||
return 0;
|
||||
}) as unknown as Mock;
|
||||
|
||||
|
||||
@@ -47,6 +47,14 @@ const CategoryIcon: Record<TaskCategory, typeof Zap> = {
|
||||
testing: FileCode
|
||||
};
|
||||
|
||||
// Phases where stuck detection should be skipped (terminal states + initial planning)
|
||||
// Defined outside component to avoid recreation on every render
|
||||
const STUCK_CHECK_SKIP_PHASES = ['complete', 'failed', 'planning'] as const;
|
||||
|
||||
function shouldSkipStuckCheck(phase: string | undefined): boolean {
|
||||
return STUCK_CHECK_SKIP_PHASES.includes(phase as typeof STUCK_CHECK_SKIP_PHASES[number]);
|
||||
}
|
||||
|
||||
interface TaskCardProps {
|
||||
task: Task;
|
||||
onClick: () => void;
|
||||
@@ -184,11 +192,11 @@ export const TaskCard = memo(function TaskCard({
|
||||
|
||||
// Memoized stuck check function to avoid recreating on every render
|
||||
const performStuckCheck = useCallback(() => {
|
||||
// IMPORTANT: If the execution phase is 'complete' or 'failed', the task is NOT stuck.
|
||||
// It means the process has finished and status update is pending.
|
||||
// This prevents false-positive "stuck" indicators when the process exits normally.
|
||||
const currentPhase = task.executionProgress?.phase;
|
||||
if (currentPhase === 'complete' || currentPhase === 'failed') {
|
||||
if (shouldSkipStuckCheck(currentPhase)) {
|
||||
if (window.DEBUG) {
|
||||
console.log(`[TaskCard] Stuck check skipped for ${task.id} - phase is '${currentPhase}' (planning/terminal phases don't need process verification)`);
|
||||
}
|
||||
setIsStuck(false);
|
||||
return;
|
||||
}
|
||||
@@ -198,7 +206,7 @@ export const TaskCard = memo(function TaskCard({
|
||||
checkTaskRunning(task.id).then((actuallyRunning) => {
|
||||
// Double-check the phase again in case it changed while waiting
|
||||
const latestPhase = task.executionProgress?.phase;
|
||||
if (latestPhase === 'complete' || latestPhase === 'failed') {
|
||||
if (shouldSkipStuckCheck(latestPhase)) {
|
||||
setIsStuck(false);
|
||||
} else {
|
||||
setIsStuck(!actuallyRunning);
|
||||
|
||||
@@ -107,23 +107,39 @@ export function useTaskDetail({ task }: UseTaskDetailOptions) {
|
||||
useEffect(() => {
|
||||
let timeoutId: NodeJS.Timeout | undefined;
|
||||
|
||||
// IMPORTANT: If execution phase is 'complete' or 'failed', the task is NOT stuck.
|
||||
// It means the process has finished and status update is pending.
|
||||
// This prevents false-positive "stuck" indicators when the process exits normally.
|
||||
const isPhaseTerminal = executionPhase === 'complete' || executionPhase === 'failed';
|
||||
if (isPhaseTerminal) {
|
||||
// IMPORTANT: Check !isActiveTask FIRST before any phase checks
|
||||
// This ensures hasCheckedRunning is always reset when task stops,
|
||||
// even if the task stops while in 'planning' phase
|
||||
if (!isActiveTask) {
|
||||
setIsStuck(false);
|
||||
setHasCheckedRunning(false);
|
||||
return;
|
||||
}
|
||||
|
||||
// Task is active from here on
|
||||
|
||||
// 'planning' phase: Skip stuck check but don't set hasCheckedRunning
|
||||
// (allows stuck detection when task transitions to 'coding')
|
||||
if (executionPhase === 'planning') {
|
||||
setIsStuck(false);
|
||||
return;
|
||||
}
|
||||
|
||||
// Terminal phases: Task finished, no more stuck checks needed
|
||||
if (executionPhase === 'complete' || executionPhase === 'failed') {
|
||||
setIsStuck(false);
|
||||
setHasCheckedRunning(true);
|
||||
return;
|
||||
}
|
||||
|
||||
if (isActiveTask && !hasCheckedRunning) {
|
||||
// Active task in coding/validation phase - check if stuck
|
||||
if (!hasCheckedRunning) {
|
||||
// Wait 2 seconds before checking - gives process time to spawn and register
|
||||
timeoutId = setTimeout(() => {
|
||||
checkTaskRunning(task.id).then((actuallyRunning) => {
|
||||
// Double-check the phase in case it changed while waiting
|
||||
const latestPhase = task.executionProgress?.phase;
|
||||
if (latestPhase === 'complete' || latestPhase === 'failed') {
|
||||
if (latestPhase === 'complete' || latestPhase === 'failed' || latestPhase === 'planning') {
|
||||
setIsStuck(false);
|
||||
} else {
|
||||
setIsStuck(!actuallyRunning);
|
||||
@@ -131,9 +147,6 @@ export function useTaskDetail({ task }: UseTaskDetailOptions) {
|
||||
setHasCheckedRunning(true);
|
||||
});
|
||||
}, 2000);
|
||||
} else if (!isActiveTask) {
|
||||
setIsStuck(false);
|
||||
setHasCheckedRunning(false);
|
||||
}
|
||||
|
||||
return () => {
|
||||
|
||||
Reference in New Issue
Block a user