root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(engine): resume execution after review revise

Browse Source
This commit is contained in:
Phil Larson
2026-05-29 10:59:25 -07:00
parent b265d6b395
commit 6b57e7cbf6
6 changed files with 91 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
packages/engine/src/__tests__/executor-step-session.test.ts
View File

@@ -193,7 +193,7 @@ describe("Workflow Steps Execution", () => {
});
describe("FN-5436: pending-review skip on no-fn_task_done exit", () => {
it("parks in-review immediately when code review REVISE is pending", async () => {
it("does not park in-review when code review REVISE requires more executor work", async () => {
const store = createMockStore();
const baseTask = {
id: "FN-5436-A",
@@ -240,22 +240,20 @@ describe("Workflow Steps Execution", () => {
await executor.execute(baseTask as any);
expect(mockedCreateFnAgent).toHaveBeenCalledTimes(1);
expect(mockedCreateFnAgent).toHaveBeenCalledTimes(4);
expect(store.updateTask).toHaveBeenCalledWith("FN-5436-A", expect.objectContaining({
status: "failed",
error: "Agent finished without calling fn_task_done (after 3 retries)",
}));
expect(store.moveTask).toHaveBeenCalledWith("FN-5436-A", "todo", { preserveProgress: true });
expect(store.moveTask).not.toHaveBeenCalledWith("FN-5436-A", "in-review");
expect(store.updateTask).not.toHaveBeenCalledWith("FN-5436-A", {
status: "failed",
error: "executor-exit-while-review-pending",
});
expect(store.updateTask).not.toHaveBeenCalledWith("FN-5436-A", expect.objectContaining({ taskDoneRetryCount: expect.anything() }));
expect(store.moveTask).toHaveBeenCalledWith("FN-5436-A", "in-review");
expect(store.logEntry).toHaveBeenCalledWith(
"FN-5436-A",
expect.stringContaining("blocked on pending review (code-review-revise-outstanding)"),
undefined,
expect.objectContaining({ agentId: "executor" }),
);
expect(onError).not.toHaveBeenCalledWith(
expect(onError).toHaveBeenCalledWith(
expect.objectContaining({ id: "FN-5436-A" }),
expect.objectContaining({ message: "executor-exit-while-review-pending" }),
expect.objectContaining({ message: "Agent finished without calling fn_task_done (after 3 retries)" }),
);
});

25
packages/engine/src/__tests__/merge-error-recovery.test.ts
View File

@@ -710,6 +710,31 @@ describe("ProjectEngine merge error recovery", () => {
expect(store.moveTask).toHaveBeenCalledWith(TASK_ID, "done");
});
it("auto-finalizes merge-confirmed tasks with stale transient merging status", async () => {
const store = makeStore({
tasks: [
makeTask({
mergeDetails: { mergeConfirmed: true },
status: "merging",
error: "stale transient merge state",
}),
],
});
const engine = createEngine(store);
await runMergeCycle(engine);
expect(store.updateTask).toHaveBeenCalledWith(TASK_ID, { paused: false, status: null, error: null });
expect(store.moveTask).toHaveBeenCalledWith(TASK_ID, "done");
expect(store.updateTask).not.toHaveBeenCalledWith(
TASK_ID,
expect.objectContaining({
status: "failed",
error: expect.stringContaining("finalization blocked"),
}),
);
});
it("does not park merge-confirmed tasks as failed when finalize loses in-review ownership", async () => {
const store = makeStore({
tasks: [

36
packages/engine/src/__tests__/self-healing.test.ts
View File

@@ -4002,6 +4002,42 @@ describe("SelfHealingManager", () => {
managerWithRecovery.stop();
});
it("auto-finalizes merge-confirmed tasks with stale transient merging status", async () => {
const managerWithRecovery = new SelfHealingManager(store, {
rootDir: "/tmp/test-project",
});
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
{
id: "FN-354",
column: "in-review",
paused: false,
status: "merging",
error: "stale transient merge state",
mergeDetails: {
mergeConfirmed: true,
mergedAt: "2026-01-01T00:00:00.000Z",
},
steps: [{ status: "done" }],
workflowStepResults: [],
log: [],
},
]);
const result = await managerWithRecovery.recoverMergedReviewTasks();
expect(result).toBe(1);
expect(store.updateTask).toHaveBeenCalledWith("FN-354", {
paused: false,
status: null,
error: null,
mergeRetries: 0,
});
expect(store.moveTask).toHaveBeenCalledWith("FN-354", "done");
managerWithRecovery.stop();
});
});
describe("recoverStuckMergeDeadlocks", () => {

7
packages/engine/src/executor.ts
View File

@@ -276,7 +276,6 @@ type PendingReviewBlockResult =
| {
blocked: true;
reason:
| "code-review-revise-outstanding"
| "review-request-without-verdict"
| "code-review-rethink-or-unavailable-outstanding"
| "code-review-unavailable-blocking";
@@ -286,7 +285,7 @@ type PendingReviewBlockResult =
function detectPendingReviewBlock(
task: Task,
codeReviewVerdicts: Map<number, ReviewVerdict>,
_codeReviewVerdicts: Map<number, ReviewVerdict>,
): PendingReviewBlockResult {
const inProgressStepIndices: number[] = [];
for (let stepIndex = 0; stepIndex < task.steps.length; stepIndex++) {
@@ -305,10 +304,6 @@ function detectPendingReviewBlock(
.filter((action): action is string => Boolean(action));
for (const stepIndex of inProgressStepIndices) {
if (codeReviewVerdicts.get(stepIndex) === "REVISE") {
return { blocked: true, reason: "code-review-revise-outstanding", stepIndex };
}
const stepDisplay = stepIndex + 1;
const codeRequest = `code review requested for Step ${stepDisplay}`;
const planRequest = `plan review requested for Step ${stepDisplay}`;

10
packages/engine/src/project-engine.ts
View File

@@ -1597,7 +1597,15 @@ export class ProjectEngine {
this.internalEnqueueMerge(taskId);
continue;
}
const blockerReason = getTaskHardMergeBlocker(task as Task);
const blockerReason = getTaskHardMergeBlocker({
...(task as Task),
// Merge-confirmed tasks have already landed. Treat stale merge
// in-flight statuses as soft state to clear during finalization,
// not hard blockers that park an otherwise confirmed merge as failed.
paused: false,
status: task.status === "merging" || task.status === "merging-pr" ? undefined : task.status,
error: undefined,
});
if (blockerReason) {
await store.updateTask(taskId, {
status: "failed",

11
packages/engine/src/self-healing.ts
View File

@@ -3731,7 +3731,7 @@ export class SelfHealingManager {
const dep = taskById.get(depId);
// listTasks excludes soft-deleted rows, so missing dependency IDs are
// treated as resolved here by design.
return dep && dep.column !== "done" && dep.column !== "in-review" && dep.column !== "archived";
return dep && !dep.deletedAt && dep.column !== "done" && dep.column !== "in-review" && dep.column !== "archived";
});
const overlapBlocker = task.overlapBlockedBy ? taskById.get(task.overlapBlockedBy) : undefined;
const hasActiveOverlapBlocker = Boolean(
@@ -3762,6 +3762,9 @@ export class SelfHealingManager {
reasonCode = "missing-blocker";
reason = `blocker ${blockerId} missing`;
}
} else if (blocker.deletedAt) {
reasonCode = "soft-deleted-blocker";
reason = `blocker ${blockerId} soft-deleted at ${blocker.deletedAt}`;
} else if (blocker.column === "done") {
reasonCode = "blocker-done";
reason = `blocker ${blockerId} is done`;
@@ -5612,6 +5615,12 @@ export class SelfHealingManager {
try {
const hardBlocker = getTaskHardMergeBlocker({
...task,
// Merge-confirmed tasks have already landed. Treat stale merge
// in-flight statuses as soft state to clear during finalization,
// not hard blockers that park an otherwise confirmed merge as failed.
paused: false,
status: task.status === "merging" || task.status === "merging-pr" ? undefined : task.status,
error: undefined,
steps: task.steps ?? [],
workflowStepResults: task.workflowStepResults,
});