fix(spec)!: refuse a decision branch with no expression — absent or null — at all three doors (#19961)

.changeset/19961-decision-branch-expression-absent-refused.md

@@ -0,0 +1,61 @@
+---
+'@objectstack/spec': minor
+'@objectstack/lint': minor
+---
+
+fix(spec)!: a `decision` branch with no `expression` — the key absent, or `null` — is refused at authoring (#19961)
+
+Clause-②: no (narrowing)
+
+<!-- adr-0087: registered flow-decision-branch-expression-absent-refused -->
+
+**BREAKING** — an accept-set narrowing on one authored flow-node slot, 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.** `DecisionConditionSchema` declares a branch `{ label, expression }`
+with `expression` a required `z.string()`. Nothing enforced that: a decision node's
+`config` is an open record no schema is parsed against, and the expression ledger's
+resolver skipped an absent value as "not authored". So `conditions: [{ label: 'y' }]`
+passed `FlowSchema.parse`, `AutomationEngine.registerFlow` and `objectstack validate`,
+and the run then failed at that branch — the executor evaluates every branch it
+reaches, and a branch with no `expression` is a condition with no `source`, which
+`evaluateCondition` refuses. The ledger now marks the slot `required` (reconciled
+against the schema's own `required` list), and the branch is refused at all three
+doors through the walk and the function that already refuse a blank one — by
+`FlowSchema.parse` with a `custom` issue anchored at the slot (for example
+`nodes.1.config.conditions.0.expression`), by `registerFlow` and `objectstack validate`
+through that same parse, and by `validateStackExpressions` for a stack handed to it
+directly — with one message, led by the published `PREDICATE_SLOT_STRING_REFUSAL`
+sentence. `expression: null` is refused the same way, and so is a branch that wrote
+its predicate under `condition` (the edge's spelling), which has no `expression`
+either. The Studio flow designer writes the refused shape when a branch row's
+expression cell is left empty. Where such a branch 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 |
+|:--|:--|
+| `conditions: [{ label: 'high' }]` on a `decision` node | the predicate you meant — `{ label: 'high', expression: 'record.amount > 10000' }` |
+| `conditions: [{ label: 'high', condition: 'record.amount > 10000' }]` | the same predicate under `expression` |
+| `conditions: [{ label: 'high', expression: null }]` | the predicate you meant, or `expression: 'false'` to keep the branch and never take it |
+
+**One-line fix:** write the predicate under `expression`. `expression: 'false'` keeps
+the branch and its label and never takes it — a change of behaviour, not a preserved
+one: a run that reached the branch used to FAIL there, and now routes on to the next
+branch or the declared fallback. ⚠️ Do not drop a decision's only branch: the node
+then routes by its out-edges alone, and the out-edge that branch labelled is no
+longer held back.
+
+**Unchanged.** A branch carrying a non-blank predicate parses, registers and
+validates as before; a blank one keeps its refusal and its own prescription
+(`flow-predicate-slot-blank-string-refused`); a `decision` with no `conditions`, or
+an empty list, still routes by its out-edges; an absent screen field `visibleWhen`
+is still legal (that slot is not required); and `PREDICATE_SLOT_STRING_REFUSAL`
+keeps its name and its text.

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

@@ -15,6 +15,7 @@ import {
   ASSIGNMENT_VALUE_ENVELOPE_REFUSAL,
   PREDICATE_SLOT_STRING_REFUSAL,
   STRUCTURAL_CONDITION_SHAPE_REFUSAL,
+  predicateSlotRefusal,
 } from '@objectstack/spec/automation';
 
 import {
@@ -4398,6 +4399,75 @@ describe('a blank string in a ledger predicate slot (#17493)', () => {
   });
 });
 
+/**
+ * [#19961] A `decision` branch with no `expression`, at the THIRD door:
+ * `objectstack validate`'s expression pass.
+ *
+ * `DecisionConditionSchema` declares `expression` `z.string()`, not optional,
+ * but `conditions: [{ label: 'y' }]` reported NOTHING here: the resolver skipped
+ * the absent value as "not authored", and `checkDeclaredPredicate` returned
+ * early on `null` / absent besides. The run then failed at the branch. The
+ * ledger now marks the slot `required`, the resolver emits the absent value
+ * there, and this pass refuses it through `predicateSlotRefusal` — the same
+ * call, the same message, as `FlowSchema.parse` and `registerFlow`.
+ *
+ * The table is the one those two doors run (in `spec` and `service-automation`):
+ * nothing, a blank string, a real predicate — asserted by `where`, severity and
+ * the full message, read off the spec's own function.
+ *
+ * ⚠️ Through the CLI, `objectstack validate` meets the absent value first at
+ * its schema step (`FlowSchema.parse` refuses it there, with the same message).
+ * This pass is what answers for a stack handed to `validateStackExpressions`
+ * directly, and it is what these pins drive.
+ */
+describe('a decision branch with no `expression` (#19961)', () => {
+  const flowStack = (...branches: Record<string, unknown>[]) => ({
+    flows: [{
+      name: 'absent_flow',
+      nodes: [{ id: 'start', type: 'start' }, { id: 'check', type: 'decision', config: { conditions: branches } }],
+      edges: [],
+    }],
+  });
+  const errorsOf = (stack: unknown) =>
+    validateStackExpressions(stack as never).filter((i) => (i.severity ?? 'error') === 'error');
+  const WHERE_0 = "flow 'absent_flow' · node 'check' (decision) decision branch expression at config.conditions[0].expression";
+
+  it.each([
+    { name: 'no `expression` key — the #19961 shape', branch: { label: 'y' }, refused: true, refusedWith: undefined },
+    { name: '`expression: null`', branch: { label: 'y', expression: null }, refused: true, refusedWith: null },
+    { name: 'the predicate under the edge\'s spelling `condition`', branch: { label: 'y', condition: 'true' }, refused: true, refusedWith: undefined },
+    { name: 'a blank string — the #17493 control', branch: { label: 'y', expression: '   ' }, refused: true, refusedWith: '   ' },
+    { name: 'a real predicate — the accept control', branch: { label: 'y', expression: 'true' }, refused: false, refusedWith: undefined },
+  ] as Array<{ name: string; branch: Record<string, unknown>; refused: boolean; refusedWith: unknown }>)('$name', ({ branch, refused, refusedWith }) => {
+    const found = errorsOf(flowStack(branch));
+    if (!refused) {
+      expect(found).toHaveLength(0);
+      return;
+    }
+    expect(found).toHaveLength(1);
+    expect(found[0].severity).toBe('error');
+    expect(found[0].where).toBe(WHERE_0);
+    expect(found[0].message).toBe(predicateSlotRefusal(refusedWith)!.message);
+    expect(found[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true);
+  });
+
+  it('names WHICH branch: an absent second branch is located at index 1, the valid first one is not', () => {
+    const found = errorsOf(flowStack({ label: 'a', expression: 'amount > 1' }, { label: 'b' }));
+    expect(found.map((i) => i.where)).toEqual([
+      "flow 'absent_flow' · node 'check' (decision) decision branch expression at config.conditions[1].expression",
+    ]);
+    expect(found[0].source).toBe('');
+  });
+
+  it('CONTROL — a decision with no branch, and a screen field with no `visibleWhen`, report nothing', () => {
+    expect(errorsOf({ flows: [{ name: 'f', nodes: [{ id: 'check', type: 'decision', config: {} }], edges: [] }] })).toHaveLength(0);
+    expect(errorsOf(flowStack())).toHaveLength(0);
+    expect(errorsOf({
+      flows: [{ name: 'f', nodes: [{ id: 'form', type: 'screen', config: { fields: [{ name: 'amount', type: 'number' }] } }], edges: [] }],
+    })).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

@@ -1401,7 +1401,14 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] {
    * a field-existence pass would report every field name as unknown.
    */
   const checkDeclaredPredicate = (where: string, raw: unknown): { refused: boolean } => {
-    if (raw == null) return { refused: false };
+    // [#19961] No `raw == null` early return: whether an absent value is a
+    // finding is the resolver's call, not this pass's. It emits absent / `null`
+    // only for a `required` ledger slot (a `decision` branch's `expression`),
+    // and there it is a refusal — the one `predicateSlotRefusal` gives the
+    // other two doors. An early return here answered "valid" for the very
+    // value `FlowSchema.parse` refuses, for any caller of
+    // `validateStackExpressions` that did not parse first.
+    //
     // [#15572] The slot is declared bare CEL TEXT, so a non-string — the
     // `{ dialect, source }` envelope above all — is refused on SHAPE before
     // anything tries to read a source out of it. The refusal is the spec's,

packages/services/service-automation/src/builtin/config-expression-ledger.test.ts

@@ -48,6 +48,8 @@ interface SchemaNode {
   items?: SchemaNode;
   /** `true` = an open map with untyped values; an object = the schema every value takes. */
   additionalProperties?: boolean | SchemaNode;
+  /** The keys of `properties` this object requires (JSON Schema `required`). */
+  required?: string[];
   xExpression?: string;
 }
 
@@ -83,9 +85,9 @@ const ROLE_BY_MARKER: Record<string, FlowNodeExpressionRole> = {
 function collectExpressionProps(
   schema: SchemaNode | undefined,
   prefix = '',
-): { path: string; marker: string }[] {
+): { path: string; marker: string; required: boolean }[] {
   if (!schema || typeof schema !== 'object') return [];
-  const out: { path: string; marker: string }[] = [];
+  const out: { path: string; marker: string; required: boolean }[] = [];
 
   if (schema.properties) {
     for (const [key, prop] of Object.entries(schema.properties)) {
@@ -96,7 +98,11 @@ function collectExpressionProps(
       const here = prefix
         ? `${prefix}.${key}${isObjectArray ? '[]' : ''}`
         : `${key}${isObjectArray ? '[]' : ''}`;
-      if (typeof prop.xExpression === 'string') out.push({ path: here, marker: prop.xExpression });
+      // [#19961] Whether the declaring object REQUIRES the slot — read off the
+      // same schema the marker is, so the ledger's `required` flag is
+      // reconciled against the contract rather than restated beside it.
+      const required = Array.isArray(schema.required) && schema.required.includes(key);
+      if (typeof prop.xExpression === 'string') out.push({ path: here, marker: prop.xExpression, required });
       if (isObjectArray) out.push(...collectExpressionProps(prop.items, here));
       else out.push(...collectExpressionProps(prop, here));
     }
@@ -110,7 +116,8 @@ function collectExpressionProps(
   const values = schema.additionalProperties;
   if (values && typeof values === 'object') {
     const here = prefix ? `${prefix}.*` : '*';
-    if (typeof values.xExpression === 'string') out.push({ path: here, marker: values.xExpression });
+    // A map value is never "required": the map's keys are the author's own.
+    if (typeof values.xExpression === 'string') out.push({ path: here, marker: values.xExpression, required: false });
     out.push(...collectExpressionProps(values, here));
   }
   return out;
@@ -119,7 +126,7 @@ function collectExpressionProps(
 const engine = new AutomationEngine(silentLogger());
 installBuiltinNodes(engine, ctx());
 
-type DeclaredSlot = { nodeType: string; path: string; role: FlowNodeExpressionRole };
+type DeclaredSlot = { nodeType: string; path: string; role: FlowNodeExpressionRole; required: boolean };
 
 /** Resolve an `xExpression` marker to its ledger role, failing loudly on an unknown one. */
 function roleOf(nodeType: string, path: string, marker: string): FlowNodeExpressionRole {
@@ -137,8 +144,8 @@ function declaredFromDescriptors(): DeclaredSlot[] {
   const found: DeclaredSlot[] = [];
   for (const descriptor of engine.getActionDescriptors()) {
     const schema = descriptor.configSchema as SchemaNode | undefined;
-    for (const { path, marker } of collectExpressionProps(schema)) {
-      found.push({ nodeType: descriptor.type, path, role: roleOf(descriptor.type, path, marker) });
+    for (const { path, marker, required } of collectExpressionProps(schema)) {
+      found.push({ nodeType: descriptor.type, path, role: roleOf(descriptor.type, path, marker), required });
     }
   }
   return found;
@@ -163,8 +170,8 @@ function declaredFromDescriptors(): DeclaredSlot[] {
 function declaredFromSchemalessConfigs(): DeclaredSlot[] {
   const found: DeclaredSlot[] = [];
   for (const [nodeType, json] of Object.entries(getSchemalessNodeConfigJsonSchemas())) {
-    for (const { path, marker } of collectExpressionProps(json as SchemaNode)) {
-      found.push({ nodeType, path, role: roleOf(nodeType, path, marker) });
+    for (const { path, marker, required } of collectExpressionProps(json as SchemaNode)) {
+      found.push({ nodeType, path, role: roleOf(nodeType, path, marker), required });
     }
   }
   return found;
@@ -213,6 +220,33 @@ describe('configSchema ↔ expression-ledger reconciliation (#4027)', () => {
     expect(stale, 'stale ledger entries — no descriptor or schemaless schema declares these').toEqual([]);
   });
 
+  /**
+   * [#19961] The ledger's `required` flag is what makes the resolver emit an
+   * ABSENT value for the doors to refuse — so it must say exactly what the
+   * declaring channel's `required` list says, in both directions. A flag the
+   * contract does not back would refuse a legal omission (an absent
+   * `visibleWhen` shows the field); a requirement the ledger misses is the
+   * #19961 shape again — declared required, admitted absent at every door.
+   *
+   * Reconciled over the `predicate` role, the one role the flag acts on: the
+   * resolver emits an absent value only for a required PREDICATE slot. The
+   * channels require two `flow-template` slots too (`loop.collection`,
+   * `map.collection`), and no door refuses their absence — their executors
+   * parse their own config — so the flag stays off there, and the second
+   * assertion pins that it is never set on another role.
+   */
+  it('the ledger marks `required` exactly the predicate slots the declaring channel requires (#19961)', () => {
+    const declared = declaredEverywhere().filter((d) => d.role === 'predicate');
+    const requiredByChannel = declared.filter((d) => d.required).map(key).sort();
+    const requiredByLedger = FLOW_NODE_EXPRESSION_PATHS.filter((e) => e.role === 'predicate' && e.required).map(key).sort();
+    expect(requiredByLedger, 'ledger `required` flags disagree with the declaring channel').toEqual(requiredByChannel);
+    expect(FLOW_NODE_EXPRESSION_PATHS.filter((e) => e.role !== 'predicate' && e.required).map(key), '`required` acts on the predicate role only').toEqual([]);
+    // Non-vacuous: the one required predicate slot there is today is derived,
+    // not assumed — and the optional one (`visibleWhen`) is derived as optional.
+    expect(requiredByChannel).toEqual(['decision.conditions[].expression (predicate)']);
+    expect(declared.filter((d) => !d.required).map(key)).toEqual(['screen.fields[].visibleWhen (predicate)']);
+  });
+
   it('decision.conditions[].expression is covered — the #4439 hole', () => {
     const decision = FLOW_NODE_EXPRESSION_PATHS.find(
       (e) => e.nodeType === 'decision' && e.path === 'conditions[].expression',