service-automation: refuse a CEL envelope in a z.string() predicate slot at build time

.changeset/decision-predicate-envelope-refused.md

@@ -0,0 +1,16 @@
+---
+"@objectstack/spec": minor
+"@objectstack/service-automation": minor
+"@objectstack/lint": minor
+---
+
+A flow predicate authored as a CEL envelope is now refused at build time, instead of running unread by either validator.
+
+A `predicate`-role expression slot holds **bare CEL text** — `DecisionConditionSchema.expression` is declared `z.string()`, and so is a screen field's `visibleWhen`. An author who instead wrote the `{ dialect, source }` expression *envelope* there reached a shape nothing could see: a flow node's `config` is an open `z.record(z.unknown())` that no Zod schema is parsed against, the unknown-key walk exempts the schemaless node types on purpose (`decision` publishes no descriptor `configSchema`), and the expression ledger's `predicate` arm skipped every non-string as "a type violation for the schema pass to report" — a schema pass that, for those node types, does not exist. `registerFlow` accepted the flow, `objectstack validate` reported nothing, and the evaluator was the only layer that ever read the predicate.
+
+- `resolveFlowNodeExpressions` now emits a non-string sitting in a `predicate` slot, and the new `predicateSlotRefusal` / `PREDICATE_SLOT_STRING_REFUSAL` say why it is refused — one notion, derived once, read by both validators so build time and author time cannot disagree about the shape. `flow-template` slots keep the old rule: no validator implements that dialect, so a finding there is one nobody could judge.
+- `registerFlow` throws, naming the node, the slot and the index, and attributing the finding to the envelope's own `source`. `objectstack validate` reports the same refusal as a located `error`.
+
+**String predicates are untouched, deliberately.** A whitespace-only string still means "not authored" on both sides, exactly as before; what a non-empty string *says* is still judged by `validateExpression('predicate', …)`, brace trap and all. Only the shape moved.
+
+An app that authored an envelope in one of these slots now fails to register with a message naming the slot; the fix is to write the predicate as bare CEL text (`record.rating >= 4`). The `{ dialect, source }` envelope remains the `value`-role spelling, on the `assignment` node's `assignments` map.

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

@@ -11,7 +11,7 @@ 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 } from '@objectstack/spec/automation';
+import { ASSIGNMENT_VALUE_ENVELOPE_REFUSAL, PREDICATE_SLOT_STRING_REFUSAL } from '@objectstack/spec/automation';
 
 import {
   validateStackExpressions,
@@ -2117,6 +2117,69 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
       expect(issues.filter(i => i.where.includes('conditions'))).toHaveLength(0);
     });
 
+    /**
+     * [#15572] The same slot, authored as a CEL **envelope** rather than as the
+     * bare CEL text it is declared to hold (`DecisionConditionSchema.expression`
+     * is `z.string()`). Before this, `objectstack validate` reported nothing:
+     * the ledger's `predicate` arm emitted strings only, skipping the envelope
+     * as "a type violation for the schema pass to report" — and `decision`
+     * publishes no descriptor `configSchema`, so no schema pass ever ran.
+     *
+     * ⚠️ Read the RED CONTROL below before trusting any zero here. The reading
+     * this card was filed on used a second envelope as its control — a control
+     * that could itself have been the answer, which makes it no control at all
+     * — so this test drives a form that DOES report (the brace-trap string, on
+     * this very slot, through this very call) in the same run as the forms
+     * under test. Without it, a harness that reached the slot and a harness
+     * that reached nothing would read identically.
+     */
+    describe('a predicate slot authored as an expression envelope (#15572)', () => {
+      const atSlot = (expression: unknown) =>
+        validateStackExpressions(decisionFlow(expression as string))
+          .filter(i => i.where.includes('conditions[0].expression'));
+
+      it('RED CONTROL — the brace-trap string on this slot still reports', () => {
+        // Not an envelope, so it cannot be "the answer" to what is being
+        // measured; it proves only that this call reaches this slot and can
+        // emit. If this ever goes to zero, every zero below is void.
+        const control = atSlot("{lead_record.status} == 'converted'");
+        expect(control).toHaveLength(1);
+        expect(control[0].severity).toBe('error');
+        expect(control[0].message).toContain('template brace');
+      });
+
+      it('reports a `{ dialect, source }` envelope, whatever the source says', () => {
+        for (const envelope of [
+          { dialect: 'cel', source: '   ' },            // the silent-`false` shape
+          { dialect: 'cel', source: 'rows.map(r,' },    // the throwing shape
+          { dialect: 'cel', ast: { kind: 'const' } },   // no `source` at all
+        ]) {
+          const found = atSlot(envelope);
+          expect(found).toHaveLength(1);
+          expect(found[0].severity).toBe('error');
+          expect(found[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true);
+          expect(found[0].where).toContain('decision branch expression');
+        }
+      });
+
+      it('attributes the finding to the envelope’s own source when it has one', () => {
+        expect(atSlot({ dialect: 'cel', source: 'rows.map(r,' })[0].source).toBe('rows.map(r,');
+        expect(atSlot({ dialect: 'cel', ast: { kind: 'const' } })[0].source).toBe('');
+      });
+
+      it('reports every other non-string on the same rule, naming what it found', () => {
+        expect(atSlot(42)[0].message).toContain('Found a number');
+        expect(atSlot(['a > 1'])[0].message).toContain('Found an array');
+      });
+
+      it('leaves string predicates alone — including the whitespace-only one', () => {
+        // The card states this boundary explicitly so nobody "fixes" it: a
+        // whitespace-only STRING is "not authored" on both sides and stays so.
+        expect(atSlot('   ')).toHaveLength(0);
+        expect(atSlot("lead_record.status == 'converted'")).toHaveLength(0);
+      });
+    });
+
     it('leaves a correct single-brace loop collection alone', () => {
       // `loop.collection` is the single-brace `{var}` flow-interpolation dialect,
       // where braces are CORRECT. It is recorded in the ledger as `flow-template`
@@ -2850,6 +2913,12 @@ describe('validateStackExpressions — reads only keys the spec declares (meta-t
       // keys; the metadata keys it judges are `AssignmentValueSchema`'s, read
       // by the schema and not by name here.
       'shape',
+      // [#15572] The spec's shared refusal for a non-string in a predicate
+      // slot. Its keys are that helper's own `{ message, source }` — never
+      // metadata keys — and it is named to stay clear of the `message` /
+      // `source` receivers for the reason the entry above it records: a local
+      // called `message` here would be excused into masking a genuine read.
+      'shapeRefusal',
       // [#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

packages/lint/src/validate-expressions.ts

@@ -79,7 +79,7 @@
  */
 
 import { validateExpression, collectCelRootIdentifiers, parseCelToAst, SCOPE_ROOTS } from '@objectstack/formula';
-import { collectFlowGraphs, resolveFlowNodeExpressions } from '@objectstack/spec/automation';
+import { collectFlowGraphs, predicateSlotRefusal, resolveFlowNodeExpressions } 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
@@ -1065,11 +1065,22 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] {
    * bind the *screen's own* collected values, not the trigger record's fields, so
    * a field-existence pass would report every field name as unknown.
    */
-  const checkDeclaredPredicate = (where: string, raw: unknown): void => {
-    if (raw == null) return;
+  const checkDeclaredPredicate = (where: string, raw: unknown): { refused: boolean } => {
+    if (raw == null) return { refused: false };
+    // [#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,
+    // shared with the engine's `registerFlow` pass: `error`, because that pass
+    // throws, and a shape build refuses must not pass author time.
+    const shapeRefusal = predicateSlotRefusal(raw);
+    if (shapeRefusal) {
+      issues.push({ where, message: shapeRefusal.message, source: shapeRefusal.source, severity: 'error' });
+      return { refused: true };
+    }
     const res = validateExpression('predicate', raw as string | { dialect?: string; source?: string });
     for (const e of res.errors) issues.push({ where, message: e.message, source: e.source, severity: 'error' });
     for (const w of res.warnings) issues.push({ where, message: w.message, source: w.source, severity: 'warning' });
+    return { refused: false };
   };
 
   /**
@@ -1184,7 +1195,10 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] {
             continue;
           }
           if (found.entry.role !== 'predicate') continue;
-          checkDeclaredPredicate(slotWhere, found.value);
+          // [#15572] A slot refused on SHAPE gets no second diagnostic: the
+          // shadowing warning is about which scope a CEL source resolves in,
+          // and a value that is not CEL text has no source to resolve.
+          if (checkDeclaredPredicate(slotWhere, found.value).refused) continue;
           // [#14288] The shadowing warning is about the SCOPE an expression is
           // evaluated in, not about which key it was authored under — so it
           // belongs on every `predicate` slot the ledger declares, not just the

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

@@ -281,17 +281,26 @@ describe('resolveFlowNodeExpressions — path resolution (#4027)', () => {
     expect(found[0].entry.role).toBe('flow-template');
   });
 
-  it('skips absent, empty and non-string values rather than inventing findings', () => {
+  it('skips absent and empty values rather than inventing findings', () => {
     expect(resolveFlowNodeExpressions('screen', {})).toEqual([]);
     expect(resolveFlowNodeExpressions('screen', { fields: [] })).toEqual([]);
     expect(resolveFlowNodeExpressions('screen', { fields: [{ visibleWhen: '   ' }] })).toEqual([]);
-    // A non-string in an expression slot is a type violation for the schema pass
-    // to report — not something to hand to a parser.
-    expect(resolveFlowNodeExpressions('screen', { fields: [{ visibleWhen: true }] })).toEqual([]);
     // A repeater authored as a non-array must not throw.
     expect(resolveFlowNodeExpressions('screen', { fields: 'nope' })).toEqual([]);
   });
 
+  // [#15572] A NON-string in a predicate slot is no longer skipped. It was
+  // skipped as "a type violation for the schema pass to report", and for the
+  // schemaless node types — `decision` publishes no descriptor `configSchema`,
+  // so `validateNodeConfigKeys` exempts it and nothing parses its config
+  // against `DecisionConfigSchema` — that schema pass does not exist. The value
+  // is emitted so `registerFlow` and `objectstack validate` can refuse it.
+  it('emits a non-string in a predicate slot for the consumer to refuse (#15572)', () => {
+    expect(resolveFlowNodeExpressions('screen', { fields: [{ visibleWhen: true }] })
+      .map((f) => [f.path, f.value, f.entry.role]))
+      .toEqual([['fields[0].visibleWhen', true, 'predicate']]);
+  });
+
   it('resolves each decision branch predicate, with its index (#4439)', () => {
     const found = resolveFlowNodeExpressions('decision', {
       conditions: [