Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
39 changes: 39 additions & 0 deletions .changeset/script-node-converged-to-function.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down Expand Up @@ -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/);
}
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: [] },
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -950,7 +950,10 @@ function nodeSummary(node: FlowNode): 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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).`;
}
Expand Down
Loading