Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,11 @@ export function registerTaskExecutionHandlers(

console.warn('[TASK_START] Found task:', task.specId, 'status:', task.status, 'reviewReason:', task.reviewReason, 'subtasks:', task.subtasks.length);

// Clear stale tracking state from any previous execution so that:
// - terminalEventSeen doesn't suppress future PROCESS_EXITED events
// - lastSequenceByTask doesn't drop events from the new process
taskStateManager.prepareForRestart(taskId);

// Check if implementation_plan.json has valid subtasks BEFORE XState handling.
// This is more reliable than task.subtasks.length which may not be loaded yet.
const specsBaseDir = getSpecsDir(project.autoBuildPath);
Expand Down Expand Up @@ -360,6 +365,9 @@ export function registerTaskExecutionHandlers(
task,
project
);

// Clear stale tracking state so a subsequent restart works correctly
taskStateManager.prepareForRestart(taskId);
Comment on lines +368 to +370
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's understand the file structure and find the taskMachine definition
find . -type f -name "*.ts" -path "*/ipc-handlers/task/*" | head -20

Repository: AndyMik90/Auto-Claude

Length of output: 792


🏁 Script executed:

# Search for the backlog state definition as requested
rg -n 'backlog' --type ts -A 10 -B 2

Repository: AndyMik90/Auto-Claude

Length of output: 50378


🏁 Script executed:

# Find where the taskMachine is defined
rg -n 'taskMachine\s*=' --type ts -l | head -10

Repository: AndyMik90/Auto-Claude

Length of output: 120


🏁 Script executed:

# Look at the execution-handlers file to see the context around lines 368-370
wc -l apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts

Repository: AndyMik90/Auto-Claude

Length of output: 132


🏁 Script executed:

sed -n '360,380p' apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts

Repository: AndyMik90/Auto-Claude

Length of output: 467


🏁 Script executed:

# Find the taskMachine definition
cat apps/frontend/src/shared/state-machines/task-machine.ts

Repository: AndyMik90/Auto-Claude

Length of output: 7701


🏁 Script executed:

# Search for prepareForRestart calls
rg -n 'prepareForRestart' apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts

Repository: AndyMik90/Auto-Claude

Length of output: 343


🏁 Script executed:

# Search for handleProcessExited definition and terminalEventSeen
rg -n 'handleProcessExited|terminalEventSeen' apps/frontend/src/main/ipc-handlers/task/ -A 5

Repository: AndyMik90/Auto-Claude

Length of output: 835


🏁 Script executed:

sed -n '175,190p' apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts

Repository: AndyMik90/Auto-Claude

Length of output: 828


🏁 Script executed:

sed -n '535,550p' apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts

Repository: AndyMik90/Auto-Claude

Length of output: 884


🏁 Script executed:

sed -n '710,730p' apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts

Repository: AndyMik90/Auto-Claude

Length of output: 1166


🏁 Script executed:

sed -n '1145,1160p' apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts

Repository: AndyMik90/Auto-Claude

Length of output: 768


🏁 Script executed:

# Now let's find the handleProcessExited and terminalEventSeen implementation
rg -n 'handleProcessExited|terminalEventSeen' apps/frontend/src/main/task-state-manager.ts -A 10

Repository: AndyMik90/Auto-Claude

Length of output: 3268


🏁 Script executed:

# Check if there are any other callers of handleProcessExited to understand the flow
rg -n 'handleProcessExited' apps/frontend/src/main/ -B 3 -A 1

Repository: AndyMik90/Auto-Claude

Length of output: 4367


🏁 Script executed:

# Verify the backlog state doesn't have PROCESS_EXITED handler by extracting just that section
sed -n '50,65p' apps/frontend/src/shared/state-machines/task-machine.ts

Repository: AndyMik90/Auto-Claude

Length of output: 525


Remove prepareForRestart from TASK_STOP handler

The prepareForRestart(taskId) call at line 370 is redundant and logically inconsistent. After handleUiEvent(USER_STOPPED) transitions the actor to its backlog state, clearing terminalEventSeen here allows a stale PROCESS_EXITED event from the killed process to reach the actor—where it will be silently ignored since the backlog state doesn't define a handler for it. Every subsequent restart path already calls prepareForRestart before starting a new process (TASK_START line 183, TASK_UPDATE_STATUS line 718, TASK_RECOVER_STUCK line 1150), so the state is clean regardless. Delete the call and let the next start handler reset the tracking state at the right moment.

🔧 Proposed fix
     taskStateManager.handleUiEvent(
       taskId,
       { type: 'USER_STOPPED', hasPlan },
       task,
       project
     );
-
-    // Clear stale tracking state so a subsequent restart works correctly
-    taskStateManager.prepareForRestart(taskId);
   });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts` around lines
368 - 370, Remove the redundant call to
taskStateManager.prepareForRestart(taskId) from the TASK_STOP handler: locate
the TASK_STOP handling block where handleUiEvent(USER_STOPPED) is invoked and
delete the prepareForRestart(taskId) invocation so that terminalEventSeen
remains set and any late PROCESS_EXITED events are handled/ignored correctly by
the backlog state; rely on existing calls to taskStateManager.prepareForRestart
in TASK_START, TASK_UPDATE_STATUS, and TASK_RECOVER_STUCK to reset tracking
before a new run.

});

/**
Expand Down Expand Up @@ -529,6 +537,9 @@ export function registerTaskExecutionHandlers(
return { success: false, error: 'Failed to write QA fix request file' };
}

// Clear stale tracking state before starting new QA process
taskStateManager.prepareForRestart(taskId);

// Restart QA process - use worktree path if it exists, otherwise main project
// The QA process needs to run where the implementation_plan.json with completed subtasks is
const qaProjectPath = hasWorktree ? worktreePath : project.path;
Expand Down Expand Up @@ -703,6 +714,8 @@ export function registerTaskExecutionHandlers(

// Auto-start task when status changes to 'in_progress' and no process is running
if (status === 'in_progress' && !agentManager.isRunning(taskId)) {
// Clear stale tracking state before starting a new process
taskStateManager.prepareForRestart(taskId);
const mainWindow = getMainWindow();

// Check git status before auto-starting
Expand Down Expand Up @@ -1133,6 +1146,8 @@ export function registerTaskExecutionHandlers(
// Auto-restart the task if requested
let autoRestarted = false;
if (autoRestart) {
// Clear stale tracking state before restarting
taskStateManager.prepareForRestart(taskId);
Comment on lines +1149 to +1150
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

prepareForRestart placement is correct; note that no XState event is sent before the new process starts

The call correctly clears terminalEventSeen and lastSequenceByTask before startTaskExecution/startSpecCreation. However, unlike TASK_START and TASK_UPDATE_STATUS, this auto-restart path sends no XState event (e.g., USER_RESUMED) to transition the actor before the new process begins. If the actor is in error or a mid-execution state, the machine may not accept events from the fresh process correctly.

This is a pre-existing design choice (outside the scope of this PR), but worth tracking separately.

Would you like me to open a follow-up issue to add an appropriate handleUiEvent call in the TASK_RECOVER_STUCK auto-restart path?

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts` around lines
1149 - 1150, The auto-restart path calls
taskStateManager.prepareForRestart(taskId) but does not send an XState UI event
before launching the new process, so the actor can remain in an invalid state;
add a call to handleUiEvent(...) in the TASK_RECOVER_STUCK auto-restart path
(immediately after prepareForRestart and before invoking
startTaskExecution/startSpecCreation) to dispatch the appropriate UI event
(e.g., USER_RESUMED or a TASK_RECOVERED/TASK_RECOVER_STUCK event) so the actor
transitions to a receptive state prior to the new process starting.

// Check git status before auto-restarting
const gitStatusForRestart = checkGitStatus(project.path);
if (!gitStatusForRestart.isGitRepo || !gitStatusForRestart.hasCommits) {
Expand Down
12 changes: 12 additions & 0 deletions apps/frontend/src/main/task-state-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,18 @@ export class TaskStateManager {
return this.getCurrentState(taskId) === 'plan_review';
}

/**
* Reset tracking state for a task that is about to be restarted.
* Clears terminalEventSeen (so process exits aren't swallowed) and
* lastSequenceByTask (so events from the new process aren't dropped
* as duplicates). Does NOT stop or remove the XState actor, since
* the caller may still need to send events to it.
*/
prepareForRestart(taskId: string): void {
this.terminalEventSeen.delete(taskId);
this.lastSequenceByTask.delete(taskId);
}

clearTask(taskId: string): void {
this.lastSequenceByTask.delete(taskId);
this.lastStateByTask.delete(taskId);
Expand Down
Loading