Fix incorrect divergence warnings on checkout

.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/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

@@ -193,10 +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 --workspace ${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

@@ -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 --workspace ${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);

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;
+};

packages/cli/src/projects/list.ts

@@ -7,6 +7,7 @@ import * as o from '../options';
 import * as po from './options';
 
 import type { Opts } from './options';
+import abort from '../util/abort';
 
 export type ProjectListOptions = Pick<Opts, 'log' | 'workspace'>;
 
@@ -34,7 +35,9 @@ export const handler = async (options: ProjectListOptions, logger: Logger) => {
     // eg, this will happen if there's no openfn.yaml file
     // basically we need the workspace to return a reason
     // (again, I'm thinking of removing the validation entirely)
-    throw new Error('No OpenFn projects found');
+    abort(logger, `No OpenFn projects found at ${options.workspace}`, {
+      fix: 'Run this command from a folder with an openfn.yaml file, or pass --workspace to set the workspace root',
+    });
   }
 
   logger.always(`Available openfn projects\n\n${workspace

packages/cli/src/projects/merge.ts

@@ -171,6 +171,8 @@ export const handler = async (options: MergeOptions, logger: Logger) => {
       workspace: workspacePath,
       project: options.outputPath ? finalPath : final.id,
       log: options.log,
+      // after the merge, we have to force the output to be checked out, ignoring divergence
+      force: true,
     },
     logger
   );

packages/cli/src/projects/util.ts

@@ -259,14 +259,15 @@ export const updateForkedFrom = (proj: Project) => {
   return proj;
 };
 
+// Compare a project to its version hashed when forked
+// This tells us whether the project was edited since it was created
 export const findLocallyChangedWorkflows = async (
   workspace: Workspace,
   project: Project,
   ifNoForkedFrom: 'assume-ok' | 'assume-diverged' = 'assume-diverged'
 ) => {
   // Check openfn.yaml for the forked_from versions
   const { forked_from } = workspace.activeProject ?? {};
-
   // If there are no forked_from references, we have no baseline
   // so assume everything has changed
   if (!forked_from || Object.keys(forked_from).length === 0) {

packages/cli/src/util/abort.ts

@@ -16,32 +16,32 @@ interface CLIFriendlyError extends Error {
 export default (
   logger: Logger,
   reason: string,
-  error?: CLIFriendlyError,
+  error?: Partial<CLIFriendlyError>,
   help?: string
 ) => {
   const e = new AbortError(reason);
   logger.break();
   logger.error(reason);
   if (error) {
-    logger.error(error.message);
-    logger.break();
+    if (error.message) {
+      logger.error(error.message);
+      logger.break();
+    }
 
     if (error.details) {
-      logger.error('ERROR DETAILS:');
       logger.error(error.details);
       logger.break();
     }
     if (error.fix) {
-      logger.error('FIX HINT:');
-      logger.error(error.fix);
+      logger.always(error.fix);
       logger.break();
     }
   }
   if (help) {
     logger.always(help);
   }
   logger.break();
-  logger.error('Critical error: aborting command');
+  // logger.error('Critical error: aborting command');
 
   process.exitCode = 1;
 

packages/cli/test/compile/compile.test.ts

@@ -89,7 +89,6 @@ test.serial('throw an AbortError if a job is uncompilable', async (t) => {
 
   t.assert(logger._find('error', /unexpected token/i));
   t.assert(logger._find('always', /check the syntax of the job expression/i));
-  t.assert(logger._find('error', /critical error: aborting command/i));
 });
 
 test.serial(
@@ -111,7 +110,6 @@ test.serial(
 
     t.assert(logger._find('error', /unexpected token/i));
     t.assert(logger._find('always', /check the syntax of the job expression/i));
-    t.assert(logger._find('error', /critical error: aborting command/i));
   }
 );