From 27e5c780b33cee81aa2cd0c6efa4d97cc632c3d1 Mon Sep 17 00:00:00 2001 From: Charlie Kolb Date: Tue, 11 Nov 2025 10:32:18 +0100 Subject: [PATCH] feat(core): Add workflow history grouping framework (no-changelog) (#21480) --- packages/workflow/src/workflow-diff.ts | 202 ++++++++++++++- packages/workflow/test/workflow-diff.test.ts | 258 ++++++++++++++++++- 2 files changed, 455 insertions(+), 5 deletions(-) diff --git a/packages/workflow/src/workflow-diff.ts b/packages/workflow/src/workflow-diff.ts index 782f2631ab9..a4a23752886 100644 --- a/packages/workflow/src/workflow-diff.ts +++ b/packages/workflow/src/workflow-diff.ts @@ -1,5 +1,11 @@ import isEqual from 'lodash/isEqual'; import pick from 'lodash/pick'; +import type { INode, IWorkflowBase } from '.'; + +export type DiffableNode = Pick; +export type DiffableWorkflow = { + nodes: N[]; +}; export const enum NodeDiffStatus { Eq = 'equal', @@ -15,7 +21,7 @@ export type NodeDiff = { export type WorkflowDiff = Map>; -export function compareNodes( +export function compareNodes( base: T | undefined, target: T | undefined, ): boolean { @@ -27,7 +33,7 @@ export function compareNodes( return isEqual(baseNode, targetNode); } -export function compareWorkflowsNodes( +export function compareWorkflowsNodes( base: T[], target: T[], nodesEqual: (base: T | undefined, target: T | undefined) => boolean = compareNodes, @@ -62,3 +68,195 @@ export function compareWorkflowsNodes( return diff; } + +function mergeNodeDiff( + prev: NodeDiffStatus, + next: NodeDiffStatus, +): NodeDiffStatus | 'undone' | 'invariant broken' { + switch (prev) { + case NodeDiffStatus.Added: + switch (next) { + case NodeDiffStatus.Added: + return 'invariant broken'; + case NodeDiffStatus.Deleted: + return 'undone'; + default: + return NodeDiffStatus.Added; + } + case NodeDiffStatus.Deleted: + switch (next) { + case NodeDiffStatus.Added: + return NodeDiffStatus.Modified; + default: + return 'invariant broken'; + } + case NodeDiffStatus.Eq: + switch (next) { + case NodeDiffStatus.Added: + return 'invariant broken'; + default: + return next; + } + case NodeDiffStatus.Modified: + switch (next) { + case NodeDiffStatus.Added: + return 'invariant broken'; + case NodeDiffStatus.Deleted: + return NodeDiffStatus.Deleted; + default: + return NodeDiffStatus.Modified; + } + } +} + +export class WorkflowChangeSet { + constructor(public nodes: WorkflowDiff = new Map()) {} + + hasChanges() { + for (const nodeDiff of this.nodes.values()) { + if (nodeDiff.status !== NodeDiffStatus.Eq) return true; + } + return false; + } + + mergeNext(wcs: WorkflowChangeSet) { + for (const [key, diff] of wcs.nodes) { + const existing = this.nodes.get(key); + if (existing) { + const diffStatus = mergeNodeDiff(existing.status, diff.status); + if (diffStatus === 'invariant broken') { + throw new Error('invariant broken'); + } + if (diffStatus === 'undone') { + this.nodes.delete(key); + } else { + this.nodes.set(key, { ...diff, status: diffStatus }); + } + } else { + this.nodes.set(key, { ...diff, status: NodeDiffStatus.Added }); + } + } + } +} + +// determines whether the second node is a "superset" of the first one, i.e. whether no data +// is lost if we were to cleanse the first node +function nodeIsAdditive(prevNode: T, nextNode: T) { + const { parameters: prevParams, ...prev } = prevNode; + const { parameters: nextParams, ...next } = nextNode; + + // abort if the nodes don't match besides parameters + if (!compareNodes({ ...prev, parameters: {} }, { ...next, parameters: {} })) return false; + + const params = Object.keys(prevParams); + // abort if prev has some field next does not have + if (params.some((x) => !Object.prototype.hasOwnProperty.call(nextParams, x))) return false; + + for (const key of params) { + const left = prevParams[key]; + const right = nextParams[key]; + // non-strings must be exactly equal to not be lost data + if (typeof left === 'string' && typeof right === 'string') { + // strings must only be contained in the new string + if (!right.includes(left)) return false; + } else if (left !== right) return false; + } + + return true; +} + +function mergeAdditiveChanges( + _prev: GroupedWorkflowHistory>, + next: GroupedWorkflowHistory>, + diff: WorkflowDiff, +) { + for (const d of diff.values()) { + if (d.status === NodeDiffStatus.Deleted) return false; + if (d.status === NodeDiffStatus.Added) continue; + const nextNode = next.from.nodes.find((x) => x.name === d.node.name); + if (!nextNode) throw new Error('invariant broken'); + if (d.status === NodeDiffStatus.Modified && !nodeIsAdditive(d.node, nextNode)) return false; + } + return true; +} + +export const RULES = { + mergeAdditiveChanges, +}; + +type GroupedWorkflowHistory> = { + workflowChangeSet: WorkflowChangeSet; + groupedWorkflows: W[]; + from: W; + to: W; +}; + +function compareWorkflows( + previous: W, + next: W, +): GroupedWorkflowHistory { + const nodesDiff = compareWorkflowsNodes(previous.nodes, next.nodes); + const workflowChangeSet = new WorkflowChangeSet(nodesDiff); + return { + workflowChangeSet, + groupedWorkflows: [], + from: previous, + to: next, + }; +} + +export type DiffRule< + W extends IWorkflowBase = IWorkflowBase, + N extends W['nodes'][number] = W['nodes'][number], +> = ( + prev: GroupedWorkflowHistory, + next: GroupedWorkflowHistory, + diff: WorkflowDiff, +) => boolean; + +export function groupWorkflows( + workflows: W[], + rules: Array>, +): Array> { + if (workflows.length === 0) return []; + if (workflows.length === 1) { + return [ + { + workflowChangeSet: new WorkflowChangeSet(), + groupedWorkflows: [], + from: workflows[0], + to: workflows[0], + }, + ]; + } + + const diffs: Array> = []; + + for (let i = 0; i < workflows.length - 1; ++i) { + diffs.push(compareWorkflows(workflows[i], workflows[i + 1])); + } + let prevDiffsLength = diffs.length; + do { + prevDiffsLength = diffs.length; + const n = diffs.length; + for (let i = n - 1; i > 0; --i) { + const diff = compareWorkflowsNodes(diffs[i - 1].from.nodes, diffs[i].to.nodes); + for (const rule of rules) { + const shouldMerge = rule(diffs[i - 1], diffs[i], diff); + if (shouldMerge) { + const right = diffs.pop(); + if (!right) throw new Error('invariant broken'); + + // merge diffs + diffs[i - 1].workflowChangeSet.mergeNext(right.workflowChangeSet); + diffs[i - 1].groupedWorkflows.push(diffs[i - 1].to); + diffs[i - 1].groupedWorkflows.push(...right.groupedWorkflows); + diffs[i - 1].to = right.to; + break; + } + } + } + } while (prevDiffsLength !== diffs.length); + + return diffs; +} diff --git a/packages/workflow/test/workflow-diff.test.ts b/packages/workflow/test/workflow-diff.test.ts index c85a0f5f195..301f37c45da 100644 --- a/packages/workflow/test/workflow-diff.test.ts +++ b/packages/workflow/test/workflow-diff.test.ts @@ -1,4 +1,16 @@ -import { compareNodes, compareWorkflowsNodes, NodeDiffStatus } from '../src/workflow-diff'; +import { mock } from 'vitest-mock-extended'; + +import { type INodeParameters, type IWorkflowBase } from '../src'; +import { + compareNodes, + compareWorkflowsNodes, + groupWorkflows, + NodeDiffStatus, + RULES, + WorkflowChangeSet, + type DiffableNode, + type DiffRule, +} from '../src/workflow-diff'; describe('NodeDiffStatus', () => { it('should have correct enum values', () => { @@ -30,7 +42,7 @@ describe('compareNodes', () => { typeVersion: number; webhookId: string; credentials: Record; - parameters: Record; + parameters: INodeParameters; position: [number, number]; disabled: boolean; }; @@ -149,7 +161,7 @@ describe('compareWorkflowsNodes', () => { typeVersion: number; webhookId: string; credentials: Record; - parameters: Record; + parameters: INodeParameters; }; it('should detect equal nodes', () => { @@ -261,3 +273,243 @@ describe('compareWorkflowsNodes', () => { expect(diff.get('4')?.status).toBe(NodeDiffStatus.Added); }); }); + +describe('groupWorkflows', () => { + const node1 = mock({ id: '1', parameters: { a: 1 } }); + const node2 = mock({ id: '2', parameters: { a: 2 } }); + + let rules: DiffRule[] = []; + let workflows: IWorkflowBase[]; + beforeEach(() => { + rules = []; + workflows = []; + }); + describe('basic grouping', () => { + it('should group workflows with no changes', () => { + workflows = mock<[IWorkflowBase, IWorkflowBase]>([ + { id: '1', nodes: [node1, node2] }, + { id: '1', nodes: [node1, node2] }, + ]); + const grouped = groupWorkflows(workflows, rules); + + expect(grouped.length).toBe(1); + expect(grouped[0].from).toEqual(workflows[0]); + expect(grouped[0].to).toEqual(workflows[1]); + expect(grouped[0].workflowChangeSet.nodes.size).toBe(2); + expect(grouped[0].workflowChangeSet.nodes.get(node1.id)?.status).toBe(NodeDiffStatus.Eq); + expect(grouped[0].workflowChangeSet.nodes.get(node2.id)?.status).toBe(NodeDiffStatus.Eq); + expect(grouped[0].groupedWorkflows).toEqual([]); + }); + + it('should group workflows with added nodes', () => { + workflows = mock<[IWorkflowBase, IWorkflowBase]>([ + { id: '1', nodes: [node1] }, + { id: '1', nodes: [node1, node2] }, + ]); + + const grouped = groupWorkflows(workflows, rules); + + expect(grouped.length).toBe(1); + expect(grouped[0].from).toEqual(workflows[0]); + expect(grouped[0].to).toEqual(workflows[1]); + expect(grouped[0].workflowChangeSet.nodes.size).toBe(2); + expect(grouped[0].workflowChangeSet.nodes.get(node1.id)?.status).toBe(NodeDiffStatus.Eq); + expect(grouped[0].workflowChangeSet.nodes.get(node2.id)?.status).toBe(NodeDiffStatus.Added); + expect(grouped[0].groupedWorkflows).toEqual([]); + }); + + it('should group workflows with deleted nodes', () => { + workflows = mock<[IWorkflowBase, IWorkflowBase]>([ + { id: '1', nodes: [node1, node2] }, + { id: '1', nodes: [node1] }, + ]); + + const grouped = groupWorkflows(workflows, rules); + + expect(grouped.length).toBe(1); + expect(grouped[0].from).toEqual(workflows[0]); + expect(grouped[0].to).toEqual(workflows[1]); + expect(grouped[0].workflowChangeSet.nodes.size).toBe(2); + expect(grouped[0].workflowChangeSet.nodes.get(node1.id)?.status).toBe(NodeDiffStatus.Eq); + expect(grouped[0].workflowChangeSet.nodes.get(node2.id)?.status).toBe(NodeDiffStatus.Deleted); + + expect(grouped[0].groupedWorkflows).toEqual([]); + }); + + it('should group workflows with modified nodes', () => { + const modifiedNode2 = { id: '2', parameter: { a: 3 } }; + workflows = mock<[IWorkflowBase, IWorkflowBase]>([ + { id: '1', nodes: [node1, node2] }, + { id: '1', nodes: [node1, modifiedNode2] }, + ]); + + const grouped = groupWorkflows(workflows, rules); + + expect(grouped.length).toBe(1); + expect(grouped[0].from).toEqual(workflows[0]); + expect(grouped[0].to).toEqual(workflows[1]); + expect(grouped[0].workflowChangeSet.nodes.size).toBe(2); + expect(grouped[0].workflowChangeSet.nodes.get(node1.id)?.status).toBe(NodeDiffStatus.Eq); + expect(grouped[0].workflowChangeSet.nodes.get(modifiedNode2.id)?.status).toBe( + NodeDiffStatus.Modified, + ); + expect(grouped[0].groupedWorkflows).toEqual([]); + }); + + it('should handle multiple workflow groups', () => { + workflows = mock([ + { id: '1', nodes: [node1] }, + { id: '1', nodes: [node1, node2] }, + { id: '1', nodes: [node1] }, + ]); + + const grouped = groupWorkflows(workflows, rules); + + expect(grouped.length).toBe(2); + expect(grouped[0].from).toEqual(workflows[0]); + expect(grouped[0].to).toEqual(workflows[1]); + expect(grouped[0].workflowChangeSet.nodes.size).toBe(2); + expect(grouped[0].workflowChangeSet.nodes.get(node1.id)?.status).toBe(NodeDiffStatus.Eq); + expect(grouped[0].workflowChangeSet.nodes.get(node2.id)?.status).toBe(NodeDiffStatus.Added); + + expect(grouped[1].from).toEqual(workflows[1]); + expect(grouped[1].to).toEqual(workflows[2]); + expect(grouped[1].workflowChangeSet.nodes.size).toBe(2); + expect(grouped[1].workflowChangeSet.nodes.get(node1.id)?.status).toBe(NodeDiffStatus.Eq); + expect(grouped[1].workflowChangeSet.nodes.get(node2.id)?.status).toBe(NodeDiffStatus.Deleted); + }); + + it('should handle empty workflows array', () => { + const grouped = groupWorkflows(workflows, rules); + + expect(grouped.length).toBe(0); + }); + + it('should handle single workflow', () => { + const workflows = mock([{ id: '1', nodes: [node1] }]); + + const grouped = groupWorkflows(workflows, rules); + + expect(grouped.length).toBe(1); + expect(grouped[0].from).toEqual(workflows[0]); + expect(grouped[0].to).toEqual(workflows[0]); + expect(grouped[0].workflowChangeSet.nodes.size).toBe(0); + expect(grouped[0].groupedWorkflows).toEqual([]); + }); + }); + describe('rules', () => { + it('should not apply an inapplicable rule', () => { + workflows = mock([ + { id: '1', nodes: [node1] }, + { id: '1', nodes: [node1, node2] }, + { id: '1', nodes: [node1] }, + ]); + + const alwaysMergeRule: DiffRule = (_l, _r) => false; + const rules = [alwaysMergeRule]; + + const grouped = groupWorkflows(workflows, rules); + + expect(grouped.length).toBe(2); + expect(grouped[0].from).toEqual(workflows[0]); + expect(grouped[0].to).toEqual(workflows[1]); + expect(grouped[0].groupedWorkflows).toEqual([]); + expect(grouped[1].from).toEqual(workflows[1]); + expect(grouped[1].to).toEqual(workflows[2]); + expect(grouped[1].groupedWorkflows).toEqual([]); + }); + it('should apply a given rule', () => { + workflows = mock([ + { id: '1', nodes: [node1] }, + { id: '1', nodes: [node1, node2] }, + { id: '1', nodes: [node1] }, + ]); + + const alwaysMergeRule: DiffRule = (_l, _r) => true; + const rules = [alwaysMergeRule]; + + const grouped = groupWorkflows(workflows, rules); + + expect(grouped.length).toBe(1); + expect(grouped[0].from).toEqual(workflows[0]); + expect(grouped[0].to).toEqual(workflows[2]); + expect(grouped[0].groupedWorkflows).toEqual([workflows[1]]); + expect(grouped[0].workflowChangeSet.nodes.size).toBe(1); + expect(grouped[0].workflowChangeSet.nodes.get(node1.id)?.status).toBe(NodeDiffStatus.Eq); + }); + describe('mergeAdditiveChanges', () => { + const createWorkflow = (id: string, nodes: DiffableNode[]): IWorkflowBase => { + return { + id, + nodes, + } as IWorkflowBase; + }; + + test.each([ + { + description: 'should return true when all changes are additive', + baseWorkflow: createWorkflow('1', [{ id: '1', parameters: { a: 'value1' }, name: 'n1' }]), + nextWorkflow: createWorkflow('1', [ + { id: '1', parameters: { a: 'value1', b: 'value2' }, name: 'n1' }, + ]), + expected: true, + }, + { + description: 'should return false when a node is deleted', + baseWorkflow: createWorkflow('1', [{ id: '1', parameters: { a: 'value1' }, name: 'n1' }]), + nextWorkflow: createWorkflow('1', []), + expected: false, + }, + { + description: 'should return false when a node is modified non-additively', + baseWorkflow: createWorkflow('1', [{ id: '1', parameters: { a: 'value1' }, name: 'n1' }]), + nextWorkflow: createWorkflow('1', [ + { id: '1', parameters: { a: 'differentValue' }, name: 'n1' }, + ]), + expected: false, + }, + { + description: 'should return true when a node is added', + baseWorkflow: createWorkflow('1', [{ id: '1', parameters: { a: 'value1' }, name: 'n1' }]), + nextWorkflow: createWorkflow('1', [ + { id: '1', parameters: { a: 'value1' }, name: 'n1' }, + { id: '2', parameters: { b: 'value2' }, name: 'n1' }, + ]), + expected: true, + }, + { + description: 'should return false when a node is modified and loses data', + baseWorkflow: createWorkflow('1', [ + { id: '1', parameters: { a: 'value1', b: 'value2' }, name: 'n1' }, + ]), + nextWorkflow: createWorkflow('1', [{ id: '1', parameters: { a: 'value1' }, name: 'n1' }]), + expected: false, + }, + { + description: 'should handle empty workflows', + baseWorkflow: createWorkflow('1', []), + nextWorkflow: createWorkflow('1', []), + expected: true, + }, + ])('$description', ({ baseWorkflow, nextWorkflow, expected }) => { + const result = RULES.mergeAdditiveChanges( + { + from: baseWorkflow, + to: baseWorkflow, + groupedWorkflows: [], + workflowChangeSet: new WorkflowChangeSet(), + }, + { + from: nextWorkflow, + to: nextWorkflow, + groupedWorkflows: [], + workflowChangeSet: new WorkflowChangeSet(), + }, + compareWorkflowsNodes(baseWorkflow.nodes, nextWorkflow.nodes), + ); + + expect(result).toEqual(expected); + }); + }); + }); +});