Merge & Sync improvements

.changeset/few-cobras-pick.md

@@ -0,0 +1,7 @@
+---
+'@openfn/cli': minor
+---
+
+Allow users to specifiy which workflows to deploy or merge by passing `-w`.
+
+NOTE: the `-w` alias has been repurposed from `--workspace` to `--workflow`. This may affect your local development environment. If so, just expand `-w` to `--workspace`.

.changeset/hungry-lights-love.md

@@ -0,0 +1,5 @@
+---
+'@openfn/lexicon': patch
+---
+
+Allow project.alias to be null

.changeset/late-needles-scream.md

@@ -0,0 +1,5 @@
+---
+'@openfn/cli': patch
+---
+
+Fix an issue on checkout where incorrect divergence warnings can be shown

.changeset/some-squids-follow.md

@@ -0,0 +1,5 @@
+---
+'@openfn/lightning-mock': patch
+---
+
+Ensure that projects added as JSON are deeply cloned, preventing scribbles

.changeset/yummy-balloons-search.md

@@ -0,0 +1,5 @@
+---
+'@openfn/project': patch
+---
+
+Set the correct alias on the checked out project

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

@@ -22,7 +22,6 @@ test.serial('expression not found', async (t) => {
   const stdlogs = extractLogs(stdout);
   assertLog(t, stdlogs, /expression not found/i);
   assertLog(t, stdlogs, /failed to load the expression from blah.js/i);
-  assertLog(t, stdlogs, /critical error: aborting command/i);
 });
 
 test.serial('workflow not found', async (t) => {
@@ -33,7 +32,6 @@ test.serial('workflow not found', async (t) => {
 
   assertLog(t, stdlogs, /workflow not found/i);
   assertLog(t, stdlogs, /failed to load a workflow from blah.json/i);
-  assertLog(t, stdlogs, /critical error: aborting command/i);
 });
 
 test.serial('job contains invalid js', async (t) => {
@@ -45,7 +43,6 @@ test.serial('job contains invalid js', async (t) => {
   assertLog(t, stdlogs, /failed to compile job/i);
   assertLog(t, stdlogs, /unexpected token \(2:10\)/i);
   assertLog(t, stdlogs, /check the syntax of the job expression/i);
-  assertLog(t, stdlogs, /critical error: aborting command/i);
 });
 
 // TODO this should really mention which job threw the error
@@ -60,7 +57,6 @@ test.serial('workflow references a job with invalid js', async (t) => {
   assertLog(t, stdlogs, /failed to compile job/i);
   assertLog(t, stdlogs, /unexpected token \(2:10\)/i);
   assertLog(t, stdlogs, /check the syntax of the job expression/i);
-  assertLog(t, stdlogs, /critical error: aborting command/i);
 });
 
 test.serial("can't find an expression referenced in a workflow", async (t) => {
@@ -77,7 +73,6 @@ test.serial("can't find an expression referenced in a workflow", async (t) => {
     stdlogs,
     /This workflow references a file which cannot be found at does-not-exist.js/i
   );
-  assertLog(t, stdlogs, /critical error: aborting command/i);
 });
 
 test.serial("can't find config referenced in a workflow", async (t) => {
@@ -98,7 +93,6 @@ test.serial("can't find config referenced in a workflow", async (t) => {
     stdlogs,
     /This workflow references a file which cannot be found at does-not-exist.js/i
   );
-  assertLog(t, stdlogs, /critical error: aborting command/i);
 });
 
 test.serial('circular workflow', async (t) => {
@@ -141,7 +135,6 @@ test.serial('invalid end (ambiguous)', async (t) => {
   const stdlogs = extractLogs(stdout);
 
   assertLog(t, stdlogs, /Error: end pattern matched multiple steps/i);
-  assertLog(t, stdlogs, /aborting/i);
 });
 
 // These test error outputs within valid workflows

integration-tests/cli/test/project-v1.test.ts

@@ -108,7 +108,9 @@ workflows:
 const projectsPath = path.resolve(TMP_DIR);
 
 test.before(async () => {
-  // await rm(TMP_DIR, { recursive: true });
+  try {
+    await rm(TMP_DIR, { recursive: true });
+  } catch (e) {}
   await mkdir(`${TMP_DIR}/.projects`, { recursive: true });
 
   await writeFile(`${TMP_DIR}/openfn.yaml`, '');
@@ -120,7 +122,7 @@ test.before(async () => {
 });
 
 test.serial('list available projects', async (t) => {
-  const { stdout } = await run(`openfn projects -w ${projectsPath}`);
+  const { stdout } = await run(`openfn projects --workspace ${projectsPath}`);
 
   t.regex(stdout, /hello-world/);
   t.regex(stdout, /8dbc4349-52b4-4bf2-be10-fdf06da52c46/);
@@ -130,7 +132,7 @@ test.serial('list available projects', async (t) => {
 
 // checkout a project from a yaml file
 test.serial('Checkout a project', async (t) => {
-  await run(`openfn checkout hello-world -w ${projectsPath}`);
+  await run(`openfn checkout hello-world --workspace ${projectsPath}`);
 
   // check workflow.yaml
   const workflowYaml = await readFile(
@@ -167,7 +169,7 @@ steps:
 // note: order of tests is important here
 test.serial('execute a workflow from the checked out project', async (t) => {
   // cheeky bonus test of checkout by alias
-  await run(`openfn checkout main -w ${projectsPath}`);
+  await run(`openfn checkout main --workspace ${projectsPath}`);
 
   // execute a workflow
   const { stdout } = await run(
@@ -191,8 +193,9 @@ test.serial('merge a project', async (t) => {
   t.is(initial, 'fn(() => ({ x: 1}))');
 
   // Run the merge
-  await run(`openfn merge hello-world-staging -w ${projectsPath} --force`);
-
+  const { stdout } = await run(
+    `openfn merge hello-world-staging --workspace ${projectsPath} --force --log debug`
+  );
   // Check the step is updated
   const merged = await readStep();
   t.is(merged, "log('hello world')");

integration-tests/cli/test/project-v2.test.ts

@@ -131,15 +131,15 @@ test.before(async () => {
 });
 
 test.serial('list available projects', async (t) => {
-  const { stdout } = await run(`openfn projects -w ${TMP_DIR}`);
+  const { stdout } = await run(`openfn projects --workspace ${TMP_DIR}`);
   t.regex(stdout, /sandboxing-simple/);
   t.regex(stdout, /a272a529-716a-4de7-a01c-a082916c6d23/);
   t.regex(stdout, /staging/);
   t.regex(stdout, /bc6629fb-7dc8-4b28-93af-901e2bd58dc4/);
 });
 
 test.serial('Checkout a project', async (t) => {
-  await run(`openfn checkout staging -w ${TMP_DIR}`);
+  await run(`openfn checkout staging --workspace ${TMP_DIR}`);
 
   // check workflow.yaml
   const workflowYaml = await readFile(
@@ -174,12 +174,13 @@ steps:
 
 test.serial('execute a workflow from the checked out project', async (t) => {
   // cheeky bonus test of checkout by alias
-  await run(`openfn checkout main -w ${TMP_DIR}`);
+  await run(`openfn checkout main --workspace ${TMP_DIR} --force`);
 
   // execute a workflow
-  await run(
+  const { stdout } = await run(
     `openfn hello-workflow  -o ${TMP_DIR}/output.json  --workspace ${TMP_DIR}`
   );
+  console.log(stdout);
 
   const output = await readFile(`${TMP_DIR}/output.json`, 'utf8');
   const finalState = JSON.parse(output);
@@ -189,7 +190,7 @@ test.serial('execute a workflow from the checked out project', async (t) => {
 test.serial(
   'execute a workflow from the checked out project with a credential map',
   async (t) => {
-    await run(`openfn checkout main --log debug -w ${TMP_DIR}`);
+    await run(`openfn checkout main --log debug --workspace ${TMP_DIR}`);
 
     // Modify the checked out workflow code
     await writeFile(
@@ -248,7 +249,7 @@ test.serial(
     // Important: the collection value MUST be as string
     server.collections.upsert('stuff', 'x', JSON.stringify({ id: 'x' }));
 
-    await run(`openfn checkout main --log debug -w ${TMP_DIR}`);
+    await run(`openfn checkout main --log debug --workspace ${TMP_DIR}`);
 
     // Modify the checked out workflow code
     await writeFile(
@@ -313,7 +314,7 @@ workspace:
 );
 
 test.serial('merge a project', async (t) => {
-  await run(`openfn checkout main -w ${TMP_DIR}`);
+  await run(`openfn checkout main --workspace ${TMP_DIR}`);
 
   const readStep = () =>
     readFile(
@@ -326,7 +327,9 @@ test.serial('merge a project', async (t) => {
   t.is(initial, 'fn(() => ({ x: 1}))');
 
   // Run the merge
-  const { stdout } = await run(`openfn merge staging -w ${TMP_DIR} --force`);
+  const { stdout } = await run(
+    `openfn merge staging --workspace ${TMP_DIR} --force`
+  );
 
   // Check the step is updated
   const merged = await readStep();

packages/cli/src/projects/checkout.ts

@@ -10,12 +10,9 @@ import * as o from '../options';
 import * as po from './options';
 
 import type { Opts } from './options';
-import {
-  findLocallyChangedWorkflows,
-  tidyWorkflowDir,
-  updateForkedFrom,
-} from './util';
+import { tidyWorkflowDir, updateForkedFrom } from './util';
 import { createProjectCredentials } from './create-credentials';
+import abort from '../util/abort';
 
 export type CheckoutOptions = Pick<
   Opts,
@@ -52,7 +49,11 @@ export const handler = async (options: CheckoutOptions, logger?: Logger) => {
   // TODO: try to retain the endpoint for the projects
   const { project: _, ...config } = workspace.getConfig() as any;
 
-  const currentProject = await workspace.getCheckedOutProject();
+  const localProject = await workspace.getCheckedOutProject(
+    // TODO not sold on this assignment - I think my test case must be wrong
+    workspace.activeProject?.alias as any
+  );
+
   // get the project
   let switchProject;
   if (/\.(yaml|json)$/.test(projectIdentifier)) {
@@ -71,34 +72,33 @@ export const handler = async (options: CheckoutOptions, logger?: Logger) => {
       `Project with id ${projectIdentifier} not found in the workspace`
     );
   }
+  logger?.info(`Checking out ${switchProject.alias}`);
 
   // get the current state of the checked out project
   try {
-    const localProject = await Project.from('fs', {
-      root: options.workspace || '.',
-    });
-    logger?.success(`Loaded local project ${localProject.alias}`);
-    const changed = await findLocallyChangedWorkflows(
-      workspace,
-      localProject,
-      'assume-ok'
-    );
-    if (changed.length && !options.force) {
-      logger?.break();
-      logger?.warn(
-        'WARNING: detected changes on your currently checked-out project'
-      );
-      logger?.warn(
-        `Changes may be lost by checking out ${localProject.alias} right now`
+    // If there's no project checked out, there's nothing to compare
+    if (localProject?.workflows.length) {
+      logger?.info(
+        `Loaded currently checked out project ${localProject.alias} to check for untracked changes`
       );
-      logger?.warn(`Pass --force or -f to override this warning and continue`);
-      // TODO log to run with force
-      // TODO need to implement a save function
-      const e = new Error(
-        `The currently checked out project has diverged! Changes may be lost`
-      );
-      delete e.stack;
-      throw e;
+      // TODO is alias robust here? Should we get by alias and domain?
+      const tracked = workspace.get(localProject.alias ?? localProject.id);
+      const changed = hasUntrackedChanges(localProject, tracked);
+      logger?.debug(changed);
+      if (changed.length && !options.force) {
+        const err = {
+          details: `Changes may be lost by checking out ${
+            localProject.alias ?? localProject.id
+          } right now`,
+          // TODO how can users save changes? Not really possible right now
+          fix: 'Pass --force or -f to override this warning and continue',
+        };
+        abort(
+          logger!,
+          `${switchProject.alias} has diverged from ${localProject.alias}!`,
+          err
+        );
+      }
     }
   } catch (e: any) {
     if (e.message.match('ENOENT')) {
@@ -113,7 +113,7 @@ export const handler = async (options: CheckoutOptions, logger?: Logger) => {
   if (options.clean) {
     await rimraf(workspace.workflowsPath);
   } else {
-    await tidyWorkflowDir(currentProject, switchProject, false, workspacePath);
+    await tidyWorkflowDir(localProject, switchProject, false, workspacePath);
   }
 
   // write the forked from map
@@ -137,3 +137,47 @@ export const handler = async (options: CheckoutOptions, logger?: Logger) => {
 
   logger?.success(`Expanded project to ${workspacePath}`);
 };
+
+// This function will tell us if the active/checked out project
+// has any changes compared to the tracked state file
+// It implies that changes will be lost on checkout
+// (later, users can save a project to an arbitrary save file and so this may not be true)
+const hasUntrackedChanges = (
+  activeProject: Project,
+  tracked?: Project | null
+) => {
+  if (!tracked) {
+    // if there's no tracking we can't compare
+    // should we log a warning then?
+    return [];
+  }
+
+  const changedWorkflows: Array<{
+    id: string;
+    type: 'new' | 'changed' | 'removed';
+  }> = [];
+
+  // Check for changed and added workflows
+  for (const workflow of activeProject.workflows) {
+    const trackedWorkflow = tracked.getWorkflow(workflow.id);
+    if (!trackedWorkflow) {
+      // this is a new workflow added locally
+      changedWorkflows.push({ id: workflow.id, type: 'new' });
+      continue;
+    }
+
+    if (!tracked.canMergeInto(activeProject)) {
+      changedWorkflows.push({ id: workflow.id, type: 'changed' });
+    }
+  }
+
+  // Check for removed workflows
+  for (const workflow of tracked.workflows) {
+    const localWorkflow = activeProject.getWorkflow(workflow.id);
+    if (!localWorkflow) {
+      changedWorkflows.push({ id: workflow.id, type: 'removed' });
+    }
+  }
+
+  return changedWorkflows;
+};