service-automation: refuse a structural flow condition that is neither CEL text nor an expression

.changeset/structural-condition-shape-refused.md

@@ -0,0 +1,17 @@
+---
+"@objectstack/spec": minor
+"@objectstack/service-automation": minor
+"@objectstack/lint": minor
+---
+
+A flow condition that is neither CEL text nor an expression is now refused at build time, instead of being read as an empty condition and answering a silent `false`.
+
+`evaluateCondition` derives its source as `typeof expression === 'string' ? expression : (expression?.source ?? '')`. For a value that is neither — a number, a boolean, an array — the read yields `undefined`, the `??` supplies `''`, and the empty-source arm returns **`false`**: the "an unauthored branch must not open" rule, applied to a value that was very much authored. Measured: a `decision` node carrying `config: { condition: 42 }` **registered clean** and executed `success: true` with nothing said at any layer; `{ source: 1 }` did not even get that far and threw a bare `TypeError: exprStr.trim is not a function` out of the validator. `config.condition` is also the key a **start node's trigger gate** is read from, so the same value could gate a whole flow shut forever with no signal to the author.
+
+- The new `structuralConditionRefusal` / `STRUCTURAL_CONDITION_SHAPE_REFUSAL` in `@objectstack/spec/automation` are the single shared notion of why, read by both validators so build time and author time cannot disagree about the shape. `registerFlow` throws, naming the node or edge and attributing the finding; `objectstack validate` reports the same refusal as a located `error`.
+
+**This is deliberately NOT the `predicate`-slot rule, and the difference is measured.** A ledger `predicate` slot (`decision.conditions[].expression`, a screen field's `visibleWhen`) is declared `z.string()`, so `PREDICATE_SLOT_STRING_REFUSAL` refuses every non-string including an envelope. Neither structural slot is declared that way: `FlowEdgeSchema.condition` is `ExpressionInputSchema`, whose string arm **transforms into** `{ dialect: 'cel', source }` — so after `FlowSchema.parse` every authored edge condition *is* an envelope — and `FlowNodeSchema.config` is an open `z.record` that passes an envelope written at `config.condition` through verbatim, where `evaluateCondition` evaluates it correctly. Both shapes stay accepted here; an envelope with no `dialect`, and an `ast`-carrying one (`ExpressionSchema`'s own `source`-or-`ast` rule), stay accepted too.
+
+**Strings are untouched, deliberately.** A whitespace-only condition still means "not authored" and still answers `false` on both sides — consistent behaviour, ruled correct, not a defect. What a non-empty string *says* is still `validateExpression('predicate', …)`'s verdict, brace trap and all. Only the shape moved.
+
+An app that authored a number, a boolean, an array or a source-less object in a node or edge `condition` now fails to register with a message naming the site; the fix is to write the condition as bare CEL text (`record.rating >= 4`) or as an expression envelope.

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

@@ -11,7 +11,11 @@ import { FieldSchema, ObjectSchema, SelectOptionSchema } from '@objectstack/spec
 import { SharingRuleSchema } from '@objectstack/spec/security';
 // [#15137] The published refusal sentence a `value`-slot finding must lead
 // with — asserted from the spec's own export, never re-spelled in a test.
-import { ASSIGNMENT_VALUE_ENVELOPE_REFUSAL, PREDICATE_SLOT_STRING_REFUSAL } from '@objectstack/spec/automation';
+import {
+  ASSIGNMENT_VALUE_ENVELOPE_REFUSAL,
+  PREDICATE_SLOT_STRING_REFUSAL,
+  STRUCTURAL_CONDITION_SHAPE_REFUSAL,
+} from '@objectstack/spec/automation';
 
 import {
   validateStackExpressions,
@@ -3794,3 +3798,125 @@ describe('assignment value envelope — located findings (#15137)', () => {
     expect(issues.filter((i) => i.where.includes('assignment value'))).toHaveLength(0);
   });
 });
+
+/**
+ * [#15662] The STRUCTURAL condition arm at author time — `objectstack
+ * validate`'s half of the refusal `registerFlow` throws.
+ *
+ * The gap: `evaluateCondition` reads its source as
+ * `typeof expression === 'string' ? expression : (expression?.source ?? '')`,
+ * so a value that is neither a string nor envelope-shaped lands in the
+ * empty-source arm and returns a silent `false`. Measured on `main`: `42`,
+ * `true` and `['a']` at a node's `config.condition` registered clean and ran
+ * `success: true` with nothing reported anywhere — on the same key the start
+ * node's TRIGGER GATE is read from.
+ *
+ * ⚠️ The rule is deliberately NOT `PREDICATE_SLOT_STRING_REFUSAL`. That arm's
+ * slots are declared `z.string()`; these are not. `FlowEdgeSchema.condition` is
+ * `ExpressionInputSchema`, so an envelope is a first-class authored spelling
+ * here, and a node's open `z.record` config passes one through verbatim. The
+ * controls below are those two shapes staying green.
+ */
+describe('structural condition shape (#15662)', () => {
+  const objects = [
+    { name: 'crm_lead', fields: { rating: { type: 'number' }, status: { type: 'text' } } },
+  ];
+
+  const flowWith = (opts: { startCondition?: unknown; decisionCondition?: unknown; edgeCondition?: unknown }) => ({
+    objects,
+    flows: [{
+      name: 'gate_flow',
+      nodes: [
+        {
+          id: 'start', type: 'start',
+          config: {
+            objectName: 'crm_lead',
+            ...('startCondition' in opts ? { condition: opts.startCondition } : {}),
+          },
+        },
+        {
+          id: 'branch', type: 'decision',
+          config: { ...('decisionCondition' in opts ? { condition: opts.decisionCondition } : {}) },
+        },
+      ],
+      edges: [{
+        id: 'e1', source: 'start', target: 'branch',
+        ...('edgeCondition' in opts ? { condition: opts.edgeCondition } : {}),
+      }],
+    }],
+  });
+
+  const condIssues = (opts: Parameters<typeof flowWith>[0], site: string) =>
+    validateStackExpressions(flowWith(opts)).filter((i) => i.where.includes(site));
+
+  it('RED CONTROL — the brace-trap string on this very arm still reports', () => {
+    // Not a shape violation, so it cannot be "the answer" to what is measured
+    // below; it proves this call reaches this slot and can emit. If it ever
+    // goes to zero, every zero in this block is void.
+    const control = condIssues({ decisionCondition: '{record.rating} >= 4' }, "node 'branch'");
+    expect(control).toHaveLength(1);
+    expect(control[0].severity).toBe('error');
+    expect(control[0].message).toContain('template brace');
+  });
+
+  it('reports every measured silent value on a node condition, naming what it found', () => {
+    for (const [value, found] of [[42, 'Found a number'], [true, 'Found a boolean'], [['a'], 'Found an array']] as const) {
+      const issues = condIssues({ decisionCondition: value }, "node 'branch'");
+      expect(issues).toHaveLength(1);
+      expect(issues[0].severity).toBe('error');
+      expect(issues[0].message.startsWith(STRUCTURAL_CONDITION_SHAPE_REFUSAL)).toBe(true);
+      expect(issues[0].message).toContain(found);
+      expect(issues[0].where).toContain("flow 'gate_flow'");
+    }
+  });
+
+  it('reports the same on the START node trigger gate', () => {
+    const issues = condIssues({ startCondition: 42 }, "node 'start'");
+    expect(issues).toHaveLength(1);
+    expect(issues[0].message.startsWith(STRUCTURAL_CONDITION_SHAPE_REFUSAL)).toBe(true);
+  });
+
+  it('reports the same on an edge condition', () => {
+    const issues = condIssues({ edgeCondition: ['a'] }, "edge 'e1'");
+    expect(issues).toHaveLength(1);
+    expect(issues[0].message.startsWith(STRUCTURAL_CONDITION_SHAPE_REFUSAL)).toBe(true);
+  });
+
+  it('refuses an object that is neither text nor an expression', () => {
+    const issues = condIssues({ decisionCondition: { source: 1 } }, "node 'branch'");
+    expect(issues).toHaveLength(1);
+    expect(issues[0].message).toContain('neither a string `source` nor an `ast`');
+    // A non-string `source` is what is being refused, so it is never the
+    // attribution.
+    expect(issues[0].source).toBe('');
+  });
+
+  it('refuses ONCE — the value-reading passes do not re-report it as an empty condition', () => {
+    expect(condIssues({ decisionCondition: 42 }, 'condition')).toHaveLength(1);
+  });
+
+  describe('CONTROLS — the shapes that must stay accepted', () => {
+    it('an expression ENVELOPE on a node condition is legitimate here', () => {
+      expect(condIssues({ decisionCondition: { dialect: 'cel', source: 'record.rating >= 4' } }, "node 'branch'"))
+        .toHaveLength(0);
+      // No dialect — `evaluateCondition` reads it as CEL, so does this.
+      expect(condIssues({ decisionCondition: { source: 'record.rating >= 4' } }, "node 'branch'"))
+        .toHaveLength(0);
+    });
+
+    it('an envelope on an EDGE is legitimate — every parsed edge condition is one', () => {
+      expect(condIssues({ edgeCondition: { dialect: 'cel', source: 'record.rating >= 4' } }, "edge 'e1'"))
+        .toHaveLength(0);
+      expect(condIssues({ edgeCondition: 'record.rating >= 4' }, "edge 'e1'")).toHaveLength(0);
+    });
+
+    it('a whitespace-only STRING is untouched — ruled correct, not a defect', () => {
+      expect(condIssues({ decisionCondition: '   ' }, "node 'branch'")).toHaveLength(0);
+      expect(condIssues({ edgeCondition: '   ' }, "edge 'e1'")).toHaveLength(0);
+    });
+
+    it('a clean bare-CEL condition still passes', () => {
+      expect(condIssues({ decisionCondition: 'record.rating >= 4' }, "node 'branch'")).toHaveLength(0);
+    });
+  });
+});

packages/lint/src/validate-expressions.ts

@@ -79,7 +79,12 @@
  */
 
 import { validateExpression, collectCelRootIdentifiers, parseCelToAst, SCOPE_ROOTS } from '@objectstack/formula';
-import { collectFlowGraphs, predicateSlotRefusal, resolveFlowNodeExpressions } from '@objectstack/spec/automation';
+import {
+  collectFlowGraphs,
+  predicateSlotRefusal,
+  resolveFlowNodeExpressions,
+  structuralConditionRefusal,
+} from '@objectstack/spec/automation';
 // [#15137] The `value`-role half. Same two published primitives the engine
 // composes at `registerFlow` (`AutomationEngine.valueEnvelopeRefusals`), in the
 // same order: the SHAPE rule lives in the spec's `AssignmentValueSchema` (it
@@ -1156,12 +1161,44 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] {
       }
     };
 
+    /**
+     * [#15662] A STRUCTURAL condition (`config.condition` on a node,
+     * `edge.condition`) refused on SHAPE before anything reads a source out of
+     * it. The refusal is the spec's, shared with the engine's `registerFlow`
+     * pass: `error`, because that pass throws, and a shape build refuses must
+     * not pass author time.
+     *
+     * ⚠️ Deliberately NOT `predicateSlotRefusal`, which the declared-slot arm
+     * above uses. A ledger `predicate` slot is declared `z.string()`; neither
+     * structural slot is — `FlowEdgeSchema.condition` is
+     * `ExpressionInputSchema`, so an envelope is the shape the parse itself
+     * produces, and a node's `config` is an open `z.record` that passes one
+     * through verbatim. Both are admitted here; a value that is neither text
+     * nor an expression is not, because the evaluator reads it as an EMPTY
+     * condition and answers a silent `false`.
+     *
+     * @returns whether the slot was refused, so the caller can skip the
+     *   value-reading passes that would otherwise re-report it as an empty one.
+     */
+    const checkStructuralCondition = (where: string, raw: unknown): { refused: boolean } => {
+      if (raw == null) return { refused: false };
+      const shapeRefusal = structuralConditionRefusal(raw);
+      if (shapeRefusal) {
+        issues.push({ where, message: shapeRefusal.message, source: shapeRefusal.source, severity: 'error' });
+        return { refused: true };
+      }
+      return { refused: false };
+    };
+
     for (const graph of graphs) {
       const at = graph.scope ? `flow '${flowName}' · ${graph.scope}` : `flow '${flowName}'`;
       for (const node of graph.nodes as unknown as AnyRec[]) {
         const cfg = (node.config ?? {}) as AnyRec;
-        check(`${at} · node '${node.id}' (${node.type}) condition`, cfg.condition, objectName);
-        warnShadowedFieldReads(`${at} · node '${node.id}' (${node.type}) condition`, cfg.condition);
+        const nodeCondWhere = `${at} · node '${node.id}' (${node.type}) condition`;
+        if (!checkStructuralCondition(nodeCondWhere, cfg.condition).refused) {
+          check(nodeCondWhere, cfg.condition, objectName);
+          warnShadowedFieldReads(nodeCondWhere, cfg.condition);
+        }
 
         // Descriptor-declared expression slots (#4027). Before this, the traversal
         // hardcoded `condition` and assumed every other node string was a `{var}`
@@ -1276,8 +1313,11 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] {
         }
       }
       for (const edge of graph.edges as unknown as AnyRec[]) {
-        check(`${at} · edge '${edge.id}' (${edge.source}→${edge.target}) condition`, edge.condition, objectName);
-        warnShadowedFieldReads(`${at} · edge '${edge.id}' (${edge.source}→${edge.target}) condition`, edge.condition);
+        const edgeCondWhere = `${at} · edge '${edge.id}' (${edge.source}→${edge.target}) condition`;
+        if (!checkStructuralCondition(edgeCondWhere, edge.condition).refused) {
+          check(edgeCondWhere, edge.condition, objectName);
+          warnShadowedFieldReads(edgeCondWhere, edge.condition);
+        }
       }
       // No `checkNullGuards` on node/edge conditions — and NOT for the reason
       // #4811 first recorded (#4811 re-measured it). The stated blocker was the

packages/services/service-automation/src/engine.ts

@@ -25,7 +25,7 @@ import { FlowSchema, FLOW_STRUCTURAL_NODE_TYPES, validateControlFlow, collectFlo
 // `validate-flow-trigger-readiness`, so the runtime cannot drift from what
 // authoring accepted. See `resolveTriggerBinding`.
 import { resolveFlowTriggerKind } from '@objectstack/spec/automation';
-import { predicateSlotRefusal, resolveFlowNodeExpressions } from '@objectstack/spec/automation';
+import { predicateSlotRefusal, resolveFlowNodeExpressions, structuralConditionRefusal } from '@objectstack/spec/automation';
 // [#15137] The `value`-role half of the ledger. Both halves of "is this envelope
 // well-formed?" are IMPORTED, never re-spelled here: the shape rule is
 // `AssignmentValueSchema` (spec, #14149 — it refuses a non-`cel` dialect and the
@@ -7108,6 +7108,40 @@ export class AutomationEngine implements IAutomationService {
             }
         };
 
+        /**
+         * [#15662] The STRUCTURAL condition surfaces — `config.condition` on any
+         * node and `edge.condition` — refused on SHAPE before anything tries to
+         * read a source out of them.
+         *
+         * ⚠️ Not `predicateSlotRefusal`, the ledger arm's rule, and the
+         * difference is measured rather than assumed: `FlowEdgeSchema.condition`
+         * is `ExpressionInputSchema`, whose string arm transforms into
+         * `{ dialect: 'cel', source }`, so after `FlowSchema.parse` EVERY
+         * authored edge condition is an envelope — the ledger rule here would
+         * refuse every conditional edge in every flow. An envelope written at a
+         * node's `config.condition` is likewise passed through verbatim by the
+         * open `z.record` and evaluated correctly (#4336). Both are legitimate;
+         * `structuralConditionRefusal` admits them.
+         *
+         * What it refuses is the value that is neither text nor an expression.
+         * `evaluateCondition` reads `expression?.source ?? ''` and the
+         * empty-source arm answers `false` — "an unauthored branch must not
+         * open", applied to a value that was authored — so `42` / `true` /
+         * `['a']` registered clean and ran silently, on the same key the start
+         * node's trigger gate is read from. Same severity as a malformed
+         * predicate (this throws): the reject set of registration and the reject
+         * set of evaluation must be one set.
+         */
+        const checkStructuralCondition = (where: string, raw: unknown): void => {
+            if (raw == null) return;
+            const shapeRefusal = structuralConditionRefusal(raw);
+            if (shapeRefusal) {
+                failures.push(`  • ${where}: ${shapeRefusal.message}\n      source: \`${shapeRefusal.source}\``);
+                return;
+            }
+            check(where, raw);
+        };
+
         // #4347 — every graph in the flow, not just the top-level arrays. An
         // ADR-0031 container keeps a whole sub-graph in its `config`, so
         // iterating `flow.nodes`/`flow.edges` checked PART of the flow while
@@ -7120,7 +7154,7 @@ export class AutomationEngine implements IAutomationService {
             for (const node of graph.nodes) {
                 const cfg = (node.config ?? {}) as Record<string, unknown>;
                 // start-node trigger gate + decision/branch predicates live in config.condition
-                check(`${at}node '${node.id}' (${node.type}) condition`, cfg.condition);
+                checkStructuralCondition(`${at}node '${node.id}' (${node.type}) condition`, cfg.condition);
 
                 // Descriptor-declared expression slots (#4027). The ledger names them
                 // per node type and carries the dialect each one takes, so a declared
@@ -7177,7 +7211,7 @@ export class AutomationEngine implements IAutomationService {
                 }
             }
             for (const edge of graph.edges) {
-                check(`${at}edge '${edge.id}' (${edge.source}→${edge.target}) condition`, edge.condition as unknown);
+                checkStructuralCondition(`${at}edge '${edge.id}' (${edge.source}→${edge.target}) condition`, edge.condition as unknown);
             }
         }