fix: use null instead of undefined to clear worktree/branch in executor updateTask calls
The store's updateTask treats `undefined` as "no change" and `null` as "clear field". Six places in executor.ts passed `undefined` when intending to clear worktree/branch, leaving stale references that caused tasks to get stuck in in-progress with no active session after stuck-kills, pauses, and transient error retries. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -4690,7 +4690,7 @@ describe("task_add_dep tool", () => {
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-DEP", "triage");
|
||||
|
||||
// Worktree and status should be cleared
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-DEP", { worktree: undefined, status: undefined });
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-DEP", { worktree: null, status: null });
|
||||
|
||||
// Task should NOT be marked as failed
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith("FN-DEP", { status: "failed" });
|
||||
@@ -5074,7 +5074,7 @@ describe("TaskExecutor bounded recovery retries", () => {
|
||||
);
|
||||
expect(store.moveTask).not.toHaveBeenCalledWith("FN-001", "in-review");
|
||||
// Executor now handles the requeue in its finally block
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-001", { status: "stuck-killed" });
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-001", { status: "stuck-killed", worktree: null, branch: null });
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "todo");
|
||||
});
|
||||
|
||||
@@ -5108,7 +5108,7 @@ describe("TaskExecutor bounded recovery retries", () => {
|
||||
|
||||
// Should NOT requeue or mark as failed (budget handler already did that)
|
||||
expect(store.moveTask).not.toHaveBeenCalledWith("FN-001", "todo");
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith("FN-001", { status: "stuck-killed" });
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith("FN-001", { status: "stuck-killed", worktree: null, branch: null });
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith(
|
||||
"FN-001",
|
||||
expect.objectContaining({ status: "failed" }),
|
||||
@@ -5147,7 +5147,7 @@ describe("TaskExecutor bounded recovery retries", () => {
|
||||
// Should NOT call moveTask because task is already in todo
|
||||
expect(store.moveTask).not.toHaveBeenCalledWith("FN-001", "todo");
|
||||
// Should still clean up and mark as stuck-killed
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-001", { status: "stuck-killed" });
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-001", { status: "stuck-killed", worktree: null, branch: null });
|
||||
});
|
||||
|
||||
it("clears recovery metadata after successful run completes", async () => {
|
||||
|
||||
@@ -984,8 +984,8 @@ export class TaskExecutor {
|
||||
await this.store.updateTask(task.id, {
|
||||
recoveryRetryCount: decision.nextState.recoveryRetryCount,
|
||||
nextRecoveryAt: decision.nextState.nextRecoveryAt,
|
||||
worktree: undefined,
|
||||
branch: undefined,
|
||||
worktree: null,
|
||||
branch: null,
|
||||
});
|
||||
await this.store.moveTask(task.id, "todo");
|
||||
stuckRequeue = null; // Prevent outer finally from re-processing
|
||||
@@ -1026,7 +1026,7 @@ export class TaskExecutor {
|
||||
execSync(`git worktree remove "${worktreePath}" --force`, { cwd: this.rootDir, stdio: "pipe" });
|
||||
} catch {}
|
||||
}
|
||||
await this.store.updateTask(task.id, { status: "stuck-killed", worktree: undefined, branch: undefined });
|
||||
await this.store.updateTask(task.id, { status: "stuck-killed", worktree: null, branch: null });
|
||||
if (task.column !== "todo") {
|
||||
await this.store.moveTask(task.id, "todo");
|
||||
executorLog.log(`${task.id} moved to todo for retry after stuck kill`);
|
||||
@@ -1471,7 +1471,7 @@ export class TaskExecutor {
|
||||
executorLog.warn(`Failed to remove old worktree ${worktreePath}: ${cleanupErr.message}`);
|
||||
}
|
||||
}
|
||||
await this.store.updateTask(task.id, { worktree: undefined, branch: undefined });
|
||||
await this.store.updateTask(task.id, { worktree: null, branch: null });
|
||||
await this.store.logEntry(task.id, "Execution paused — agent terminated, moved to todo");
|
||||
await this.store.moveTask(task.id, "todo");
|
||||
} else if (this.stuckAborted.has(task.id)) {
|
||||
@@ -1555,8 +1555,8 @@ export class TaskExecutor {
|
||||
await this.store.updateTask(task.id, {
|
||||
recoveryRetryCount: decision.nextState.recoveryRetryCount,
|
||||
nextRecoveryAt: decision.nextState.nextRecoveryAt,
|
||||
worktree: undefined,
|
||||
branch: undefined,
|
||||
worktree: null,
|
||||
branch: null,
|
||||
});
|
||||
await this.store.moveTask(task.id, "todo");
|
||||
return;
|
||||
@@ -1606,7 +1606,7 @@ export class TaskExecutor {
|
||||
executorLog.warn(`Failed to remove old worktree ${worktreePath}: ${cleanupErr.message}`);
|
||||
}
|
||||
}
|
||||
await this.store.updateTask(task.id, { status: "stuck-killed", worktree: undefined, branch: undefined });
|
||||
await this.store.updateTask(task.id, { status: "stuck-killed", worktree: null, branch: null });
|
||||
// Only move to todo if not already there. The task.column check uses the
|
||||
// captured task object from execute() start — if the task was already in "todo"
|
||||
// when execute() started (e.g., resumed orphan), we skip the redundant move.
|
||||
@@ -2030,7 +2030,7 @@ export class TaskExecutor {
|
||||
this.activeWorktrees.delete(taskId);
|
||||
|
||||
// Update task: clear worktree and status, move to triage
|
||||
await this.store.updateTask(taskId, { worktree: undefined, status: undefined });
|
||||
await this.store.updateTask(taskId, { worktree: null, status: null });
|
||||
await this.store.moveTask(taskId, "triage");
|
||||
await this.store.logEntry(taskId, "Execution stopped — work discarded, moved to triage for re-specification");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user