From 52d64fa66e03ca3897a941d6c6202978074e419b Mon Sep 17 00:00:00 2001 From: Phil Larson Date: Sun, 26 Jul 2026 23:13:21 -0700 Subject: [PATCH] fix(engine): project CE steps after review handoff (#2464) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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. ## 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. --- .../ce-review-handoff-step-projection.md | 7 ++ .../__tests__/executor-graph-boundary.test.ts | 74 ++++++++++++++++++- packages/engine/src/executor.ts | 11 ++- 3 files changed, 86 insertions(+), 6 deletions(-) create mode 100644 .changeset/ce-review-handoff-step-projection.md diff --git a/.changeset/ce-review-handoff-step-projection.md b/.changeset/ce-review-handoff-step-projection.md new file mode 100644 index 0000000000..0a229e0d61 --- /dev/null +++ b/.changeset/ce-review-handoff-step-projection.md @@ -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. diff --git a/packages/engine/src/__tests__/executor-graph-boundary.test.ts b/packages/engine/src/__tests__/executor-graph-boundary.test.ts index 2d6d297657..dee50bb853 100644 --- a/packages/engine/src/__tests__/executor-graph-boundary.test.ts +++ b/packages/engine/src/__tests__/executor-graph-boundary.test.ts @@ -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; 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; 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(); + }); }); diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index bdad63a116..bd10a9bf4c 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -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,