diff --git a/.changeset/fuzzy-ravens-retry.md b/.changeset/fuzzy-ravens-retry.md new file mode 100644 index 0000000000..30eed661a0 --- /dev/null +++ b/.changeset/fuzzy-ravens-retry.md @@ -0,0 +1,5 @@ +--- +"@runfusion/fusion": patch +--- + +Fix retry handling for stranded in-review tasks whose status is unset by allowing retry when execution is incomplete or a merge retry has already been attempted. diff --git a/packages/cli/src/__tests__/extension.test.ts b/packages/cli/src/__tests__/extension.test.ts index 1893b70ce8..2815f7ee0b 100644 --- a/packages/cli/src/__tests__/extension.test.ts +++ b/packages/cli/src/__tests__/extension.test.ts @@ -3124,6 +3124,153 @@ describe("fn pi extension (runnable structured-output regression slice)", () => expect(updated?.mergeRetries).toBe(0); }); + it("moves status-none in-review task with incomplete steps to todo preserving progress", async () => { + const store = new TaskStore(tmpDir); + await store.init(); + + const task = await store.createTask({ + title: "status-none execution-stalled task", + description: "test", + column: "todo", + }); + await store.updateTask(task.id, { + steps: [ + { name: "Step 0", status: "done" }, + { name: "Step 1", status: "in-progress" }, + { name: "Step 2", status: "pending" }, + ], + }); + await store.moveTask(task.id, "in-progress"); + await store.moveTask(task.id, "in-review"); + await store.updateTask(task.id, { status: null, error: "stalled without failed status", mergeRetries: 5 }); + + const retryTool = api.tools.get("fn_task_retry")!; + const result = await retryTool.execute("retry-status-none-exec", { id: task.id }, undefined, undefined, makeCtx(tmpDir)); + + expect(result.isError).toBeFalsy(); + expect(result.details.newColumn).toBe("todo"); + + const updated = await store.getTask(task.id); + expect(updated?.column).toBe("todo"); + expect(updated?.status).toBeFalsy(); + expect(updated?.error).toBeFalsy(); + expect(updated?.steps[1].status).toBe("in-progress"); + expect(updated?.mergeRetries).toBe(5); + }); + + it("moves status-none zero-step in-review task with no merge attempts to todo", async () => { + const store = new TaskStore(tmpDir); + await store.init(); + + const task = await store.createTask({ + title: "status-none zero-step execution-stalled task", + description: "test", + column: "todo", + }); + await writeFile(join(tmpDir, ".fusion", "tasks", task.id, "PROMPT.md"), "# status-none zero-step execution-stalled task\n\nNo steps yet.\n"); + await store.updateTask(task.id, { steps: [] }); + await store.moveTask(task.id, "in-progress"); + await store.moveTask(task.id, "in-review"); + await store.updateTask(task.id, { status: null, error: "stalled before planning steps", mergeRetries: 0, steps: [] }); + + const retryTool = api.tools.get("fn_task_retry")!; + const result = await retryTool.execute("retry-status-none-zero-step-exec", { id: task.id }, undefined, undefined, makeCtx(tmpDir)); + + expect(result.isError).toBeFalsy(); + expect(result.details.newColumn).toBe("todo"); + + const updated = await store.getTask(task.id); + expect(updated?.column).toBe("todo"); + expect(updated?.status).toBeFalsy(); + expect(updated?.error).toBeFalsy(); + expect(updated?.steps).toEqual([]); + expect(updated?.mergeRetries).toBe(0); + }); + + it("keeps status-none in-review task with prior merge attempts in-review and resets merge state", async () => { + const store = new TaskStore(tmpDir); + await store.init(); + + const task = await store.createTask({ + title: "status-none merge-stalled task", + description: "test", + column: "todo", + }); + await store.updateTask(task.id, { + steps: [ + { name: "Step 0", status: "done" }, + { name: "Step 1", status: "done" }, + ], + }); + await store.moveTask(task.id, "in-progress"); + await store.moveTask(task.id, "in-review"); + await store.updateTask(task.id, { status: null, error: "merge retry exhausted without failed status", mergeRetries: 2 }); + + const retryTool = api.tools.get("fn_task_retry")!; + const result = await retryTool.execute("retry-status-none-merge", { id: task.id }, undefined, undefined, makeCtx(tmpDir)); + + expect(result.isError).toBeFalsy(); + expect(result.details.newColumn).toBe("in-review"); + + const updated = await store.getTask(task.id); + expect(updated?.column).toBe("in-review"); + expect(updated?.status).toBeFalsy(); + expect(updated?.error).toBeFalsy(); + expect(updated?.mergeRetries).toBe(0); + }); + + it("rejects status-none in-review task with completed steps and no merge attempts", async () => { + const store = new TaskStore(tmpDir); + await store.init(); + + const task = await store.createTask({ + title: "status-none completed task with no merge attempts", + description: "test", + column: "todo", + }); + await store.updateTask(task.id, { + steps: [ + { name: "Step 0", status: "done" }, + { name: "Step 1", status: "done" }, + ], + }); + await store.moveTask(task.id, "in-progress"); + await store.moveTask(task.id, "in-review"); + await store.updateTask(task.id, { status: null, mergeRetries: 0 }); + + const retryTool = api.tools.get("fn_task_retry")!; + const result = await retryTool.execute("retry-status-none-no-merge", { id: task.id }, undefined, undefined, makeCtx(tmpDir)); + + expect(result.isError).toBe(true); + expect(result.content[0].text).toContain("not in a retryable state"); + + const updated = await store.getTask(task.id); + expect(updated?.column).toBe("in-review"); + expect(updated?.mergeRetries).toBe(0); + }); + + it("rejects non-review task with status none", async () => { + const store = new TaskStore(tmpDir); + await store.init(); + + const task = await store.createTask({ + title: "status-none todo task", + description: "test", + column: "todo", + }); + await store.updateTask(task.id, { status: null, steps: [{ name: "Step 0", status: "pending" }] }); + + const retryTool = api.tools.get("fn_task_retry")!; + const result = await retryTool.execute("retry-status-none-todo", { id: task.id }, undefined, undefined, makeCtx(tmpDir)); + + expect(result.isError).toBe(true); + expect(result.content[0].text).toContain("not in a retryable state"); + + const updated = await store.getTask(task.id); + expect(updated?.column).toBe("todo"); + expect(updated?.status).toBeFalsy(); + }); + it("moves non-review failed task to todo and resets all retry counters", async () => { const store = new TaskStore(tmpDir); await store.init(); diff --git a/packages/cli/src/commands/__tests__/task.test.ts b/packages/cli/src/commands/__tests__/task.test.ts index 567fd9b291..87f07e74b8 100644 --- a/packages/cli/src/commands/__tests__/task.test.ts +++ b/packages/cli/src/commands/__tests__/task.test.ts @@ -24,7 +24,15 @@ vi.mock("node:child_process", async () => { new Promise((resolve) => { execFn(cmd, opts, (_err: any, stdout: string, stderr: string) => resolve({ stdout, stderr })); }); - return { exec: execFn }; + const execFileFn = vi.fn((_file: string, _args: any, opts: any, cb: any) => { + const callback = typeof opts === "function" ? opts : cb; + if (typeof callback === "function") callback(null, "", ""); + }) as any; + execFileFn[promisify.custom] = (file: string, args?: any, opts?: any) => + new Promise((resolve) => { + execFileFn(file, args, opts, (_err: any, stdout: string, stderr: string) => resolve({ stdout, stderr })); + }); + return { exec: execFn, execFile: execFileFn }; }); // Mock @fusion/core before importing the module under test @@ -39,9 +47,16 @@ vi.mock("@fusion/core", async (importActual) => { done: "Done", }; + const TaskStoreMock = vi.fn(function () {}); + const taskStoreMockImplementation = TaskStoreMock.mockImplementation.bind(TaskStoreMock); + TaskStoreMock.mockImplementation = ((impl: (...args: any[]) => unknown) => + taskStoreMockImplementation(function (this: unknown, ...args: any[]) { + return impl(...args); + })) as typeof TaskStoreMock.mockImplementation; + return { ...actual, - TaskStore: vi.fn(), + TaskStore: TaskStoreMock, COLUMNS, COLUMN_LABELS, runDeterministicDuplicateGuard: vi.fn(), @@ -2379,13 +2394,18 @@ describe("runTaskRetry", () => { mockMoveTask = vi.fn(); mockLogEntry = vi.fn(); - (TaskStore as unknown as ReturnType).mockImplementation(() => ({ - init: vi.fn(), - getTask: mockGetTask, - updateTask: mockUpdateTask, - moveTask: mockMoveTask, - logEntry: mockLogEntry, - })); + vi.mocked(resolveProject).mockResolvedValue({ + projectId: "proj_test", + projectPath: "/test", + projectName: "test", + isRegistered: true, + store: { + getTask: mockGetTask, + updateTask: mockUpdateTask, + moveTask: mockMoveTask, + logEntry: mockLogEntry, + } as unknown as TaskStore, + }); }); afterEach(() => { @@ -2514,6 +2534,115 @@ describe("runTaskRetry", () => { expect(successLine![0]).toContain("FN-001"); expect(successLine![0]).toContain("todo"); }); + + it("retries stranded in-review task with status none and incomplete steps to todo preserving progress", async () => { + mockGetTask.mockResolvedValueOnce(makeTask({ + id: "FN-001", + status: null, + column: "in-review", + mergeRetries: 4, + steps: [ + { name: "Step 0", status: "done" }, + { name: "Step 1", status: "in-progress" }, + { name: "Step 2", status: "pending" }, + ], + })); + mockUpdateTask.mockResolvedValueOnce(makeTask({ id: "FN-001", status: undefined, error: undefined })); + mockMoveTask.mockResolvedValueOnce(makeTask({ id: "FN-001", column: "todo" })); + mockLogEntry.mockResolvedValueOnce(makeTask({ id: "FN-001" })); + + await runTaskRetry("FN-001"); + + expect(mockUpdateTask).toHaveBeenCalledWith("FN-001", expect.objectContaining({ + status: null, + error: null, + })); + expect(mockUpdateTask.mock.calls[0][1]).not.toHaveProperty("mergeRetries"); + expect(mockMoveTask).toHaveBeenCalledWith("FN-001", "todo", { preserveProgress: true }); + expect(mockLogEntry).toHaveBeenCalledWith( + "FN-001", + "Retry requested from CLI (stranded in-review execution retry → todo, preserving progress)", + ); + }); + + it("retries stranded zero-step in-review task with no merge attempts to todo", async () => { + mockGetTask.mockResolvedValueOnce(makeTask({ + id: "FN-001", + status: null, + column: "in-review", + mergeRetries: 0, + steps: [], + })); + mockUpdateTask.mockResolvedValueOnce(makeTask({ id: "FN-001", status: undefined, error: undefined })); + mockMoveTask.mockResolvedValueOnce(makeTask({ id: "FN-001", column: "todo" })); + mockLogEntry.mockResolvedValueOnce(makeTask({ id: "FN-001" })); + + await runTaskRetry("FN-001"); + + expect(mockMoveTask).toHaveBeenCalledWith("FN-001", "todo", { preserveProgress: true }); + expect(mockLogEntry).toHaveBeenCalledWith( + "FN-001", + "Retry requested from CLI (stranded in-review execution retry → todo, preserving progress)", + ); + }); + + it("retries stranded in-review task with status none and prior merge attempts as merge retry", async () => { + mockGetTask.mockResolvedValueOnce(makeTask({ + id: "FN-001", + status: null, + column: "in-review", + mergeRetries: 2, + steps: [ + { name: "Step 0", status: "done" }, + { name: "Step 1", status: "done" }, + ], + })); + mockUpdateTask.mockResolvedValueOnce(makeTask({ id: "FN-001", status: undefined, error: undefined })); + mockLogEntry.mockResolvedValueOnce(makeTask({ id: "FN-001" })); + + await runTaskRetry("FN-001"); + + expect(mockUpdateTask).toHaveBeenCalledWith("FN-001", expect.objectContaining({ + status: null, + error: null, + mergeRetries: 0, + })); + expect(mockMoveTask).not.toHaveBeenCalled(); + expect(mockLogEntry).toHaveBeenCalledWith( + "FN-001", + "Retry requested from CLI (in-review merge retry, mergeRetries reset)", + ); + }); + + it("rejects stranded in-review task with status none, completed steps, and no merge attempts", async () => { + mockGetTask.mockResolvedValueOnce(makeTask({ + id: "FN-001", + status: null, + column: "in-review", + mergeRetries: 0, + steps: [ + { name: "Step 0", status: "done" }, + { name: "Step 1", status: "done" }, + ], + })); + + await expect(runTaskRetry("FN-001")).rejects.toThrow("Task FN-001 is not in a retryable state (status: none)"); + expect(mockUpdateTask).not.toHaveBeenCalled(); + expect(mockMoveTask).not.toHaveBeenCalled(); + }); + + it("rejects non-review task with status none", async () => { + mockGetTask.mockResolvedValueOnce(makeTask({ + id: "FN-001", + status: null, + column: "todo", + steps: [{ name: "Step 0", status: "pending" }], + })); + + await expect(runTaskRetry("FN-001")).rejects.toThrow("Task FN-001 is not in a retryable state (status: none)"); + expect(mockUpdateTask).not.toHaveBeenCalled(); + expect(mockMoveTask).not.toHaveBeenCalled(); + }); }); // --- Logs Tests --- diff --git a/packages/cli/src/commands/task.ts b/packages/cli/src/commands/task.ts index a2cf2eb755..641f737f2a 100644 --- a/packages/cli/src/commands/task.ts +++ b/packages/cli/src/commands/task.ts @@ -1027,13 +1027,70 @@ export async function runTaskRetry(id: string, projectName?: string) { throw new Error(`Task ${id} not found`); } + const isInReviewStatusNone = + task.column === "in-review" && (task.status === null || task.status === undefined); + const hasIncompleteSteps = task.steps.some( + (s: { status: string }) => s.status === "pending" || s.status === "in-progress", + ); + // FN-4130 / PR #59 follow-up: zero-step review failures with no merge attempts + // (`mergeRetries ?? 0 === 0`) failed during execution, not merge finalization. + const isExecutionFailureInReview = + hasIncompleteSteps || (task.steps.length === 0 && (task.mergeRetries ?? 0) === 0); + const isInReviewExecutionStall = isInReviewStatusNone && isExecutionFailureInReview; + const isInReviewMergeRetryStall = isInReviewStatusNone && (task.mergeRetries ?? 0) > 0; + const isInReviewRetry = + task.column === "in-review" && + (task.status === "failed" || + task.status === "stuck-killed" || + isInReviewExecutionStall || + isInReviewMergeRetryStall); + // Validate task is in a retryable state - if (task.status !== 'failed' && task.status !== 'stuck-killed') { + if (task.status !== 'failed' && task.status !== 'stuck-killed' && !isInReviewRetry) { throw new Error(`Task ${id} is not in a retryable state (status: ${task.status || 'none'})`); } const autoPauseClearPatch = buildAutoPauseClearPatch(task); const clearedDeadlockAutoPause = Object.keys(autoPauseClearPatch).length > 0; + const retryLogSuffix = clearedDeadlockAutoPause ? ", cleared deadlock auto-pause" : ""; + + // In-review retry: distinguish between execution failures (incomplete steps) + // and merge failures (all steps done). + if (isInReviewRetry) { + if (isExecutionFailureInReview) { + await store.updateTask(id, { + status: null, + error: null, + ...autoPauseClearPatch, + ...buildManualRetryResetPatch(), + }); + await store.logEntry( + id, + isInReviewExecutionStall + ? `Retry requested from CLI (stranded in-review execution retry → todo, preserving progress${retryLogSuffix})` + : `Retry requested from CLI (execution failure in-review → todo, preserving progress${retryLogSuffix})`, + ); + await store.moveTask(id, "todo", { preserveProgress: true }); + + console.log(); + console.log(` ✓ Retried ${id} → todo (execution failure, preserving step progress)`); + console.log(); + return; + } + + await store.updateTask(id, { + status: null, + error: null, + ...autoPauseClearPatch, + ...buildManualRetryResetPatch({ resetMergeRetries: true }), + }); + await store.logEntry(id, `Retry requested from CLI (in-review merge retry, mergeRetries reset${retryLogSuffix})`); + + console.log(); + console.log(` ✓ Retried ${id} → in-review (merge retry state cleared)`); + console.log(); + return; + } // Clear failure state and stale branch refs so retry can choose a fresh base. await store.updateTask(id, { diff --git a/packages/cli/src/extension.ts b/packages/cli/src/extension.ts index 79f50ec327..8d1039819c 100644 --- a/packages/cli/src/extension.ts +++ b/packages/cli/src/extension.ts @@ -1022,7 +1022,7 @@ export default function kbExtension(pi: ExtensionAPI) { promptSnippet: "Retry a failed Fusion task (clears error, moves to todo or stays in in-review)", promptGuidelines: [ "Use when a task has failed and needs to be retried", - "Only tasks in 'failed' or 'stuck-killed' state can be retried", + "Only tasks in 'failed' or 'stuck-killed' state, plus stranded in-review tasks with incomplete execution or prior merge attempts, can be retried", "In-review tasks with incomplete steps (pending/in-progress) move to todo with preserveProgress so execution can resume", "In-review tasks with all steps done stay in in-review and reset merge retry state for auto-merge re-attempt", "Tasks in other columns are moved to the todo column with error state cleared", @@ -1046,8 +1046,26 @@ export default function kbExtension(pi: ExtensionAPI) { }; } + const isInReviewStatusNone = + task.column === "in-review" && (task.status === null || task.status === undefined); + const hasIncompleteSteps = task.steps.some( + (s: { status: string }) => s.status === "pending" || s.status === "in-progress", + ); + // FN-4130 / PR #59 follow-up: zero-step review failures with no merge attempts + // (`mergeRetries ?? 0 === 0`) failed during execution, not merge finalization. + const isExecutionFailureInReview = + hasIncompleteSteps || (task.steps.length === 0 && (task.mergeRetries ?? 0) === 0); + const isInReviewExecutionStall = isInReviewStatusNone && isExecutionFailureInReview; + const isInReviewMergeRetryStall = isInReviewStatusNone && (task.mergeRetries ?? 0) > 0; + const isInReviewRetry = + task.column === "in-review" && + (task.status === "failed" || + task.status === "stuck-killed" || + isInReviewExecutionStall || + isInReviewMergeRetryStall); + // Validate task is in a retryable state - if (task.status !== 'failed' && task.status !== 'stuck-killed') { + if (task.status !== 'failed' && task.status !== 'stuck-killed' && !isInReviewRetry) { return { content: [{ type: "text", text: `Task ${params.id} is not in a retryable state (status: ${task.status || 'none'})` }], isError: true, @@ -1060,15 +1078,7 @@ export default function kbExtension(pi: ExtensionAPI) { const retryLogSuffix = clearedDeadlockAutoPause ? ", cleared deadlock auto-pause" : ""; // In-review retry: distinguish between execution failures and merge failures. - if (task.column === 'in-review') { - const hasIncompleteSteps = task.steps.some( - (s: { status: string }) => s.status === "pending" || s.status === "in-progress", - ); - // FN-4130 / PR #59 follow-up: zero-step review failures with no merge attempts - // (`mergeRetries ?? 0 === 0`) failed during execution, not merge finalization. - const isExecutionFailureInReview = - hasIncompleteSteps || (task.steps.length === 0 && (task.mergeRetries ?? 0) === 0); - + if (isInReviewRetry) { if (isExecutionFailureInReview) { await store.updateTask(params.id, { status: null, @@ -1076,7 +1086,12 @@ export default function kbExtension(pi: ExtensionAPI) { ...autoPauseClearPatch, ...buildManualRetryResetPatch(), }); - await store.logEntry(params.id, `Retry requested via Fusion extension (execution failure in-review → todo, preserving progress${retryLogSuffix})`); + await store.logEntry( + params.id, + isInReviewExecutionStall + ? `Retry requested via Fusion extension (stranded in-review execution retry → todo, preserving progress${retryLogSuffix})` + : `Retry requested via Fusion extension (execution failure in-review → todo, preserving progress${retryLogSuffix})`, + ); await store.moveTask(params.id, "todo", { preserveProgress: true }); return { content: [{ type: "text", text: `Retried ${params.id} → todo (execution failure, preserving step progress)` }], diff --git a/packages/dashboard/src/__tests__/routes-tasks-ops.test.ts b/packages/dashboard/src/__tests__/routes-tasks-ops.test.ts index 06198a4942..7c13caadcd 100644 --- a/packages/dashboard/src/__tests__/routes-tasks-ops.test.ts +++ b/packages/dashboard/src/__tests__/routes-tasks-ops.test.ts @@ -470,6 +470,140 @@ describe("POST /tasks/:id/retry", () => { expect(updateCall).not.toHaveProperty("mergeRetries"); }); + it("retries status-none in-review task with incomplete steps by moving to todo", async () => { + const reviewTask = { + ...FAKE_TASK_DETAIL, + column: "in-review" as const, + status: null, + mergeRetries: 4, + steps: [ + { name: "Step 0", status: "done" }, + { name: "Step 1", status: "in-progress" }, + { name: "Step 2", status: "pending" }, + ], + }; + const movedTask = { ...reviewTask, column: "todo" as const, status: undefined }; + (store.getTask as ReturnType).mockResolvedValueOnce(reviewTask); + (store.updateTask as ReturnType).mockResolvedValue(reviewTask); + (store.moveTask as ReturnType).mockResolvedValue(movedTask); + + const res = await REQUEST(buildApp(), "POST", "/api/tasks/KB-001/retry", JSON.stringify({}), { + "Content-Type": "application/json", + }); + + expect(res.status).toBe(200); + expect(store.updateTask).toHaveBeenCalledWith("KB-001", { + status: null, + error: null, + ...buildManualRetryResetPatch(), + }); + expect(store.moveTask).toHaveBeenCalledWith("KB-001", "todo", { preserveProgress: true }); + expect(store.logEntry).toHaveBeenCalledWith( + "KB-001", + "Retry requested from dashboard (stranded in-review execution retry → todo, preserving progress)", + ); + }); + + it("retries status-none zero-step in-review task with no merge attempts by moving to todo", async () => { + const reviewTask = { + ...FAKE_TASK_DETAIL, + column: "in-review" as const, + status: null, + steps: [], + mergeRetries: 0, + }; + const movedTask = { ...reviewTask, column: "todo" as const, status: undefined }; + (store.getTask as ReturnType).mockResolvedValueOnce(reviewTask); + (store.updateTask as ReturnType).mockResolvedValue(reviewTask); + (store.moveTask as ReturnType).mockResolvedValue(movedTask); + + const res = await REQUEST(buildApp(), "POST", "/api/tasks/KB-001/retry", JSON.stringify({}), { + "Content-Type": "application/json", + }); + + expect(res.status).toBe(200); + expect(store.moveTask).toHaveBeenCalledWith("KB-001", "todo", { preserveProgress: true }); + expect(store.logEntry).toHaveBeenCalledWith( + "KB-001", + "Retry requested from dashboard (stranded in-review execution retry → todo, preserving progress)", + ); + }); + + it("retries status-none in-review task with prior merge attempts by staying in-review", async () => { + const reviewTask = { + ...FAKE_TASK_DETAIL, + column: "in-review" as const, + status: null, + mergeRetries: 2, + steps: [ + { name: "Step 0", status: "done" }, + { name: "Step 1", status: "done" }, + ], + }; + (store.getTask as ReturnType) + .mockResolvedValueOnce(reviewTask) + .mockResolvedValueOnce({ ...reviewTask, status: undefined, mergeRetries: 0 }); + (store.updateTask as ReturnType).mockResolvedValue(reviewTask); + + const res = await REQUEST(buildApp(), "POST", "/api/tasks/KB-001/retry", JSON.stringify({}), { + "Content-Type": "application/json", + }); + + expect(res.status).toBe(200); + expect(store.updateTask).toHaveBeenCalledWith("KB-001", { + status: null, + error: null, + ...buildManualRetryResetPatch({ resetMergeRetries: true }), + }); + expect(store.moveTask).not.toHaveBeenCalled(); + expect(store.logEntry).toHaveBeenCalledWith( + "KB-001", + "Retry requested from dashboard (in-review merge retry, mergeRetries reset)", + ); + }); + + it("returns 400 for status-none in-review task with completed steps and no merge attempts", async () => { + const reviewTask = { + ...FAKE_TASK_DETAIL, + column: "in-review" as const, + status: null, + mergeRetries: 0, + steps: [ + { name: "Step 0", status: "done" }, + { name: "Step 1", status: "done" }, + ], + }; + (store.getTask as ReturnType).mockResolvedValueOnce(reviewTask); + + const res = await REQUEST(buildApp(), "POST", "/api/tasks/KB-001/retry", JSON.stringify({}), { + "Content-Type": "application/json", + }); + + expect(res.status).toBe(400); + expect(res.body.error).toContain("not in a retryable state"); + expect(store.updateTask).not.toHaveBeenCalled(); + expect(store.moveTask).not.toHaveBeenCalled(); + }); + + it("returns 400 for non-review task with status none", async () => { + const task = { + ...FAKE_TASK_DETAIL, + column: "todo" as const, + status: null, + steps: [{ name: "Step 0", status: "pending" }], + }; + (store.getTask as ReturnType).mockResolvedValueOnce(task); + + const res = await REQUEST(buildApp(), "POST", "/api/tasks/KB-001/retry", JSON.stringify({}), { + "Content-Type": "application/json", + }); + + expect(res.status).toBe(400); + expect(res.body.error).toContain("not in a retryable state"); + expect(store.updateTask).not.toHaveBeenCalled(); + expect(store.moveTask).not.toHaveBeenCalled(); + }); + it("preserves worktree/branch when retrying in-review task", async () => { const reviewTask = { ...FAKE_TASK_DETAIL, diff --git a/packages/dashboard/src/routes/register-task-workflow-routes.ts b/packages/dashboard/src/routes/register-task-workflow-routes.ts index 1f8324bebc..e5160abb2b 100644 --- a/packages/dashboard/src/routes/register-task-workflow-routes.ts +++ b/packages/dashboard/src/routes/register-task-workflow-routes.ts @@ -1541,10 +1541,24 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork task.status === "planning" || task.status === "needs-replan" || (task.stuckKillCount ?? 0) > 0); + const isInReviewStatusNone = + task.column === "in-review" && (task.status === null || task.status === undefined); + const hasIncompleteSteps = task.steps.some( + (s: { status: string }) => s.status === "pending" || s.status === "in-progress", + ); + // FN-4130 / PR #59 follow-up: zero-step review failures with no merge attempts + // (`mergeRetries ?? 0 === 0`) failed during execution, not merge finalization. + const isExecutionFailureInReview = + hasIncompleteSteps || (task.steps.length === 0 && (task.mergeRetries ?? 0) === 0); + const isInReviewExecutionStall = isInReviewStatusNone && isExecutionFailureInReview; + const isInReviewMergeRetryStall = isInReviewStatusNone && (task.mergeRetries ?? 0) > 0; const isInReviewRetry = task.column === "in-review" && - (task.status === "failed" || task.status === "stuck-killed"); - if (task.status !== "failed" && task.status !== "stuck-killed" && !retrySpecification) { + (task.status === "failed" || + task.status === "stuck-killed" || + isInReviewExecutionStall || + isInReviewMergeRetryStall); + if (task.status !== "failed" && task.status !== "stuck-killed" && !retrySpecification && !isInReviewRetry) { throw badRequest(`Task is not in a retryable state (current status: ${task.status || 'none'})`); } @@ -1555,14 +1569,6 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork // In-review retry: distinguish between execution failures (incomplete steps) // and merge failures (all steps done). if (isInReviewRetry) { - const hasIncompleteSteps = task.steps.some( - (s: { status: string }) => s.status === "pending" || s.status === "in-progress", - ); - // FN-4130 / PR #59 follow-up: zero-step review failures with no merge attempts - // (`mergeRetries ?? 0 === 0`) failed during execution, not merge finalization. - const isExecutionFailureInReview = - hasIncompleteSteps || (task.steps.length === 0 && (task.mergeRetries ?? 0) === 0); - if (isExecutionFailureInReview) { await scopedStore.updateTask(req.params.id, { status: null, @@ -1572,7 +1578,9 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork }); await scopedStore.logEntry( req.params.id, - `Retry requested from dashboard (execution failure in-review → todo, preserving progress${retryLogSuffix})`, + isInReviewExecutionStall + ? `Retry requested from dashboard (stranded in-review execution retry → todo, preserving progress${retryLogSuffix})` + : `Retry requested from dashboard (execution failure in-review → todo, preserving progress${retryLogSuffix})`, ); const updated = await scopedStore.moveTask(req.params.id, "todo", { preserveProgress: true }); res.json(updated);