From 875cfad136f08532198b5798164fad075c6f9670 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Mon, 29 Jun 2026 09:01:00 -0700 Subject: [PATCH] fix(FN-7231): require merge proof before workflow done --- .changeset/fn-7231-merge-proof-guard.md | 7 ++ .../__tests__/merger-merge-lifecycle.test.ts | 65 +++++++++++++------ .../engine/src/auto-merge-finalization.ts | 27 ++++++-- 3 files changed, 75 insertions(+), 24 deletions(-) create mode 100644 .changeset/fn-7231-merge-proof-guard.md diff --git a/.changeset/fn-7231-merge-proof-guard.md b/.changeset/fn-7231-merge-proof-guard.md new file mode 100644 index 0000000000..853cbd5aa1 --- /dev/null +++ b/.changeset/fn-7231-merge-proof-guard.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Prevent workflow tasks from reaching Done without durable merge confirmation. +category: fix +dev: Workflow graph merge finalization now requires mergeConfirmed proof before accepting done/no-op states. diff --git a/packages/engine/src/__tests__/merger-merge-lifecycle.test.ts b/packages/engine/src/__tests__/merger-merge-lifecycle.test.ts index dccd637923..857730833f 100644 --- a/packages/engine/src/__tests__/merger-merge-lifecycle.test.ts +++ b/packages/engine/src/__tests__/merger-merge-lifecycle.test.ts @@ -315,7 +315,7 @@ describe("auto-merge proven finalization helper", () => { ); }); - it("treats a successful merged result as finalization proof even before mergeConfirmed is persisted", async () => { + it("blocks loose merged results that lack durable merge confirmation", async () => { const strandedTask = { id: "FN-MERGED-PROOF", title: "Merged proof", @@ -333,11 +333,6 @@ describe("auto-merge proven finalization helper", () => { updatedAt: new Date().toISOString(), mergeDetails: undefined, } as Task; - const doneTask = { - ...strandedTask, - column: "done", - mergeDetails: { mergeConfirmed: true, commitSha: "abc123" }, - } as Task; const store = createMockStore(strandedTask) as unknown as TaskStore & { getTask: ReturnType; updateTask: ReturnType; @@ -345,7 +340,6 @@ describe("auto-merge proven finalization helper", () => { recordRunAuditEvent: ReturnType; }; store.getTask.mockResolvedValue(strandedTask); - store.moveTask.mockResolvedValue(doneTask); const result = await finalizeProvenAutoMergeTask({ store, @@ -354,18 +348,13 @@ describe("auto-merge proven finalization helper", () => { source: "workflow-graph-merge-finalize", }); - expect(result.outcome).toBe("done"); - expect(store.updateTask).toHaveBeenCalledWith( - "FN-MERGED-PROOF", - expect.objectContaining({ - mergeDetails: expect.objectContaining({ commitSha: "abc123", mergeConfirmed: true }), - }), - ); - expect(store.moveTask).toHaveBeenCalledWith( - "FN-MERGED-PROOF", - "done", - expect.objectContaining({ recoveryRehome: true, preserveProgress: true }), - ); + expect(result).toEqual(expect.objectContaining({ outcome: "blocked", reason: "missing-merge-confirmation" })); + expect(store.updateTask).not.toHaveBeenCalled(); + expect(store.moveTask).not.toHaveBeenCalled(); + expect(store.recordRunAuditEvent).toHaveBeenCalledWith(expect.objectContaining({ + mutationType: "task:auto-merge-finalize-column-mismatch-no-action", + metadata: expect.objectContaining({ previousColumn: "in-progress", reason: "missing-merge-confirmation" }), + })); }); it("treats already-done landed rows as idempotent success", async () => { @@ -405,6 +394,44 @@ describe("auto-merge proven finalization helper", () => { expect(store.recordRunAuditEvent).not.toHaveBeenCalled(); }); + it("blocks already-done rows that lack merge confirmation instead of accepting the column", async () => { + const doneTask = { + id: "FN-DONE-NOPROOF", + title: "Already done without proof", + description: "Test", + column: "done", + dependencies: [], + steps: [{ status: "done" }], + currentStep: 0, + log: [], + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + mergeDetails: { mergeConfirmed: false, noOpMerge: true }, + } as Task; + const store = createMockStore(doneTask) as unknown as TaskStore & { + getTask: ReturnType; + updateTask: ReturnType; + moveTask: ReturnType; + recordRunAuditEvent: ReturnType; + }; + store.getTask.mockResolvedValue(doneTask); + + const result = await finalizeProvenAutoMergeTask({ + store, + taskId: "FN-DONE-NOPROOF", + result: { task: doneTask, ok: true, merged: true, noOp: true } as MergeResult, + source: "workflow-graph-merge-finalize", + }); + + expect(result).toEqual(expect.objectContaining({ outcome: "blocked", reason: "done-without-merge-confirmation" })); + expect(store.updateTask).not.toHaveBeenCalled(); + expect(store.moveTask).not.toHaveBeenCalled(); + expect(store.recordRunAuditEvent).toHaveBeenCalledWith(expect.objectContaining({ + mutationType: "task:auto-merge-finalize-column-mismatch-no-action", + metadata: expect.objectContaining({ previousColumn: "done", reason: "done-without-merge-confirmation" }), + })); + }); + it("diagnoses rows without merge proof instead of finalizing them", async () => { const unprovenTask = { id: "FN-NOPROOF", diff --git a/packages/engine/src/auto-merge-finalization.ts b/packages/engine/src/auto-merge-finalization.ts index ec99e1a132..1331310922 100644 --- a/packages/engine/src/auto-merge-finalization.ts +++ b/packages/engine/src/auto-merge-finalization.ts @@ -67,11 +67,11 @@ async function recordFinalizationAudit(args: { function buildFinalizationMergeDetails(task: Task, result?: MergeResult): NonNullable { const mergedAt = task.mergeDetails?.mergedAt ?? new Date().toISOString(); /* - * FNXC:WorkflowMerge 2026-06-29-08:33: - * Workflow graph merge nodes receive the direct merge result shape. Some merge callers prove landing with `merged:true` before durable task metadata is refreshed, so finalization must promote that result into `mergeConfirmed` instead of failing the graph at the merge-finalize boundary. + * FNXC:WorkflowMerge 2026-06-29-09:04: + * Workflow graph merge finalization must never promote loose `merged:true` or `noOp:true` results into durable merge proof. A task can reach `done` only when the merger records `mergeConfirmed:true`; otherwise replay/recovery must block so the branch is merged instead of bypassed. */ const mergeConfirmed = - result?.mergeConfirmed === true || result?.merged === true || task.mergeDetails?.mergeConfirmed === true; + result?.mergeConfirmed === true || task.mergeDetails?.mergeConfirmed === true; return { ...(task.mergeDetails ?? {}), ...(result?.commitSha ? { commitSha: result.commitSha } : {}), @@ -83,10 +83,14 @@ function buildFinalizationMergeDetails(task: Task, result?: MergeResult): NonNul ...(result?.mergeCommitMessage ? { mergeCommitMessage: result.mergeCommitMessage } : {}), mergedAt, mergeConfirmed, - ...(result?.noOp ? { noOpMerge: true, noOpReason: result.reason } : {}), + ...(result?.noOp && mergeConfirmed ? { noOpMerge: true, noOpReason: result.reason } : {}), }; } +function hasDurableMergeProof(task: Task, result?: MergeResult): boolean { + return task.mergeDetails?.mergeConfirmed === true || result?.mergeConfirmed === true; +} + /** * FNXC:AutoMergeLifecycle 2026-06-22-19:28: * Proven auto-merge completion must refresh the authoritative row before moving to done because the merge CAS and queue retry paths can leave a landed task in todo with stale queued/overlap state. Use TaskStore recovery rehome for those column mismatches so completion remains idempotent without direct database surgery. @@ -107,12 +111,25 @@ export async function finalizeProvenAutoMergeTask({ } if (latest.column === "done") { + if (!hasDurableMergeProof(latest, result)) { + const reason = "done-without-merge-confirmation"; + await recordFinalizationAudit({ + store, + audit, + task: latest, + type: "task:auto-merge-finalize-column-mismatch-no-action", + reason, + auditAgentId, + auditPhase, + }); + return { outcome: "blocked", task: latest, previousColumn: "done", reason }; + } if (result) result.task = latest; return { outcome: "already-done", task: latest, previousColumn: "done" }; } const mergeDetails = buildFinalizationMergeDetails(latest, result); - const hasProof = mergeDetails.mergeConfirmed === true || result?.mergeConfirmed === true || result?.merged === true || result?.noOp === true; + const hasProof = hasDurableMergeProof({ ...latest, mergeDetails } as Task, result); if (!hasProof) { const reason = "missing-merge-confirmation"; await recordFinalizationAudit({