feat(lint,docs): has(x) 不是 null 守卫 —— 发布期拒绝未守卫的可空比较 (#4763)

.changeset/has-is-not-a-null-guard-lint.md

@@ -0,0 +1,42 @@
+---
+"@objectstack/lint": minor
+---
+
+feat(lint): `has(x)` 不是 null 守卫 —— 发布期直接拒绝未守卫的可空比较 (#4763)
+
+CEL 的 `has(x)` 问的是**键是否存在**。自 #4649 起,谓词读到的记录对对象声明的每个
+字段都是**全量**的:一个声明了却存 `NULL` 的列同样"存在",所以
+`has(record.end_date)` 对声明字段恒为 `true`,什么也没告诉作者。于是这个读起来
+像守卫的写法根本不是守卫:
+
+```text
+has(record.start_date) && has(record.end_date) && record.end_date < record.start_date
+```
+
+它会走到 `null < null`,CEL 没有对应重载,整个谓词中断。#4761 之前中断被吞掉
+(规则跳过,一条 WARN),也就是说**这一形状的规则在任何含 null 值的行上从未生效
+过**——它写在元数据里、读起来完全正确、却什么都没有强制执行。#4761 把运行时改成
+fail-closed 之后,当场就在我们自己的两个示例对象里抓到了它。
+
+运行时拒绝是兜底,不是该学到这件事的地方:作者会在真实数据(很可能是生产数据)
+上收到一个 400,离写下规则可能已经过去几个月。而这个错误**仅凭元数据就可判定**
+——谓词的 AST 加上对象声明的字段类型,就足以判断某个操作数是否可能为 null。按
+AGENTS.md PD #12(在创作期拒绝,不要在消费端容忍),它属于发布闸门。
+
+**新增闸门(error,直接拒绝,没有降级开关)。** `os build` / `os validate` /
+`os lint` 与运行时发布闸门共用的 `validateStackExpressions` 现在会拒绝这样的谓词:
+对**声明为可空**的字段(没有 `required: true`、没有 `defaultValue`、没有默认选项、
+不是 autonumber)应用**排序**(`< <= > >=`)或**算术**(`+ - * / %`,含一元 `-`)
+运算符,而该操作数没有被同一布尔分支内支配它的 `!= null` / `== null` / `!isBlank()`
+显式判空所守卫。`has(x)` **刻意不**计入守卫——这正是本规则存在的理由。错误信息点名
+规则、操作数与修法,收尾句逐字取自 `rule-validator.ts` 的 `unevaluableRuleError`,
+两道闸门措辞完全一致。
+
+覆盖面(有意划定,而不是含糊地覆盖一半):对象**校验规则**(含 `conditional` 规则
+`then` / `otherwise` 里嵌套的谓词)与**生命周期 hook 的 `condition`** ——即真正由 CEL
+在全量记录上求值、会 fail-closed 的两类面。共享规则条件(下推成 SQL 过滤,`NULL > x`
+是三值逻辑,不会 fault)、flow 的扁平作用域条件(裸标识符可能是 flow 变量)与
+`Field.formula`(有自己的 #3306 `guard ? value : null` 处理)不在此列。
+
+对**未声明**键的 `has()` 完全不受影响——那才是它的正当用途:区分"这次 PATCH 里
+根本没提到这个键"与"显式写了 null"。示例应用无需改动即通过新闸门。

content/docs/data-modeling/validation.mdx

@@ -59,6 +59,10 @@ export const Order = ObjectSchema.create({
 Condition expressions are **CEL** (evaluated by `@objectstack/formula`), not Salesforce-style formulas. Reference the incoming record via `record.<field>`, use `==`/`!=`, `&&`/`||`, and helpers like `isBlank(x)` and `has(record.field)`. A string condition is accepted as authoring shorthand and normalized to `{ dialect: 'cel', source }` at build time.
 </Callout>
 
+<Callout type="warn">
+**`has(x)` is not a null guard.** Predicates see a record that is *total* over the object's declared fields, so `has(record.end_date)` is true even when the value is `NULL` — `has(a) && has(b) && a < b` then reaches `null < null`, CEL has no overload, and the whole rule aborts (the write is rejected fail-closed). Write `record.start_date != null && record.end_date != null && record.end_date < record.start_date` instead. Since #4763 the `has()` form is **rejected at build/publish**: an ordering or arithmetic operator over a declared nullable field needs a real `!= null` guard. `has()` over an *undeclared* key — "was this in the PATCH at all?" — is untouched.
+</Callout>
+
 ## Common Properties
 
 All validation types share these base properties:

packages/lint/package.json

@@ -25,6 +25,7 @@
     "typecheck": "tsc --noEmit"
   },
   "dependencies": {
+    "@marcbachmann/cel-js": "^8.0.0",
     "@objectstack/formula": "workspace:*",
     "@objectstack/sdui-parser": "workspace:*",
     "@objectstack/spec": "workspace:*",

packages/lint/src/index.ts

@@ -27,6 +27,16 @@ export type { WidgetBindingFinding, WidgetBindingSeverity } from './validate-wid
 export { validateStackExpressions } from './validate-expressions.js';
 export type { ExprIssue } from './validate-expressions.js';
 
+// #4763 — `has(x)` reads as a null guard and is not one. The decision procedure
+// is exported on its own so other authoring surfaces (cloud graph-lint, the AI
+// authoring path) reuse ONE verdict instead of re-deriving it.
+export {
+  findUnguardedNullableOperands,
+  nullGuardMessage,
+  NULL_GUARD_HINT,
+} from './validate-null-guards.js';
+export type { NullGuardFinding, NullGuardOptions } from './validate-null-guards.js';
+
 export { validateListViewMode, LIST_VIEW_FILTERS_IN_VIEWS_MODE } from './validate-list-view-mode.js';
 
 // [ADR-0078] The functional-completeness gate. All judgement lives in the shared

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

@@ -341,7 +341,13 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
             close_date: { type: 'date' },
             expected: { type: 'formula', formula: 'record.amount * record.probability / 100' },
           },
-          validations: [{ name: 'future', expression: 'record.close_date >= today()' }],
+          // The `!= null` guard is load-bearing since #4763: `close_date` is a
+          // declared NULLABLE field, and an un-guarded `>=` over it faults at
+          // runtime (`null >= timestamp` has no overload) — the null-guard gate
+          // rejects that shape at authoring now. Soundness (this block's
+          // subject) and null-guarding are separate verdicts; the predicate has
+          // to satisfy both to produce zero issues.
+          validations: [{ name: 'future', expression: 'record.close_date != null && record.close_date >= today()' }],
         }],
       });
       expect(issues).toHaveLength(0);
@@ -758,3 +764,203 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
     });
   });
 });
+
+// ───────────────────────────────────────────────────────────────────────
+// #4763 — `has(x)` reads as a null guard and is not one.
+//
+// Scope note (constraint of the issue, pinned here so it stays a decision):
+// this gate walks the AUTHORED METADATA the stack carries — object validation
+// rules and lifecycle-hook conditions. It never reads source files, so the
+// deliberately-bad fixtures in `packages/objectql/src/validation/rule-*.test.ts`
+// (which pin the runtime's fail-closed behaviour and MUST keep the bad shape)
+// are structurally out of its reach.
+// ───────────────────────────────────────────────────────────────────────
+describe('null-guard gate (#4763)', () => {
+  // Mirrors `showcase_project`: dates and money are declared but nullable;
+  // `status` carries a default option and `name` is required, so neither can
+  // be null and neither may ever be flagged.
+  const project = {
+    name: 'showcase_project',
+    fields: {
+      name: { type: 'text', required: true },
+      status: { type: 'select', options: [{ value: 'planned', default: true }, { value: 'active' }] },
+      start_date: { type: 'date' },
+      end_date: { type: 'date' },
+      budget: { type: 'currency' },
+      spent: { type: 'currency', defaultValue: 0 },
+    },
+  };
+  const withRule = (rule: Record<string, unknown>) =>
+    validateStackExpressions({ objects: [{ ...project, validations: [rule] }] });
+
+  it('REJECTS the `has(a) && has(b) && a < b` shape over nullable declared fields', () => {
+    const issues = withRule({
+      type: 'script',
+      name: 'end_after_start',
+      condition: 'has(record.start_date) && has(record.end_date) && record.end_date < record.start_date',
+    });
+    expect(issues.length).toBeGreaterThan(0);
+    expect(issues.every((i) => (i.severity ?? 'error') === 'error')).toBe(true);
+    const joined = issues.map((i) => i.message).join('\n');
+    // names the rule …
+    expect(joined).toContain("validation rule 'end_after_start'");
+    // … the operand …
+    expect(joined).toContain('record.end_date');
+    expect(joined).toContain('record.start_date');
+    // … and the fix, in the runtime's own words.
+    expect(joined).toContain("Guard it with '!= null'");
+    expect(joined).toContain('has(x)');
+    expect(issues[0].where).toContain("object 'showcase_project'");
+  });
+
+  it('ACCEPTS the `!= null` form (the fix #4761 landed in the examples)', () => {
+    expect(
+      withRule({
+        type: 'script',
+        name: 'end_after_start',
+        condition:
+          'record.start_date != null && record.end_date != null && record.end_date < record.start_date',
+      }),
+    ).toHaveLength(0);
+  });
+
+  it('ACCEPTS a guarded arithmetic predicate (showcase `spent_within_budget`)', () => {
+    expect(
+      withRule({
+        type: 'script',
+        name: 'spent_within_budget',
+        condition: 'record.budget != null && record.spent != null && record.spent > record.budget * 1.2',
+      }),
+    ).toHaveLength(0);
+  });
+
+  it('never flags a required field or one with a default (`spent`, `status`, `name`)', () => {
+    expect(
+      withRule({ type: 'script', name: 'spend_positive', condition: 'record.spent > 0' }),
+    ).toHaveLength(0);
+  });
+
+  it('reaches the predicates nested in a `conditional` rule’s then/otherwise', () => {
+    const issues = withRule({
+      type: 'conditional',
+      name: 'budget_sanity',
+      when: "record.status == 'active'",
+      then: { type: 'script', name: 'over_budget', condition: 'has(record.budget) && record.budget > 1' },
+    });
+    expect(issues.length).toBe(1);
+    expect(issues[0].message).toContain('record.budget');
+    expect(issues[0].where).toContain("'budget_sanity' then → 'over_budget'");
+  });
+
+  // Negative-case pin: the real `showcase_account` rule pair. Both use `has()`
+  // — legitimately, to tell "key absent from the PATCH" apart from "explicit
+  // null" — and both compare with EQUALITY only. They must stay legal; a rule
+  // that flags them is too broad.
+  it('leaves `showcase_account.churn_reason_consistency` alone (legitimate `has()`)', () => {
+    const issues = validateStackExpressions({
+      objects: [{
+        name: 'showcase_account',
+        fields: { status: { type: 'select', options: [{ value: 'churned' }] }, churn_reason: { type: 'text' } },
+        validations: [{
+          type: 'conditional',
+          name: 'churn_reason_consistency',
+          when: "record.status == 'churned'",
+          then: {
+            type: 'script',
+            name: 'churn_reason_present',
+            condition: "!has(record.churn_reason) || record.churn_reason == null || record.churn_reason == ''",
+          },
+          otherwise: {
+            type: 'script',
+            name: 'churn_reason_absent',
+            condition: "has(record.churn_reason) && record.churn_reason != null && record.churn_reason != ''",
+          },
+        }],
+      }],
+    });
+    expect(issues).toHaveLength(0);
+  });
+
+  describe('hook conditions — the third instance the issue named', () => {
+    const hookStack = (condition: string) => ({
+      objects: [project],
+      hooks: [{ name: 'project_budget_alert', object: 'showcase_project', condition }],
+    });
+
+    // Regression pin. `examples/app-showcase/src/data/hooks/index.ts` carried
+    // `has(record.spent) && has(record.budget) && record.spent > record.budget`
+    // until #4770/#4786 corrected it. This asserts the bad shape cannot come
+    // back: it is red today, and would have been red before that fix.
+    it('REJECTS the pre-#4786 showcase hook shape', () => {
+      const issues = validateStackExpressions(
+        hookStack('has(record.spent) && has(record.budget) && record.spent > record.budget'),
+      );
+      expect(issues.length).toBeGreaterThan(0);
+      expect(issues[0].where).toContain("hook 'project_budget_alert'");
+      expect(issues.map((i) => i.message).join('\n')).toContain('record.budget');
+    });
+
+    it('ACCEPTS the corrected shape now on `main`', () => {
+      expect(
+        validateStackExpressions(
+          hookStack('record.spent != null && record.budget != null && record.spent > record.budget'),
+        ),
+      ).toHaveLength(0);
+    });
+
+    it('applies per target for a multi-object hook', () => {
+      const issues = validateStackExpressions({
+        objects: [project, { name: 'other_obj', fields: { budget: { type: 'currency', required: true } } }],
+        hooks: [{ name: 'multi', object: ['showcase_project', 'other_obj'], condition: 'record.budget > 1' }],
+      });
+      // Only the object that declares `budget` nullable is flagged.
+      expect(issues).toHaveLength(1);
+      expect(issues[0].where).toContain('showcase_project');
+    });
+  });
+
+  describe('surfaces deliberately NOT covered', () => {
+    it('leaves sharing-rule conditions alone (compiled to a SQL filter, never faults)', () => {
+      expect(
+        validateStackExpressions({
+          objects: [project],
+          sharingRules: [{
+            name: 'big_budget',
+            object: 'showcase_project',
+            condition: "record.status == 'active' && record.budget > 100000",
+          }],
+        }),
+      ).toHaveLength(0);
+    });
+
+    it('leaves flattened flow conditions alone (a bare id may be a flow variable)', () => {
+      expect(
+        validateStackExpressions({
+          objects: [project],
+          flows: [{
+            name: 'escalate',
+            nodes: [
+              { id: 'start', type: 'start', config: { objectName: 'showcase_project' } },
+              { id: 'd', type: 'decision', config: { condition: 'record.budget > 100000' } },
+            ],
+            edges: [],
+          }],
+        }),
+      ).toHaveLength(0);
+    });
+
+    it('leaves `Field.formula` expressions alone (blessed `guard ? value : null`, #3306)', () => {
+      expect(
+        validateStackExpressions({
+          objects: [{
+            ...project,
+            fields: {
+              ...project.fields,
+              remaining: { type: 'formula', formula: 'record.budget - record.spent' },
+            },
+          }],
+        }),
+      ).toHaveLength(0);
+    });
+  });
+});