diff --git a/.changeset/large-windows-move.md b/.changeset/large-windows-move.md new file mode 100644 index 000000000..86b540b9e --- /dev/null +++ b/.changeset/large-windows-move.md @@ -0,0 +1,5 @@ +--- +'@openfn/cli': patch +--- + +Fix loading of workflow.yaml files diff --git a/packages/cli/src/util/load-plan.ts b/packages/cli/src/util/load-plan.ts index 00dba22db..93cc92002 100644 --- a/packages/cli/src/util/load-plan.ts +++ b/packages/cli/src/util/load-plan.ts @@ -35,8 +35,12 @@ const loadPlan = async ( if (options.path && /ya?ml$/.test(options.path)) { const content = await fs.readFile(path.resolve(options.path), 'utf-8'); const workflow = yamlToJson(content); + // Temporarily support workflow.yaml files without a top workflow key + // This was released in july 2025 but it's actually wrong! + // we'll support it for a while for back-compat, but this need removing in 2027 + const plan = workflow.workflow ? workflow : { workflow }; options.baseDir = dirname(options.path); - return loadXPlan({ workflow }, options, logger); + return loadXPlan(plan, options, logger); } // Run a workflow from a project, with a path and workflow name diff --git a/packages/cli/test/util/load-plan.test.ts b/packages/cli/test/util/load-plan.test.ts index 6e50dde63..c6e3486dd 100644 --- a/packages/cli/test/util/load-plan.test.ts +++ b/packages/cli/test/util/load-plan.test.ts @@ -4,11 +4,7 @@ import { createMockLogger } from '@openfn/logger'; import type { Job } from '@openfn/lexicon'; import loadPlan from '../../src/util/load-plan'; -import { - collectionsEndpoint, - collectionsVersion, - Opts, -} from '../../src/options'; +import { Opts } from '../../src/options'; const logger = createMockLogger(undefined, { level: 'debug' }); @@ -47,8 +43,6 @@ test.afterEach(() => { mock.restore(); }); -// TODO: add some tests for handling yaml stuff - test.serial('expression: load a plan from an expression.js', async (t) => { const opts = { expressionPath: 'test/job.js', @@ -429,3 +423,57 @@ test.serial('xplan: append collections', async (t) => { collections_token: opts.apiKey, }); }); + +// This is basically an invalid workflow yaml that was accidentally supported +// Preserving for back compat for now +test.serial( + 'xplan: load a workflow.yaml (without top workflow key)', + async (t) => { + mock({ + 'test/wf.yaml': ` +name: wf +steps: + - id: a + adaptors: [] + expression: x() +`, + }); + const opts = { + path: 'test/wf.yaml', // TODO should this work with workflow path as well? + expandAdaptors: true, + plan: {}, + }; + + const plan = await loadPlan(opts, logger); + + t.truthy(plan); + // Note that options are lost in this design! + t.deepEqual(plan, { workflow: sampleXPlan.workflow, options: {} }); + } +); + +test.serial('xplan: load a proper workflow.yaml', async (t) => { + mock({ + 'test/wf.yaml': ` +workflow: + name: wf + steps: + - id: a + adaptors: [] + expression: x() +options: + start: a +`, + }); + const opts = { + path: 'test/wf.yaml', // TODO should this work with workflow path as well? + expandAdaptors: true, + plan: {}, + }; + + const plan = await loadPlan(opts, logger); + + t.truthy(plan); + // Note that options are lost in this design! + t.deepEqual(plan, sampleXPlan); +}); diff --git a/packages/project/src/Workflow.ts b/packages/project/src/Workflow.ts index 2a5e3c98b..45a90c275 100644 --- a/packages/project/src/Workflow.ts +++ b/packages/project/src/Workflow.ts @@ -13,11 +13,12 @@ class Workflow { index: any; name?: string; + start?: string; id: string; openfn?: l.WorkflowMeta; - options: any; // TODO + options: l.WorkflowOptions; - constructor(workflow: l.Workflow) { + constructor(workflow: l.Workflow, options: l.WorkflowOptions = {}) { this.index = { steps: {}, // steps by id edges: {}, // edges by from-id id @@ -30,7 +31,7 @@ class Workflow { // history needs to be on workflow object. this.workflow.history = workflow.history?.length ? workflow.history : []; - const { id, name, openfn, steps, history, ...options } = workflow; + const { id, name, openfn } = workflow; if (!(id || name)) { throw new Error('A Workflow MUST have a name or id'); } @@ -159,7 +160,10 @@ class Workflow { } toJSON(): Object { - return clone(this.workflow); + return { + ...clone(this.workflow), + options: this.options, + }; } getUUIDMap(): Record { diff --git a/packages/project/src/parse/from-app-state.ts b/packages/project/src/parse/from-app-state.ts index 5224d6fc3..7db607944 100644 --- a/packages/project/src/parse/from-app-state.ts +++ b/packages/project/src/parse/from-app-state.ts @@ -7,6 +7,7 @@ import { Project } from '../Project'; import renameKeys from '../util/rename-keys'; import slugify from '../util/slugify'; import ensureJson from '../util/ensure-json'; +import Workflow from '../Workflow'; export type fromAppStateConfig = Partial & { format?: 'yaml' | 'json'; @@ -96,6 +97,7 @@ export const mapWorkflow = (workflow: Provisioner.Workflow) => { name: workflow.name, steps: [], history: workflow.version_history ?? [], + openfn: renameKeys(remoteProps, { id: 'uuid' }), }; if (workflow.name) { @@ -161,5 +163,10 @@ export const mapWorkflow = (workflow: Provisioner.Workflow) => { mapped.steps.push(s); }); - return mapped; + // TODO do we need to load other options from the state file? + const options = { + start: 'trigger', + }; + + return new Workflow(mapped, options); }; diff --git a/packages/project/src/serialize/to-fs.ts b/packages/project/src/serialize/to-fs.ts index ca9f59956..0e5284366 100644 --- a/packages/project/src/serialize/to-fs.ts +++ b/packages/project/src/serialize/to-fs.ts @@ -50,7 +50,6 @@ export const extractWorkflow = (project: Project, workflowId: string) => { name: workflow.name, // Note: if no options are defined, options will serialize to an empty object // Not crazy about this - maybe we should do something better? Or do we like the consistency? - options: workflow.options, steps: workflow.steps.map((step) => { const { openfn, expression, next, ...mapped } = step; if (expression) { @@ -65,7 +64,11 @@ export const extractWorkflow = (project: Project, workflowId: string) => { return mapped; }), }; - return handleOutput(wf, path, format!); + return handleOutput( + { workflow: wf, options: workflow.options }, + path, + format! + ); }; // extracts an expression.js from a workflow in project diff --git a/packages/project/test/fixtures/sample-v2-project.ts b/packages/project/test/fixtures/sample-v2-project.ts index 049ccb27e..fffb95932 100644 --- a/packages/project/test/fixtures/sample-v2-project.ts +++ b/packages/project/test/fixtures/sample-v2-project.ts @@ -29,6 +29,7 @@ export const json: SerializedProject = { ], name: 'Workflow', id: 'workflow', + options: { start: 'trigger' }, openfn: { uuid: 1 }, history: [], }, @@ -65,6 +66,7 @@ workflows: name: Workflow id: workflow openfn: - uuid: 1 + uuid: 1 history: [] + start: trigger `; diff --git a/packages/project/test/parse/from-app-state.test.ts b/packages/project/test/parse/from-app-state.test.ts index 6f509bc04..913e517fe 100644 --- a/packages/project/test/parse/from-app-state.test.ts +++ b/packages/project/test/parse/from-app-state.test.ts @@ -86,6 +86,7 @@ test('should create a Project from prov state with a workflow', (t) => { id: 'my-workflow', name: 'My Workflow', history: [], + start: 'trigger', steps: [ { id: 'trigger', diff --git a/packages/project/test/parse/from-project.test.ts b/packages/project/test/parse/from-project.test.ts index d6c57644d..c86b0cf1d 100644 --- a/packages/project/test/parse/from-project.test.ts +++ b/packages/project/test/parse/from-project.test.ts @@ -86,6 +86,7 @@ test('import from a v2 project as JSON', async (t) => { uuid: 1, }, history: [], + start: 'trigger', steps: [ { name: 'b', @@ -141,6 +142,7 @@ test('import from a v2 project as YAML', async (t) => { uuid: 1, }, history: [], + start: 'trigger', steps: [ { name: 'b', diff --git a/packages/project/test/serialize/to-fs.test.ts b/packages/project/test/serialize/to-fs.test.ts index 6f558ae20..44761d3ee 100644 --- a/packages/project/test/serialize/to-fs.test.ts +++ b/packages/project/test/serialize/to-fs.test.ts @@ -1,6 +1,7 @@ import test from 'ava'; import { Project } from '../../src/Project'; import toFs, { extractWorkflow } from '../../src/serialize/to-fs'; +import Workflow from '../../src/Workflow'; const step = { id: 'step', @@ -31,16 +32,54 @@ test('extractWorkflow: single simple workflow (yaml by default)', (t) => { const { path, content } = extractWorkflow(project, 'my-workflow'); t.is(path, 'workflows/my-workflow/my-workflow.yaml'); - // TODO is the empty options object correct here?? t.deepEqual( content, - `id: my-workflow -name: My Workflow + `workflow: + id: my-workflow + name: My Workflow + steps: + - id: step + adaptor: "@openfn/language-common@latest" + expression: ./step.js options: {} -steps: - - id: step - adaptor: "@openfn/language-common@latest" - expression: ./step.js +` + ); +}); + +test('extractWorkflow: workflow with options', (t) => { + const project = new Project({ + workflows: [ + new Workflow( + { + // TODO I need to fix this name/id conflict + // the local workflow id is a slugified form of the name + id: 'my-workflow', + name: 'My Workflow', + steps: [step], + // should be ignored because this lives in the project file + openfn: { + id: '72ca3eb0-042c-47a0-a2a1-a545ed4a8406', + }, + }, + { start: 'step' } + ), + ], + }); + + const { path, content } = extractWorkflow(project, 'my-workflow'); + t.is(path, 'workflows/my-workflow/my-workflow.yaml'); + + t.deepEqual( + content, + `workflow: + id: my-workflow + name: My Workflow + steps: + - id: step + adaptor: "@openfn/language-common@latest" + expression: ./step.js +options: + start: step ` ); }); @@ -58,7 +97,7 @@ test('extractWorkflow: single simple workflow with an edge', (t) => { id: 'step1', next: { step2: { - condition: true, + condition: 'always', openfn: { // should be excluded! uuid: 1, @@ -88,26 +127,28 @@ test('extractWorkflow: single simple workflow with an edge', (t) => { t.is(path, 'workflows/my-workflow/my-workflow.json'); t.deepEqual(JSON.parse(content), { - id: 'my-workflow', - name: 'My Workflow', - options: {}, - steps: [ - { - id: 'step1', - expression: './step1.js', - adaptor: '@openfn/language-common@latest', - next: { - step2: { - condition: true, + workflow: { + id: 'my-workflow', + name: 'My Workflow', + steps: [ + { + id: 'step1', + expression: './step1.js', + adaptor: '@openfn/language-common@latest', + next: { + step2: { + condition: 'always', + }, }, }, - }, - { - id: 'step2', - expression: './step2.js', - adaptor: '@openfn/language-common@latest', - }, - ], + { + id: 'step2', + expression: './step2.js', + adaptor: '@openfn/language-common@latest', + }, + ], + }, + options: {}, }); }); @@ -142,17 +183,19 @@ test('extractWorkflow: single simple workflow with random edge property', (t) => t.is(path, 'workflows/my-workflow/my-workflow.json'); t.deepEqual(JSON.parse(content), { - id: 'my-workflow', - name: 'My Workflow', + workflow: { + id: 'my-workflow', + name: 'My Workflow', + steps: [ + { + id: 'step', + expression: './step.js', + adaptor: '@openfn/language-common@latest', + foo: 'bar', + }, + ], + }, options: {}, - steps: [ - { - id: 'step', - expression: './step.js', - adaptor: '@openfn/language-common@latest', - foo: 'bar', - }, - ], }); }); @@ -177,7 +220,7 @@ test('extractWorkflow: single simple workflow with custom root', (t) => { config ); - const { path, content } = extractWorkflow(project, 'my-workflow'); + const { path } = extractWorkflow(project, 'my-workflow'); t.is(path, 'openfn/wfs/my-workflow/my-workflow.json'); }); @@ -223,7 +266,9 @@ test('toFs: extract a project with 1 workflow and 1 step', (t) => { }, }); - const workflow = JSON.parse(files['workflows/my-workflow/my-workflow.json']); + const { workflow } = JSON.parse( + files['workflows/my-workflow/my-workflow.json'] + ); t.is(workflow.id, 'my-workflow'); t.is(workflow.steps.length, 1); diff --git a/packages/project/test/serialize/to-project.test.ts b/packages/project/test/serialize/to-project.test.ts index 533350059..fec0b6052 100644 --- a/packages/project/test/serialize/to-project.test.ts +++ b/packages/project/test/serialize/to-project.test.ts @@ -1,8 +1,9 @@ import test from 'ava'; import { Project } from '../../src/Project'; -import generateWorkflow, { generateProject } from '../../src/gen/generator'; +import generateWorkflow from '../../src/gen/generator'; import * as v2 from '../fixtures/sample-v2-project'; +import Workflow from '../../src/Workflow'; const createProject = () => { const proj = new Project({ @@ -20,12 +21,15 @@ const createProject = () => { allow_support_access: false, }, workflows: [ - generateWorkflow( - 'trigger(type=webhook)-b(expression="fn()",adaptor=common,project_credential_id=x)', - { - uuidSeed: 1, - openfnUuid: true, - } + new Workflow( + generateWorkflow( + 'trigger(type=webhook)-b(expression="fn()",adaptor=common,project_credential_id=x)', + { + uuidSeed: 1, + openfnUuid: true, + } + ), + { start: 'trigger' } ), ], }); @@ -34,7 +38,7 @@ const createProject = () => { return proj; }; -test('should serialize to YAML format v2 project by default', (t) => { +test.only('should serialize to YAML format v2 project by default', (t) => { const proj = createProject(); const yaml = proj.serialize('project');