fix(lint): null-guard gate accuracy follow-ups: the runtime door's bare start-guard gap recorded, the summary-nulls presumption stated, an insert-safe guard for previous

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

@@ -1,3 +1,3 @@
 ---
 '@objectstack/lint': minor
 ---
@@ -12,6 +12,6 @@
 `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.
+- **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, so a bare start-condition guard proves nothing there: a flow whose start condition is `budget != null` and whose edge compares `record.budget > 200000` passes `os validate` (where the flow's bound set is provable) and is refused by the runtime publish gate. Spell the start guard `record.budget != null` and both accept it.
 - **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.
+- **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. That presumes `os migrate summary-nulls` ran: on a database upgraded in place without it, a parent row stored before roll-ups were seeded at create time, and no child of it written since, still reads `null`.

.changeset/29491-previous-guard-hint.md

@@ -0,0 +1,10 @@
+---
+'@objectstack/lint': patch
+---
+
+fix(lint): the null-guard message names the guard that holds on an insert for a `previous.` operand
+
+Clause-②: no
+Metadata-protocol: none
+
+The publish-time null-guard gate (`os build`, `os validate`, `os lint` and the runtime publish gate) reports an ordering or arithmetic operator applied to a nullable declared field with no `!= null` test in front of it, and tells the author to add one. For a `previous.FIELD` operand that advice alone faults on an insert, where `previous` is itself `null` (on a validation rule, a hook condition and a record-triggered flow's create leg alike). The message for such an operand now ends: guard it with `previous != null && previous.FIELD != null`. The shared sentence before it is unchanged and still reads word for word as the runtime's. Which predicates are refused does not change. Nothing to migrate.

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

@@ -8,7 +8,7 @@
 
 import { describe, it, expect } from 'vitest';
 
-import { validateStackExpressions } from './validate-expressions.js';
+import { runStackExpressionPasses, 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.
@@ -91,6 +91,33 @@ describe('flow conditions under the null-guard gate (#29026)', () => {
     })])).toEqual([]);
   });
 
+  it('records the two doors disagreeing on a BARE start guard: the runtime publish gate refuses what `os validate` accepts (#29491)', () => {
+    const guardedBy = (condition: string) => flow({
+      nodes: [start({ condition }), { 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: 'record.budget > 200000' },
+        { id: 'e3', source: 'triage', target: 'calm', isDefault: true },
+      ],
+    });
+    const atRuntimeGate = (f: Rec) =>
+      runStackExpressionPasses({ objects: [project], flows: [f] }, { runtimeWriteType: 'flow' })
+        .filter((i) => i.message.includes('declares as nullable'));
+
+    // A bare guard: the build door proves `record.budget` from it, the runtime
+    // door cannot (its bound set is not provable), so only the runtime refuses.
+    const bare = guardedBy('budget != null');
+    expect(nullGuardIssues([bare])).toEqual([]);
+    const refused = atRuntimeGate(bare);
+    expect(refused).toHaveLength(1);
+    expect(refused[0]!.where).toContain("edge 'e2'");
+
+    // The `record.` spelling of the same guard is accepted at both doors.
+    const spelled = guardedBy('record.budget != null');
+    expect(nullGuardIssues([spelled])).toEqual([]);
+    expect(atRuntimeGate(spelled)).toEqual([]);
+  });
+
   it('judges a `decision` branch expression', () => {
     const issues = nullGuardIssues([flow({
       nodes: [

packages/lint/src/validate-expressions.ts

@@ -499,10 +499,19 @@ function isNullableField(def: AnyRec): boolean {
 /**
  * [#29026] A `count` / `sum` roll-up summary is always valued: zero children is
  * zero. `@objectstack/objectql`'s `summaryEmptySetValue` is the authority — the
- * engine seeds a new parent row with it, falls back to it when an aggregate
- * over no rows answers `null`, and backfills a stored `NULL` with it — and this
- * package may not import a runtime, so the two functions are named here. A
- * `min` / `max` / `avg` over no rows IS `null`, and stays nullable. Measured on
+ * engine seeds a new parent row with it at create time
+ * (`initializeSummaryFields`) and falls back to it when a child write's
+ * recompute over no rows answers `null` — and this package may not import a
+ * runtime, so the two functions are named here. A `min` / `max` / `avg` over
+ * no rows IS `null`, and stays nullable.
+ *
+ * This judgment PRESUMES the one-off `os migrate summary-nulls` ran. Nothing
+ * backfills a stored `NULL` automatically: a parent row stored before the
+ * create-time seed shipped, and never visited by a child write since, keeps
+ * its `NULL` until that migration (`summary-backfill.ts`) recomputes it. On a
+ * database upgraded in place without it, such a row reads `null`, and a
+ * predicate this gate passed faults on it — a validation rule then refuses the
+ * write. Measured on
  * a record-triggered run: `showcase_expense_report.total_amount` (a `sum`)
  * reads `0` on a report with no lines
  * (`examples/app-showcase/test/notify-recipient-optional-fields.test.ts`).

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

@@ -289,6 +289,30 @@ describe('nullGuardMessage', () => {
     expect(msg).toContain(NULL_GUARD_HINT);
   });
 
+  it('names the guard that holds on an insert for a `previous.` operand, and only for one (#29491)', () => {
+    const fullGuard = '`previous != null && previous.budget != null`';
+    const [prior] = find('previous.budget > 5');
+    for (const outcome of ['fail-closed', 'run-fails'] as const) {
+      const msg = nullGuardMessage("validation rule 'r'", 'showcase_project', prior!, outcome);
+      // The shared sentence stays verbatim, and the clause follows it.
+      expect(msg).toContain(`${NULL_GUARD_HINT} On an insert \`previous\` is itself null`);
+      expect(msg).toContain(`guard it with ${fullGuard}.`);
+    }
+    const [current] = find('record.budget > 5');
+    expect(nullGuardMessage("validation rule 'r'", 'showcase_project', current!)).not.toContain('previous');
+    const [bare] = findUnguardedNullableOperands('budget > 5', { nullableFields, bareFields: new Set(['budget']) });
+    expect(nullGuardMessage('edge', 'showcase_project', bare!, 'run-fails')).not.toContain('previous');
+
+    // The guard it names passes this gate, and is the one that evaluates on
+    // an insert, where `previous` is bound to null: the `!= null` test alone
+    // faults there.
+    expect(find('previous != null && previous.budget != null && previous.budget > 5')).toEqual([]);
+    const onInsert = (source: string) =>
+      celEngine.evaluate({ dialect: 'cel', source }, { record: { budget: 1 }, extra: { previous: null } });
+    expect(onInsert('previous != null && previous.budget != null && previous.budget > 5')).toMatchObject({ ok: true, value: false });
+    expect(onInsert('previous.budget != null && previous.budget > 5').ok).toBe(false);
+  });
+
   it('closes with the runtime rejection wording verbatim (one voice, two gates)', () => {
     // Lifted from `unevaluableRuleError` in
     // packages/objectql/src/validation/rule-validator.ts — if that text moves,

packages/lint/src/validate-null-guards.ts

@@ -90,7 +90,7 @@
  * | field `requiredWhen`           | TOTAL   | same `merged` in `evaluateValidationRules` — fail-OPEN, so an unguarded predicate enforces NOTHING in silence | covered (#4811) |
  * | field `readonlyWhen`           | TOTAL   | `rule-validator.ts` `readonlyWhenBindings` materialises BOTH roots (#4953 clause 1, landed in #6454) | excluded — NOT on totality; see below |
  * | action `visible` / `disabled`  | sparse  | evaluated client-side; no materialization exists in `objectui` | excluded (decided — #4953 clause 2) |
- * | flow start / edge / `decision` condition — record-triggered flow | TOTAL | `record-change-trigger.ts` `buildContext` layers `previous` under the payload/after-row then runs BOTH `record` and `previous` through a structural-mirror `materializeDeclaredFields` (#4953 clause 1's other half, services lane); `seedRunVariables` flattens that same total record to bare names | covered (#29026), outcome `run-fails`; see below |
+ * | flow start / edge / `decision` condition — record-triggered flow | TOTAL | `record-change-trigger.ts` `buildContext` layers `previous` under the payload/after-row then runs BOTH `record` and `previous` through a structural-mirror `materializeDeclaredFields` (#4953 clause 1's other half, services lane); `seedRunVariables` flattens that same total record to bare names | covered (#29026), outcome `run-fails`; the runtime publish gate is stricter on a bare start guard; see below |
  * | the same slots, any other flow | sparse or unknown | the inbound hook hands the request body, an action an empty record carrying at most `id`, a `map` item its own keys; nothing materializes them | excluded |
  * | sharing-rule `condition`       | n/a     | compiled to a SQL filter; `NULL > x` is three-valued, never faults | excluded |
  * | field `expression` (`Field.formula`) | n/a | product judgement, not a wiring gap — see below              | excluded |
@@ -266,6 +266,21 @@
  *         before any node, whatever the entrance, so what it proves non-null
  *         is presumed at every edge and `decision` branch of the flow
  *         ({@link provenNonNullKeys}); an author does not re-guard downstream.
+ *         **The two doors disagree on a BARE start guard, and the runtime
+ *         publish gate is the stricter one.** `os validate` sees the flow's
+ *         entrances, so where the bound set is provable a bare
+ *         `budget != null` start condition proves `record.budget` (point 2)
+ *         and an edge comparing `record.budget > 200000` passes. The runtime
+ *         publish gate judges the written flow alone
+ *         (`perWriteSnapshotEntrance`): the bound set is not provable there,
+ *         no bare name is judged, the bare guard proves nothing, and the same
+ *         edge is refused. The stricter side stands: with the bound set not
+ *         provable, a bare name is not provably the field, and deriving the
+ *         presumption from it would be unsound. Nobody has measured a sound
+ *         way for that door to stand down on such a finding instead, so it
+ *         does not. Spell the start guard `record.budget != null` and
+ *         both doors accept the flow. Pinned in
+ *         `validate-expressions.flow-null-guards.test.ts`.
  *      4. **Outcome `run-fails`, not `fail-closed`.** `evaluateCondition`
  *         throws on the fault (ADR-0032 §1c) and the record-change trigger
  *         isolates the write, so the run fails and the write lands.
@@ -275,7 +290,10 @@
  *    `previous.x != null` itself faults there; the full guard is
  *    `previous != null && previous.x != null`. The gate still reports the
  *    unguarded `previous.x` (it faults on the update leg's null), and
- *    `previous != null` alone is not accepted as guarding `previous.x`.
+ *    `previous != null` alone is not accepted as guarding `previous.x`. The
+ *    message names that full guard for a `previous.` operand
+ *    ({@link PREVIOUS_GUARD_CLAUSE}), on every surface: a validation rule
+ *    and a hook condition bind `previous` to `null` on an insert too.
  *    Other declared predicate slots (a `screen` field's `visibleWhen`) are
  *    not judged: their fault does not take this outcome, and nobody has
  *    measured what it does take.
@@ -304,6 +322,18 @@ export const NULL_GUARD_HINT =
   `Guard it with '!= null'` +
   ` — 'has(x)' does NOT do that: a declared field holding null is still PRESENT, so has(x) is true.`;
 
+/**
+ * The clause {@link nullGuardMessage} adds after {@link NULL_GUARD_HINT} for a
+ * `previous.<f>` operand, and only for one. An insert binds `previous` itself to
+ * `null` (`previousBinding` in the rule validator, `seedRunVariables` on a
+ * flow's create leg), so `previous.<f> != null` faults there too and the guard
+ * that holds on both legs names `previous` first. Kept out of the shared hint,
+ * which stays verbatim with the runtime's sentence.
+ */
+const PREVIOUS_GUARD_CLAUSE = (operand: string): string =>
+  ` On an insert \`previous\` is itself null, so \`${operand} != null\` alone faults there too:` +
+  ` guard it with \`previous != null && ${operand} != null\`.`;
+
 /** Ordering + arithmetic operators — the ones with no `null` overload. */
 const FAULTING_BINARY_OPS = new Set(['<', '<=', '>', '>=', '+', '-', '*', '/', '%']);
 
@@ -770,6 +800,7 @@ export function nullGuardMessage(
     `as nullable (no \`required: true\`, no \`defaultValue\`).${hasNote} At runtime the operand is ` +
     `null, CEL has no \`${finding.operator}\` overload for null, and the whole predicate aborts — ` +
     `${OUTCOME_CLAUSE[outcome]}. ` +
-    `The predicate compares a value that is null. ${NULL_GUARD_HINT}`
+    `The predicate compares a value that is null. ${NULL_GUARD_HINT}` +
+    (finding.operand.startsWith('previous.') ? PREVIOUS_GUARD_CLAUSE(finding.operand) : '')
   );
 }