Deploy: fixes to support version history properly

.changeset/moody-ducks-warn.md

@@ -0,0 +1,5 @@
+---
+'@openfn/project': patch
+---
+
+Support forked_from metadata key in openfn.yaml

integration-tests/cli/test/sync.test.ts

@@ -30,7 +30,10 @@ const initWorkspace = (t: any) => {
   };
 };
 
-const gen = (name = 'patients', workflows = ['trigger-job(body="fn()")']) => {
+const gen = (
+  name = 'patients',
+  workflows = ['trigger-job(expression="fn()")']
+) => {
   // generate a project
   const project = generateProject(name, workflows, {
     openfnUuid: true,
@@ -44,7 +47,7 @@ test('fetch a new project', async (t) => {
   const { workspace, read } = initWorkspace(t);
   const project = gen();
 
-  await run(
+  const { stdout } = await run(
     `openfn project fetch \
        --workspace ${workspace} \
        --endpoint ${endpoint} \
@@ -239,7 +242,7 @@ test('pull an update to project', async (t) => {
 test('checkout by alias', async (t) => {
   const { workspace, read } = initWorkspace(t);
   const main = gen();
-  const staging = gen('patients-staging', ['trigger-job(body="fn(x)")']);
+  const staging = gen('patients-staging', ['trigger-job(expression="fn(x)")']);
 
   await run(
     `openfn project fetch \

packages/cli/CHANGELOG.md

@@ -4,7 +4,7 @@
 
 ### Minor Changes
 
-- 8b9f402: fetch: allow state files to be writtem to JSON with --format
+- 8b9f402: fetch: allow state files to be written to JSON with --format
 
 ### Patch Changes
 

packages/cli/src/projects/checkout.ts

@@ -10,7 +10,7 @@ import * as o from '../options';
 import * as po from './options';
 
 import type { Opts } from './options';
-import { tidyWorkflowDir } from './util';
+import { tidyWorkflowDir, updateForkedFrom } from './util';
 
 export type CheckoutOptions = Pick<
   Opts,
@@ -69,7 +69,11 @@ export const handler = async (options: CheckoutOptions, logger: Logger) => {
     await tidyWorkflowDir(currentProject!, switchProject);
   }
 
+  // write the forked from map
+  updateForkedFrom(switchProject);
+
   // expand project into directory
+  // TODO: only write files with a diff
   const files: any = switchProject.serialize('fs');
   for (const f in files) {
     if (files[f]) {

packages/cli/src/projects/deploy.ts

@@ -1,6 +1,8 @@
 import yargs from 'yargs';
-import Project from '@openfn/project';
+import Project, { Workspace } from '@openfn/project';
 import c from 'chalk';
+import { writeFile } from 'node:fs/promises';
+import path from 'node:path';
 
 import * as o from '../options';
 import * as o2 from './options';
@@ -10,6 +12,7 @@ import {
   fetchProject,
   serialize,
   getSerializePath,
+  updateForkedFrom,
 } from './util';
 import { build, ensure } from '../util/command-builders';
 
@@ -64,27 +67,54 @@ export const command: yargs.CommandModule<DeployOptions> = {
   handler: ensure('project-deploy', options),
 };
 
+export const hasRemoteDiverged = (
+  local: Project,
+  remote: Project
+): string[] | null => {
+  let diverged: string[] | null = null;
+
+  const refs = local.cli.forked_from ?? {};
+
+  // for each workflow, check that the local fetched_from is the head of the remote history
+  for (const wf of local.workflows) {
+    if (wf.id in refs) {
+      const forkedVersion = refs[wf.id];
+      const remoteVersion = remote.getWorkflow(wf.id)?.history.at(-1);
+      if (forkedVersion !== remoteVersion) {
+        diverged ??= [];
+        diverged.push(wf.id);
+      }
+    } else {
+      // TODO what if there's no forked from for this workflow?
+      // Do we assume divergence because we don't know? Do we warn?
+    }
+  }
+
+  // TODO what if a workflow is removed locally?
+
+  return diverged;
+};
+
 export async function handler(options: DeployOptions, logger: Logger) {
   logger.warn(
     'WARNING: the project deploy command is in BETA and may not be stable. Use cautiously on production projects.'
   );
   const config = loadAppAuthConfig(options, logger);
 
-  // TODO: allow users to specify which project to deploy
-  // Should be able to take any project.yaml file via id, uuid, alias or path
-  // Note that it's a little wierd to deploy a project you haven't checked out,
-  // so put good safeguards here
   logger.info('Attempting to load checked-out project from workspace');
 
-  // TODO this doesn't have a history!
-  // loading from the fs the history isn't available
+  // TODO this is the hard way to load the local alias
+  // We need track alias in openfn.yaml to make this easier (and tracked in from fs)
+  const ws = new Workspace(options.workspace || '.');
+  const { alias } = ws.getActiveProject()!;
+  // TODO this doesn't have an alias
   const localProject = await Project.from('fs', {
     root: options.workspace || '.',
+    alias,
   });
 
-  // TODO if there's no local metadata, the user must pass a UUID or alias to post to
-
   logger.success(`Loaded local project ${printProjectName(localProject)}`);
+
   // First step, fetch the latest version and write
   // this may throw!
   let remoteProject: Project;
@@ -122,7 +152,8 @@ Pass --force to override this error and deploy anyway.`);
     return false;
   }
 
-  const diffs = reportDiff(remoteProject!, localProject, logger);
+  // TODO: what if remote diff and the version checked disagree for some reason?
+  const diffs = reportDiff(localProject, remoteProject, logger);
   if (!diffs.length) {
     logger.success('Nothing to deploy');
     return;
@@ -132,39 +163,51 @@ Pass --force to override this error and deploy anyway.`);
 
   // Skip divergence testing if the remote has no history in its workflows
   // (this will only happen on older versions of lightning)
+  // TODO now maybe skip if there's no forked_from
   const skipVersionTest =
-    localProject.workflows.find((wf) => wf.history.length === 0) ||
+    // localProject.workflows.find((wf) => wf.history.length === 0) ||
     remoteProject.workflows.find((wf) => wf.history.length === 0);
 
+  // localProject.workflows.forEach((w) => console.log(w.history));
+
   if (skipVersionTest) {
     logger.warn(
       'Skipping compatibility check as no local version history detected'
     );
-    logger.warn('Pushing these changes may overrite changes made to the app');
-  } else if (!localProject.canMergeInto(remoteProject!)) {
-    if (!options.force) {
-      logger.error(`Error: Projects have diverged!
+    logger.warn('Pushing these changes may overwrite changes made to the app');
+  } else {
+    const divergentWorkflows = hasRemoteDiverged(localProject, remoteProject!);
+    if (divergentWorkflows) {
+      logger.warn(
+        `The following workflows have diverged: ${divergentWorkflows}`
+      );
+      if (!options.force) {
+        logger.error(`Error: Projects have diverged!
 
-The remote project has been edited since the local project was branched. Changes may be lost.
+  The remote project has been edited since the local project was branched. Changes may be lost.
 
-Pass --force to override this error and deploy anyway.`);
-      return;
+  Pass --force to override this error and deploy anyway.`);
+        return;
+      } else {
+        logger.warn(
+          'Remote project has not diverged from local project! Pushing anyway as -f passed'
+        );
+      }
     } else {
-      logger.warn(
-        'Remote project has not diverged from local project! Pushing anyway as -f passed'
+      logger.info(
+        'Remote project has not diverged from local project - it is safe to deploy 🎉'
       );
     }
-  } else {
-    logger.info(
-      'Remote project has not diverged from local project - it is safe to deploy 🎉'
-    );
   }
 
   logger.info('Merging changes into remote project');
+  // TODO I would like to log which workflows are being updated
   const merged = Project.merge(localProject, remoteProject!, {
     mode: 'replace',
     force: true,
+    onlyUpdated: true,
   });
+
   // generate state for the provisioner
   const state = merged.serialize('state', {
     format: 'json',
@@ -180,6 +223,8 @@ Pass --force to override this error and deploy anyway.`);
   // TODO not totally sold on endpoint handling right now
   config.endpoint ??= localProject.openfn?.endpoint!;
 
+  // TODO: I want to report diff HERE, after the merged state and stuff has been built
+
   if (options.dryRun) {
     logger.always('dryRun option set: skipping upload step');
   } else {
@@ -218,17 +263,24 @@ Pass --force to override this error and deploy anyway.`);
       merged.config
     );
 
+    updateForkedFrom(finalProject);
+    const configData = finalProject.generateConfig();
+    await writeFile(
+      path.resolve(options.workspace!, configData.path),
+      configData.content
+    );
+
+    // TODO why is alias wrong here?
     const finalOutputPath = getSerializePath(localProject, options.workspace!);
-    logger.debug('Updating local project at ', finalOutputPath);
-    await serialize(finalProject, finalOutputPath);
+    const fullFinalPath = await serialize(finalProject, finalOutputPath);
+    logger.debug('Updated local project at ', fullFinalPath);
   }
 
   logger.success('Updated project at', config.endpoint);
 }
 
 export const reportDiff = (local: Project, remote: Project, logger: Logger) => {
   const diffs = remote.diff(local);
-
   if (diffs.length === 0) {
     logger.info('No workflow changes detected');
     return diffs;
@@ -267,3 +319,4 @@ export const reportDiff = (local: Project, remote: Project, logger: Logger) => {
 
   return diffs;
 };
+``;

packages/cli/src/projects/fetch.ts

@@ -254,7 +254,7 @@ export async function fetchRemoteProject(
         options.project
       } to UUID ${projectUUID} from local project ${printProjectName(
         localProject
-      )}}`
+      )}`
     );
   }
 
@@ -321,19 +321,5 @@ To ignore this error and override the local file, pass --force (-f)
 
       throw error;
     }
-
-    const hasAnyHistory = remoteProject.workflows.find(
-      (w) => w.workflow.history?.length
-    );
-
-    // Skip version checking if:
-    const skipVersionCheck =
-      options.force || // The user forced the checkout
-      !hasAnyHistory; // the remote project has no history (can happen in old apps)
-
-    if (!skipVersionCheck && !remoteProject.canMergeInto(localProject!)) {
-      // TODO allow rename
-      throw new Error('Error! An incompatible project exists at this location');
-    }
   }
 }

packages/cli/src/projects/util.ts

@@ -158,10 +158,20 @@ export async function deployProject(
     });
 
     if (!response.ok) {
-      const body = await response.json();
-
+      logger?.error(`Deploy failed with code `, response.status);
       logger?.error('Failed to deploy project:');
-      logger?.error(JSON.stringify(body, null, 2));
+
+      const contentType = response.headers.get('content-type');
+
+      if (contentType.match('application/json ')) {
+        const body = await response.json();
+        logger?.error(JSON.stringify(body, null, 2));
+      } else {
+        const content = await response.text();
+        // TODO html errors are too long to be useful... figure this out later
+        logger?.error(content);
+      }
+
       throw new CLIError(
         `Failed to deploy project ${state.name}: ${response.status}`
       );
@@ -216,3 +226,14 @@ export async function tidyWorkflowDir(
   // Return and sort for testing
   return toRemove.sort();
 }
+
+export const updateForkedFrom = (proj: Project) => {
+  proj.cli.forked_from = proj.workflows.reduce((obj: any, wf) => {
+    if (wf.history.length) {
+      obj[wf.id] = wf.history.at(-1);
+    }
+    return obj;
+  }, {});
+
+  return proj;
+};