feat: abort merge when there's going to be data lost during a merge

packages/cli/src/merge/command.ts

@@ -1,6 +1,6 @@
 import yargs from 'yargs';
 import { Opts } from '../options';
-import { ensure, build } from '../util/command-builders';
+import { ensure, build, override } from '../util/command-builders';
 import * as o from '../options';
 
 export type MergeOptions = Required<
@@ -12,6 +12,7 @@ export type MergeOptions = Required<
     | 'removeUnmapped'
     | 'workflowMappings'
     | 'log'
+    | 'force'
   >
 >;
 
@@ -21,6 +22,9 @@ const options = [
   o.removeUnmapped,
   o.workflowMappings,
   o.log,
+  override(o.force, {
+    description: 'Force a merge even when workflows are incompatible',
+  }),
 ];
 
 const mergeCommand: yargs.CommandModule = {

packages/cli/src/merge/handler.ts

@@ -55,6 +55,7 @@ const mergeHandler = async (options: MergeOptions, logger: Logger) => {
   const final = Project.merge(sourceProject, targetProject, {
     removeUnmapped: options.removeUnmapped,
     workflowMappings: options.workflowMappings,
+    force: options.force,
   });
   const yaml = final.serialize('state', { format: 'yaml' });
   await fs.writeFile(finalPath, yaml);
@@ -65,6 +66,7 @@ const mergeHandler = async (options: MergeOptions, logger: Logger) => {
       command: 'checkout',
       projectPath: commandPath,
       projectName: final.name || '',
+      log: options.log,
     },
     logger
   );

packages/lexicon/core.d.ts

@@ -83,6 +83,9 @@ export type Workflow = {
   globals?: string;
 
   openfn?: OpenFnMetadata;
+
+  // holds history information of a workflow
+  history?: string[]
 };
 
 export type StepId = string;

packages/project/src/Workflow.ts

@@ -30,6 +30,9 @@ class Workflow {
 
     this.workflow = clone(workflow);
 
+    // history needs to be on workflow object.
+    this.workflow.history = workflow.history?.length ? workflow.history : [];
+
     const { id, name, openfn, steps, ...options } = workflow;
     if (!(id || name)) {
       throw new Error('A Workflow MUST have a name or id');
@@ -167,6 +170,19 @@ class Workflow {
   getVersionHash() {
     return generateHash(this);
   }
+
+  pushHistory(versionHash: string) {
+    this.workflow.history?.push(versionHash);
+  }
+
+  // return true if the current workflow can be merged into the target workflow without losing any changes
+  canMergeInto(target: Workflow) {
+    if (!target.workflow.history.length) return true;
+    const targetHead =
+      target.workflow.history[target.workflow.history.length - 1];
+    if (this.workflow.history.indexOf(targetHead) > -1) return true;
+    return false;
+  }
 }
 
 export default Workflow;

packages/project/src/merge/merge-project.ts

@@ -1,17 +1,16 @@
-import { Workflow } from '@openfn/lexicon';
 import { defaultsDeep, isEmpty } from 'lodash-es';
 
 import { Project } from '../Project';
 import { mergeWorkflows } from './merge-node';
 import mapUuids from './map-uuids';
 import baseMerge from '../util/base-merge';
 import getDuplicates from '../util/get-duplicates';
+import Workflow from '../Workflow';
 
 export type MergeProjectOptions = Partial<{
   workflowMappings: Record<string, string>; // <source, target>
   removeUnmapped: boolean;
-
-  force: boolean; // TODO not implemented yet
+  force: boolean;
 }>;
 
 /**
@@ -56,6 +55,27 @@ export function merge(
     return !!options?.workflowMappings[w.id];
   });
 
+  // mergeability
+  const potentialConflicts: Record<string, string> = {};
+  for (const sourceWorkflow of sourceWorkflows) {
+    const targetId =
+      options.workflowMappings?.[sourceWorkflow.id] ?? sourceWorkflow.id;
+    const targetWorkflow = target.getWorkflow(targetId);
+    if (targetWorkflow && !sourceWorkflow.canMergeInto(targetWorkflow)) {
+      potentialConflicts[sourceWorkflow.name] = targetWorkflow?.name;
+    }
+  }
+
+  if (Object.keys(potentialConflicts).length && !options?.force) {
+    throw new Error(
+      `The below workflows can't be merged directly without losing data\n${Object.entries(
+        potentialConflicts
+      )
+        .map(([from, to]) => `${from} → ${to}`)
+        .join('\n')}\nPass --force to force the merge anyway`
+    );
+  }
+
   for (const sourceWorkflow of sourceWorkflows) {
     const targetId =
       options.workflowMappings?.[sourceWorkflow.id] ?? sourceWorkflow.id;

packages/project/test/gen/fixtures.ts

@@ -4,6 +4,7 @@
 export const ab = {
   id: 'workflow',
   name: 'Workflow',
+  history: [],
   steps: [
     {
       id: 'a',

packages/project/test/gen/generator.test.ts

@@ -120,6 +120,7 @@ test('it should generate a simple workflow with any letter', (t) => {
   const expected = {
     id: 'workflow',
     name: 'Workflow',
+    history: [],
     steps: [
       {
         id: 'x',
@@ -152,6 +153,7 @@ test('it should generate a simple workflow with words, numbers and underscores',
   const expected = {
     id: 'workflow',
     name: 'Workflow',
+    history: [],
     steps: [
       {
         id: 'node_1',
@@ -184,6 +186,7 @@ test('it should generate two node pairs', (t) => {
   const expected = {
     id: 'workflow',
     name: 'Workflow',
+    history: [],
     steps: [
       {
         id: 'a',
@@ -231,6 +234,7 @@ test('it should generate two node pairs from one parent', (t) => {
   const expected = {
     id: 'workflow',
     name: 'Workflow',
+    history: [],
     steps: [
       {
         id: 'a',
@@ -276,6 +280,7 @@ test('it should generate several node pairs', (t) => {
   const expected = {
     id: 'workflow',
     name: 'Workflow',
+    history: [],
     steps: [
       {
         id: 'a',
@@ -439,6 +444,7 @@ test('it should generate a simple workflow with mapped uuids', (t) => {
   const expected = {
     id: 'workflow',
     name: 'Workflow',
+    history: [],
     steps: [
       {
         id: 'a',
@@ -481,6 +487,7 @@ test('it should generate a project with uuids', (t) => {
   const expected = {
     id: 'workflow',
     name: 'Workflow',
+    history: [],
     steps: [
       {
         id: 'a',

packages/project/test/parse/from-app-state.test.ts

@@ -114,6 +114,7 @@ test('should create a Project from prov state with a workflow', (t) => {
   t.deepEqual(project.workflows[0].toJSON(), {
     id: 'my-workflow',
     name: 'My Workflow',
+    history: [],
     steps: [
       {
         id: 'trigger',

packages/project/test/project.test.ts

@@ -2,7 +2,7 @@
 import test from 'ava';
 import type { Provisioner } from '@openfn/lexicon/lightning';
 import { Project } from '../src/Project';
-import generateWorkflow from '../src/gen/generator';
+import generateWorkflow, { generateProject } from '../src/gen/generator';
 
 // TODO move to fixtures and re-use?
 // Or use util function instead?
@@ -154,3 +154,34 @@ test('should return UUIDs for everything', (t) => {
     },
   });
 });
+
+test('incompatible-merge: should throw error when merge is incompatible', (t) => {
+  const source = generateWorkflow('trigger-x');
+  source.pushHistory(source.getVersionHash());
+  const target = generateWorkflow('trigger-y');
+  target.pushHistory(target.getVersionHash());
+
+  t.false(source.canMergeInto(target));
+
+  const sourceProject = new Project({ workflows: [source] });
+  const targetProject = new Project({ workflows: [target] });
+  t.throws(() => Project.merge(sourceProject, targetProject), {
+    message: `The below workflows can't be merged directly without losing data\nWorkflow → Workflow\nPass --force to force the merge anyway`,
+  });
+});
+
+test('incompatible-merge-force: should ignore incompatiblity and merge when forced', (t) => {
+  // same as the above test with force
+  const source = generateWorkflow('trigger-x');
+  source.pushHistory(source.getVersionHash());
+  const target = generateWorkflow('trigger-y');
+  target.pushHistory(target.getVersionHash());
+
+  t.false(source.canMergeInto(target));
+
+  const sourceProject = new Project({ workflows: [source] });
+  const targetProject = new Project({ workflows: [target] });
+  t.notThrows(() =>
+    Project.merge(sourceProject, targetProject, { force: true })
+  );
+});

packages/project/test/workflow.test.ts

@@ -1,9 +1,11 @@
 import { randomUUID } from 'node:crypto';
 import test from 'ava';
 import Workflow from '../src/Workflow';
+import { generateWorkflow } from '../src';
 
 const simpleWorkflow = {
   id: 'my-workflow',
+  history: [],
   name: 'My Workflow',
   steps: [
     {
@@ -205,3 +207,76 @@ test('map uuids to ids', (t) => {
   t.deepEqual(w.index.id[uuid_ac], 'a-c');
   t.deepEqual(w.index.id[uuid_bc], 'b-c');
 });
+
+test('canMergeInto: can merge empty histories', (t) => {
+  const sourceWf = generateWorkflow('trigger-x');
+  const targetWf = generateWorkflow('trigger-y');
+
+  t.deepEqual(sourceWf.workflow.history, []);
+  t.deepEqual(targetWf.workflow.history, []);
+
+  t.true(sourceWf.canMergeInto(targetWf)); // allowed
+});
+
+test('canMergeInto: can merge empty target history', (t) => {
+  const sourceWf = generateWorkflow('trigger-x');
+  const targetWf = generateWorkflow('trigger-y');
+  sourceWf.workflow.history = ['history-1'];
+
+  t.is(sourceWf.workflow.history.length, 1);
+  t.deepEqual(targetWf.workflow.history, []);
+
+  t.true(sourceWf.canMergeInto(targetWf)); // allowed
+});
+
+test('canMergeInto: cannot merge empty source history', (t) => {
+  const sourceWf = generateWorkflow('trigger-x');
+  const targetWf = generateWorkflow('trigger-y');
+  targetWf.workflow.history = ['history-1'];
+
+  t.is(targetWf.workflow.history.length, 1);
+  t.deepEqual(sourceWf.workflow.history, []);
+
+  t.false(sourceWf.canMergeInto(targetWf)); // not allowed
+});
+
+test('canMergeInto: can merge target head in source', (t) => {
+  const sourceWf = generateWorkflow('trigger-x');
+  const targetWf = generateWorkflow('trigger-y');
+  targetWf.workflow.history = ['history-1', 'history-3', 'history-5'];
+  sourceWf.workflow.history = ['history-2', 'history-5', 'history-6'];
+
+  t.true(sourceWf.canMergeInto(targetWf)); // allowed
+});
+
+test('canMergeInto: cannot merge target head not in source', (t) => {
+  const sourceWf = generateWorkflow('trigger-x');
+  const targetWf = generateWorkflow('trigger-y');
+  targetWf.workflow.history = ['history-1', 'history-3', 'history-5'];
+  sourceWf.workflow.history = ['history-2', 'history-4', 'history-6'];
+
+  t.false(sourceWf.canMergeInto(targetWf)); // not allowed
+});
+
+test('canMergeInto: cannot merge source head in target', (t) => {
+  const sourceWf = generateWorkflow('trigger-x');
+  const targetWf = generateWorkflow('trigger-y');
+  targetWf.workflow.history = [
+    'history-1',
+    'history-3',
+    'history-6',
+    'history-5',
+  ];
+  sourceWf.workflow.history = ['history-2', 'history-4', 'history-6'];
+
+  t.false(sourceWf.canMergeInto(targetWf)); // not allowed
+});
+
+test('canMergeInto: can merge same heads', (t) => {
+  const sourceWf = generateWorkflow('trigger-x');
+  const targetWf = generateWorkflow('trigger-y');
+  targetWf.workflow.history = ['history-1', 'history-3', 'history-6'];
+  sourceWf.workflow.history = ['history-2', 'history-4', 'history-6'];
+
+  t.true(sourceWf.canMergeInto(targetWf)); // allowed
+});