fix(engine): project CE steps after review handoff (#2464)
## Summary - reconcile successful graph-native workflow results with pending task checklist steps even when review handoff already moved the card into the merge column - preserve terminal, paused, and no-redundant-move behavior - cover the real Compound Engineering post-review-handoff state with a regression test ## Root cause Compound Engineering runs `review-handoff` before `merge`. Review handoff moves the task to `in-review`, which is also the merge column. `ensureWorkflowMergeBoundaryTask()` returned immediately for cards already in that column, before projecting successful `workflowStepResults` onto legacy `Task.steps[]`. The merger then saw `0/N` and rejected approved work with `task has incomplete steps`. ## Verification - RED: regression test failed before the fix because `store.updateTask` was never called - GREEN: `executor-graph-boundary.test.ts` — 6 passed - relevant non-PostgreSQL set — 31 passed, 5 PostgreSQL tests explicitly skipped - `@fusion/engine` typecheck passed - changeset format passed - `git diff --check` passed ## Baseline note `ce-workflow-step-executor.test.ts` currently has three failures on clean `origin/main` after FN-8601 foreach-proof hardening. The same failures reproduce without this patch and are not regressions from this change. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved reconciliation after review handoff by projecting completed step results onto the legacy checklist when reaching the merge column. * Prevented tasks from being marked approved with incomplete step counts (including “0/N” style states). * Reduced unnecessary merge failures and deadlock/pause scenarios when merge-column progress was already recorded. * **Tests** * Added coverage for execute-and-merge workflows, ensuring merge-boundary resolution updates pending steps without moving the task. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
7
.changeset/ce-review-handoff-step-projection.md
Normal file
7
.changeset/ce-review-handoff-step-projection.md
Normal file
@@ -0,0 +1,7 @@
|
|||||||
|
---
|
||||||
|
"@runfusion/fusion": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
summary: Graph-native workflows now reconcile completed task steps after review handoff reaches the merge column.
|
||||||
|
category: fix
|
||||||
|
dev: `ensureWorkflowMergeBoundaryTask` evaluates successful node-result proof and projects it onto the legacy checklist before applying its already-at-merge-column no-op. This prevents Compound Engineering tasks from reaching approved review at `0/N`, failing merge with `task has incomplete steps`, and deadlock-pausing.
|
||||||
@@ -36,7 +36,36 @@ function benchmarkIr(): WorkflowIr {
|
|||||||
} as WorkflowIr;
|
} as WorkflowIr;
|
||||||
}
|
}
|
||||||
|
|
||||||
function makeExecutor(opts: { selection?: { workflowId: string; stepIds: string[] }; ir?: WorkflowIr; taskColumn?: string }) {
|
function executeIr(): WorkflowIr {
|
||||||
|
return {
|
||||||
|
version: "v2",
|
||||||
|
name: "execute then merge",
|
||||||
|
columns: [
|
||||||
|
{ id: "in-progress", name: "In progress", traits: [] },
|
||||||
|
{ id: "in-review", name: "In review", traits: [{ trait: "merge" }, { trait: "merge-blocker" }] },
|
||||||
|
],
|
||||||
|
nodes: [
|
||||||
|
{ id: "execute", kind: "prompt", column: "in-progress", config: { seam: "execute" } },
|
||||||
|
{ id: "merge", kind: "merge-gate", column: "in-review" },
|
||||||
|
],
|
||||||
|
edges: [{ from: "execute", to: "merge", condition: "success" }],
|
||||||
|
} as WorkflowIr;
|
||||||
|
}
|
||||||
|
|
||||||
|
function makeExecutor(opts: {
|
||||||
|
selection?: { workflowId: string; stepIds: string[] };
|
||||||
|
ir?: WorkflowIr;
|
||||||
|
taskColumn?: string;
|
||||||
|
steps?: Array<{ id: string; title: string; status: "pending" | "done" }>;
|
||||||
|
workflowStepResults?: Array<{
|
||||||
|
workflowStepId: string;
|
||||||
|
workflowStepName: string;
|
||||||
|
source: "node";
|
||||||
|
phase: "pre-merge";
|
||||||
|
status: "passed";
|
||||||
|
completedAt: string;
|
||||||
|
}>;
|
||||||
|
}) {
|
||||||
const store = createMockStore() as unknown as Record<string, unknown>;
|
const store = createMockStore() as unknown as Record<string, unknown>;
|
||||||
const liveTask = {
|
const liveTask = {
|
||||||
id: "FN-B1",
|
id: "FN-B1",
|
||||||
@@ -44,7 +73,8 @@ function makeExecutor(opts: { selection?: { workflowId: string; stepIds: string[
|
|||||||
description: "",
|
description: "",
|
||||||
column: opts.taskColumn ?? "in-review",
|
column: opts.taskColumn ?? "in-review",
|
||||||
dependencies: [],
|
dependencies: [],
|
||||||
steps: [],
|
steps: opts.steps ?? [],
|
||||||
|
workflowStepResults: opts.workflowStepResults ?? [],
|
||||||
currentStep: 0,
|
currentStep: 0,
|
||||||
log: [],
|
log: [],
|
||||||
prompt: "# t",
|
prompt: "# t",
|
||||||
@@ -109,4 +139,44 @@ describe("U5a — IR-driven merge boundary (scenario 1)", () => {
|
|||||||
const moveTask = store.moveTask as ReturnType<typeof vi.fn>;
|
const moveTask = store.moveTask as ReturnType<typeof vi.fn>;
|
||||||
expect(moveTask).not.toHaveBeenCalled();
|
expect(moveTask).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/*
|
||||||
|
FNXC:WorkflowLifecycle 2026-07-26-22:59:
|
||||||
|
Successful pre-merge proof must still project graph-native results onto legacy steps after review handoff has already moved the card into the merge column; the projection must not trigger a redundant move.
|
||||||
|
*/
|
||||||
|
it("projects graph-native completion after review handoff already moved the card to the merge column", async () => {
|
||||||
|
const pendingSteps = [
|
||||||
|
{ id: "0", title: "Preflight", status: "pending" as const },
|
||||||
|
{ id: "1", title: "Implement", status: "pending" as const },
|
||||||
|
];
|
||||||
|
const { executor, store, liveTask } = makeExecutor({
|
||||||
|
selection: { workflowId: "custom:execute", stepIds: [] },
|
||||||
|
ir: executeIr(),
|
||||||
|
taskColumn: "in-review",
|
||||||
|
steps: pendingSteps,
|
||||||
|
workflowStepResults: [{
|
||||||
|
workflowStepId: "execute",
|
||||||
|
workflowStepName: "Execute",
|
||||||
|
source: "node",
|
||||||
|
phase: "pre-merge",
|
||||||
|
status: "passed",
|
||||||
|
completedAt: new Date().toISOString(),
|
||||||
|
}],
|
||||||
|
});
|
||||||
|
|
||||||
|
await executor.ensureWorkflowMergeBoundaryTask(
|
||||||
|
liveTask,
|
||||||
|
{ reason: "workflow-merge-boundary", nodeId: "merge", workflowId: "custom:execute", runId: "r1" },
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(store.updateTask).toHaveBeenCalledWith(
|
||||||
|
"FN-B1",
|
||||||
|
{
|
||||||
|
steps: pendingSteps.map((step) => ({ ...step, status: "done" })),
|
||||||
|
currentStep: 1,
|
||||||
|
},
|
||||||
|
undefined,
|
||||||
|
);
|
||||||
|
expect(store.moveTask).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -7623,10 +7623,12 @@ export class TaskExecutor {
|
|||||||
*/
|
*/
|
||||||
const targetColumn = await this.resolveMergeBoundaryColumn(task.id, metadata.nodeId);
|
const targetColumn = await this.resolveMergeBoundaryColumn(task.id, metadata.nodeId);
|
||||||
|
|
||||||
// Already at the merge column, or in the terminal (done/complete) column:
|
/*
|
||||||
// nothing to do. builtin:coding's targetColumn is `in-review`, so this stays
|
FNXC:WorkflowMerge 2026-07-26-22:59:
|
||||||
// byte-identical to the pre-cutover `in-review || done` guard.
|
A prior review handoff can move a graph-native workflow into its merge column before this boundary projects successful node results onto the legacy checklist. Preserve the no-move behavior, but do not return until the projection has run.
|
||||||
if (live.column === targetColumn || live.column === "done") return live;
|
*/
|
||||||
|
const alreadyAtMergeColumn = live.column === targetColumn;
|
||||||
|
if (live.column === "done") return live;
|
||||||
if (live.paused || live.userPaused) return live;
|
if (live.paused || live.userPaused) return live;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
@@ -7669,6 +7671,7 @@ export class TaskExecutor {
|
|||||||
this.getRunContextFor(live.id),
|
this.getRunContextFor(live.id),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
if (alreadyAtMergeColumn) return live;
|
||||||
const moveOptions = {
|
const moveOptions = {
|
||||||
preserveProgress: true,
|
preserveProgress: true,
|
||||||
moveSource: "engine" as const,
|
moveSource: "engine" as const,
|
||||||
|
|||||||
Reference in New Issue
Block a user