diff --git a/src/__tests__/unit/services/release-stale-task-pods.test.ts b/src/__tests__/unit/services/release-stale-task-pods.test.ts index 4c18d1a0d0..c2ee602633 100644 --- a/src/__tests__/unit/services/release-stale-task-pods.test.ts +++ b/src/__tests__/unit/services/release-stale-task-pods.test.ts @@ -537,7 +537,7 @@ describe("releaseStaleTaskPods", () => { vi.useRealTimers(); }); - test("should not halt tasks with open PRs even without pods", async () => { + test("should halt stale IN_PROGRESS tasks without pods even when open PR exists", async () => { const now = new Date("2024-10-24T12:00:00Z"); vi.setSystemTime(now); @@ -567,19 +567,32 @@ describe("releaseStaleTaskPods", () => { ]; vi.mocked(mockDb.task.findMany).mockResolvedValue(staleTasks as any); + vi.mocked(mockDb.task.update).mockResolvedValue({} as any); const result = await releaseStaleTaskPods(); // Should NOT call releaseTaskPod (no pod to release) expect(mockReleaseTaskPod).not.toHaveBeenCalled(); - // Should NOT call haltTask (has open PR) - expect(mockDb.task.update).not.toHaveBeenCalled(); + // Should call haltTask — pod is already gone, open PR is no reason to skip halting + expect(mockDb.task.update).toHaveBeenCalledWith({ + where: { id: "task-1" }, + data: { + workflowStatus: "HALTED", + workflowCompletedAt: expect.any(Date), + }, + select: { + workflowStartedAt: true, + workflowCompletedAt: true, + featureId: true, + workspace: { select: { slug: true } }, + }, + }); - // Verify result - no pods released, no tasks halted + // Verify result - no pods released, 1 task halted expect(result.success).toBe(true); expect(result.podsReleased).toBe(0); - expect(result.tasksHalted).toBe(0); + expect(result.tasksHalted).toBe(1); vi.useRealTimers(); }); diff --git a/src/services/task-coordinator-cron.ts b/src/services/task-coordinator-cron.ts index 28bac827c8..d1cd340a2a 100644 --- a/src/services/task-coordinator-cron.ts +++ b/src/services/task-coordinator-cron.ts @@ -357,20 +357,26 @@ export async function releaseStaleTaskPods(): Promise<{ return content?.status && content.status !== "DONE" && content.status !== "CANCELLED"; }); - if (hasOpenPr) { - console.log( - `[ReleaseStaleTaskPods] Skipping halt for task ${task.id} - has open PR` - ); - } + // Base eligibility: stale IN_PROGRESS, not already HALTED + const isHaltEligible = task.status === "IN_PROGRESS" && task.workflowStatus !== "HALTED"; - // Determine if this task should be halted (only IN_PROGRESS tasks not already halted, and no open PR) - const shouldHalt = task.status === "IN_PROGRESS" && task.workflowStatus !== "HALTED" && !hasOpenPr; + // With-pod: protect active reviewer sessions — don't halt if a PR is still open + const shouldHaltWithPod = isHaltEligible && !hasOpenPr; + + // No-pod: pod is already gone, nothing to protect — always halt if eligible + const shouldHaltNoPod = isHaltEligible; if (task.podId) { // Task has a pod - use releaseTaskPod to release it - // If should halt: set to HALTED + // If should halt: set to HALTED (skip if PR still open — protect active reviewer session) // If already done/failed/etc: pass null to preserve original workflowStatus - const newWorkflowStatus = shouldHalt ? "HALTED" : null; + const newWorkflowStatus = shouldHaltWithPod ? "HALTED" : null; + + if (task.podId && hasOpenPr) { + console.log( + `[ReleaseStaleTaskPods] Skipping halt for task ${task.id} - has open PR` + ); + } const result = await releaseTaskPod({ taskId: task.id, @@ -386,20 +392,20 @@ export async function releaseStaleTaskPods(): Promise<{ podsReleased++; } - if (shouldHalt && (result.success || result.taskCleared)) { + if (shouldHaltWithPod && (result.success || result.taskCleared)) { tasksHalted++; } console.log( `[ReleaseStaleTaskPods] Processed task ${task.id} (status: ${task.status}, workflowStatus: ${task.workflowStatus}): ` + - `pod released: ${result.podDropped}, halted: ${shouldHalt}, reassigned: ${result.reassigned || false}` + + `pod released: ${result.podDropped}, halted: ${shouldHaltWithPod}, reassigned: ${result.reassigned || false}` + (hasOpenPr ? `, skipped halt due to open PR` : "") ); if (!result.success && result.error) { throw new Error(result.error); } - } else if (shouldHalt) { + } else if (shouldHaltNoPod) { // Task has no pod but is stale IN_PROGRESS - just halt it await haltTask(task.id, false); tasksHalted++;