feat(lint): the publish-time null-guard gate judges a record-triggered flow's start, edge and decision-branch conditions

.changeset/29026-flow-condition-null-guard.md

@@ -0,0 +1,17 @@
+---
+'@objectstack/lint': minor
+---
+
+feat(lint): the publish-time null-guard gate judges a record-triggered flow's start, edge and decision-branch conditions
+
+Clause-②: no (narrowing)
+Metadata-protocol: narrowed
+
+<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no export or type of the package's public surface is retired: the publish-time null-guard gate now judges three predicate slots of a record-triggered flow that it already judges on validation rules and hook conditions. A flow it now refuses is fixed in the flow's own expression (add the `!= null` test the message names), so there is nothing for `objectstack migrate meta` to rewrite and no tombstone. -->
+
+`os build`, `os validate`, `os lint` and the runtime publish gate now refuse a record-triggered flow (start `triggerType: 'record-…'`) whose start condition, edge `condition` or `decision` branch `expression` applies an ordering (`< <= > >=`) or arithmetic (`+ - * / %`) operator to a nullable declared field of the trigger object with no `!= null` / `== null` / `!isBlank()` test in front of it. The run's `record` and `previous` carry every declared field, nulls included, so such a comparison faults with `no such overload` on the first record where the field is empty, and the run fails there (the write that fired it still lands).
+
+- **Who moves.** A flow condition such as `budget > 200000` on an optional `budget`. Fix: `budget != null && budget > 200000`, or put the `!= null` test in the flow's start condition, which gates every run, so every edge and branch after it may compare freely. `has(record.budget)` is not a guard: the key is always present.
+- **Bare names count.** The run spreads the record's fields to top level, so a bare `budget` is read as `record.budget`, unless the flow declares a variable or a node id of that name, or has a node that can bind names nobody declared (a `script`, `connector_action`, `wait`, `subflow` or `map`); those bare names are not judged. At the runtime publish gate bare names are not judged.
+- **Not judged.** Flows with no record trigger (an `api` flow's request body, an action's or `map` item's record are not made total), a `screen` field's `visibleWhen`, and `config.condition` on any node but `start`.
+- **Fewer findings elsewhere.** A `count` or `sum` roll-up summary is now always valued for this gate (zero children is zero), on validation rules and hook conditions too; `min` / `max` / `avg` roll-ups stay nullable.

packages/lint/src/validate-expressions.flow-null-guards.test.ts

@@ -0,0 +1,203 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+//
+// #29026 — the null-guard gate on a record-triggered flow's start, edge and
+// `decision` branch conditions. `validate-null-guards.test.ts` pins the
+// decision procedure; this file pins the wiring: which flows, which slots,
+// which bare names, and what the start condition proves downstream. The
+// ledger row in `validate-null-guards.ts` carries the runtime measurements.
+
+import { describe, it, expect } from 'vitest';
+
+import { validateStackExpressions } from './validate-expressions.js';
+
+// `budget` and `owner` are optional, as on `showcase_project`; `name` is
+// required and `spent` has a default, so neither may ever be flagged.
+const project = {
+  name: 'showcase_project',
+  fields: {
+    name: { type: 'text', required: true },
+    owner: { type: 'text' },
+    health: { type: 'select', options: [{ value: 'green', default: true }, { value: 'red' }] },
+    budget: { type: 'currency' },
+    spent: { type: 'currency', defaultValue: 0 },
+  },
+};
+
+type Rec = Record<string, unknown>;
+
+const start = (config: Rec = {}): Rec => ({
+  id: 'start',
+  type: 'start',
+  config: { objectName: 'showcase_project', triggerType: 'record-after-update', ...config },
+});
+
+const flow = (overrides: Rec): Rec => ({
+  name: 'escalate',
+  label: 'Escalate',
+  type: 'autolaunched',
+  nodes: [start(), { id: 'triage', type: 'decision' }, { id: 'alert', type: 'end' }, { id: 'calm', type: 'end' }],
+  edges: [
+    { id: 'e1', source: 'start', target: 'triage' },
+    { id: 'e2', source: 'triage', target: 'alert', condition: 'budget > 200000' },
+    { id: 'e3', source: 'triage', target: 'calm', isDefault: true },
+  ],
+  ...overrides,
+});
+
+/** The null-guard findings alone — other passes judge other things about these flows. */
+const nullGuardIssues = (flows: Rec[]) =>
+  validateStackExpressions({ objects: [project], flows }).filter((i) => i.message.includes('declares as nullable'));
+
+describe('flow conditions under the null-guard gate (#29026)', () => {
+  it('rejects the bare unguarded comparison that failed the showcase escalation run', () => {
+    const issues = nullGuardIssues([flow({})]);
+    expect(issues).toHaveLength(1);
+    const [issue] = issues;
+    expect(issue!.severity).toBe('error');
+    expect(issue!.where).toContain("edge 'e2'");
+    expect(issue!.path).toBe('flows[0].edges[1].condition');
+    expect(issue!.message).toContain('`budget`');
+    expect(issue!.message).toContain('the flow run fails at this condition');
+    expect(issue!.message).toContain("Guard it with '!= null'");
+  });
+
+  it('rejects the `record.` spelling too, and accepts either guarded spelling', () => {
+    const edges = (condition: string) => [
+      { id: 'e1', source: 'start', target: 'triage' },
+      { id: 'e2', source: 'triage', target: 'alert', condition },
+      { id: 'e3', source: 'triage', target: 'calm', isDefault: true },
+    ];
+    expect(nullGuardIssues([flow({ edges: edges('record.budget > 200000') })])).toHaveLength(1);
+    expect(nullGuardIssues([flow({ edges: edges('record.budget != null && record.budget > 200000') })])).toEqual([]);
+    expect(nullGuardIssues([flow({ edges: edges('budget != null && budget > 200000') })])).toEqual([]);
+    // `has()` is true on the materialized null, so it is no guard here either.
+    const hasOnly = nullGuardIssues([flow({ edges: edges('has(record.budget) && record.budget > 200000') })]);
+    expect(hasOnly).toHaveLength(1);
+    expect(hasOnly[0]!.message).toContain('`has(record.budget)` does not guard it');
+  });
+
+  it('judges the start condition, and presumes what it proves at every edge downstream', () => {
+    const unguardedStart = nullGuardIssues([flow({
+      nodes: [start({ condition: 'health == "red" && budget > 100000' }), { id: 'triage', type: 'decision' }, { id: 'alert', type: 'end' }, { id: 'calm', type: 'end' }],
+      edges: [{ id: 'e1', source: 'start', target: 'triage' }],
+    })]);
+    expect(unguardedStart).toHaveLength(1);
+    expect(unguardedStart[0]!.where).toContain("node 'start' (start) condition");
+    expect(unguardedStart[0]!.path).toBe('flows[0].nodes[0].config.condition');
+
+    // The start condition gates every run, so its guard holds at e2.
+    expect(nullGuardIssues([flow({
+      nodes: [start({ condition: 'budget != null && budget > 100000' }), { id: 'triage', type: 'decision' }, { id: 'alert', type: 'end' }, { id: 'calm', type: 'end' }],
+    })])).toEqual([]);
+  });
+
+  it('judges a `decision` branch expression', () => {
+    const issues = nullGuardIssues([flow({
+      nodes: [
+        start(),
+        { id: 'triage', type: 'decision', config: { conditions: [{ label: 'critical', expression: 'record.budget > 200000' }, { label: 'normal', expression: 'true' }] } },
+        { id: 'alert', type: 'end' },
+        { id: 'calm', type: 'end' },
+      ],
+      edges: [{ id: 'e1', source: 'start', target: 'triage' }],
+    })]);
+    expect(issues).toHaveLength(1);
+    expect(issues[0]!.where).toContain('conditions[0].expression');
+  });
+
+  it('judges an edge inside a container region', () => {
+    const issues = nullGuardIssues([flow({
+      nodes: [
+        start(),
+        {
+          id: 'fan',
+          type: 'parallel',
+          config: {
+            branches: [{
+              name: 'Owner',
+              nodes: [{ id: 'gate', type: 'decision' }, { id: 'ping', type: 'end' }],
+              edges: [{ id: 'b1', source: 'gate', target: 'ping', condition: 'record.budget * 2 > 10' }],
+            }],
+          },
+        },
+      ],
+      edges: [{ id: 'e1', source: 'start', target: 'fan' }],
+    })]);
+    expect(issues).toHaveLength(1);
+    expect(issues[0]!.where).toContain("edge 'b1'");
+  });
+
+  it('leaves a flow with no record trigger alone: nothing makes its record total', () => {
+    const noTrigger = flow({ nodes: [{ id: 'start', type: 'start', config: { objectName: 'showcase_project' } }, { id: 'triage', type: 'decision' }, { id: 'alert', type: 'end' }, { id: 'calm', type: 'end' }] });
+    expect(nullGuardIssues([noTrigger])).toEqual([]);
+    const api = flow({ type: 'api', nodes: [{ id: 'start', type: 'start', config: { objectName: 'showcase_project', triggerType: 'api' } }, { id: 'triage', type: 'decision' }, { id: 'alert', type: 'end' }, { id: 'calm', type: 'end' }] });
+    expect(nullGuardIssues([api])).toEqual([]);
+  });
+
+  it('leaves a bare name a declared variable holds alone — the variable wins — and still judges `record.`', () => {
+    const variables = [{ name: 'budget', type: 'number', defaultValue: 5 }];
+    expect(nullGuardIssues([flow({ variables })])).toEqual([]);
+    const issues = nullGuardIssues([flow({
+      variables,
+      edges: [
+        { id: 'e1', source: 'start', target: 'triage' },
+        { id: 'e2', source: 'triage', target: 'alert', condition: 'record.budget > 200000' },
+        { id: 'e3', source: 'triage', target: 'calm', isDefault: true },
+      ],
+    })]);
+    expect(issues).toHaveLength(1);
+  });
+
+  it('leaves bare names alone on a flow whose nodes can bind names nobody declared', () => {
+    const withScript = (condition: string) => flow({
+      nodes: [start(), { id: 'calc', type: 'script', config: { function: 'calc' } }, { id: 'triage', type: 'decision' }, { id: 'alert', type: 'end' }, { id: 'calm', type: 'end' }],
+      edges: [
+        { id: 'e0', source: 'start', target: 'calc' },
+        { id: 'e1', source: 'calc', target: 'triage' },
+        { id: 'e2', source: 'triage', target: 'alert', condition },
+        { id: 'e3', source: 'triage', target: 'calm', isDefault: true },
+      ],
+    });
+    expect(nullGuardIssues([withScript('budget > 200000')])).toEqual([]);
+    expect(nullGuardIssues([withScript('record.budget > 200000')])).toHaveLength(1);
+  });
+
+  it('does not judge an inert `config.condition` on a non-start node', () => {
+    expect(nullGuardIssues([flow({
+      nodes: [start(), { id: 'triage', type: 'decision', config: { condition: 'record.budget > 1' } }, { id: 'alert', type: 'end' }, { id: 'calm', type: 'end' }],
+      edges: [{ id: 'e1', source: 'start', target: 'triage' }],
+    })])).toEqual([]);
+  });
+
+  it('treats a `count` / `sum` roll-up as always valued, and a `max` one as nullable', () => {
+    // Zero children is zero for `count` / `sum` (objectql's
+    // `summaryEmptySetValue`); `min` / `max` / `avg` over no rows is `null`.
+    const report = {
+      name: 'expense_report',
+      fields: {
+        status: { type: 'select', options: [{ value: 'draft', default: true }, { value: 'submitted' }] },
+        total_amount: { type: 'summary', summaryOperations: { object: 'expense_line', field: 'amount', function: 'sum' } },
+        line_count: { type: 'summary', summaryOperations: { object: 'expense_line', function: 'count' } },
+        largest_line: { type: 'summary', summaryOperations: { object: 'expense_line', field: 'amount', function: 'max' } },
+      },
+    };
+    const quorum = (condition: string): Rec => ({
+      name: 'quorum',
+      label: 'Quorum',
+      type: 'autolaunched',
+      nodes: [{ id: 'start', type: 'start', config: { objectName: 'expense_report', triggerType: 'record-after-update', condition } }, { id: 'end', type: 'end' }],
+      edges: [{ id: 'e1', source: 'start', target: 'end' }],
+    });
+    const judge = (condition: string) =>
+      validateStackExpressions({ objects: [report], flows: [quorum(condition)] })
+        .filter((i) => i.message.includes('declares as nullable'));
+    expect(judge('status == "submitted" && total_amount >= 5000 && line_count > 1')).toEqual([]);
+    expect(judge('largest_line > 1000').map((i) => i.message)).toEqual([expect.stringContaining('`largest_line`')]);
+  });
+
+  it('never flags a non-nullable field', () => {
+    expect(nullGuardIssues([flow({
+      edges: [{ id: 'e1', source: 'start', target: 'triage' }, { id: 'e2', source: 'triage', target: 'alert', condition: 'spent > 5 && size(name) > 1' }],
+    })])).toEqual([]);
+  });
+});

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

@@ -37,8 +37,11 @@ import type { ExprIssue } from './validate-expressions.js';
 import { validateSemanticRoles, SEMANTIC_ROLE_FIELD_UNPROVISIONED } from './validate-semantic-roles.js';
 
 describe('validateStackExpressions (ADR-0032 build-time)', () => {
+  // [#29026] `rating` is required: a record-triggered flow comparing an
+  // optional field with no `!= null` in front is refused by the null-guard
+  // gate, and this suite's flows compare `rating` to pin other verdicts.
   const objects = [
-    { name: 'crm_lead', fields: { rating: { type: 'number' }, status: { type: 'text' } } },
+    { name: 'crm_lead', fields: { rating: { type: 'number', required: true }, status: { type: 'text' } } },
   ];
 
   it('passes a clean stack', () => {
@@ -3207,28 +3210,30 @@ describe('null-guard gate (#4763)', () => {
       ).toHaveLength(0);
     });
 
-    // #4811 re-measured the reason this one is excluded. It is NOT the
-    // flattened scope (this gate never resolves a bare identifier — only
-    // `record.<f>`/`previous.<f>`, and a record-triggered run binds both
-    // roots — #22677). It is that `record-change-trigger.ts` seeds the flow's
-    // record as `{ ...(inputData ?? {}), ...after }` (spelled `inputDoc` until
-    // #5671 dropped that alias read) with no `materializeDeclaredFields`,
-    // so a declared column the write never mentioned is an ABSENT key — and on
-    // an absent key the `!= null` this gate prescribes faults exactly like the
-    // comparison it was meant to guard.
-    it('leaves flow conditions alone (trigger record is not total over declared fields)', () => {
+    // RE-JUDGED IN PLACE (#29026). This pinned `toHaveLength(0)` while the
+    // record-change trigger seeded the flow's record sparse; since #4953 it
+    // materializes every declared field, so a record-triggered flow's edge is
+    // covered and `validate-expressions.flow-null-guards.test.ts` pins the
+    // wiring. What stays out is pinned here: a non-start node's
+    // `config.condition` (inert — nothing evaluates it), and a flow with no
+    // record trigger, whose record nothing makes total.
+    it('judges a record-triggered flow edge, and leaves an inert node condition and an untriggered flow alone', () => {
+      const flow = (triggerType?: string) => ({
+        name: 'escalate',
+        nodes: [
+          { id: 'start', type: 'start', config: { objectName: 'showcase_project', ...(triggerType ? { triggerType } : {}) } },
+          { id: 'd', type: 'decision', config: { condition: 'record.budget > 100000' } },
+        ],
+        edges: [{ id: 'e1', source: 'd', target: 'end', condition: 'record.spent > record.budget' }],
+      });
+      const triggered = validateStackExpressions({ objects: [project], flows: [flow('record-after-update')] });
+      expect(triggered.map((i) => i.where)).toEqual([expect.stringContaining("edge 'e1'")]);
+      expect(triggered[0]!.message).toContain('`record.budget`');
+      // (With no trigger the run binds no `record` at all, which the flow CEL
+      // root judge reports; that is its verdict, not this gate's.)
       expect(
-        validateStackExpressions({
-          objects: [project],
-          flows: [{
-            name: 'escalate',
-            nodes: [
-              { id: 'start', type: 'start', config: { objectName: 'showcase_project', triggerType: 'record-after-update' } },
-              { id: 'd', type: 'decision', config: { condition: 'record.budget > 100000' } },
-            ],
-            edges: [{ id: 'e1', source: 'd', target: 'end', condition: 'record.spent > record.budget' }],
-          }],
-        }),
+        validateStackExpressions({ objects: [project], flows: [flow()] })
+          .filter((i) => i.message.includes('declares as nullable')),
       ).toHaveLength(0);
     });
 
@@ -3435,10 +3440,20 @@ const READ_SURFACES: Array<{ receiver: string; expected: string[]; declaredBy: s
   {
     receiver: 'def',
     // `referenceTo` is absent — likewise.
-    expected: ['defaultValue', 'options', 'reference', 'required', 'type'],
+    // [#29026] `summaryOperations` joined: a `count` / `sum` roll-up is
+    // always valued for the null-guard gate, and its function is read below.
+    expected: ['defaultValue', 'options', 'reference', 'required', 'summaryOperations', 'type'],
     declaredBy: 'FieldSchema',
     keys: () => Object.keys(FieldSchema.shape),
   },
+  {
+    // [#29026] The roll-up's aggregate function — the null-guard gate's
+    // nullability judgment reads it and nothing else here.
+    receiver: 'summaryOperations',
+    expected: ['function'],
+    declaredBy: 'FieldSchema.summaryOperations',
+    keys: () => shapeKeysOf(FieldSchema.shape.summaryOperations),
+  },
   {
     // [#24798] One `bulkActionDefs[]` entry: its `confirmText` template slot,
     // and its `name` for the location (its params are read by the spec's
@@ -3712,6 +3727,11 @@ describe('validateStackExpressions — reads only keys the spec declares (meta-t
       // node walk's own sites. Its keys are `Map.prototype.set` / `get`, never
       // metadata keys; the config it describes is read off the tabled `cfg`.
       'nodeSlotKeys',
+      // [#29026] The flow null-guard wiring: the flow CEL root judge's own
+      // result (`provable`), the flow's declared-name set and the gate's
+      // bare-name set. Their keys are that module's result key and Set
+      // methods, never metadata keys.
+      'rootScope', 'declaredVariables', 'nullGuardBare',
     ]);
     expect(receivers.filter((r) => !tabled.has(r) && !PLUMBING.has(r))).toEqual([]);
   });
@@ -4618,8 +4638,10 @@ describe('assignment value envelope — located findings (#15137)', () => {
  * controls below are those two shapes staying green.
  */
 describe('structural condition shape (#15662)', () => {
+  // [#29026] `rating` is required so the record-triggered null-guard gate
+  // stays out of the shape verdicts this suite pins (its controls compare it).
   const objects = [
-    { name: 'crm_lead', fields: { rating: { type: 'number' }, status: { type: 'text' } } },
+    { name: 'crm_lead', fields: { rating: { type: 'number', required: true }, status: { type: 'text' } } },
   ];
 
   const flowWith = (opts: { startCondition?: unknown; decisionCondition?: unknown; edgeCondition?: unknown }) => ({
@@ -4785,8 +4807,10 @@ describe('structural condition shape (#15662)', () => {
  * one shared refusal to prevent.
  */
 describe('blank structural condition (#17495)', () => {
+  // [#29026] `rating` is required so the record-triggered null-guard gate
+  // stays out of the shape verdicts this suite pins (its controls compare it).
   const objects = [
-    { name: 'crm_lead', fields: { rating: { type: 'number' }, status: { type: 'text' } } },
+    { name: 'crm_lead', fields: { rating: { type: 'number', required: true }, status: { type: 'text' } } },
   ];
 
   const flowWith = (opts: { startCondition?: unknown; decisionCondition?: unknown; edgeCondition?: unknown }) => ({