fix(objectql): 字段 readonlyWhen 求值补 materializeDeclaredFields —— 服务端第三个接缝 (#4953)

.changeset/readonly-when-total-record.md

@@ -0,0 +1,38 @@
+---
+"@objectstack/objectql": patch
+---
+
+fix(objectql): 字段 `readonlyWhen` 在服务端看到的记录改为「对象声明的全量形状」(#4953)
+
+`materializeDeclaredFields`(#1871 / #4649)此前只接在两个求值接缝上:
+`evaluateValidationRules`(对象级校验规则、字段 `requiredWhen`、option
+`visibleWhen`)与生命周期 hook 的 `condition`。**字段 `readonlyWhen` 不在其中** ——
+写入路径上的 `stripReadonlyWhenFields` / `stripReadonlyWhenFieldsMulti` 直接把
+`{ ...previous, ...data }` 交给 CEL 求值。
+
+后果是同一个字段上的两条谓词对「记录是什么」给出相反答案:``requiredWhen:
+P`record.approved_at == null` `` 是一条可用的守卫,而写在同一字段上的
+``readonlyWhen: P`record.approved_at == null` `` 只要驱动没把 `approved_at`
+这一列回读出来就会 fault;**而 `readonlyWhen` fault 是 fail-open**,于是作者声明
+为冻结的字段被照常写入。某次写入是否被拦,取决于驱动回读了哪些列 —— 作者既看不见
+也控制不了的存储细节。
+
+本次把这两个 strip 的 `record` 与 `previous` 两个根都过 `materializeDeclaredFields`,
+按维护者 2026-08-06 裁决(#4953)统一**服务端**接缝。
+
+**这是一次可见的行为变化,方向如下:**
+
+- 稀疏行上原本 fault→放行的谓词现在正常求值,谓词为真则改动被剥离(即恢复本应生效的
+  只读约束)。`record.x == null` / `!= null` / `previous.x == null` 都属此类。
+- 相应地,`has(record.<已声明字段>)` 在全量绑定下恒为 `true`(物化出的 `null` 是一个
+  「存在且值为 null」的键,这是 CEL 自身的规则),`!has(record.<已声明字段>)` 恒为
+  `false`。因此以 `readonlyWhen: !has(record.x)` 表达「x 为空时冻结」的写法**不再锁住
+  字段** —— 它原本也不是一条保证(在回读全部列的驱动上它从来不锁),现在它变成确定的
+  `false`。要表达「为空时冻结」请改写为 `record.x == null`(即 `@objectstack/lint`
+  null-guard 闸门一直建议的写法)。
+
+未改动的部分:`readonlyWhen` 的 fail-open 策略本身;#4889 的 `parent` 未绑定 ⇒
+**LOCKED** 判定(`parent` 是另一个对象的行,不做物化);对象级 `script` /
+`cross_field` 自 #4649 起的 fail-closed;INSERT 仍不走 `readonlyWhen` 剥离。
+未读到前序行时(引擎未取或行已不存在)**不做**物化 —— 那样不是补齐缺失值,而是
+凭空捏造一个与库中行相矛盾的值。

packages/objectql/src/declared-fields.ts

@@ -3,15 +3,28 @@
 /**
  * Make a record TOTAL over an object's DECLARED fields.
  *
- * Shared by the two places that evaluate a CEL expression against "the
- * record": object-level validation predicates
- * (`validation/rule-validator.ts`, #1871 / #4649) and declarative hook
- * `condition`s (`hook-wrappers.ts`, #4770). They used to disagree — a
- * predicate saw a total record while a hook condition saw only the fields the
- * current write happened to carry — which is precisely the drift this module
- * exists to prevent: an author cannot be expected to know that the same
- * `record.done == true` means two different things depending on which surface
- * reads it.
+ * Shared by the SERVER-side places that evaluate a CEL expression against "the
+ * record": object-level validation predicates + field `requiredWhen` +
+ * option `visibleWhen` (`validation/rule-validator.ts`, #1871 / #4649),
+ * declarative hook `condition`s (`hook-wrappers.ts`, #4770), and the field
+ * `readonlyWhen` strips on the write path (`validation/rule-validator.ts`
+ * `readonlyWhenBindings`, #4953). They used to disagree — a predicate saw a
+ * total record while a hook condition saw only the fields the current write
+ * happened to carry — which is precisely the drift this module exists to
+ * prevent: an author cannot be expected to know that the same `record.done ==
+ * true` means two different things depending on which surface reads it.
+ *
+ * Two bindings are still sparse, and the difference between them matters:
+ *
+ *  - The flow trigger record (`packages/triggers/trigger-record-change`) is a
+ *    server seam the same ruling puts on this list; it is simply not wired yet
+ *    (services lane, #4953 item 1's other half). Do not read its absence as a
+ *    decision.
+ *  - objectui's action `visible` / `disabled` binds whatever record the client
+ *    already fetched. That one is a DECISION (#4953 item 2): making it total
+ *    would mean every REST read padding out all declared columns, so it stays
+ *    sparse and is documented as sparse — an author on that surface guards with
+ *    `has()`, not `!= null`.
  *
  * CEL is strict about missing keys: `record.x` on a record that does not carry
  * the key `x` aborts the whole expression with `No such key`, which is NOT the

packages/objectql/src/validation/rule-validator.test.ts

@@ -452,6 +452,195 @@ describe('hasParentScopedRequiredWhen (#4977 gate)', () => {
   });
 });
 
+// #4953 — the record a `readonlyWhen` predicate sees is TOTAL over the object's
+// DECLARED fields, like the two seams that were materialised in #4649/#4770.
+// Before this, `stripReadonlyWhenFields` merged `{...previous, ...data}` raw, so
+// a predicate reading a declared column the DRIVER did not echo back faulted —
+// and a faulting `readonlyWhen` fails open, i.e. WROTE the field the author
+// declared frozen. Which columns come back is a storage property no author can
+// see, so the same declaration was enforced or not depending on the driver.
+const sparseLockFields = {
+  fields: {
+    notes: { type: 'text' },
+    approved_at: { type: 'datetime' },
+    // "while nothing has been approved, the amount is frozen" — the `== null`
+    // spelling #4649 made the supported one (and the null-guard gate prescribes).
+    amount: { type: 'currency', readonlyWhen: 'record.approved_at == null' },
+  },
+};
+
+/** A prior row from a driver that stores only the columns a write touched. */
+const sparsePrior = () => ({ id: 'r1', amount: 100 });
+/** The same row from a driver that returns every declared column. */
+const totalPrior = (approvedAt: unknown) => ({ id: 'r1', amount: 100, notes: null, approved_at: approvedAt });
+
+describe('readonlyWhen binds a TOTAL record (#4953)', () => {
+  it('evaluates `record.<declared> == null` on a SPARSE prior instead of faulting through', () => {
+    // THE bug. Pre-#4953: `No such key: approved_at` ⇒ fail-open ⇒ amount written.
+    const warnings: string[] = [];
+    const out = stripReadonlyWhenFields(sparseLockFields, { amount: 999 }, sparsePrior(), {
+      warn: (m: string) => warnings.push(m),
+    } as never);
+    expect(out).toEqual({});
+    expect(warnings.some((w) => w.includes('failed to evaluate'))).toBe(false);
+    expect(warnings.some((w) => w.includes('is read-only (readonlyWhen)'))).toBe(true);
+  });
+
+  it('still KEEPS the change when the materialised value makes the predicate FALSE', () => {
+    // Materialising is not "lock everything": the row HAS an approval date, so
+    // the lock is off and the legitimate edit lands.
+    expect(
+      stripReadonlyWhenFields(sparseLockFields, { amount: 999 }, { id: 'r1', amount: 100, approved_at: '2026-01-01' }),
+    ).toEqual({ amount: 999 });
+  });
+
+  it('reads the same verdict on a sparse prior as on a total one (the point)', () => {
+    // One declaration, two drivers, one answer. This equality is the guarantee;
+    // before #4953 the left side kept the change and the right side stripped it.
+    const sparse = stripReadonlyWhenFields(sparseLockFields, { amount: 999 }, sparsePrior());
+    const total = stripReadonlyWhenFields(sparseLockFields, { amount: 999 }, totalPrior(null));
+    expect(sparse).toEqual(total);
+    expect(sparse).toEqual({});
+  });
+
+  it('materialises the `previous` root too, not just `record`', () => {
+    const schema = { fields: { ...sparseLockFields.fields, amount: { type: 'currency', readonlyWhen: 'previous.approved_at == null' } } };
+    expect(stripReadonlyWhenFields(schema, { amount: 999 }, sparsePrior())).toEqual({});
+    expect(stripReadonlyWhenFields(schema, { amount: 999 }, { id: 'r1', amount: 100, approved_at: '2026-01-01' })).toEqual({ amount: 999 });
+  });
+
+  it('applies on the BULK path identically — one payload, N sparse rows', () => {
+    // A bulk write must not judge the same predicate by a different record
+    // shape than a single-id write does.
+    expect(stripReadonlyWhenFieldsMulti(sparseLockFields, { amount: 999 }, [sparsePrior()])).toEqual({});
+    // ≥1 locked row still drops it for the batch; no locked row still writes.
+    expect(stripReadonlyWhenFieldsMulti(sparseLockFields, { amount: 999 }, [
+      { id: 'r1', amount: 1, approved_at: '2026-01-01' },
+      sparsePrior(),
+    ])).toEqual({});
+    expect(stripReadonlyWhenFieldsMulti(sparseLockFields, { amount: 999 }, [
+      { id: 'r1', amount: 1, approved_at: '2026-01-01' },
+      { id: 'r2', amount: 2, approved_at: '2026-02-02' },
+    ])).toEqual({ amount: 999 });
+  });
+
+  it('does NOT materialise when the prior row is not in hand (no fabrication)', () => {
+    // `declared-fields.ts`'s standing rule: without the persisted state,
+    // defaulting a declared field to null would FABRICATE a value that
+    // contradicts the stored row. So this case keeps the historical fault →
+    // fail-open exit, and the engine avoids it by fetching the prior row
+    // whenever the object declares a readonlyWhen field (`needsPriorRecord`).
+    const warnings: string[] = [];
+    const out = stripReadonlyWhenFields(sparseLockFields, { amount: 999 }, null, {
+      warn: (m: string) => warnings.push(m),
+    } as never);
+    expect(out).toEqual({ amount: 999 });
+    expect(warnings.some((w) => w.includes('failed to evaluate — change allowed through'))).toBe(true);
+  });
+
+  it('never mutates the caller\'s prior record (it is the engine\'s hookContext.previous)', () => {
+    const prior = sparsePrior();
+    stripReadonlyWhenFields(sparseLockFields, { amount: 999 }, prior);
+    expect('approved_at' in prior).toBe(false);
+    expect('notes' in prior).toBe(false);
+    const rows = [sparsePrior()];
+    stripReadonlyWhenFieldsMulti(sparseLockFields, { amount: 999 }, rows);
+    expect('approved_at' in rows[0]!).toBe(false);
+  });
+
+  it('leaves the fail-open branch ALIVE — an ordering comparison still faults over a total record', () => {
+    // `null < null` is `no such overload`, so materialising does not make every
+    // predicate evaluable. This is exactly why the null-guard gate exists.
+    const warnings: string[] = [];
+    const out = stripReadonlyWhenFields(
+      { fields: { ...sparseLockFields.fields, amount: { type: 'currency', readonlyWhen: 'record.notes < record.approved_at' } } },
+      { amount: 999 },
+      sparsePrior(),
+      { warn: (m: string) => warnings.push(m) } as never,
+    );
+    expect(out).toEqual({ amount: 999 });
+    expect(warnings.some((w) => w.includes('failed to evaluate — change allowed through'))).toBe(true);
+  });
+
+  it('keeps fail-OPEN for an UNDECLARED key — materialising covers declared fields only', () => {
+    // The #4649 line, unmoved: a typo must stay unevaluable so it is reported,
+    // not silently read as null.
+    const warnings: string[] = [];
+    expect(stripReadonlyWhenFields(
+      { fields: { amount: { type: 'currency', readonlyWhen: 'record.stauts == null' } } },
+      { amount: 999 },
+      { id: 'r1', amount: 100 },
+      { warn: (m: string) => warnings.push(m) } as never,
+    )).toEqual({ amount: 999 });
+    expect(warnings.some((w) => w.includes('failed to evaluate — change allowed through'))).toBe(true);
+  });
+
+  // ── the consequence that moves the OTHER way, pinned rather than discovered ──
+  it('`has(record.<declared>)` is uniformly TRUE — so it locks even on a sparse prior', () => {
+    // CEL's own rule: a materialised `null` is a PRESENT key holding null.
+    // `has()` therefore guards against an UNDECLARED key, not an empty value.
+    expect(stripReadonlyWhenFields(
+      { fields: { ...sparseLockFields.fields, amount: { type: 'currency', readonlyWhen: 'has(record.approved_at)' } } },
+      { amount: 999 },
+      sparsePrior(),
+    )).toEqual({});
+  });
+
+  it('`!has(record.<declared>)` is uniformly FALSE — a lock spelled that way STOPS locking', () => {
+    // The one verdict this change moves toward "allowed": pre-#4953 the sparse
+    // binding made `!has(...)` true and the field was stripped. It was never a
+    // guarantee — on a driver returning all columns the same declaration never
+    // locked anything — so the flip replaces a storage-dependent verdict with a
+    // deterministic one, and the deterministic answer is FALSE. An author who
+    // means "while the field is empty" writes `== null` (the spelling
+    // @objectstack/lint's null-guard gate prescribes).
+    expect(stripReadonlyWhenFields(
+      { fields: { ...sparseLockFields.fields, amount: { type: 'currency', readonlyWhen: '!has(record.approved_at)' } } },
+      { amount: 999 },
+      sparsePrior(),
+    )).toEqual({ amount: 999 });
+  });
+
+  // ── blast radius: nothing else at this write gate moves ─────────────────
+  it('does not touch the object-level rules — `script` / `cross_field` stay fail-CLOSED (#4649)', () => {
+    const withRules = {
+      fields: { ...sparseLockFields.fields },
+      validations: [
+        { type: 'script', name: 'typo_rule', message: 'nope', condition: 'record.stauts == null' },
+      ],
+    };
+    expect(() => evaluateValidationRules(withRules as never, { amount: 1 }, 'update', {
+      previous: { id: 'r1', amount: 100 },
+    } as never)).toThrow(/could not be evaluated/);
+    const crossField = {
+      fields: { ...sparseLockFields.fields },
+      validations: [
+        { type: 'cross_field', name: 'typo_cross', message: 'nope', condition: 'record.stauts == null' },
+      ],
+    };
+    expect(() => evaluateValidationRules(crossField as never, { amount: 1 }, 'update', {
+      previous: { id: 'r1', amount: 100 },
+    } as never)).toThrow(/could not be evaluated/);
+  });
+
+  it('does not disturb the #4889 parent binding: unbound root still LOCKS, parent stays unmaterialised', () => {
+    // `parent` is a row of ANOTHER object — this function has no declared-field
+    // list for it — and an ABSENT parent is the signal #4889 depends on.
+    const warnings: string[] = [];
+    expect(stripReadonlyWhenFields(invoiceLineFields, { quantity: 9999 }, { id: 'l1', invoice: 'inv1' }, {
+      warn: (m: string) => warnings.push(m),
+    } as never)).toEqual({});
+    expect(warnings.some((w) => w.includes("reads 'parent'") && w.includes('LOCKED'))).toBe(true);
+    // A parent that IS bound but does not carry the key stays a fault (no
+    // materialisation of the header): fail-open, the change goes through.
+    const warnings2: string[] = [];
+    expect(stripReadonlyWhenFields(invoiceLineFields, { quantity: 9999 }, { id: 'l1', invoice: 'inv1' }, {
+      warn: (m: string) => warnings2.push(m),
+    } as never, { id: 'inv1' })).toEqual({ quantity: 9999 });
+    expect(warnings2.some((w) => w.includes('failed to evaluate — change allowed through'))).toBe(true);
+  });
+});
+
 // #2948 — static `readonly:true` write enforcement (caller-supplied only).
 const stampedFields = {
   fields: {