feat(automation): resolve & validate script-node callables; function registration (#1870) (#1903)

os-zhuang · claude · web-flow · commit dc8b2defc562 · 2026-06-15T14:33:03.000+05:00
* feat(automation): resolve & validate script-node callables; function registration path (#1870) A flow `script` node pointing at an unregistered callable — or declaring no `actionType`/`function` at all — built fine and silently did nothing at runtime. Close the gap on three surfaces: - Runtime: the built-in `script` executor resolves its target (built-in side-effect → registered function → else fail loud). The old `(no-op handler)` success path is removed, so an unwired callable can no longer quietly skip the step. - Registration path: add `AutomationEngine.setFunctionResolver()`/ `resolveFunction()` and bridge it in the plugin's `start()` to ObjectQL's `resolveFunction` (fed by `bundle.functions` / `defineStack({ functions })`). A `script` node can now invoke an authored function by name: `{ type: 'script', config: { function: 'my_fn', inputs: { … } } }`. - Build: `objectstack build` flags a `script` node that names no callable (the `actionType: undefined` repro). Existence is checked at runtime — functions are code, not serialized into the artifact. Tests: new script-executor suite (function resolution, loud failure, bare actionType shorthand, thrown-function surfacing) + CLI build-validation cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(automation): recognize inline `config.script` so it isn't flagged as an empty no-op (#1870) CI Build Core caught that the build-time check (and the runtime executor) treated a `script` node carrying inline JS in `config.script` as target-less, breaking example-crm/showcase builds. Inline `config.script` is a distinct, recognized form — the built-in runtime has no server-side JS sandbox so it does NOT execute it (a separate gap, out of #1870's callable-resolution scope). Now: - build validation accepts `function` | `actionType` | non-empty `script`; - the runtime executor warns loudly that inline scripts aren't executed (and points to `defineStack({ functions })`) instead of failing the step or silently succeeding. Verified example-crm, app-showcase, app-todo all build clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
diff --git a/.changeset/flow-script-callable-validation.md b/.changeset/flow-script-callable-validation.md
@@ -0,0 +1,26 @@
+---
+"@objectstack/service-automation": minor
+"@objectstack/cli": patch
+---
+
+feat(automation): resolve & validate `script`-node callables; first-class function registration (#1870)
+
+A flow `script` node that pointed at an unregistered callable (or declared no
+`actionType`/`function` at all) built fine and silently did nothing at runtime.
+Two changes close that gap:
+
+- **Loud runtime resolution.** The built-in `script` executor now resolves its
+  target in order — built-in side-effect (`email`/`slack`) → a registered
+  function (`config.function`, or a bare `config.actionType` that matches no
+  built-in) → otherwise **fail the step loudly**. The old `(no-op handler)`
+  success path is gone, so an unwired callable can no longer quietly skip.
+- **First-class registration path.** `AutomationEngine.setFunctionResolver()` /
+  `resolveFunction()` bridge flow nodes to the host function registry. The
+  automation plugin wires it to ObjectQL's `resolveFunction` (populated from
+  `bundle.functions` / `defineStack({ functions })`), so an authored package can
+  register a function and call it from a `script` node:
+  `{ type: 'script', config: { function: 'my_fn', inputs: { … } } }`.
+- **Build-time structural check.** `objectstack build` now flags a `script` node
+  that declares neither `actionType` nor `function` (the `actionType: undefined`
+  repro). Function *existence* is verified at runtime — functions are code, not
+  serialized into the artifact.
diff --git a/packages/cli/src/utils/validate-expressions.test.ts b/packages/cli/src/utils/validate-expressions.test.ts
@@ -66,4 +66,37 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
     expect(issues).toHaveLength(1);
     expect(issues[0].where).toContain("validation 'r1'");
   });
+
+  // #1870 — a `script` node that names no callable is a silent no-op.
+  it('flags a script node that declares neither actionType nor function (#1870)', () => {
+    const issues = validateStackExpressions({
+      flows: [{
+        name: 'helpdesk_flow',
+        nodes: [
+          { id: 'start', type: 'start', config: {} },
+          { id: 'triage', type: 'script', config: { actionType: undefined } },
+        ],
+        edges: [],
+      }],
+    });
+    expect(issues).toHaveLength(1);
+    expect(issues[0].where).toContain("node 'triage' (script) callable");
+    expect(issues[0].message).toMatch(/neither .*actionType.* nor .*function/);
+  });
+
+  it('accepts a script node that names a built-in action or a function (#1870)', () => {
+    const issues = validateStackExpressions({
+      flows: [{
+        name: 'helpdesk_flow',
+        nodes: [
+          { id: 'start', type: 'start', config: {} },
+          { id: 'mail', type: 'script', config: { actionType: 'email' } },
+          { id: 'triage', type: 'script', config: { function: 'helpdesk.aiTriageStub' } },
+          { id: 'inline', type: 'script', config: { script: 'variables.x = 1;' } },
+        ],
+        edges: [],
+      }],
+    });
+    expect(issues).toHaveLength(0);
+  });
 });
diff --git a/packages/cli/src/utils/validate-expressions.ts b/packages/cli/src/utils/validate-expressions.ts
@@ -78,6 +78,29 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] {
     for (const node of nodes) {
       const cfg = (node.config ?? {}) as AnyRec;
       check(`flow '${flowName}' · node '${node.id}' (${node.type}) condition`, cfg.condition, objectName);
+      // #1870 — a `script` node must declare a callable target (`actionType` or
+      // `function`). A node with neither is a silent no-op that otherwise passes
+      // build. (Function *existence* isn't checkable here — functions are code,
+      // not serialized into the artifact — so this is a structural check; the
+      // runtime verifies the named function is actually registered.)
+      if (node.type === 'script') {
+        const fn = typeof cfg.function === 'string' ? cfg.function.trim() : '';
+        const action = typeof cfg.actionType === 'string' ? cfg.actionType.trim() : '';
+        // Inline `config.script` (a JS body) is also a declared form — the
+        // built-in runtime doesn't execute it (warned at run time), but the node
+        // is not the empty no-op this check targets, so don't flag it.
+        const inline = typeof cfg.script === 'string' ? cfg.script.trim() : '';
+        if (!fn && !action && !inline) {
+          issues.push({
+            where: `flow '${flowName}' · node '${node.id}' (script) callable`,
+            message:
+              `script node declares neither \`actionType\` nor \`function\` — it would do nothing at runtime. ` +
+              `Name a built-in action (e.g. \`actionType: 'email'\`) or a registered function ` +
+              `(\`function: 'my_fn'\`, registered via \`defineStack({ functions })\`).`,
+            source: JSON.stringify({ id: node.id, type: node.type, config: cfg }),
+          });
+        }
+      }
     }
     for (const edge of edges) {
       check(`flow '${flowName}' · edge '${edge.id}' (${edge.source}→${edge.target}) condition`, edge.condition, objectName);
diff --git a/packages/services/service-automation/src/builtin/screen-nodes.test.ts b/packages/services/service-automation/src/builtin/screen-nodes.test.ts
@@ -0,0 +1,110 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+
+import { describe, it, expect, beforeEach } from 'vitest';
+import { AutomationEngine, type FlowFunctionHandler } from '../engine.js';
+import { registerScreenNodes } from './screen-nodes.js';
+
+function createTestLogger() {
+    return {
+        info: () => {},
+        warn: () => {},
+        error: () => {},
+        debug: () => {},
+        child: () => createTestLogger(),
+    } as any;
+}
+
+function createCtx() {
+    return { logger: createTestLogger(), getService: () => undefined } as any;
+}
+
+/** A one-`script`-node flow whose script node carries `config`. */
+function scriptFlow(config: Record<string, unknown>) {
+    return {
+        name: 'script_flow',
+        label: 'Script Flow',
+        type: 'autolaunched' as const,
+        nodes: [
+            { id: 'start', type: 'start' as const, label: 'Start' },
+            { id: 'run', type: 'script' as const, label: 'Run', config },
+            { id: 'end', type: 'end' as const, label: 'End' },
+        ],
+        edges: [
+            { id: 'e1', source: 'start', target: 'run' },
+            { id: 'e2', source: 'run', target: 'end' },
+        ],
+    };
+}
+
+describe('script node (#1870 — callable resolution)', () => {
+    let engine: AutomationEngine;
+
+    beforeEach(() => {
+        engine = new AutomationEngine(createTestLogger());
+        registerScreenNodes(engine, createCtx());
+    });
+
+    it('runs the built-in email side-effect', async () => {
+        engine.registerFlow('script_flow', scriptFlow({ actionType: 'email', template: 't', recipients: ['a'] }));
+        const result = await engine.execute('script_flow', {} as any);
+        expect(result.success).toBe(true);
+    });
+
+    it('invokes a registered function and captures its return value as output', async () => {
+        const calls: Array<Record<string, unknown>> = [];
+        const fn: FlowFunctionHandler = (c) => {
+            calls.push(c.input);
+            return { triaged: true, priority: 'high' };
+        };
+        engine.setFunctionResolver((name) => (name === 'helpdesk.aiTriageStub' ? fn : undefined));
+
+        engine.registerFlow('script_flow', scriptFlow({
+            function: 'helpdesk.aiTriageStub',
+            inputs: { ticket: 't_1' },
+        }));
+        const result = await engine.execute('script_flow', {} as any);
+
+        expect(result.success).toBe(true);
+        expect(calls).toEqual([{ ticket: 't_1' }]);
+    });
+
+    it('resolves a bare actionType that matches no built-in as a function name', async () => {
+        let called = false;
+        engine.setFunctionResolver((name) => (name === 'pm.aiRiskAssessmentStub' ? (() => { called = true; return 1; }) : undefined));
+        engine.registerFlow('script_flow', scriptFlow({ actionType: 'pm.aiRiskAssessmentStub' }));
+        const result = await engine.execute('script_flow', {} as any);
+        expect(result.success).toBe(true);
+        expect(called).toBe(true);
+    });
+
+    it('FAILS LOUDLY for an unregistered function instead of silently no-op (#1870)', async () => {
+        // No resolver wired → nothing resolves.
+        engine.registerFlow('script_flow', scriptFlow({ function: 'helpdesk.aiTriageStub' }));
+        const result = await engine.execute('script_flow', {} as any);
+        expect(result.success).toBe(false);
+        expect(result.error).toMatch(/aiTriageStub/);
+        expect(result.error).toMatch(/no function named|not a built-in/i);
+    });
+
+    it('recognizes inline config.script as a no-op (not a loud failure) — built-in runtime has no JS sandbox', async () => {
+        engine.registerFlow('script_flow', scriptFlow({ script: 'variables.x = 1;', outputVariables: ['x'] }));
+        const result = await engine.execute('script_flow', {} as any);
+        // Recognized form: succeeds (doesn't fail loud), but is documented as not executed.
+        expect(result.success).toBe(true);
+    });
+
+    it('FAILS LOUDLY when the script node declares no target at all (actionType: undefined repro)', async () => {
+        engine.registerFlow('script_flow', scriptFlow({ actionType: undefined }));
+        const result = await engine.execute('script_flow', {} as any);
+        expect(result.success).toBe(false);
+        expect(result.error).toMatch(/neither .*actionType.* nor .*function|nothing to run/i);
+    });
+
+    it('surfaces a thrown function as a loud step failure', async () => {
+        engine.setFunctionResolver(() => () => { throw new Error('boom'); });
+        engine.registerFlow('script_flow', scriptFlow({ function: 'explode' }));
+        const result = await engine.execute('script_flow', {} as any);
+        expect(result.success).toBe(false);
+        expect(result.error).toMatch(/explode.*failed|failed.*boom|boom/i);
+    });
+});
diff --git a/packages/services/service-automation/src/builtin/screen-nodes.ts b/packages/services/service-automation/src/builtin/screen-nodes.ts
@@ -16,10 +16,22 @@ import type { AutomationEngine } from '../engine.js';
  *   as bare flow variables). A field-less screen — or one with
  *   `waitForInput === false` — stays a server pass-through (input vars, if any,
  *   are already injected from `context.params`).
- * - 'script' nodes dispatch by `config.actionType`. Currently only 'email'
- *   has a (logger-backed) implementation; unknown action types still succeed
- *   so flows can continue and downstream nodes can react.
+ * - 'script' nodes name a callable to run (#1870):
+ *     - `config.actionType` selecting a built-in side-effect ('email', 'slack',
+ *       logger-backed), or
+ *     - `config.function` (or a bare `actionType` that matches no built-in)
+ *       naming a registered function — resolved via `engine.resolveFunction()`,
+ *       which the host bridges to `bundle.functions` / `defineStack({ functions })`.
+ *   A target that resolves to neither fails the step LOUDLY rather than the old
+ *   silent "no-op handler" success, so an unwired callable can't quietly skip.
  */
+
+/**
+ * Built-in `script` side-effect action types with a (logger-backed) handler.
+ * Anything else is treated as a registered-function name (#1870).
+ */
+const SCRIPT_BUILTIN_ACTION_TYPES = new Set(['email', 'slack']);
+
 export function registerScreenNodes(engine: AutomationEngine, ctx: PluginContext): void {
     // screen — server-side pass-through (input vars already injected by engine).
     engine.registerNodeExecutor({
@@ -71,26 +83,78 @@ export function registerScreenNodes(engine: AutomationEngine, ctx: PluginContext
         description: 'Run a custom script action.',
         icon: 'code', category: 'logic', source: 'builtin',
       }),
-      async execute(node, _variables, _context) {
+      async execute(node, variables, context) {
         const cfg = (node.config ?? {}) as Record<string, unknown>;
-        const actionType = (cfg.actionType as string | undefined) ?? 'noop';
-        if (actionType === 'email') {
+        const fnName = typeof cfg.function === 'string' && cfg.function.trim() ? cfg.function.trim() : undefined;
+        const actionType = typeof cfg.actionType === 'string' && cfg.actionType.trim() ? cfg.actionType.trim() : undefined;
+
+        // Built-in side-effect actions keep their logger-backed behavior — but
+        // only when an explicit `function` isn't set (that always wins).
+        if (!fnName && actionType && SCRIPT_BUILTIN_ACTION_TYPES.has(actionType)) {
           ctx.logger.info(
-            `[Script:email] template=${String(cfg.template)} ` +
+            `[Script:${actionType}] template=${String(cfg.template)} ` +
               `recipients=${JSON.stringify(cfg.recipients)} ` +
               `vars=${JSON.stringify(cfg.variables)}`,
           );
           return {
             success: true,
-            output: {
-              actionType,
-              template: cfg.template,
-              recipients: cfg.recipients,
-            },
+            output: { actionType, template: cfg.template, recipients: cfg.recipients },
+          };
+        }
+
+        // Inline `config.script` (a JS source body) is a distinct, recognized
+        // form — but the built-in runtime has no server-side JS sandbox, so it
+        // does not execute it. Warn loudly (not a silent success) and steer the
+        // author to the supported path — a registered function — rather than
+        // failing the flow. Executing inline scripts is a separate capability,
+        // out of #1870's callable-resolution scope.
+        const inlineScript = typeof cfg.script === 'string' && cfg.script.trim() ? cfg.script : undefined;
+        if (!fnName && inlineScript) {
+          ctx.logger.warn(
+            `[Script] node '${node.id}': inline \`config.script\` is not executed by the built-in runtime ` +
+              `(no server-side JS sandbox) — this node is a no-op. To run server logic, move it into a ` +
+              `registered function and call it via \`config.function\` + \`defineStack({ functions })\`.`,
+          );
+          return { success: true, output: { script: 'not-executed' } };
+        }
+
+        // Otherwise the node names a function to invoke. `function` is canonical;
+        // a bare `actionType` that matched no built-in is accepted as a shorthand
+        // function name (so templates that point a node straight at e.g.
+        // `helpdesk.aiTriageStub` resolve).
+        const target = fnName ?? actionType;
+        if (!target) {
+          // Defense in depth: registerFlow already rejects this structurally
+          // (#1870), so reaching here means a node bypassed registration.
+          return {
+            success: false,
+            error:
+              `script node '${node.id}': declares neither \`actionType\` nor \`function\` — nothing to run.`,
+          };
+        }
+
+        const handler = engine.resolveFunction(target);
+        if (!handler) {
+          return {
+            success: false,
+            error:
+              `script node '${node.id}': '${target}' is not a built-in action ` +
+              `(${[...SCRIPT_BUILTIN_ACTION_TYPES].join(', ')}) and no function named '${target}' is registered. ` +
+              `Register it via \`defineStack({ functions: { '${target}': fn } })\`, or fix the name (#1870).`,
+          };
+        }
+
+        // Map declared inputs (`config.inputs` | `config.input`) to the function.
+        const input = (cfg.inputs ?? cfg.input ?? {}) as Record<string, unknown>;
+        try {
+          const result = await handler({ input, variables, automation: context, logger: ctx.logger });
+          return { success: true, output: { function: target, result } };
+        } catch (err) {
+          return {
+            success: false,
+            error: `script function '${target}' (node '${node.id}') failed: ${(err as Error).message}`,
           };
         }
-        ctx.logger.info(`[Script:${actionType}] node=${node.id} executed (no-op handler)`);
-        return { success: true, output: { actionType } };
       },
     });
 
diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts
@@ -167,6 +167,39 @@ export interface RegisteredConnector {
     readonly handlers: Record<string, ConnectorActionHandler>;
 }
 
+/**
+ * Context handed to a named handler function invoked from a `script` node
+ * (#1870). Mirrors {@link ConnectorActionContext} but carries the node's mapped
+ * `input` so the function reads its arguments without reaching into the raw
+ * variable map. The function's return value becomes the node output.
+ */
+export interface FlowFunctionContext {
+    /** Inputs mapped from the node's `config.inputs` (already in scope). */
+    readonly input: Record<string, unknown>;
+    /** Live flow variable map — read prior-node output / write results. */
+    readonly variables: Map<string, unknown>;
+    /** The flow execution / trigger context. */
+    readonly automation: AutomationContext;
+    readonly logger: Logger;
+}
+
+/**
+ * A named handler function callable from a `script` node. Returns the node's
+ * output (any JSON-serializable value); returning `undefined` yields an empty
+ * output. Authored packages contribute these via `defineStack({ functions })`,
+ * which the host bridges in through {@link AutomationEngine.setFunctionResolver}.
+ */
+export type FlowFunctionHandler = (ctx: FlowFunctionContext) => unknown | Promise<unknown>;
+
+/**
+ * Resolves a function name to its handler. Injected by the host (the automation
+ * plugin bridges it to ObjectQL's `resolveFunction`, fed by `bundle.functions`),
+ * so the engine stays decoupled from any specific function registry. Returns
+ * `undefined` for an unknown name, letting the `script` node fail the step
+ * loudly instead of silently no-op'ing (#1870).
+ */
+export type FlowFunctionResolver = (name: string) => FlowFunctionHandler | undefined;
+
 /**
  * A designer-facing view of one connector action — identity + its JSON-Schema
  * input/output. The runtime handler is intentionally omitted; this is metadata.
@@ -353,6 +386,8 @@ export class AutomationEngine implements IAutomationService {
     private boundFlowTriggers = new Map<string, string>();
     /** Connectors registered by integration plugins, keyed by connector name (ADR-0018 §Addendum). */
     private connectors = new Map<string, RegisteredConnector>();
+    /** Bridge to the host function registry for `script`-node calls (#1870), if wired. */
+    private functionResolver: FlowFunctionResolver | null = null;
     private executionLogs: ExecutionLogEntry[] = [];
     private readonly maxLogSize: number;
     private logger: Logger;
@@ -697,6 +732,26 @@ export class AutomationEngine implements IAutomationService {
         return this.connectors.get(connectorId)?.handlers[actionId];
     }
 
+    /**
+     * Wire the engine to the host's named-function registry (#1870). The
+     * automation plugin calls this in `start()` with a resolver backed by
+     * ObjectQL's `resolveFunction` (populated from `bundle.functions` /
+     * `defineStack({ functions })`), so a `script` node can invoke an
+     * authored function by name. Passing `null` detaches the bridge.
+     */
+    setFunctionResolver(resolver: FlowFunctionResolver | null): void {
+        this.functionResolver = resolver;
+    }
+
+    /**
+     * Resolve a named function for a `script` node. Returns `undefined` when no
+     * resolver is wired or the name is unregistered — the node then fails the
+     * step with a clear error rather than silently no-op'ing.
+     */
+    resolveFunction(name: string): FlowFunctionHandler | undefined {
+        return this.functionResolver?.(name) ?? undefined;
+    }
+
     /** Get all registered connector names. */
     getRegisteredConnectors(): string[] {
         return [...this.connectors.keys()];
diff --git a/packages/services/service-automation/src/plugin.ts b/packages/services/service-automation/src/plugin.ts
diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts
