fix(spec)!: refuse a flow node config its executor cannot run — a required key left out, or a decision branch list it cannot read — at all three doors (#20316)

.changeset/20316-flow-node-config-required-keys-refused.md

@@ -0,0 +1,82 @@
+---
+'@objectstack/spec': minor
+'@objectstack/lint': minor
+---
+
+fix(spec)!: a flow node config its executor cannot run — a key its contract requires, left out, or a decision branch list it cannot read — is refused at authoring (#20316)
+
+Clause-②: no (narrowing)
+
+<!-- adr-0087: registered flow-node-config-required-keys-refused -->
+
+**BREAKING** — an accept-set narrowing on authored flow-node `config`, shipped as
+`minor` under the launch-window convention (`check-changeset-no-major` refuses
+`major` until GA; breaking-ness is carried by this banner and the ADR-0087
+disposition above, not by the level).
+
+**What changed.** A flow node's `config` is an open record, so what its executor
+requires was checked by no build door. `FlowSchema.parse`, `AutomationEngine.registerFlow`
+and `objectstack validate` all admitted a node that left out a key its executor
+contract requires — and the executor's own contract parse then refused the node on
+every run that reached it. A `decision` branch with no `label` was worse: it never
+failed, the matched branch reported no label, and traversal took EVERY out-edge, so
+the flow ran green down the wrong paths. All three doors now refuse these shapes
+through one judge, `flowNodeConfigRefusals` (new in `@objectstack/spec/automation`):
+
+- **A key a builtin's executor contract requires, left out.** Each builtin node's
+  config is parsed against the very contract its executor parses against
+  (`getBuiltinNodeConfigContracts()`, new, reconciled against the executors' own parse
+  calls), and only the keys left out are kept — a present value of the wrong type, and
+  an undeclared key, are judged where they were before. The keys: `objectName` on
+  `get_record` / `create_record` / `update_record` / `delete_record`; `recipients` on
+  `notify` (and `title` when there is no `template`); `url` on `http`; `function` on
+  `script`; `flowName` on `subflow`; `collection` and `flowName` on `map`;
+  `collection` on a `loop` that has a `body`; `branches` on `parallel`; `try` on
+  `try_catch`; and on `screen`, each field's `name`, each option's `value` and
+  `label`, and a `lookup` field's `reference`. A key a rule of the contract requires
+  (the `notify` title, the `lookup` reference) is refused in the contract's own words.
+- **A `decision` branch list its executor cannot read.** `conditions` present and not
+  `null` must be an array; every branch must be an object; every branch's `label` must
+  be a non-blank string (absent, `null`, blank or non-text all name no out-edge).
+
+Each refusal is a `custom` issue anchored at the key (`nodes.1.config.objectName`,
+`nodes.1.config.fields.0.name`, `nodes.1.config.conditions.0.label`, or the region
+path `nodes.1.config.body.nodes.0.config…`), met at `registerFlow` and
+`objectstack validate` through that same parse, and reported by
+`validateStackExpressions` for a stack handed to it directly. The refusal codes join
+`FLOW_SLOT_REFUSAL_CODES`: `node-config-key-missing`, `node-config-key-required-by-rule`,
+`decision-conditions-not-array`, `decision-branch-not-object`,
+`decision-branch-label-missing`.
+
+The Studio flow designer writes refused shapes when a node is added and saved before
+it is configured, when a decision branch row's label cell is left empty, and when a
+screen field row's name cell is left empty. Where such a node already sits, the whole
+flow is refused: registered from the metadata registry or `sys_metadata` at boot, it
+is skipped with a `failed to register flow` warn naming it while the flows beside it
+register; a `defineStack({ flows })` source throws `StackSchemaInvalidError` for the
+whole stack; an artifact file is refused whole at load.
+
+## FROM → TO
+
+| you wrote | write instead |
+|:--|:--|
+| `{ type: 'get_record', config: { outputVariable: 'rows' } }` | the object it reads — `config: { objectName: 'account', outputVariable: 'rows' }` (the same for `create_record` / `update_record` / `delete_record`) |
+| `{ type: 'loop', config: { body: { … } } }` | the array it iterates — `config: { collection: '{rows}', body: { … } }` |
+| `{ type: 'map', config: { flowName: 'per_row' } }` | `config: { collection: '{rows}', flowName: 'per_row' }` |
+| `{ type: 'http', config: { method: 'GET' } }` | `config: { url: 'https://api.example.com/v1/items', method: 'GET' }` |
+| `{ type: 'script' }` | the registered function it calls — `config: { function: 'recalc_totals' }` |
+| `{ type: 'notify', config: { recipients: ['{record.owner}'] } }` | a content source — `title: 'Deal won'`, or a `template` |
+| `conditions: [{ expression: 'record.amount > 1000' }]` on a `decision` | the out-edge it routes to — `[{ label: 'large', expression: 'record.amount > 1000' }]`, beside an out-edge labelled `large` |
+| `conditions: ['record.amount > 1000']` | `[{ label: 'large', expression: 'record.amount > 1000' }]` |
+
+**One-line fix:** write the key the node was meant to carry. To branch on the
+out-edges instead of on `conditions`, delete `conditions` and put each predicate on its
+edge's `condition`.
+
+**Unchanged.** A node carrying every key its contract requires parses, registers and
+validates as before; a legacy flat-graph `loop` (no `body`) still needs no
+`collection`; a `decision` with no `conditions`, `conditions: null` or an empty list
+still routes by its out-edges; `assignment`, `wait`, `connector_action` and plugin node
+types are not judged by this rule; and a key spelled by a D2 alias (`object`, `flow`,
+`functionName`, …) is still canonicalized before `registerFlow` and `objectstack
+validate` judge it.

packages/lint/src/validate-expressions.test.ts

@@ -15,6 +15,7 @@ import {
   ASSIGNMENT_VALUE_ENVELOPE_REFUSAL,
   PREDICATE_SLOT_STRING_REFUSAL,
   STRUCTURAL_CONDITION_SHAPE_REFUSAL,
+  flowNodeConfigRefusals,
   predicateSlotRefusal,
 } from '@objectstack/spec/automation';
 
@@ -2291,7 +2292,7 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
       const badRegion = () => ({
         nodes: [
           { id: 'gate', type: 'decision', config: { condition: '{record.rating} >= 4' } },
-          { id: 'act', type: 'update_record' },
+          { id: 'act', type: 'update_record', config: { objectName: 'crm_lead' } },
         ],
         edges: [{ id: 'b1', source: 'gate', target: 'act', condition: '{record.status} == "open"' }],
       });
@@ -2341,7 +2342,7 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
             body: {
               nodes: [
                 { id: 'gate', type: 'decision', config: { condition: 'record.rating >= 4' } },
-                { id: 'act', type: 'update_record' },
+                { id: 'act', type: 'update_record', config: { objectName: 'crm_lead' } },
               ],
               edges: [{ id: 'b1', source: 'gate', target: 'act', condition: 'record.status == "open"' }],
             },
@@ -3010,6 +3011,11 @@ describe('validateStackExpressions — reads only keys the spec declares (meta-t
       // are `success` / `error`, SafeParseResult's own, so that excuse covers
       // both locals for one reason and masks no metadata read either.
       'blankRefusal',
+      // [#20316] The spec's node-config judge, one refusal at a time. Its keys
+      // are that helper's own `{ code, params, message, source, path }` —
+      // never metadata keys — and it is named to stay clear of the `message` /
+      // `source` receivers for the reason the two entries above record.
+      'configRefusal',
       // [#14089] NOT a receiver at all — the tail of the `'./flow-variable-scope.js'`
       // import specifier, which this scan cannot tell from `scope.j…`. The two
       // entries above it in this set (`fields`, `guards`) are the same artefact
@@ -4468,6 +4474,66 @@ describe('a decision branch with no `expression` (#19961)', () => {
   });
 });
 
+/**
+ * [#20316] What a node's executor needs its `config` to carry, at the THIRD
+ * door: `objectstack validate`'s expression pass.
+ *
+ * A key the node's executor contract requires, left out, and a `decision`
+ * branch list the executor cannot read, reported NOTHING here — the flow then
+ * registered and every run that reached the node failed there, or (a branch
+ * with no `label`) ran green down every out-edge. This pass now refuses them
+ * through `flowNodeConfigRefusals` — the same call, the same message, as
+ * `FlowSchema.parse` and `registerFlow`.
+ *
+ * ⚠️ Through the CLI, `objectstack validate` meets these shapes first at its
+ * schema step (`FlowSchema.parse` refuses them there, with the same message).
+ * This pass answers for a stack handed to `validateStackExpressions` directly,
+ * and it is what these pins drive.
+ */
+describe('node config an executor requires (#20316)', () => {
+  const stackWith = (node: Record<string, unknown>) => ({
+    flows: [{ name: 'config_flow', nodes: [{ id: 'start', type: 'start' }, { id: 'n', ...node }], edges: [] }],
+  });
+  const errorsOf = (stack: unknown) =>
+    validateStackExpressions(stack as never).filter((i) => (i.severity ?? 'error') === 'error');
+
+  it.each([
+    { type: 'loop', whole: { collection: '{rows}', body: { nodes: [{ id: 'b', type: 'assignment' }], edges: [] } }, key: 'collection' },
+    { type: 'map', whole: { collection: '{rows}', flowName: 'child_flow' }, key: 'collection' },
+    { type: 'get_record', whole: { objectName: 'account' }, key: 'objectName' },
+    { type: 'http', whole: { url: 'https://example.com/hook' }, key: 'url' },
+    { type: 'subflow', whole: { flowName: 'child_flow' }, key: 'flowName' },
+  ] as Array<{ type: string; whole: Record<string, unknown>; key: string }>)('$type without `$key` is refused; with it, nothing is', ({ type, whole, key }) => {
+    expect(errorsOf(stackWith({ type, config: whole }))).toHaveLength(0);
+    const authored = { ...whole };
+    delete authored[key];
+    const found = errorsOf(stackWith({ type, config: authored }));
+    expect(found.map((i) => [i.where, i.message, i.source])).toEqual([
+      [`flow 'config_flow' · node 'n' (${type}) config.${key}`, flowNodeConfigRefusals(type, authored)[0].message, ''],
+    ]);
+  });
+
+  it('a decision branch with no `label` is refused at the label; its labelled twin is not', () => {
+    const found = errorsOf(stackWith({ type: 'decision', config: { conditions: [{ expression: 'true' }] } }));
+    expect(found.map((i) => [i.where, i.message])).toEqual([[
+      "flow 'config_flow' · node 'n' (decision) config.conditions[0].label",
+      flowNodeConfigRefusals('decision', { conditions: [{ expression: 'true' }] })[0].message,
+    ]]);
+    expect(errorsOf(stackWith({ type: 'decision', config: { conditions: [{ label: 'y', expression: 'true' }] } }))).toHaveLength(0);
+  });
+
+  it('a decision branch that is a bare string is refused once, as a branch', () => {
+    const found = errorsOf(stackWith({ type: 'decision', config: { conditions: ['true'] } }));
+    expect(found.map((i) => i.where)).toEqual(["flow 'config_flow' · node 'n' (decision) config.conditions[0]"]);
+  });
+
+  it('a `script` with no `function` is ONE finding, the callable check\'s — it reads the pre-conversion spellings this pass may be handed', () => {
+    const found = errorsOf(stackWith({ type: 'script', config: {} }));
+    expect(found.map((i) => i.where)).toEqual(["flow 'config_flow' · node 'n' (script) callable"]);
+    expect(errorsOf(stackWith({ type: 'script', config: { functionName: 'recalc_totals' } }))).toHaveLength(0);
+  });
+});
+
 /**
  * [#20078] A field-level predicate that reads THROUGH a reference field is
  * refused at authoring, with the repair that is true for the root it reads.

packages/lint/src/validate-expressions.ts

@@ -87,6 +87,7 @@ import {
 } from '@objectstack/formula';
 import {
   collectFlowGraphs,
+  flowNodeConfigRefusals,
   predicateSlotRefusal,
   resolveFlowNodeExpressions,
   resolveFlowNodeValueSlots,
@@ -1620,6 +1621,28 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] {
         // `loop.collection`). The ledger records them regardless, so the
         // reconciliation ratchet still sees the marker.
         const nodeType = typeof node.type === 'string' ? node.type : '';
+        // [#20316] What the node's executor needs its `config` to carry — a key
+        // its contract requires, left out, and a `decision` branch list it
+        // cannot read. The spec's one judge, the same call `FlowSchema.parse`
+        // makes (and `registerFlow` meets through that parse), so a stack
+        // handed to `validateStackExpressions` without a parse in front of it
+        // is held to the same bar. `error`: the flow would register and then
+        // refuse — or, for a branch with no label, misroute — every run.
+        const configRefusals = flowNodeConfigRefusals(nodeType, node.config)
+          // A `script`'s `function` stays the callable check's below (#1870,
+          // #4343): this pass may be handed a pre-conversion source, and that
+          // check reads what such a source spells — the `functionName` alias,
+          // the retired dispatch keys — and names each, where the judge would
+          // only see `function` absent.
+          .filter((configRefusal) => !(nodeType === 'script' && configRefusal.path === 'function'));
+        for (const configRefusal of configRefusals) {
+          issues.push({
+            where: `${at} · node '${node.id}' (${nodeType}) config.${configRefusal.path}`,
+            message: configRefusal.message,
+            source: configRefusal.source,
+            severity: 'error',
+          });
+        }
         for (const found of resolveFlowNodeExpressions(nodeType, cfg)) {
           const slotWhere = `${at} · node '${node.id}' (${nodeType}) ${found.entry.label} at config.${found.path}`;
           // [#15137] `value` slots are checkable too, by their own rule — see

packages/runtime/src/domains/automation-flow-clone.test.ts

@@ -100,7 +100,7 @@ function packagedExemplar(): Record<string, unknown> {
                     condition: 'record.stage == "negotiation"',
                 },
             },
-            { id: 'notify', type: 'notify', label: 'Notify manager', config: { channel: 'email', to: '{record.manager_email}' } },
+            { id: 'notify', type: 'notify', label: 'Notify manager', config: { channel: 'email', to: '{record.manager_email}', title: 'Deal in negotiation' } },
             { id: 'end', type: 'end', label: 'End' },
         ],
         edges: [

packages/runtime/src/domains/automation-put-post-error-parity.test.ts

@@ -45,7 +45,9 @@ import { FlowSchema, validateControlFlow } from '@objectstack/spec/automation';
 import { HttpDispatcher } from '../http-dispatcher.js';
 
 /** Config keys the fake's `notify` descriptor declares (the #4277 legal set). */
-const NOTIFY_DECLARED_CONFIG_KEYS = ['message', 'recipients', 'channel'];
+// `title` joined the set when the flow parse began refusing a notify node with
+// neither `title` nor `template` (#20316) — the real notify descriptor declares it.
+const NOTIFY_DECLARED_CONFIG_KEYS = ['title', 'message', 'recipients', 'channel'];
 
 /**
  * The #4277 refusal, reproduced from `service-automation/src/engine.ts`
@@ -127,12 +129,16 @@ function makeDispatcher() {
  */
 const CTX = { request: {}, executionContext: { userId: 'user_1', systemPermissions: ['manage_metadata'] } } as any;
 
-/** A definition that is legal at every gate the fake runs. */
+/**
+ * A definition that is legal at every gate the fake runs. The notify node
+ * carries the `recipients` and `title` its executor contract requires — the
+ * flow parse refuses a node that leaves them out (#20316).
+ */
 const WELL_FORMED = {
     name: 'welcome_flow',
     label: 'Welcome',
     type: 'autolaunched',
-    nodes: [{ id: 'n', type: 'notify', label: 'Notify', config: { message: 'hi' } }],
+    nodes: [{ id: 'n', type: 'notify', label: 'Notify', config: { recipients: ['user_1'], title: 'Welcome', message: 'hi' } }],
     edges: [],
 };
 
@@ -141,12 +147,12 @@ const BAD_BODIES = {
     /** 1 — a node with no `label` (`FlowSchema.parse`). */
     missingNodeLabel: {
         ...WELL_FORMED,
-        nodes: [{ id: 'n', type: 'notify', config: { message: 'hi' } }],
+        nodes: [{ id: 'n', type: 'notify', config: { recipients: ['user_1'], title: 'Welcome', message: 'hi' } }],
     },
     /** 2 — a node key the schema does not declare (`unrecognized_keys`). */
     unknownNodeKey: {
         ...WELL_FORMED,
-        nodes: [{ id: 'n', type: 'notify', label: 'Notify', next: 'other' }],
+        nodes: [{ id: 'n', type: 'notify', label: 'Notify', config: { recipients: ['user_1'], title: 'Welcome', message: 'hi' }, next: 'other' }],
     },
     /** 3 — a `try_catch` whose `try` region is an array, not a region object. */
     malformedRegion: {
@@ -161,7 +167,7 @@ const BAD_BODIES = {
         ...WELL_FORMED,
         nodes: [{
             id: 'n', type: 'notify', label: 'Notify',
-            config: { message: 'hi', totallyBogusKey: 'oops' },
+            config: { recipients: ['user_1'], title: 'Welcome', message: 'hi', totallyBogusKey: 'oops' },
         }],
     },
 } as const;

packages/runtime/src/domains/automation-register-error-class.test.ts

@@ -74,7 +74,9 @@ import { FlowSchema, validateControlFlow } from '@objectstack/spec/automation';
 import { HttpDispatcher } from '../http-dispatcher.js';
 
 /** Config keys the fake's `notify` descriptor declares (the #4277 legal set). */
-const NOTIFY_DECLARED_CONFIG_KEYS = ['message', 'recipients', 'channel'];
+// `title` joined the set when the flow parse began refusing a notify node with
+// neither `title` nor `template` (#20316) — the real notify descriptor declares it.
+const NOTIFY_DECLARED_CONFIG_KEYS = ['title', 'message', 'recipients', 'channel'];
 
 /**
  * The #4277 refusal, reproduced from `service-automation/src/engine.ts`
@@ -159,12 +161,16 @@ function makeDispatcher(options?: { registerFlow?: (name: string, definition: un
  */
 const CTX = { request: {}, executionContext: { userId: 'user_1', systemPermissions: ['manage_metadata'] } } as any;
 
-/** A definition that is legal at every gate the fake runs. */
+/**
+ * A definition that is legal at every gate the fake runs. The notify node
+ * carries the `recipients` and `title` its executor contract requires — the
+ * flow parse refuses a node that leaves them out (#20316).
+ */
 const WELL_FORMED = {
     name: 'welcome_flow',
     label: 'Welcome',
     type: 'autolaunched',
-    nodes: [{ id: 'n', type: 'notify', label: 'Notify', config: { message: 'hi' } }],
+    nodes: [{ id: 'n', type: 'notify', label: 'Notify', config: { recipients: ['user_1'], title: 'Welcome', message: 'hi' } }],
     edges: [],
 };
 
@@ -173,12 +179,12 @@ const BAD_BODIES = {
     /** 1 — a node with no `label`. */
     missingNodeLabel: {
         ...WELL_FORMED,
-        nodes: [{ id: 'n', type: 'notify', config: { message: 'hi' } }],
+        nodes: [{ id: 'n', type: 'notify', config: { recipients: ['user_1'], title: 'Welcome', message: 'hi' } }],
     },
     /** 2 — a node key the schema does not declare. */
     unknownNodeKey: {
         ...WELL_FORMED,
-        nodes: [{ id: 'n', type: 'notify', label: 'Notify', next: 'other' }],
+        nodes: [{ id: 'n', type: 'notify', label: 'Notify', config: { recipients: ['user_1'], title: 'Welcome', message: 'hi' }, next: 'other' }],
     },
     /** 3 — a `try_catch` whose `try` region is an array, not a region object. */
     malformedRegion: {
@@ -193,7 +199,7 @@ const BAD_BODIES = {
         ...WELL_FORMED,
         nodes: [{
             id: 'n', type: 'notify', label: 'Notify',
-            config: { message: 'hi', totallyBogusKey: 'oops' },
+            config: { recipients: ['user_1'], title: 'Welcome', message: 'hi', totallyBogusKey: 'oops' },
         }],
     },
 } as const;