fix(FN-7231): require merge proof before workflow done
This commit is contained in:
7
.changeset/fn-7231-merge-proof-guard.md
Normal file
7
.changeset/fn-7231-merge-proof-guard.md
Normal file
@@ -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.
|
||||||
@@ -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 = {
|
const strandedTask = {
|
||||||
id: "FN-MERGED-PROOF",
|
id: "FN-MERGED-PROOF",
|
||||||
title: "Merged proof",
|
title: "Merged proof",
|
||||||
@@ -333,11 +333,6 @@ describe("auto-merge proven finalization helper", () => {
|
|||||||
updatedAt: new Date().toISOString(),
|
updatedAt: new Date().toISOString(),
|
||||||
mergeDetails: undefined,
|
mergeDetails: undefined,
|
||||||
} as Task;
|
} as Task;
|
||||||
const doneTask = {
|
|
||||||
...strandedTask,
|
|
||||||
column: "done",
|
|
||||||
mergeDetails: { mergeConfirmed: true, commitSha: "abc123" },
|
|
||||||
} as Task;
|
|
||||||
const store = createMockStore(strandedTask) as unknown as TaskStore & {
|
const store = createMockStore(strandedTask) as unknown as TaskStore & {
|
||||||
getTask: ReturnType<typeof vi.fn>;
|
getTask: ReturnType<typeof vi.fn>;
|
||||||
updateTask: ReturnType<typeof vi.fn>;
|
updateTask: ReturnType<typeof vi.fn>;
|
||||||
@@ -345,7 +340,6 @@ describe("auto-merge proven finalization helper", () => {
|
|||||||
recordRunAuditEvent: ReturnType<typeof vi.fn>;
|
recordRunAuditEvent: ReturnType<typeof vi.fn>;
|
||||||
};
|
};
|
||||||
store.getTask.mockResolvedValue(strandedTask);
|
store.getTask.mockResolvedValue(strandedTask);
|
||||||
store.moveTask.mockResolvedValue(doneTask);
|
|
||||||
|
|
||||||
const result = await finalizeProvenAutoMergeTask({
|
const result = await finalizeProvenAutoMergeTask({
|
||||||
store,
|
store,
|
||||||
@@ -354,18 +348,13 @@ describe("auto-merge proven finalization helper", () => {
|
|||||||
source: "workflow-graph-merge-finalize",
|
source: "workflow-graph-merge-finalize",
|
||||||
});
|
});
|
||||||
|
|
||||||
expect(result.outcome).toBe("done");
|
expect(result).toEqual(expect.objectContaining({ outcome: "blocked", reason: "missing-merge-confirmation" }));
|
||||||
expect(store.updateTask).toHaveBeenCalledWith(
|
expect(store.updateTask).not.toHaveBeenCalled();
|
||||||
"FN-MERGED-PROOF",
|
expect(store.moveTask).not.toHaveBeenCalled();
|
||||||
expect.objectContaining({
|
expect(store.recordRunAuditEvent).toHaveBeenCalledWith(expect.objectContaining({
|
||||||
mergeDetails: expect.objectContaining({ commitSha: "abc123", mergeConfirmed: true }),
|
mutationType: "task:auto-merge-finalize-column-mismatch-no-action",
|
||||||
}),
|
metadata: expect.objectContaining({ previousColumn: "in-progress", reason: "missing-merge-confirmation" }),
|
||||||
);
|
}));
|
||||||
expect(store.moveTask).toHaveBeenCalledWith(
|
|
||||||
"FN-MERGED-PROOF",
|
|
||||||
"done",
|
|
||||||
expect.objectContaining({ recoveryRehome: true, preserveProgress: true }),
|
|
||||||
);
|
|
||||||
});
|
});
|
||||||
|
|
||||||
it("treats already-done landed rows as idempotent success", async () => {
|
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();
|
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<typeof vi.fn>;
|
||||||
|
updateTask: ReturnType<typeof vi.fn>;
|
||||||
|
moveTask: ReturnType<typeof vi.fn>;
|
||||||
|
recordRunAuditEvent: ReturnType<typeof vi.fn>;
|
||||||
|
};
|
||||||
|
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 () => {
|
it("diagnoses rows without merge proof instead of finalizing them", async () => {
|
||||||
const unprovenTask = {
|
const unprovenTask = {
|
||||||
id: "FN-NOPROOF",
|
id: "FN-NOPROOF",
|
||||||
|
|||||||
@@ -67,11 +67,11 @@ async function recordFinalizationAudit(args: {
|
|||||||
function buildFinalizationMergeDetails(task: Task, result?: MergeResult): NonNullable<Task["mergeDetails"]> {
|
function buildFinalizationMergeDetails(task: Task, result?: MergeResult): NonNullable<Task["mergeDetails"]> {
|
||||||
const mergedAt = task.mergeDetails?.mergedAt ?? new Date().toISOString();
|
const mergedAt = task.mergeDetails?.mergedAt ?? new Date().toISOString();
|
||||||
/*
|
/*
|
||||||
* FNXC:WorkflowMerge 2026-06-29-08:33:
|
* FNXC:WorkflowMerge 2026-06-29-09:04:
|
||||||
* 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.
|
* 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 =
|
const mergeConfirmed =
|
||||||
result?.mergeConfirmed === true || result?.merged === true || task.mergeDetails?.mergeConfirmed === true;
|
result?.mergeConfirmed === true || task.mergeDetails?.mergeConfirmed === true;
|
||||||
return {
|
return {
|
||||||
...(task.mergeDetails ?? {}),
|
...(task.mergeDetails ?? {}),
|
||||||
...(result?.commitSha ? { commitSha: result.commitSha } : {}),
|
...(result?.commitSha ? { commitSha: result.commitSha } : {}),
|
||||||
@@ -83,10 +83,14 @@ function buildFinalizationMergeDetails(task: Task, result?: MergeResult): NonNul
|
|||||||
...(result?.mergeCommitMessage ? { mergeCommitMessage: result.mergeCommitMessage } : {}),
|
...(result?.mergeCommitMessage ? { mergeCommitMessage: result.mergeCommitMessage } : {}),
|
||||||
mergedAt,
|
mergedAt,
|
||||||
mergeConfirmed,
|
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:
|
* 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.
|
* 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 (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;
|
if (result) result.task = latest;
|
||||||
return { outcome: "already-done", task: latest, previousColumn: "done" };
|
return { outcome: "already-done", task: latest, previousColumn: "done" };
|
||||||
}
|
}
|
||||||
|
|
||||||
const mergeDetails = buildFinalizationMergeDetails(latest, result);
|
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) {
|
if (!hasProof) {
|
||||||
const reason = "missing-merge-confirmation";
|
const reason = "missing-merge-confirmation";
|
||||||
await recordFinalizationAudit({
|
await recordFinalizationAudit({
|
||||||
|
|||||||
Reference in New Issue
Block a user