feat(flow-designer)!: the script node authors a function call, and nothing else (framework#4343) (#3170)

framework#4343 retired the `script` node's other dispatch branches because none
of them ran: `actionType: 'email' | 'slack'` were logger-backed stubs that wrote
a log line, reported success and delivered nothing under any configuration —
with `template` / `recipients` / `variables` addressing a message no channel
sent — inline `config.script` was recognized and never executed (no server-side
JS sandbox), and any other `actionType` was a second spelling of `function`.

The panel follows: `Function` (required), `Inputs` and `Output variable` are
offered unconditionally, since there is no action type left to gate them behind.
All five retired keys keep a legacy render-only field labelled `(retired)` with
its replacement in the help text, so a stored node still shows everything it
carries — the rule this group already followed for the `code` / `sms` /
`notification` action types #3099 dropped.

The canvas subtitle leads with the function name (falling back to the retired
keys so an unmigrated node is never blank), and the simulator says what a retired
branch actually did instead of pretending it mocked a notification.

The reconciliation ledger spans the spec bump: on a spec that still publishes the
branches it asserts only that the form offers nothing the executor ignores; on
the spec that retires them — `SCRIPT_BUILTIN_ACTION_TYPES` disappearing is the
discriminator — the full bidirectional comparison arms itself. Verified against a
locally built framework spec: the converged panel reconciles clean both ways.


Claude-Session: https://claude.ai/code/session_01Ct9NXp2JumjKuARtQnrbPf

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-zhuang

.changeset/script-node-converged-to-function.md

@@ -0,0 +1,39 @@
+---
+"@object-ui/app-shell": minor
+---
+
+Flow designer: the `script` node authors a function call, and nothing else (framework#4343).
+
+**Breaking for authoring**, not for stored metadata: the `script` panel no longer
+offers `Action type`, `Template`, `Recipients`, `Template variables` or the inline
+`Code` body. What it offers is the function path — `Function` (required),
+`Inputs`, `Output variable` — shown unconditionally, since there is no action type
+left to gate them behind.
+
+framework#4343 retired those five keys because none of them ran. `actionType:
+'email' | 'slack'` were logger-backed stubs: they wrote a log line, reported
+success, and delivered nothing under any configuration, with `template` /
+`recipients` / `variables` addressing a message no channel sent. Inline
+`config.script` was recognized and never executed — the built-in runtime has no
+server-side JS sandbox. Any other `actionType` value was a second spelling of
+`function`. Real delivery is a **`notify`** node (the messaging service: in-app
+inbox by default, email once `@objectstack/plugin-email` is installed); Slack is a
+**`connector_action`** with the Slack connector, or an `http` node posting to a
+webhook.
+
+**Stored nodes are never hidden.** All five keys keep a legacy render-only field
+(`__legacy__` gating — the rule this group already followed for the `code` / `sms`
+/ `notification` action types objectui#3099 dropped), each labelled `(retired)`
+with its replacement in the help text. `os migrate meta --from 16` rewrites the
+metadata; a shorthand `actionType` moves into `function`, which is what it named.
+
+The flow canvas subtitle now leads with the function name (falling back to the
+retired keys so an unmigrated node is never blank), and the simulator says what a
+retired branch actually did rather than pretending it mocked a notification.
+
+The cross-repo reconciliation ledger spans the spec bump: on a spec that still
+publishes the retired branches it asserts only that the form offers nothing the
+executor ignores; on the spec that retires them (`SCRIPT_BUILTIN_ACTION_TYPES`
+disappearing is the discriminator) the full bidirectional comparison arms itself.
+Verified against a locally built framework spec: the converged panel reconciles
+clean in both directions.

packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.spec-reconciliation.test.ts

@@ -41,8 +41,9 @@ const ScriptConfigSchema = spec.ScriptConfigSchema;
 const SubflowConfigSchema = spec.SubflowConfigSchema;
 const DecisionConfigSchema = spec.DecisionConfigSchema;
 const DecisionConditionSchema = spec.DecisionConditionSchema;
+// Also the #4343 discriminator: the spec that converges `script` removes this
+// constant along with the dispatch branches it described.
 const SCRIPT_BUILTIN_ACTION_TYPES = spec.SCRIPT_BUILTIN_ACTION_TYPES as readonly string[] | undefined;
-const SCRIPT_INVOKE_FUNCTION_ACTION_TYPE = spec.SCRIPT_INVOKE_FUNCTION_ACTION_TYPE as string | undefined;
 
 /**
  * Keys a Zod object schema accepts, read straight off `.shape`.
@@ -132,20 +133,55 @@ function reconcile(type: string, zod: unknown, renderOnly: Record<string, string
   }
 }
 
-describe.skipIf(!ScriptConfigSchema)('script form ↔ ScriptConfigSchema (framework#4278)', () => {
-  it('offers exactly the executor-read keys; inline `script` stays render-only (a recognized no-op)', () => {
-    reconcile('script', ScriptConfigSchema, {
-      script: 'recognized but NOT executed by the built-in runtime (no server-side JS sandbox) — renders for stored nodes, steers authors to `function`',
-    });
+/**
+ * The `script` panel spans a spec bump, so it asserts what is true on EITHER
+ * side of it (framework#4343).
+ *
+ * The form has converged to the one thing the node does — call a registered
+ * function — and the five dispatch keys it used to offer are legacy render-only
+ * here. On the spec that retires them those keys leave the contract too
+ * (`zodKeys` drops `[REMOVED]` tombstones), so the full bidirectional ledger
+ * applies. On the spec still installed today they are live contract keys the
+ * form no longer offers, and only the "offers nothing the executor ignores"
+ * direction is meaningful — asserting the other one would demand the form keep
+ * authoring branches that never delivered anything.
+ *
+ * `SCRIPT_BUILTIN_ACTION_TYPES` is the discriminator: framework#4343 removes it
+ * along with the branches it described, so this arms itself on the bump.
+ */
+const SPEC_PREDATES_SCRIPT_CONVERGENCE = SCRIPT_BUILTIN_ACTION_TYPES !== undefined;
+
+describe.skipIf(!ScriptConfigSchema)('script form ↔ ScriptConfigSchema (framework#4278, #4343)', () => {
+  it.skipIf(SPEC_PREDATES_SCRIPT_CONVERGENCE)('offers exactly the executor-read keys', () => {
+    reconcile('script', ScriptConfigSchema);
+  });
+
+  it.skipIf(!SPEC_PREDATES_SCRIPT_CONVERGENCE)(
+    'offers nothing the executor ignores (pre-#4343 spec: the retired branches are still contract keys)',
+    () => {
+      const contract = zodKeys(ScriptConfigSchema);
+      expect(
+        offeredConfigKeys('script').filter((k) => !contract.includes(k)),
+        'script: offered by the designer form but never read by the executor',
+      ).toEqual([]);
+    },
+  );
+
+  it('offers the function path and nothing else', () => {
+    // The whole authorable surface, on either spec. `timeoutMs` is node-level,
+    // so `offeredConfigKeys` (config-rooted only) does not carry it.
+    expect(offeredConfigKeys('script')).toEqual(['function', 'inputs', 'outputVariable']);
   });
 
-  it('offers exactly the published action types: invoke_function + the built-in set', () => {
-    const actionType = fieldsForNodeType('script').find((f) => f.id === 'actionType')!;
-    expect(actionType.options!.map((o) => o.value).sort()).toEqual(
-      [SCRIPT_INVOKE_FUNCTION_ACTION_TYPE!, ...SCRIPT_BUILTIN_ACTION_TYPES!].sort(),
-    );
-    // The default is the path that runs real logic, not the no-op.
-    expect(actionType.defaultValue).toBe(SCRIPT_INVOKE_FUNCTION_ACTION_TYPE);
+  it('keeps every retired key rendering for stored nodes, without offering it', () => {
+    // Stored metadata is never hidden — the same rule that kept the inline
+    // `script` body visible after #3099 dropped it from new authoring.
+    for (const key of ['actionType', 'template', 'recipients', 'variables', 'script']) {
+      const field = fieldsForNodeType('script').find((f) => f.path[0] === 'config' && f.path[1] === key);
+      expect(field, `script: retired key '${key}' must keep a field so stored values render`).toBeDefined();
+      expect(isLegacyGated(field!), `script: '${key}' must be legacy-gated, not offered`).toBe(true);
+      expect(field!.help, `script: '${key}' must name its replacement`).toMatch(/[Rr]etired in spec 17/);
+    }
   });
 });
 

packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.test.ts

@@ -211,54 +211,55 @@ describe('loop / map collection is a template, not a CEL predicate', () => {
   });
 });
 
-describe('script node — the form authors what the executor runs (framework#4278)', () => {
+describe('script node — the form authors what the executor runs (framework#4278, #4343)', () => {
   const fields = fieldsForNodeType('script');
-  const actionType = fields.find((f) => f.id === 'actionType')!;
 
-  it('offers Call function / Email / Slack — not the broken sms / notification / no-op code', () => {
-    // `sms` / `notification` were in no dispatch set: the executor resolved
-    // them as function names and failed every run. `code` was a recognized
-    // no-op (no server-side JS sandbox). The offered options now mirror the
-    // executor's SCRIPT_BUILTIN_ACTION_TYPES + the invoke_function marker.
-    expect(actionType.options!.map((o) => o.value)).toEqual(['invoke_function', 'email', 'slack']);
-    expect(actionType.defaultValue).toBe('invoke_function');
-  });
-
-  it('authors the function path — the one that runs real logic', () => {
+  it('authors the function path — the only thing the node does', () => {
     for (const id of ['function', 'inputs', 'outputVariable']) {
       const f = fields.find((x) => x.id === id);
       expect(f, `script.${id} must be authorable`).toBeDefined();
       expect(f!.path).toEqual(['config', id]);
-      // Shown by default (invoke_function is the default action type).
+      // Unconditional now: there is no action type left to gate them behind.
+      expect(f!.showWhen).toBeUndefined();
       expect(isFieldVisible(f!, { id: 's', type: 'script' }, fields)).toBe(true);
     }
     expect(fields.find((f) => f.id === 'inputs')!.kind).toBe('keyValue');
   });
 
-  it('gates the builtin side-effect fields to email / slack', () => {
-    const template = fields.find((f) => f.id === 'template')!;
-    expect(template.showWhen).toEqual({ field: 'actionType', equals: ['email', 'slack'] });
-    expect(isFieldVisible(template, { id: 's', type: 'script', config: { actionType: 'slack' } }, fields)).toBe(true);
-    expect(isFieldVisible(template, { id: 's', type: 'script' }, fields)).toBe(false);
+  it('offers nothing else under config — the retired branches are not authorable', () => {
+    // framework#4343: `actionType`'s built-in side effects were logger-backed
+    // stubs that delivered nothing, inline `script` was never executed, and any
+    // other action type was a second spelling of `function`.
+    const offered = fields
+      .filter((f) => f.path[0] === 'config' && f.showWhen?.field !== '__legacy__')
+      .map((f) => f.id);
+    expect(offered).toEqual(['function', 'inputs', 'outputVariable']);
   });
 
   it('drops the dead plural outputVariables field (declared-but-unread — nothing ever bound it)', () => {
     expect(fields.find((f) => f.id === 'outputVariables')).toBeUndefined();
   });
 
-  it('keeps the inline script body render-only: hidden for new nodes, visible when stored', () => {
-    const script = fields.find((f) => f.id === 'script')!;
-    expect(script.showWhen).toEqual({ field: '__legacy__', equals: [] });
-    expect(isFieldVisible(script, { id: 's', type: 'script' }, fields)).toBe(false);
-    expect(
-      isFieldVisible(script, { id: 's', type: 'script', config: { script: 'return 1;' } }, fields),
-    ).toBe(true);
-  });
-
-  it('a stored legacy sms node still renders its builtin fields (stored values are never hidden)', () => {
-    const template = fields.find((f) => f.id === 'template')!;
-    const node = { id: 's', type: 'script', config: { actionType: 'sms', template: 'notify_owner' } };
-    expect(isFieldVisible(template, node, fields)).toBe(true);
+  it.each(['actionType', 'template', 'recipients', 'variables', 'script'])(
+    'keeps retired `%s` render-only: hidden for new nodes, visible when stored',
+    (id) => {
+      const field = fields.find((f) => f.id === id)!;
+      expect(field, `script.${id} must still render stored values`).toBeDefined();
+      expect(field.showWhen).toEqual({ field: '__legacy__', equals: [] });
+      expect(isFieldVisible(field, { id: 's', type: 'script' }, fields)).toBe(false);
+      expect(
+        isFieldVisible(field, { id: 's', type: 'script', config: { [id]: 'stored' } }, fields),
+      ).toBe(true);
+    },
+  );
+
+  it('a stored legacy email node still renders everything it carries', () => {
+    // Stored values are never hidden — the rule that already covered the
+    // `code` / `sms` / `notification` action types #3099 dropped.
+    const node = { id: 's', type: 'script', config: { actionType: 'email', template: 'notify_owner' } };
+    for (const id of ['actionType', 'template']) {
+      expect(isFieldVisible(fields.find((f) => f.id === id)!, node, fields), id).toBe(true);
+    }
   });
 });
 

packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts

@@ -480,70 +480,55 @@ const FLOW_NODE_CONFIG: Record<string, FlowConfigField[]> = {
     cfg('outputVariable', 'Output variable', 'text', { placeholder: 'response' }),
     { id: 'timeoutMs', path: ['timeoutMs'], label: 'Timeout (ms)', kind: 'number', placeholder: '30000' },
   ],
-  // Script — a callable step (framework#1870): call a registered function
-  // (`function` + `inputs` + `outputVariable` — the only path that runs real
-  // logic), or one of the executor's built-in side effects (email / slack —
-  // `template` / `recipients` / `variables`). The offered options mirror the
-  // spec's `SCRIPT_BUILTIN_ACTION_TYPES` + the `invoke_function` marker, and
-  // the whole group is reconciled against the spec-published
-  // `ScriptConfigSchema` (framework#4278) — before that reconciliation this
-  // group offered `sms` / `notification` (fail every run: neither is built in,
-  // so they resolve as function names), defaulted to `code` (a recognized
-  // no-op: the built-in runtime has no server-side JS sandbox), declared an
-  // `outputVariables` list nothing reads (the executor binds the singular
-  // `outputVariable`), and could not author the function path at all.
+  // Script — one thing: call a registered function (framework#1870).
+  // `function` + `inputs` + `outputVariable` is the whole authorable surface,
+  // reconciled against the spec-published `ScriptConfigSchema` (framework#4278).
   //
-  // A stored legacy node still renders completely: unknown `actionType` values
-  // (`code` / `sms` / `notification`) show as "(deprecated)" select options,
-  // and the `script` body / builtin fields surface whenever they hold a value
-  // (stored values are never hidden).
+  // framework#4343 retired the other dispatch branches, and the form follows.
+  // None of them ran: `actionType: 'email' | 'slack'` were logger-backed stubs
+  // that reported success and delivered nothing (with `template` / `recipients`
+  // / `variables` addressing a message no channel sent), inline `script` was
+  // never executed (the built-in runtime has no server-side JS sandbox), and
+  // any other `actionType` was a second spelling of `function`. Real delivery
+  // is a `notify` node; Slack is a connector (or an `http` webhook).
+  //
+  // The five keys stay as legacy render-only fields (`__legacy__` never
+  // matches, so they are never OFFERED) because a stored node must still show
+  // everything it carries — the rule this group already followed for the
+  // `code` / `sms` / `notification` action types #3099 dropped. Each carries
+  // the replacement in its help text; `os migrate meta --from 16` rewrites the
+  // stored metadata.
   script: [
-    cfg('actionType', 'Action type', 'select', {
-      options: [
-        { value: 'invoke_function', label: 'Call function' },
-        { value: 'email', label: 'Email' },
-        { value: 'slack', label: 'Slack' },
-      ],
-      defaultValue: 'invoke_function',
-      help: 'How this step runs. "Call function" invokes a registered function — the path that runs real logic.',
-    }),
     cfg('function', 'Function', 'text', {
       placeholder: 'score_lead',
-      help: 'Registered function to call — declared via defineStack({ functions }). Always wins over Action type.',
-      showWhen: { field: 'actionType', equals: ['invoke_function'] },
+      help: 'Registered function to call — declared via defineStack({ functions }). Required: it is what this step runs.',
     }),
     cfg('inputs', 'Inputs', 'keyValue', {
       help: 'Values passed to the function; {var} references resolve against the live flow variables.',
-      showWhen: { field: 'actionType', equals: ['invoke_function'] },
     }),
     cfg('outputVariable', 'Output variable', 'text', {
       placeholder: 'aiResult',
       help: "Flow variable the function's return value is bound to, for later steps.",
-      showWhen: { field: 'actionType', equals: ['invoke_function'] },
     }),
-    cfg('template', 'Template', 'reference', {
-      // Polymorphic: an email step picks from the email-template catalog; slack
-      // has no flat catalog yet, so it degrades to free text.
-      ref: { kindFrom: 'actionType', map: { email: 'email-template' } },
-      placeholder: 'case_escalated',
-      help: 'Message template id.',
-      showWhen: { field: 'actionType', equals: ['email', 'slack'] },
+    cfg('actionType', 'Action type (retired)', 'text', {
+      help: 'Retired in spec 17 — "email"/"slack" never delivered anything, and any other value was just the function name. Use a notify node for messages, a Slack connector for Slack, or move the name into Function.',
+      showWhen: { field: '__legacy__', equals: [] },
     }),
-    cfg('recipients', 'Recipients', 'stringList', {
-      help: 'One recipient per row (user id, field ref, or address).',
-      showWhen: { field: 'actionType', equals: ['email', 'slack'] },
+    cfg('template', 'Template (retired)', 'text', {
+      help: 'Retired in spec 17 — it fed a side effect that never rendered or sent a message. A notify node carries its own title/message.',
+      showWhen: { field: '__legacy__', equals: [] },
+    }),
+    cfg('recipients', 'Recipients (retired)', 'stringList', {
+      help: 'Retired in spec 17 — these addresses were logged, never messaged. Use a notify node, whose recipients reach the messaging service.',
+      showWhen: { field: '__legacy__', equals: [] },
     }),
-    cfg('variables', 'Template variables', 'keyValue', {
-      help: 'Values injected into the template.',
-      showWhen: { field: 'actionType', equals: ['email', 'slack'] },
+    cfg('variables', 'Template variables (retired)', 'keyValue', {
+      help: 'Retired in spec 17 — injected into a template nothing rendered. A notify node carries structured data in payload.',
+      showWhen: { field: '__legacy__', equals: [] },
     }),
-    // Legacy render-only (`__legacy__` never matches): the built-in runtime
-    // does NOT execute inline script bodies (no server-side JS sandbox — the
-    // executor warns and completes as a no-op), so the field is not offered
-    // for new authoring; a stored body still renders so nothing is hidden.
-    cfg('script', 'Code (not executed)', 'textarea', {
+    cfg('script', 'Code (not executed, retired)', 'textarea', {
       placeholder: 'return { ok: true };',
-      help: 'Inline scripts are NOT executed by the built-in runtime — this node is a no-op. Move the logic into a registered function and use "Call function".',
+      help: 'Retired in spec 17 — inline scripts were NEVER executed by the built-in runtime (no server-side sandbox). Move the logic into a registered function and name it in Function.',
       refMode: 'expression',
       showWhen: { field: '__legacy__', equals: [] },
     }),

packages/app-shell/src/views/metadata-admin/previews/FlowCanvas.tsx

@@ -950,7 +950,10 @@ function nodeSummary(node: FlowDesignerNode): string | undefined {
     return pick('condition');
   }
   if (node.type === 'script') {
-    return pick('actionType') || pick('template') || (c && c.script ? 'code' : undefined);
+    // The function IS the step (framework#4343). The rest are retired keys a
+    // stored node may still carry — kept as fallbacks so its subtitle is never
+    // blank before someone migrates it.
+    return pick('function') || pick('actionType') || pick('template') || (c && c.script ? 'code' : undefined);
   }
   if (node.type === 'approval') {
     const approvers = c?.approvers;

packages/app-shell/src/views/metadata-admin/previews/simulator/flow-simulator.ts

@@ -407,12 +407,16 @@ export class FlowSimulator {
 
   private mockNote(node: SimNode): string {
     if (node.type === 'script') {
+      // A script node calls a registered function and nothing else
+      // (framework#4343) — the branches below describe a stored node that has
+      // not been migrated yet, and say plainly that they never ran.
+      const fn = str(node.config?.function);
+      if (fn) return `Mocked call to '${fn}' (no function executed).`;
       const action = str(node.config?.actionType);
       if (action && action !== 'code') {
-        const recips = Array.isArray(node.config?.recipients) ? (node.config!.recipients as unknown[]).length : 0;
-        return `Mocked ${action} notification${recips ? ` to ${recips} recipient(s)` : ''}.`;
+        return `Retired '${action}' action — it never delivered anything; use a notify node (or a connector for Slack).`;
       }
-      return 'Mocked code script (no real code executed).';
+      return 'Retired inline script — the runtime never executed it; move the logic into a registered function.';
     }
     return `Mocked ${node.type.replace(/_/g, ' ')} (no backend call).`;
   }