From 308eef0463de00a4306d039677b7ab48cdd8d4db Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 31 Jul 2026 05:54:20 +0000 Subject: [PATCH] feat(flow-designer): the script node's form authors what the executor runs (framework#4278) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The script flow node's designer form is hand-written here (the engine publishes no configSchema for it, deliberately) and had drifted from the executor: of four offered actionType options, `code` was a recognized no-op, `sms` / `notification` failed every run, the plural "Output variables" field was read by nothing — and the one path that runs real logic (function + inputs + outputVariable) could not be authored. - actionType now offers Call function (default) / Email / Slack — the executor's dispatch set (SCRIPT_BUILTIN_ACTION_TYPES + the invoke_function marker). The function-path fields are first-class. - The inline `script` body becomes render-only: hidden for new nodes, shown (with a not-executed warning) whenever a stored node carries one. The dead plural outputVariables field is removed; stored values surface in the Advanced (JSON) block. - A scalar select whose stored value was dropped from the options renders it as " (deprecated)" instead of blanking it — the rule FlowObjectListField already applied to select cells. - flow-scope (data picker) and the flow simulator stop pretending the legacy outputVariables[] list binds variables — the engine never binds those names; only the singular outputVariable does. - New reconciliation test compares the hand-written script / subflow / decision groups bidirectionally against the executor-derived contracts @objectstack/spec/automation publishes for exactly this purpose, and the wait / connector_action / boundary_event groups against the FlowNodeSchema sibling blocks. `retiredKey()` tombstones are excluded from the contract key set, so a form field for a retired key fails loudly instead of false-passing — verified against the framework#4278 spec build: script/subflow/decision reconcile green, and the wait panel correctly fires on `waitEventConfig.timeoutMs` / `.onTimeout` (retired at spec 18, framework#4198), which the 18 bump must drop from the wait form. The spec-export panels feature-detect and skip on the installed rc.0 spec, arming themselves on the next bump. Verified in the preview gallery (?only=flow): the legacy code sample renders the stored body under "Code (not executed)", the function-path fields show by default, and the select offers exactly the three options. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Ki9WjmmSY19koz9hNkeL1P --- .../script-node-form-authors-what-runs.md | 36 ++++ packages/app-shell/README.md | 8 +- .../src/views/metadata-admin/i18n.ts | 13 +- .../inspectors/FlowNodeConfigField.test.tsx | 27 +++ .../inspectors/FlowNodeConfigField.tsx | 31 ++- .../inspectors/FlowStringListField.tsx | 2 +- ...ow-node-config.spec-reconciliation.test.ts | 201 ++++++++++++++++++ .../inspectors/flow-node-config.test.ts | 51 +++++ .../inspectors/flow-node-config.ts | 68 ++++-- .../inspectors/flow-scope.test.ts | 8 +- .../metadata-admin/inspectors/flow-scope.ts | 18 +- .../previews/FlowSimulatorPanel.tsx | 6 +- .../__tests__/flow-simulator.test.ts | 24 ++- .../previews/simulator/flow-sim-types.ts | 7 +- .../previews/simulator/flow-simulator.ts | 18 +- 15 files changed, 452 insertions(+), 66 deletions(-) create mode 100644 .changeset/script-node-form-authors-what-runs.md create mode 100644 packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.spec-reconciliation.test.ts diff --git a/.changeset/script-node-form-authors-what-runs.md b/.changeset/script-node-form-authors-what-runs.md new file mode 100644 index 0000000000..610094e7bc --- /dev/null +++ b/.changeset/script-node-form-authors-what-runs.md @@ -0,0 +1,36 @@ +--- +"@object-ui/app-shell": minor +--- + +feat(flow-designer): the script node's form authors what the executor runs — framework#4278 + +The `script` flow node is one of five builtins whose designer form lives only +in this package's hand-written `FLOW_NODE_CONFIG` table (the engine publishes +no `configSchema` for them, deliberately), and nothing reconciled that table +against the executor. It had drifted user-visibly: of the four `actionType` +options offered, `code` was a recognized no-op (the built-in runtime has no +server-side JS sandbox) and `sms` / `notification` failed every run (neither +is a built-in — they resolve as function names); the "Output variables" +(plural) field was read by nothing; and the one path that runs real logic — +`function` + `inputs` + `outputVariable` — could not be authored at all. + +- The `actionType` select now offers **Call function** (default) / **Email** / + **Slack**, mirroring the executor's dispatch set + (`SCRIPT_BUILTIN_ACTION_TYPES` + the `invoke_function` marker). The function + path fields (`function`, `inputs`, `outputVariable`) are first-class. +- The inline `script` body becomes render-only: hidden for new nodes (its help + states it is NOT executed and steers to a registered function), still shown + whenever a stored node carries one. The dead plural `outputVariables` field + is removed; stored values surface in the Advanced (JSON) block. +- A scalar select whose stored value was dropped from the options now renders + it as a flagged "` (deprecated)`" entry instead of blanking it — + the same rule FlowObjectListField already applied to select cells. +- The data picker (`flow-scope`) and the flow simulator stop pretending the + legacy `outputVariables[]` list binds variables — the engine never binds + those names; only the singular `outputVariable` does. +- New reconciliation test: the hand-written `script` / `subflow` / `decision` + groups are compared bidirectionally against the executor-derived config + contracts `@objectstack/spec/automation` publishes for exactly this purpose + (framework#4278), and the `wait` / `connector_action` / `boundary_event` + groups against the `FlowNodeSchema` sibling blocks. The spec-export panels + feature-detect and arm themselves on the next `@objectstack/spec` bump. diff --git a/packages/app-shell/README.md b/packages/app-shell/README.md index a5d86eceac..eecfa21c54 100644 --- a/packages/app-shell/README.md +++ b/packages/app-shell/README.md @@ -442,13 +442,15 @@ be faithfully modelled is surfaced loudly instead of faked. free-form **Set variables** editor that injects/overrides *any* variable at start, so **every branch is reachable**. A **Mock outputs** editor lets the author pin what each mocked side-effect node "returns" (written to its - `outputVariable` / `outputVariables`), so data-dependent logic downstream of a - `get_record` or `script` can be exercised too. + `outputVariable`), so data-dependent logic downstream of a `get_record` or + `script` can be exercised too. - **Semantics** — `start`/`assignment` pass through; a `decision` routes **edge-first** (first truthy outgoing `condition`, else the `isDefault` edge, else a surfaced dead-end), evaluating CEL via `@object-ui/core`'s `ExpressionEvaluator` and **surfacing eval errors** (not swallowing them); - side-effect nodes write their mock to `outputVariable` / `outputVariables[]`; + side-effect nodes write their mock to `outputVariable` (the legacy script + `outputVariables[]` list is ignored — the engine never binds those names, + framework#4278); `wait` and `screen` **pause** for manual continue; `join_gateway`, `subflow`, and `boundary_event` are marked **unsupported** (token sync / nested runs are not modelled) rather than faked. diff --git a/packages/app-shell/src/views/metadata-admin/i18n.ts b/packages/app-shell/src/views/metadata-admin/i18n.ts index 5d1d78e0fc..32abb1ae20 100644 --- a/packages/app-shell/src/views/metadata-admin/i18n.ts +++ b/packages/app-shell/src/views/metadata-admin/i18n.ts @@ -3420,14 +3420,19 @@ const FLOW_FIELD_ZH: Record> = { script: { actionType: { label: '动作类型', - help: '本步骤如何运行。原始脚本保持为“代码”。', - opts: { code: '代码', email: '邮件', sms: '短信', notification: '通知' }, + help: '本步骤如何运行。“调用函数”会调用已注册的函数 —— 真正执行逻辑的路径。', + opts: { invoke_function: '调用函数', email: '邮件', slack: 'Slack' }, }, + function: { label: '函数', help: '要调用的已注册函数 —— 经 defineStack({ functions }) 声明。优先于动作类型。' }, + inputs: { label: '输入', help: '传给函数的值;{var} 引用会按流程变量解析。' }, + outputVariable: { label: '输出变量', help: '绑定函数返回值的流程变量,供后续步骤使用。' }, template: { label: '模板', help: '消息模板 id。' }, recipients: { label: '收件人', help: '每行一个收件人(用户 id、字段引用或地址)。' }, variables: { label: '模板变量', help: '注入模板的值。' }, - script: { label: '代码', help: '脚本主体(JS/TS)。' }, - outputVariables: { label: '输出变量', help: '此脚本写回的变量名。' }, + script: { + label: '代码(不执行)', + help: '内置运行时不执行内联脚本 —— 此节点为 no-op。请把逻辑移入已注册函数并使用“调用函数”。', + }, timeoutMs: { label: '超时(毫秒)' }, }, screen: { diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeConfigField.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeConfigField.test.tsx index f89642e5bf..d83790c873 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeConfigField.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeConfigField.test.tsx @@ -100,3 +100,30 @@ describe('FlowNodeConfigField — expression vs template validation gating', () expect(screen.queryByRole('note')).toBeNull(); }); }); + +describe('FlowNodeConfigField — select keeps a stored value dropped from the options (framework#4278)', () => { + const ACTION_TYPE: FlowConfigField = { + id: 'actionType', + path: ['config', 'actionType'], + label: 'Action type', + kind: 'select', + options: [ + { value: 'invoke_function', label: 'Call function' }, + { value: 'email', label: 'Email' }, + { value: 'slack', label: 'Slack' }, + ], + }; + + it('renders a legacy stored value as a flagged "(deprecated)" option instead of blanking it', () => { + // A stored script node with the retired `sms` actionType must keep showing + // that value — the same rule FlowObjectListField applies to select cells. + render( {}} />); + expect(screen.getByText('sms (deprecated)')).toBeInTheDocument(); + }); + + it('offers only the declared options when the stored value is one of them', () => { + render( {}} />); + expect(screen.getByText('Email')).toBeInTheDocument(); + expect(screen.queryByText(/deprecated/)).toBeNull(); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeConfigField.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeConfigField.tsx index 24d0e70825..b0f441341b 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeConfigField.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeConfigField.tsx @@ -139,15 +139,28 @@ export function FlowNodeConfigField({ field, value, onCommit, disabled, locale, /> ); case 'select': - return ( - onCommit(v)} - disabled={disabled} - /> - ); + return (() => { + const current = value != null ? String(value) : ''; + const opts = field.options ?? []; + // A stored value dropped from the options (e.g. a script node's + // legacy `code` / `sms` actionType, framework#4278) must still + // render, or editing a legacy node would silently blank it. Surface + // it as selectable but flag it — it is not offered to fresh nodes. + // Same rule as FlowObjectListField's select cells (ADR-0090 D3). + const shown = + current && !opts.some((o) => o.value === current) + ? [...opts, { value: current, label: `${current} (deprecated)` }] + : opts; + return ( + onCommit(v)} + disabled={disabled} + /> + ); + })(); case 'textarea': return (
diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowStringListField.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowStringListField.tsx index a862317f93..c54083caae 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/FlowStringListField.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowStringListField.tsx @@ -2,7 +2,7 @@ /** * FlowStringListField — a repeatable single-column editor for string-array - * config (e.g. a notification's `recipients`, a script's `outputVariables`). + * config (e.g. a notification's `recipients` or `channels`). * * Mirrors FlowKeyValueField's local-draft pattern: rows live in LOCAL state * with a STABLE id and only flush to `onCommit` on blur / Enter / add / remove, diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.spec-reconciliation.test.ts b/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.spec-reconciliation.test.ts new file mode 100644 index 0000000000..bd12a72983 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.spec-reconciliation.test.ts @@ -0,0 +1,201 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * **Hand-written `FLOW_NODE_CONFIG` ↔ spec-published executor contracts** + * (framework#4278). + * + * Five builtins deliberately publish no descriptor `configSchema` (framework + * `config-schemas.test.ts`): the schema-driven online form cannot express + * their editors — decision's virtual Target column, script's + * actionType-conditional groups, the spec-structured sibling blocks. Their + * Studio form is therefore this package's hand-written table, and until #4278 + * nothing reconciled that table against what the executors actually read. + * `script` had drifted user-visibly: an `outputVariables` field nothing reads, + * `sms` / `notification` options that fail every run, a no-op `code` default, + * and no way to author the `function` / `inputs` / `outputVariable` path that + * works. + * + * The machine-readable half now lives in `@objectstack/spec/automation`: + * executor-derived config Zods for `script` / `subflow` / `decision` + * (`schemaless-node-config.zod.ts`) plus the `FlowNodeSchema` sibling blocks + * for `wait` / `connector_action` / `boundary_event`. This file is the + * objectui half of the ledger — the same bidirectional key-set comparison + * service-automation's `builtin-node-form-zod-ledger.test.ts` performs for the + * descriptor-schema'd builtins, carried across the repo seam by the + * `@objectstack/spec` dependency this package already has. + * + * The script/subflow/decision panels feature-detect their spec exports and + * skip while the installed spec predates them (they arm themselves on the next + * `@objectstack/spec` bump — see the version-alignment section of AGENTS.md); + * the sibling-block panels run against every spec version this repo supports. + */ + +import { describe, it, expect } from 'vitest'; +import * as Automation from '@objectstack/spec/automation'; +import { fieldsForNodeType, type FlowConfigField } from './flow-node-config'; + +// Feature-detected exports — absent on a spec that predates framework#4278. +// (Truthiness alone never resolves a lazySchema proxy.) +const spec = Automation as Record; +const ScriptConfigSchema = spec.ScriptConfigSchema; +const SubflowConfigSchema = spec.SubflowConfigSchema; +const DecisionConfigSchema = spec.DecisionConfigSchema; +const DecisionConditionSchema = spec.DecisionConditionSchema; +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`. + * + * `retiredKey()` tombstones are excluded: a retired key stays in the shape so + * its rejection carries the upgrade prescription (spec `shared/retired-key.ts` + * marks it with a `[REMOVED]` description), but it is NOT part of the + * authorable contract — a form field writing one produces metadata the loader + * rejects. Counting tombstones as contract keys would false-pass exactly that + * drift (e.g. `waitEventConfig.timeoutMs` / `.onTimeout`, retired at spec 18 + * by framework#4198 — this filter is what makes the `wait` panel fire on that + * bump until the form drops the two fields). + */ +function zodKeys(schema: unknown): string[] { + const shape = (schema as { shape?: Record }).shape; + expect(shape, 'expected a Zod object schema exposing .shape').toBeDefined(); + return Object.keys(shape ?? {}) + .filter((k) => { + const description = (shape![k] as { description?: string } | undefined)?.description; + return !(typeof description === 'string' && description.startsWith('[REMOVED]')); + }) + .sort(); +} + +/** Unwrap `.optional()` / `.default()` wrappers down to the object schema. */ +function unwrapped(schema: unknown): unknown { + let cur = schema as { shape?: unknown; unwrap?: () => unknown } | undefined; + for (let i = 0; cur && !cur.shape && typeof cur.unwrap === 'function' && i < 5; i++) { + cur = cur.unwrap() as typeof cur; + } + return cur; +} + +/** A field render-gated behind the never-matching `__legacy__` controller. */ +function isLegacyGated(f: FlowConfigField): boolean { + return f.showWhen?.field === '__legacy__'; +} + +/** Config keys the form OFFERS for new authoring (legacy render-only excluded). */ +function offeredConfigKeys(type: string): string[] { + return [...new Set( + fieldsForNodeType(type) + .filter((f) => f.path[0] === 'config' && !isLegacyGated(f)) + .map((f) => f.path[1]!), + )].sort(); +} + +/** + * Reconcile one node type's config-rooted form keys against its executor + * contract. `renderOnly` names contract keys the form deliberately does NOT + * offer for new authoring — each must still be present as a legacy-gated + * field so stored metadata keeps rendering, and each needs its reason here. + */ +function reconcile(type: string, zod: unknown, renderOnly: Record = {}) { + const offered = offeredConfigKeys(type); + const contract = zodKeys(zod); + + // Read by the executor, absent from the form ⇒ authorable only by hand — + // the exact shape #4278 found for script's function/inputs/outputVariable. + expect( + contract.filter((k) => !offered.includes(k) && !(k in renderOnly)), + `${type}: read by the executor but not offered by the designer form`, + ).toEqual([]); + + // Offered by the form, never read by the executor ⇒ a Studio field that + // does nothing — the #3528 / outputVariables shape. + expect( + offered.filter((k) => !contract.includes(k)), + `${type}: offered by the designer form but never read by the executor`, + ).toEqual([]); + + // Every render-only exemption must (a) be part of the executor contract and + // (b) still render for stored metadata via a legacy-gated field. + for (const key of Object.keys(renderOnly)) { + expect(contract, `${type}: render-only exemption '${key}' must be a contract key`).toContain(key); + const field = fieldsForNodeType(type).find((f) => f.path[0] === 'config' && f.path[1] === key); + expect(field, `${type}: render-only key '${key}' must keep a (legacy-gated) field`).toBeDefined(); + expect(isLegacyGated(field!), `${type}: '${key}' must be legacy-gated, not offered`).toBe(true); + } +} + +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`', + }); + }); + + 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); + }); +}); + +describe.skipIf(!SubflowConfigSchema)('subflow form ↔ SubflowConfigSchema (framework#4278)', () => { + it('offers exactly the executor-read keys', () => { + reconcile('subflow', SubflowConfigSchema); + }); +}); + +describe.skipIf(!DecisionConfigSchema)('decision form ↔ DecisionConfigSchema (framework#4278)', () => { + it('offers exactly the executor-read keys (legacy single `condition` stays render-only)', () => { + // `condition` (singular) is legacy-gated in the form and deliberately NOT + // in the contract: the decision executor never reads it — branching lives + // in `conditions[]` or on edge conditions. Legacy-gated fields are already + // excluded from `offered`, so plain reconciliation covers it. + reconcile('decision', DecisionConfigSchema); + }); + + it('branch columns match DecisionConditionSchema one level down (Target is virtual)', () => { + const conditions = fieldsForNodeType('decision').find((f) => f.id === 'conditions')!; + const columnKeys = conditions.columns!.map((c) => c.key).sort(); + const contract = zodKeys(DecisionConditionSchema); + // `target` is a VIRTUAL column — projected from / applied to the out-edges + // by flow-decision-edges, never stored on the branch — so it is the one + // legitimate column the stored-shape contract does not carry. + expect(columnKeys.filter((k) => k !== 'target')).toEqual(contract); + expect(columnKeys).toContain('target'); + }); +}); + +describe('sibling-block forms ↔ FlowNodeSchema blocks (framework#4278 ratchet)', () => { + // These blocks are published by every spec version this repo supports, so + // no feature detection: the hand-written groups for wait / connector_action + // / boundary_event must edit exactly the keys the spec block declares — + // #4161 / #4210 verified them by hand once; this keeps them verified. + const FlowNodeSchema = spec.FlowNodeSchema as { shape: Record }; + + const BLOCKS: ReadonlyArray<{ type: string; block: string }> = [ + { type: 'wait', block: 'waitEventConfig' }, + { type: 'connector_action', block: 'connectorConfig' }, + { type: 'boundary_event', block: 'boundaryConfig' }, + ]; + + it.each(BLOCKS)('$type: the $block fields match the spec block exactly', ({ type, block }) => { + const formKeys = [...new Set( + fieldsForNodeType(type) + .filter((f) => f.path[0] === block) + .map((f) => f.path[1]!), + )].sort(); + const blockKeys = zodKeys(unwrapped(FlowNodeSchema.shape[block])); + + expect( + blockKeys.filter((k) => !formKeys.includes(k)), + `${type}: declared by the spec block but absent from the designer form`, + ).toEqual([]); + expect( + formKeys.filter((k) => !blockKeys.includes(k)), + `${type}: edited by the designer form but not declared by the spec block`, + ).toEqual([]); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.test.ts b/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.test.ts index 4f58eec042..c6c157bc4f 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.test.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.test.ts @@ -211,6 +211,57 @@ describe('loop / map collection is a template, not a CEL predicate', () => { }); }); +describe('script node — the form authors what the executor runs (framework#4278)', () => { + 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', () => { + 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). + 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('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); + }); +}); + describe('notify node — first-class static config editor (#1895)', () => { const fields = fieldsForNodeType('notify'); const ids = fields.map((f) => f.id); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts b/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts index fcd3180811..2a649796f8 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts @@ -468,46 +468,72 @@ const FLOW_NODE_CONFIG: Record = { cfg('outputVariable', 'Output variable', 'text', { placeholder: 'response' }), { id: 'timeoutMs', path: ['timeoutMs'], label: 'Timeout (ms)', kind: 'number', placeholder: '30000' }, ], - // Script — overloaded in real metadata. Two observed shapes, discriminated by - // `actionType`: notification (actionType: email/sms/... → template/recipients/ - // variables) and code (no actionType → script/outputVariables). The form adapts - // on actionType; unknown values still show the code fields so nothing is lost. + // 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. + // + // 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). script: [ cfg('actionType', 'Action type', 'select', { options: [ - { value: 'code', label: 'Code' }, + { value: 'invoke_function', label: 'Call function' }, { value: 'email', label: 'Email' }, - { value: 'sms', label: 'SMS' }, - { value: 'notification', label: 'Notification' }, + { value: 'slack', label: 'Slack' }, ], - defaultValue: 'code', - help: 'How this step runs. Leave as Code for a raw script.', + 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'] }, + }), + 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; sms / - // notification have no flat catalog yet, so they degrade to free text. + // 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', 'sms', 'notification'] }, + showWhen: { field: 'actionType', equals: ['email', 'slack'] }, }), cfg('recipients', 'Recipients', 'stringList', { help: 'One recipient per row (user id, field ref, or address).', - showWhen: { field: 'actionType', equals: ['email', 'sms', 'notification'] }, + showWhen: { field: 'actionType', equals: ['email', 'slack'] }, }), cfg('variables', 'Template variables', 'keyValue', { help: 'Values injected into the template.', - showWhen: { field: 'actionType', equals: ['email', 'sms', 'notification'] }, + showWhen: { field: 'actionType', equals: ['email', 'slack'] }, }), - cfg('script', 'Code', 'textarea', { + // 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', { placeholder: 'return { ok: true };', - help: 'Script body (JS/TS).', + 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".', refMode: 'expression', - showWhen: { field: 'actionType', equals: ['code'] }, - }), - cfg('outputVariables', 'Output variables', 'stringList', { - help: 'Names of variables this script writes back.', - showWhen: { field: 'actionType', equals: ['code'] }, + showWhen: { field: '__legacy__', equals: [] }, }), { id: 'timeoutMs', path: ['timeoutMs'], label: 'Timeout (ms)', kind: 'number', placeholder: '30000' }, ], diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/flow-scope.test.ts b/packages/app-shell/src/views/metadata-admin/inspectors/flow-scope.test.ts index ddb83d73d7..7c70a09406 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/flow-scope.test.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/flow-scope.test.ts @@ -50,8 +50,12 @@ describe('nodeOutputRefs', () => { it('reads a single outputVariable', () => { expect(tokens(nodeOutputRefs({ id: 'g', type: 'get_record', config: { outputVariable: 'records' } }))).toEqual(['records']); }); - it('reads a list of outputVariables (script)', () => { - expect(tokens(nodeOutputRefs({ id: 's', type: 'script', config: { outputVariables: ['lead_score', 'qualified'] } }))).toEqual(['lead_score', 'qualified']); + it('does NOT read the legacy script outputVariables[] list (framework#4278)', () => { + // The engine never binds those names — suggesting them offered successors + // variables that never exist at run time. The script node's real output + // binding is the singular `outputVariable` on the function path. + expect(tokens(nodeOutputRefs({ id: 's', type: 'script', config: { outputVariables: ['lead_score', 'qualified'] } }))).toEqual([]); + expect(tokens(nodeOutputRefs({ id: 's', type: 'script', config: { function: 'score', outputVariable: 'lead_score' } }))).toEqual(['lead_score']); }); it('flags a loop/map iterator as a loop ref and collects its output', () => { const refs = nodeOutputRefs({ id: 'm', type: 'map', config: { iteratorVariable: 'item', outputVariable: 'results' } }); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/flow-scope.ts b/packages/app-shell/src/views/metadata-admin/inspectors/flow-scope.ts index 9ff9a81ed1..6581cb7417 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/flow-scope.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/flow-scope.ts @@ -143,11 +143,16 @@ export function flowAncestors(nodeId: string, edges: FlowEdgeLike[]): Set; const outputs: string[] = []; + // Singular only — the engine never binds the script node's legacy + // `outputVariables` list (framework#4278), so the simulator no longer + // pretends it does. if (typeof cfg.outputVariable === 'string' && cfg.outputVariable) outputs.push(cfg.outputVariable); - if (Array.isArray(cfg.outputVariables)) { - for (const o of cfg.outputVariables) if (typeof o === 'string') outputs.push(o); - } out.push({ id: n.id, label: n.label || n.id, type: n.type, outputs }); } return out; diff --git a/packages/app-shell/src/views/metadata-admin/previews/simulator/__tests__/flow-simulator.test.ts b/packages/app-shell/src/views/metadata-admin/previews/simulator/__tests__/flow-simulator.test.ts index 9aff85366f..193f3decfb 100644 --- a/packages/app-shell/src/views/metadata-admin/previews/simulator/__tests__/flow-simulator.test.ts +++ b/packages/app-shell/src/views/metadata-admin/previews/simulator/__tests__/flow-simulator.test.ts @@ -221,7 +221,27 @@ describe('FlowSimulator', () => { expect(sim.state.steps.find((s) => s.nodeId === 'g')?.status).toBe('mocked'); }); - it('writes mocked outputVariables[] for a code script', () => { + it('writes a mocked outputVariable for a function script', () => { + const sim = run( + [ + { id: 's', type: 'start' }, + { id: 'sc', type: 'script', config: { function: 'score_lead', outputVariable: 'score' } }, + { id: 'e', type: 'end' }, + ], + [ + { source: 's', target: 'sc' }, + { source: 'sc', target: 'e' }, + ], + {}, + { sc: 42 }, + ); + expect(sim.state.variables.score).toBe(42); + }); + + it('does NOT bind the legacy script outputVariables[] list (framework#4278)', () => { + // The engine never binds those names — it binds the singular + // `outputVariable` on the function path. Simulating the list taught + // authors a binding that does not exist at run time. const sim = run( [ { id: 's', type: 'start' }, @@ -235,7 +255,7 @@ describe('FlowSimulator', () => { {}, { sc: { score: 42 } }, ); - expect(sim.state.variables.score).toBe(42); + expect(sim.state.variables.score).toBeUndefined(); }); it('pauses on a wait node and resumes', () => { diff --git a/packages/app-shell/src/views/metadata-admin/previews/simulator/flow-sim-types.ts b/packages/app-shell/src/views/metadata-admin/previews/simulator/flow-sim-types.ts index d3ddbc8583..b9f3e2959e 100644 --- a/packages/app-shell/src/views/metadata-admin/previews/simulator/flow-sim-types.ts +++ b/packages/app-shell/src/views/metadata-admin/previews/simulator/flow-sim-types.ts @@ -112,8 +112,9 @@ export interface FlowValidation { } /** - * Author-supplied mock outputs, keyed by node id. The value is merged into the - * simulation variables according to the node type (single `outputVariable` or a - * `outputVariables` list). For a `screen` node it supplies the captured inputs. + * Author-supplied mock outputs, keyed by node id. The value is bound to the + * node's `outputVariable` (the engine's only output binding — the legacy + * script `outputVariables` list is ignored, framework#4278). For a `screen` + * node it supplies the captured inputs. */ export type MockResults = Record; diff --git a/packages/app-shell/src/views/metadata-admin/previews/simulator/flow-simulator.ts b/packages/app-shell/src/views/metadata-admin/previews/simulator/flow-simulator.ts index c422f2d7e6..4e8b6a41eb 100644 --- a/packages/app-shell/src/views/metadata-admin/previews/simulator/flow-simulator.ts +++ b/packages/app-shell/src/views/metadata-admin/previews/simulator/flow-simulator.ts @@ -385,7 +385,13 @@ export class FlowSimulator { return this.state.variables[key]; } - /** Apply a node's mock output to the simulation variables; returns what it wrote. */ + /** + * Apply a node's mock output to the simulation variables; returns what it + * wrote. Only the singular `outputVariable` binds — the script node's legacy + * `outputVariables` list is deliberately ignored, because the engine never + * binds those names (framework#4278); simulating them taught authors a + * binding that does not exist at run time. + */ private applyMock(node: SimNode): Record | undefined { const cfg = node.config ?? {}; const mock = this.mocks[node.id]; @@ -396,16 +402,6 @@ export class FlowSimulator { wrote[single] = mock !== undefined ? mock : {}; this.state.variables[single] = wrote[single]; } - - const list = Array.isArray(cfg.outputVariables) ? (cfg.outputVariables as unknown[]) : []; - if (list.length) { - const m = mock && typeof mock === 'object' ? (mock as Record) : {}; - for (const name of list) { - if (typeof name !== 'string') continue; - wrote[name] = name in m ? m[name] : undefined; - this.state.variables[name] = wrote[name]; - } - } return Object.keys(wrote).length ? wrote : undefined; }