Fix: renaming a workflow in git deletes it in Lightning

.changeset/rename-workflow-deploy.md

@@ -0,0 +1,6 @@
+---
+'@openfn/cli': patch
+'@openfn/project': patch
+---
+
+Fix deploying an edit to a renamed workflow deleting it from Lightning

packages/cli/src/projects/deploy.ts

@@ -25,6 +25,7 @@ import {
 import { build, ensure } from '../util/command-builders';
 import { printRichDiff } from './diff';
 import ensureCheckout from './ensure-checkout';
+import { handler as checkout } from './checkout';
 import isUuid from '../util/is-uuid';
 import { getCredentialsVisitor, remapCredentials } from './credentials-helpers';
 
@@ -639,35 +640,52 @@ export async function handler(options: DeployOptions, logger: Logger) {
     if (options.checkout !== false) {
       const workspacePath = options.workspace ?? process.cwd();
 
-      // Write the updated openfn.yaml and checkout file
-      // TODO: allow us to suppress writing this stuff
-      // (useful if posting from spec)
-      const configData = finalProject.generateConfig();
-      const configPath = path.resolve(workspacePath, configData.path);
-      await mkdir(path.dirname(configPath), { recursive: true });
-      await writeFile(configPath, configData.content);
-      await writeCheckoutFile(workspacePath, finalProject, options.branch);
-
-      // Sync collections into resources.yaml (anything else in there is left alone)
-      const resourcesPath = path.resolve(workspacePath, RESOURCES_FILE);
-      const existingResources = await readFile(resourcesPath, 'utf8').catch(
-        (e) => {
-          if (e.code !== 'ENOENT') throw e;
-          return undefined;
-        }
-      );
-      const resources = finalProject.generateResources(existingResources);
-      if (resources.content) {
-        await writeFile(resourcesPath, resources.content);
-      }
-
       // TODO if this was marked as new, we probably need to ensure a unique alias here
       const finalOutputPath = getSerializePath(
         finalProject,
         options.workspace!
       );
       const fullFinalPath = await serialize(finalProject, finalOutputPath);
       logger.debug('Updated local project at ', fullFinalPath);
+
+      if (filePath || options.workflow?.length) {
+        // workflows/ wasn't (all) deployed: rewriting it could lose local
+        // changes, so just write the updated openfn.yaml and checkout file
+        // TODO: allow us to suppress writing this stuff
+        // (useful if posting from spec)
+        const configData = finalProject.generateConfig();
+        const configPath = path.resolve(workspacePath, configData.path);
+        await mkdir(path.dirname(configPath), { recursive: true });
+        await writeFile(configPath, configData.content);
+        await writeCheckoutFile(workspacePath, finalProject, options.branch);
+
+        // Sync collections into resources.yaml (anything else in there is left alone)
+        const resourcesPath = path.resolve(workspacePath, RESOURCES_FILE);
+        const existingResources = await readFile(resourcesPath, 'utf8').catch(
+          (e) => {
+            if (e.code !== 'ENOENT') throw e;
+            return undefined;
+          }
+        );
+        const resources = finalProject.generateResources(existingResources);
+        if (resources.content) {
+          await writeFile(resourcesPath, resources.content);
+        }
+      } else {
+        // Check out what we just deployed, as pull does, so that workflows/
+        // matches Lightning (eg, a renamed workflow moves to a folder named
+        // after its new id). Everything local has just been deployed, so force
+        await checkout(
+          {
+            project: fullFinalPath,
+            workspace: workspacePath,
+            branch: options.branch,
+            force: true,
+            track: true,
+          } as any,
+          logger
+        );
+      }
     }
 
     if (options.new) {

packages/cli/test/projects/deploy.test.ts

@@ -471,6 +471,78 @@ test.serial('deploy a change to a project', async (t) => {
   t.is(Object.keys(server.state.projects).length, 1);
 });
 
+const renameWorkflow = async (folder: string) => {
+  const wfPath = `/ws/workflows/${folder}/my-workflow.yaml`;
+  const wfYaml = fs.readFileSync(wfPath, 'utf8');
+  await writeFile(
+    wfPath,
+    wfYaml.replace('name: My Workflow', 'name: My Workflow (renamed)')
+  );
+};
+
+const deployOptions = {
+  endpoint: ENDPOINT,
+  apiKey: 'test-api-key',
+  workspace: '/ws',
+  confirm: false,
+} as any;
+
+test.serial(
+  'deploy a renamed workflow, then deploy an edit to it',
+  async (t) => {
+    await setup(projectYaml);
+
+    await renameWorkflow('my-workflow');
+    await deploy(deployOptions, logger);
+
+    // the workflow is checked out under its new id
+    t.false(fs.existsSync('/ws/workflows/my-workflow'));
+    const wfYaml = fs.readFileSync(
+      '/ws/workflows/my-workflow-renamed/my-workflow-renamed.yaml',
+      'utf8'
+    );
+    t.regex(wfYaml, /^id: my-workflow-renamed$/m);
+    t.regex(readCheckout()!, /forked_from:\n  my-workflow-renamed: /);
+
+    await writeFile(
+      '/ws/workflows/my-workflow-renamed/transform-data.js',
+      'log()'
+    );
+    await deploy(deployOptions, logger);
+
+    const workflows = Object.values(
+      server.state.projects[UUID].workflows
+    ) as any[];
+    t.is(workflows.length, 1);
+    t.is(workflows[0].id, '72ca3eb0-042c-47a0-a2a1-a545ed4a8406');
+    t.is(workflows[0].name, 'My Workflow (renamed)');
+    t.is(
+      Object.values<any>(workflows[0].jobs).find(
+        (j) => j.name === 'Transform data'
+      ).body,
+      'log()'
+    );
+  }
+);
+
+test.serial(
+  'deploy a renamed workflow replaces its aliased folder with one named after the new id',
+  async (t) => {
+    await setup(projectYaml);
+    fs.renameSync('/ws/workflows/my-workflow', '/ws/workflows/sample');
+
+    await renameWorkflow('sample');
+    await deploy(deployOptions, logger);
+
+    t.false(fs.existsSync('/ws/workflows/sample'));
+    t.true(
+      fs.existsSync(
+        '/ws/workflows/my-workflow-renamed/my-workflow-renamed.yaml'
+      )
+    );
+  }
+);
+
 test.serial(
   'Error if the remote and local workflows have diverged',
   async (t) => {

packages/project/src/serialize/to-app-state.ts

@@ -89,10 +89,26 @@ export default function (
     owner: c.owner,
   }));
 
+  // A workflow both removed and kept (eg, its id changed after a rename) is
+  // a change, not a delete: never send a delete for a uuid we're also sending
+  const keptUuids = new Set(
+    project.workflows
+      .filter((w) => !w.isRemoved?.())
+      .map((w) => w.openfn?.uuid)
+      .filter(Boolean)
+  );
   state.workflows = project.workflows
+    .filter((w) => !(w.isRemoved?.() && keptUuids.has(w.openfn?.uuid)))
     .map((w) => mapWorkflow(w, project.credentials))
     .reduce((obj: any, wf) => {
-      obj[slugify(wf.name ?? wf.id)] = wf;
+      const key = slugify(wf.name ?? wf.id);
+      // don't let one workflow (or its delete) silently overwrite another
+      if (key in obj) {
+        throw new Error(
+          `More than one workflow maps to "${key}" - refusing to build a state that would lose one of them`
+        );
+      }
+      obj[key] = wf;
       return obj;
     }, {});
 

packages/project/test/serialize/to-app-state.test.ts

@@ -742,6 +742,36 @@ test('a removed workflow becomes a minimal delete: true entry', (t) => {
   });
 });
 
+test('a workflow removed and kept under the same uuid is sent as a change, not a delete', (t) => {
+  // eg, the local id drifted after a rename: the old id is removed, the new one added
+  const data = cloneDeep(removableWorkflowData);
+  // the merge lists the kept workflow before the removed one
+  data.workflows.unshift({ ...cloneDeep(data.workflows[0]), id: 'wf-renamed' });
+  const project = new Project(data);
+  project.getWorkflow('wf')!.remove();
+
+  const state = toAppState(project, { format: 'json' }) as Provisioner.Project;
+
+  t.is(Object.keys(state.workflows).length, 1);
+  t.is(state.workflows['wf'].id, 'wf-uuid');
+  t.falsy(state.workflows['wf'].delete);
+  t.is(state.workflows['wf'].jobs.step.id, 'step-uuid');
+});
+
+test('throws if a removed workflow would overwrite another with the same name', (t) => {
+  const data = cloneDeep(removableWorkflowData);
+  const added = cloneDeep(data.workflows[0]);
+  added.id = 'wf-renamed';
+  delete added.openfn; // never synced, so no uuid
+  data.workflows.push(added);
+  const project = new Project(data);
+  project.getWorkflow('wf')!.remove();
+
+  t.throws(() => toAppState(project, { format: 'json' }), {
+    message: /More than one workflow maps to "wf"/,
+  });
+});
+
 test('should serialize channels to app state', (t) => {
   const channels = [
     {