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;
|
||||
}
|
||||
|
||||
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 liveTask = {
|
||||
id: "FN-B1",
|
||||
@@ -44,7 +73,8 @@ function makeExecutor(opts: { selection?: { workflowId: string; stepIds: string[
|
||||
description: "",
|
||||
column: opts.taskColumn ?? "in-review",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
steps: opts.steps ?? [],
|
||||
workflowStepResults: opts.workflowStepResults ?? [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
prompt: "# t",
|
||||
@@ -109,4 +139,44 @@ describe("U5a — IR-driven merge boundary (scenario 1)", () => {
|
||||
const moveTask = store.moveTask as ReturnType<typeof vi.fn>;
|
||||
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);
|
||||
|
||||
// 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
|
||||
// byte-identical to the pre-cutover `in-review || done` guard.
|
||||
if (live.column === targetColumn || live.column === "done") return live;
|
||||
/*
|
||||
FNXC:WorkflowMerge 2026-07-26-22:59:
|
||||
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.
|
||||
*/
|
||||
const alreadyAtMergeColumn = live.column === targetColumn;
|
||||
if (live.column === "done") return live;
|
||||
if (live.paused || live.userPaused) return live;
|
||||
|
||||
/*
|
||||
@@ -7669,6 +7671,7 @@ export class TaskExecutor {
|
||||
this.getRunContextFor(live.id),
|
||||
);
|
||||
}
|
||||
if (alreadyAtMergeColumn) return live;
|
||||
const moveOptions = {
|
||||
preserveProgress: true,
|
||||
moveSource: "engine" as const,
|
||||
|
||||
Reference in New Issue
Block a user