Skip to content
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
2daa88c
feat: runtime support for re-usable global functions
doc-han Mar 5, 2025
a39327f
tests: using global functions
doc-han Mar 5, 2025
8b564f6
feat: cli loading of global functions
doc-han Mar 5, 2025
65cb9b8
tests: cli tests for globals
doc-han Mar 5, 2025
e1953ca
tests: scope global functions per step or job code
doc-han Mar 5, 2025
ec66bc5
refactor: rename functions to globals
doc-han Mar 5, 2025
dfe3589
refactor: update fetchfile function signature
doc-han Mar 5, 2025
54dd347
refactor: resolve comments
doc-han Mar 6, 2025
f20a602
tests: fix test
doc-han Mar 6, 2025
c929feb
refactor: update fetchFile signature
doc-han Mar 7, 2025
bf0d9ee
docs: update workflow template with globals
doc-han Mar 17, 2025
45c584e
feat: support globals in ws-worker
doc-han Mar 17, 2025
5d4c006
tests: add execute test in ws-worker for globals
doc-han Mar 17, 2025
3af2cd3
chore: add missing arg to prepareGlobals
doc-han Mar 17, 2025
ea67a70
refactor: use buildContext for context building
doc-han Mar 17, 2025
6a6bce5
tests: update globals undefined tests
doc-han Mar 17, 2025
cc41a5b
tests: global functions scoping tests
doc-han Mar 17, 2025
f7b2f6d
refactor: cleanup
doc-han Mar 17, 2025
ae78f13
feat: get named exports & pass to ignoreList of job compilation
doc-han Jun 4, 2025
0806a38
test: adaptor imports should respect ignore list
doc-han Jun 4, 2025
0407d33
test: global functions expression
doc-han Jun 4, 2025
b92f460
test: globals with relative file path
doc-han Jun 4, 2025
8b818ad
feat: add --globals argument to cli
doc-han Jun 4, 2025
712cbd6
Merge branch 'main' into farhan/re-usable-functions-across-workflow
doc-han Jun 4, 2025
fc16f7f
tests: globals via cli argument
doc-han Jun 6, 2025
5ded024
little tweaks
josephjclark Jun 15, 2025
871fdd1
remove comment
josephjclark Jun 15, 2025
9d4ece3
changesets
josephjclark Jun 15, 2025
c628121
versions
josephjclark Jun 15, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion integration-tests/cli/test/errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,11 @@ test.serial("can't find config referenced in a workflow", async (t) => {

const stdlogs = extractLogs(stdout);

assertLog(t, stdlogs, /File not found for job 1: does-not-exist.js/i);
assertLog(
t,
stdlogs,
/File not found for job configuration 1: does-not-exist.js/i
);
assertLog(
t,
stdlogs,
Expand Down
1 change: 1 addition & 0 deletions packages/cli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ A workflow has a structure like this:
{
"workflow": {
"name": "my-workflow", // human readable name used in logging
"globals": "./common-funcs.js", // code or path to functions that can be accessed in any step. globally scoped
"steps": [
{
"name": "a", // human readable name used in logging
Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export type CLIExecutionPlan = {
id?: UUID;
name?: string;
steps: Array<CLIJobNode | Trigger>;
globals?: string;
};
};

Expand Down
52 changes: 39 additions & 13 deletions packages/cli/src/util/load-plan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,11 +156,14 @@ const loadOldWorkflow = async (
};

const fetchFile = async (
jobId: string,
rootDir: string = '',
filePath: string,
fileInfo: {
name: string;
rootDir?: string;
filePath: string;
},
log: Logger
) => {
const { rootDir = '', filePath, name } = fileInfo;
try {
// Special handling for ~ feels like a necessary evil
const fullPath = filePath.startsWith('~')
Expand All @@ -172,7 +175,7 @@ const fetchFile = async (
} catch (e) {
abort(
log,
`File not found for job ${jobId}: ${filePath}`,
`File not found for ${name}: ${filePath}`,
undefined,
`This workflow references a file which cannot be found at ${filePath}\n\nPaths inside the workflow are relative to the workflow.json`
);
Expand All @@ -182,6 +185,20 @@ const fetchFile = async (
}
};

const importGlobals = async (
plan: CLIExecutionPlan,
rootDir: string,
log: Logger
) => {
const fnStr = plan.workflow?.globals;
if (fnStr && isPath(fnStr)) {
plan.workflow.globals = await fetchFile(
{ name: 'globals', rootDir, filePath: fnStr },
log
);
}
};

// TODO this is currently untested in load-plan
// (but covered a bit in execute tests)
const importExpressions = async (
Expand All @@ -204,26 +221,32 @@ const importExpressions = async (

if (expressionStr && isPath(expressionStr)) {
job.expression = await fetchFile(
job.id || `${idx}`,
rootDir,
expressionStr,
{
name: `job ${job.id || idx}`,
rootDir,
filePath: expressionStr,
},
log
);
}
if (configurationStr && isPath(configurationStr)) {
const configString = await fetchFile(
job.id || `${idx}`,
rootDir,
configurationStr,
{
name: `job configuration ${job.id || idx}`,
rootDir,
filePath: configurationStr,
},
log
);
job.configuration = JSON.parse(configString!);
}
if (stateStr && isPath(stateStr)) {
const stateString = await fetchFile(
job.id || `${idx}`,
rootDir,
stateStr,
{
name: `job state ${job.id || idx}`,
rootDir,
filePath: stateStr,
},
log
);
job.state = JSON.parse(stateString!);
Expand Down Expand Up @@ -260,6 +283,9 @@ const loadXPlan = async (
}
ensureAdaptors(plan);

// import global functions
await importGlobals(plan, options.baseDir!, logger);

// Note that baseDir should be set up in the default function
await importExpressions(plan, options.baseDir!, logger);
// expand shorthand adaptors in the workflow jobs
Expand Down
53 changes: 53 additions & 0 deletions packages/cli/test/execute/execute.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -450,3 +450,56 @@ test.serial('run a job which does not return state', async (t) => {
// Check that no error messages have been logged
t.is(logger._history.length, 0);
});

test.serial('globals: use a global function in an operation', async (t) => {
const workflow = {
workflow: {
globals: "export const prefixer = (w) => 'welcome '+w",
steps: [
{
id: 'a',
state: { data: { name: 'John' } },
expression: `${fn}fn(state=> { state.data.new = prefixer(state.data.name); return state; })`,
},
],
},
};

mockFs({
'/workflow.json': JSON.stringify(workflow),
});

const options = {
...defaultOptions,
workflowPath: '/workflow.json',
};
const result = await handler(options, logger);
t.deepEqual(result.data, { name: 'John', new: 'welcome John' });
});

test.serial('globals: get global functions from a filePath', async (t) => {
const workflow = {
workflow: {
globals: '/my-globals.js',
steps: [
{
id: 'a',
state: { data: { name: 'John' } },
expression: `${fn}fn(state=> { state.data.new = suffixer(state.data.name); return state; })`,
},
],
},
};

mockFs({
'/workflow.json': JSON.stringify(workflow),
'/my-globals.js': `export const suffixer = (w) => w + " goodbye!"`,
});

const options = {
...defaultOptions,
workflowPath: '/workflow.json',
};
const result = await handler(options, logger);
t.deepEqual(result.data, { name: 'John', new: 'John goodbye!' });
});
3 changes: 3 additions & 0 deletions packages/lexicon/core.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ export type Workflow = {
// global credentials
// (gets applied to every configuration object)
credentials?: Record<string, any>;

// a path to a file where functions are defined
globals?: string;
};

/**
Expand Down
2 changes: 2 additions & 0 deletions packages/lexicon/lightning.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ export type LightningPlan = {
edges: LightningEdge[];

options?: LightningPlanOptions;

globals?: string;
};

/**
Expand Down
4 changes: 4 additions & 0 deletions packages/runtime/src/execute/compile-plan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ export default (plan: ExecutionPlan) => {
start: options.start ?? workflow.steps[0]?.id!,
},
};

if (typeof workflow.globals === 'string')
newPlan.workflow.globals = workflow.globals;

if (workflow.credentials) {
newPlan.workflow.credentials = workflow.credentials;
}
Expand Down
33 changes: 31 additions & 2 deletions packages/runtime/src/execute/expression.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
assertSecurityKill,
AdaptorError,
} from '../errors';
import type { JobModule, ExecutionContext } from '../types';
import type { JobModule, ExecutionContext, GlobalsModule } from '../types';
import { ModuleInfoMap } from '../modules/linker';
import {
clearNullState,
Expand Down Expand Up @@ -50,15 +50,26 @@ export default (
try {
const timeout = plan.options?.timeout ?? ctx.opts.defaultRunTimeoutMs;

// prepare global functions to be injected into execution context
const globals = {
...opts.globals,
...(plan.workflow?.globals
? await prepareGlobals(plan.workflow.globals, opts)
Comment thread
doc-han marked this conversation as resolved.
Outdated
: {}),
};

// Setup an execution context
const context = buildContext(input, opts);
const context = buildContext(input, { ...opts, globals });

// FIXME: when expression isn't a string, additional stuff isn't loaded

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to look at this?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. expression in this scope is typed string | Operation[].
It seems Operation[] type is only reached via tests. CLI always has expression being a string.

If this assumption is accurate and expression in runtime is approx. always a string, then there's no problem.
Here's the condition that loads-module only when it's a string.

if (typeof expression === 'string') {
const exports = await loadModule(expression, {
...opts.linker,
// allow module paths and versions to be overriden from the defaults
modules: mergeLinkerOptions(opts.linker?.modules, moduleOverrides),
context,
log: opts.logger,
});
const operations = exports.default;
return {
operations,
...exports,
} as JobModule;
} else {
if (opts.forceSandbox) {
throw new InputError('Invalid arguments: jobs must be strings');
}
return { operations: expression as Operation[] };
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Yes, usually the expression comes in as a string, but for some unit tests we do pass functions straight in.

it's a fair constraint that if you're passing a function directly, we don't do any environment preparation. We don't load globals, we don't load a sandbox.

I don't want to leave this comment here. It's not meaningul.

  • We could move it to the prepareJob declaration. It's not a fixme, but it's correct docs that if you pass a live function, it'll bypass the sandbox, globals etc.
  • We could put out a warning in the logger. Given that it only affects tests, I don't think this is neccessary

Let's move it to prepareJob and remove the FIXME

// eg. global functions might not be loaded!
const { operations, execute } = await prepareJob(
expression,
context,
opts,
moduleOverrides
);

// Create the main reducer function
const reducer = (execute || defaultExecute)(
...operations.map((op, idx) =>
Expand Down Expand Up @@ -238,3 +249,21 @@ const prepareJob = async (
return { operations: expression as Operation[] };
}
};

const prepareGlobals = async (
source: string,
opts: Options = {}
): Promise<GlobalsModule> => {
if (typeof source === 'string' && !!source.trim()) {
const context = buildContext({}, opts);
return await loadModule(source || '', {
context,
}).catch((e) => {
// mostly syntax errors
// repackage errors and throw
e.message = `[globals] ${e.message}`;
throw e;
});
}
return {};
};
3 changes: 3 additions & 0 deletions packages/runtime/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export type Lazy<T> = string | T;

export type CompiledExecutionPlan = {
workflow: {
globals?: string;
steps: Record<StepId, CompiledStep>;
credentials?: Record<string, any>;
};
Expand All @@ -55,6 +56,8 @@ export type JobModule = {
// TODO lifecycle hooks
};

export type GlobalsModule = Record<string, any>;

type NotifyHandler = (
event: NotifyEvents,
payload: NotifyEventsLookup[typeof event]
Expand Down
37 changes: 37 additions & 0 deletions packages/runtime/test/execute/compile-plan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,43 @@ test('throw for a syntax error on a job edge', (t) => {
}
});

test('globals should be undefined if no globals are passed', (t) => {
const plan: ExecutionPlan = {
workflow: {
steps: [{ expression: 'x' }, { expression: 'y' }],
},
options: {},
};
const compiled = compilePlan(plan);
t.is(compiled.workflow.globals, undefined);
});

test('globals should be undefined if non-string is passed', (t) => {
const plan: ExecutionPlan = {
workflow: {
globals: { some: 'val' } as any,
steps: [{ expression: 'x' }, { expression: 'y' }],
},
options: {},
};
const compiled = compilePlan(plan);
t.is(compiled.workflow.globals, undefined);
});

test('global functions should perform no transformation inline code', (t) => {
const GLOBAL_FUNCTIONS =
'export const cleaner = (name) => name.replace("a", "b");';
const plan: ExecutionPlan = {
workflow: {
globals: GLOBAL_FUNCTIONS,
steps: [{ expression: 'x' }, { expression: 'y' }],
},
options: {},
};
const compiled = compilePlan(plan);
t.is(compiled.workflow.globals, GLOBAL_FUNCTIONS);
});

test('throw for multiple errors', (t) => {
const plan = {
workflow: {
Expand Down
17 changes: 17 additions & 0 deletions packages/runtime/test/execute/expression.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,23 @@ test.serial('run a live no-op job with one operation', async (t) => {
t.deepEqual(state, result);
});

test.serial('use global function defined in functions', async (t) => {
const job = `export default [state=> {state.data.pr = prefixer(state.data.name); return state;}]`;

const state = createState();
// @ts-ignore
state.data.name = 'john';
const context = createContext();
context.plan.workflow = {
steps: [] as any,
globals: "export const prefixer = (w) => 'welcome '+ w",
};

const result = await execute(context, job, state);
// @ts-ignore
t.is(result.data.pr, 'welcome ' + state.data.name);
});

test.serial('run a stringified no-op job with one operation', async (t) => {
const job = 'export default [(s) => s]';
const state = createState();
Expand Down
Loading