fix(spec)!: refuse a blank string in a flow node's predicate slot — decision branch expression, screen field visibleWhen (#17493)

.changeset/17493-predicate-slot-blank-refused.md

@@ -0,0 +1,56 @@
+---
+'@objectstack/spec': minor
+'@objectstack/service-automation': minor
+'@objectstack/lint': minor
+---
+
+fix(spec)!: a blank string in a flow node's predicate slot — a `decision` branch `expression`, a screen field `visibleWhen` — is refused at authoring (#17493)
+
+Clause-②: no (narrowing)
+
+<!-- adr-0087: registered flow-predicate-slot-blank-string-refused -->
+
+**BREAKING** — an accept-set narrowing on two authored flow-node slots, 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 `decision` node's `config.conditions[].expression` and a
+`screen` node's `config.fields[].visibleWhen` are declared bare CEL text. A string
+that is blank after trimming (`''`, `'   '`, a tab or a newline) used to be
+accepted there by `FlowSchema.parse`, `AutomationEngine.registerFlow` and
+`objectstack validate`, and was then read as "no predicate": the evaluator answers
+a blank decision predicate `false`, so that branch was not taken, and nothing said
+so. It is now refused at those doors — by `FlowSchema.parse` with a `custom` issue
+anchored at the slot (for example `nodes.1.config.conditions.0.expression`), and
+by `registerFlow` and `objectstack validate` through that same parse — with a
+message that leads with the published `PREDICATE_SLOT_STRING_REFUSAL` sentence,
+the one these slots already answered with for a non-string value. Where such a
+value 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', expression: '   ' }]` on a `decision` node | the predicate you meant — `{ label: 'high', expression: 'record.amount > 10000' }` — or, to keep what the blank did, `expression: 'false'` |
+| `fields: [{ name: 'reason', visibleWhen: '' }]` on a `screen` node | the predicate you meant — `visibleWhen: "status == 'rejected'"` — or, to keep what the blank did, drop the `visibleWhen` key |
+
+**One-line fix:** write the predicate, or keep what the blank did — `'false'` on
+a decision branch (the value the blank evaluated to), no `visibleWhen` on a
+screen field (a blank one was read as absent). ⚠️ 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. A blank structural `condition` is another case —
+see the `flow-edge-condition-evaluated-slot-source-required` migration entry.
+
+**Unchanged.** A non-blank predicate parses, registers and validates as before;
+a non-string in these slots keeps its existing refusal at `registerFlow` and
+`objectstack validate`; `edges[].condition` and a node's `config.condition` keep
+their own rule and sentence (`EVALUATED_EXPRESSION_SOURCE_REQUIRED`); and
+`AutomationEngine.evaluateCondition` still answers a blank predicate `false` for
+a caller that reaches it directly. The `PREDICATE_SLOT_STRING_REFUSAL` constant
+keeps its name and now also names the blank string, so code matching the
+constant rather than a copy of its text is unaffected.

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

@@ -2235,10 +2235,18 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
         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);
+      it('leaves non-blank string predicates alone — and refuses the whitespace-only one (#17493)', () => {
+        // RE-JUDGED IN PLACE (#17493, ruling A 5651023407), not deleted. This
+        // pinned `atSlot('   ')` at ZERO findings: "The card states this
+        // boundary explicitly so nobody 'fixes' it: a whitespace-only STRING
+        // is 'not authored' on both sides and stays so." Both sides do still
+        // agree; the ruling is that the agreement is no defence when the
+        // author's rule is silently dropped, so the blank is refused here — on
+        // the same rule and sentence as the envelope above.
+        const blank = atSlot('   ');
+        expect(blank).toHaveLength(1);
+        expect(blank[0].severity).toBe('error');
+        expect(blank[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true);
         expect(atSlot("lead_record.status == 'converted'")).toHaveLength(0);
       });
     });
@@ -4293,3 +4301,91 @@ describe('masterDetailCount — an unreadable `reference` carrier is refused (#1
     expect(parentScope[0]!.message).toMatch(/declares no `master_detail` relationships/);
   });
 });
+
+/**
+ * [#17493] (ruling A, 5651023407) — a blank string in a ledger `predicate`
+ * slot, at the THIRD door: `objectstack validate`'s expression pass.
+ *
+ * `decision`'s `config.conditions[].expression` and `screen`'s
+ * `config.fields[].visibleWhen` reported NOTHING for `''` / `'   '`: the
+ * resolver skipped the blank as "not authored", so `validateStackExpressions`
+ * never saw it, and a branch carrying it was never taken at run time. The
+ * refusal is `predicateSlotRefusal`'s — the spec's one notion, shared with
+ * `FlowSchema.parse` and `registerFlow` — so the finding leads with the same
+ * published sentence the other two doors answer with.
+ *
+ * ⚠️ Through the CLI, `objectstack validate` meets these values first at its
+ * schema step (`FlowSchema.parse` refuses them there). This pass is what
+ * answers for a stack handed to `validateStackExpressions` directly, and it is
+ * what these pins drive.
+ */
+describe('a blank string in a ledger predicate slot (#17493)', () => {
+  const flowStack = (...middle: Record<string, unknown>[]) => ({
+    flows: [{
+      name: 'blank_flow',
+      nodes: [{ id: 'start', type: 'start' }, ...middle],
+      edges: [],
+    }],
+  });
+  const decision = (expression: unknown) =>
+    ({ id: 'check', type: 'decision', config: { conditions: [{ label: 'Yes', expression }] } });
+  const screen = (visibleWhen: unknown) =>
+    ({ id: 'form', type: 'screen', config: { fields: [{ name: 'amount', type: 'number', visibleWhen }] } });
+  const errorsOf = (stack: unknown) =>
+    validateStackExpressions(stack as never).filter((i) => (i.severity ?? 'error') === 'error');
+
+  describe.each(['', '   ', '\t\n '])('the blank %j', (blank) => {
+    it('decision branch `config.conditions[].expression` — one error, located at node and branch', () => {
+      const found = errorsOf(flowStack(decision(blank)));
+      expect(found).toHaveLength(1);
+      expect(found[0].severity).toBe('error');
+      expect(found[0].where).toBe("flow 'blank_flow' · node 'check' (decision) decision branch expression at config.conditions[0].expression");
+      expect(found[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true);
+      expect(found[0].source).toBe(blank);
+    });
+
+    it('screen field `config.fields[].visibleWhen` — one error, located at node and field', () => {
+      const found = errorsOf(flowStack(screen(blank)));
+      expect(found).toHaveLength(1);
+      expect(found[0].severity).toBe('error');
+      expect(found[0].where).toBe("flow 'blank_flow' · node 'form' (screen) screen field visibleWhen at config.fields[0].visibleWhen");
+      expect(found[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true);
+    });
+  });
+
+  it('reaches a `decision` inside an ADR-0031 region body', () => {
+    const found = errorsOf(flowStack({
+      id: 'sweep', type: 'loop',
+      config: { collection: '{items}', itemVariable: 'item', body: { nodes: [decision('   ')], edges: [] } },
+    }));
+    expect(found).toHaveLength(1);
+    expect(found[0].where).toContain("loop 'sweep' body");
+    expect(found[0].where).toContain('config.conditions[0].expression');
+    expect(found[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true);
+  });
+
+  describe('CONTROLS — what this must NOT move', () => {
+    it('RED CONTROL — the brace trap on the same slot still earns its own verdict, not the blank one', () => {
+      const found = errorsOf(flowStack(decision('{amount} > 1')));
+      expect(found).toHaveLength(1);
+      expect(found[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(false);
+    });
+
+    it('a non-blank predicate and an absent one report nothing', () => {
+      expect(errorsOf(flowStack(decision('amount > 1')))).toHaveLength(0);
+      expect(errorsOf(flowStack(screen('amount > 0')))).toHaveLength(0);
+      expect(errorsOf(flowStack(screen(undefined)))).toHaveLength(0);
+    });
+
+    it('the structural `config.condition` keeps its own rule and sentence', () => {
+      const found = errorsOf(flowStack({ id: 'gate', type: 'decision', config: { condition: '   ' } }));
+      expect(found).toHaveLength(1);
+      expect(found[0].message).toContain(EVALUATED_EXPRESSION_SOURCE_REQUIRED);
+      expect(found[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(false);
+    });
+
+    it("a `flow-template` slot's blank is untouched", () => {
+      expect(errorsOf(flowStack({ id: 'sweep', type: 'loop', config: { collection: '   ' } }))).toHaveLength(0);
+    });
+  });
+});

packages/lint/src/validate-expressions.ts

@@ -1167,6 +1167,11 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] {
     // 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.
+    //
+    // [#17493] The same call refuses a BLANK string too (the resolver emits it
+    // for this role). `objectstack validate` meets that value first at its
+    // schema step — `FlowSchema.parse` refuses it — so this is the pass that
+    // answers for a stack handed to `validateStackExpressions` directly.
     const shapeRefusal = predicateSlotRefusal(raw);
     if (shapeRefusal) {
       issues.push({ where, message: shapeRefusal.message, source: shapeRefusal.source, severity: 'error' });

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

@@ -281,14 +281,28 @@ describe('resolveFlowNodeExpressions — path resolution (#4027)', () => {
     expect(found[0].entry.role).toBe('flow-template');
   });
 
-  it('skips absent and empty values rather than inventing findings', () => {
+  it('skips absent values rather than inventing findings', () => {
     expect(resolveFlowNodeExpressions('screen', {})).toEqual([]);
     expect(resolveFlowNodeExpressions('screen', { fields: [] })).toEqual([]);
-    expect(resolveFlowNodeExpressions('screen', { fields: [{ visibleWhen: '   ' }] })).toEqual([]);
     // A repeater authored as a non-array must not throw.
     expect(resolveFlowNodeExpressions('screen', { fields: 'nope' })).toEqual([]);
   });
 
+  // RE-JUDGED IN PLACE (#17493, ruling A 5651023407), not deleted. The test
+  // above used to pin a whitespace-only `visibleWhen` as skipped too — "an
+  // empty value, not a finding" — on #15572's ground that the resolver and the
+  // evaluator treated the blank the same way. That agreement was ruled no
+  // defence: a blank predicate is an author's rule that was never written. So
+  // the blank is EMITTED for a `predicate` slot, for `registerFlow` and
+  // `objectstack validate` to refuse, and still skipped for a `flow-template`
+  // one, which the ruling did not reach.
+  it('emits a blank string in a predicate slot for the consumer to refuse (#17493)', () => {
+    expect(resolveFlowNodeExpressions('screen', { fields: [{ visibleWhen: '   ' }] })
+      .map((f) => [f.path, f.value, f.entry.role]))
+      .toEqual([['fields[0].visibleWhen', '   ', 'predicate']]);
+    expect(resolveFlowNodeExpressions('loop', { collection: '   ' })).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`,

packages/services/service-automation/src/decision-predicate-envelope.test.ts

@@ -102,14 +102,29 @@ describe('decision branch predicate — envelope in a `z.string()` slot (#15572)
     });
 
     /**
-     * The boundary the card states explicitly, so nobody "fixes" it: a
-     * whitespace-only STRING predicate behaves consistently on both sides —
-     * "not authored" at the ledger, `false` at the evaluator — and is left
-     * exactly as it was. Only the envelope shape moved.
+     * RE-JUDGED IN PLACE (#17493, ruling A 5651023407) — not deleted, because
+     * the boundary it drew was a real decision and the reason it moved belongs
+     * next to it.
+     *
+     * What it pinned: "The boundary the card states explicitly, so nobody
+     * 'fixes' it: a whitespace-only STRING predicate behaves consistently on
+     * both sides — 'not authored' at the ledger, `false` at the evaluator — and
+     * is left exactly as it was. Only the envelope shape moved." So
+     * `decisionFlow('str_ws', '   ')` was asserted to register.
+     *
+     * Why it moved: the ground is still TRUE — the two sides do agree — and
+     * was ruled insufficient. A blank branch predicate is an author who meant
+     * to write a rule; the parser skipping it and the evaluator answering
+     * `false` only proves the platform did not crash, while the branch is never
+     * taken and nothing says so. The ruling is the third instance of one rule
+     * (#17322's `config.condition`, #15811's evaluated `source`), so the blank
+     * string is now refused here too — by `FlowSchema.parse` inside
+     * `registerFlow`, under this slot's own sentence. A non-blank string and an
+     * absent predicate still register, exactly as this test always said.
      */
-    it('leaves string predicates alone — including the whitespace-only one', () => {
+    it('leaves non-blank string predicates alone — and refuses the whitespace-only one (#17493)', () => {
         expect(() => engine.registerFlow('str_ok', decisionFlow('str_ok', 'record.rating >= 4'))).not.toThrow();
-        expect(() => engine.registerFlow('str_ws', decisionFlow('str_ws', '   '))).not.toThrow();
+        expect(() => engine.registerFlow('str_ws', decisionFlow('str_ws', '   '))).toThrow(PREDICATE_SLOT_STRING_REFUSAL);
         expect(() => engine.registerFlow('str_absent', decisionFlow('str_absent', undefined))).not.toThrow();
     });
 

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

@@ -9522,6 +9522,14 @@ export class AutomationEngine implements IAutomationService {
                     // evaluation must be one set, and the refusal itself is the
                     // spec's, shared with `objectstack validate` so build and
                     // author time cannot disagree about the shape.
+                    //
+                    // [#17493] The same call now refuses a BLANK string too: the
+                    // resolver emits it for this role, and `predicateSlotRefusal`
+                    // refuses it under the same sentence. ⚠️ For that value this
+                    // is the second line, not the first — `FlowSchema.parse`
+                    // (in `canonicalizeStoredFlow`, above) refuses it before
+                    // this pass runs, the same way it meets a blank
+                    // `edge.condition` before `checkStructuralCondition` does.
                     const shapeRefusal = predicateSlotRefusal(found.value);
                     if (shapeRefusal) {
                         failures.push(`  • ${slotWhere}: ${shapeRefusal.message}\n      source: \`${shapeRefusal.source}\``);