Skip to content

Commit 3fb42d2

Browse files
baozhoutaoclaude
andauthored
fix(objectql): 字段 readonlyWhen 求值补 materializeDeclaredFields (#4953) (#6454)
`readonlyWhen` 是三个服务端 CEL 求值接缝里唯一没有物化的一个:写入路径上的 `stripReadonlyWhenFields` / `stripReadonlyWhenFieldsMulti` 把 `{ ...previous, ...data }` 原样交给求值器。于是同一字段上的 `requiredWhen` (同文件、已物化)与 `readonlyWhen` 对「记录是什么」给出相反答案 —— 后者在驱动 未回读某已声明列时 fault,而 `readonlyWhen` fault 是 fail-open,作者声明为冻结的 字段被照常写入。 按维护者 2026-08-06 裁决(#4953 第 1 条 engine-core 份额)统一服务端接缝: `record`(merged)与 `previous` 两个根都过 `materializeDeclaredFields`,单行与 bulk 两条路径一致。 - `parent` 表头不物化:它是另一个对象的行,且「未绑定」正是 #4889 fail-closed 判定所依赖的信号。 - 未读到前序行时不物化(与 `evaluateValidationRules` 的 groundTruth 同规则): 那样是捏造与库中行矛盾的值,而非补齐缺失值。 - 对象级 `script` / `cross_field` 的 fail-closed 与文案不变,由测试钉住。 Claude-Session: https://claude.ai/code/session_019Q7oc7ASjh8yxyS3Yz78We Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4eeb81f commit 3fb42d2

4 files changed

Lines changed: 371 additions & 15 deletions

File tree

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

packages/objectql/src/declared-fields.ts

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

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

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,195 @@ describe('hasParentScopedRequiredWhen (#4977 gate)', () => {
452452
});
453453
});
454454

455+
// #4953 — the record a `readonlyWhen` predicate sees is TOTAL over the object's
456+
// DECLARED fields, like the two seams that were materialised in #4649/#4770.
457+
// Before this, `stripReadonlyWhenFields` merged `{...previous, ...data}` raw, so
458+
// a predicate reading a declared column the DRIVER did not echo back faulted —
459+
// and a faulting `readonlyWhen` fails open, i.e. WROTE the field the author
460+
// declared frozen. Which columns come back is a storage property no author can
461+
// see, so the same declaration was enforced or not depending on the driver.
462+
const sparseLockFields = {
463+
fields: {
464+
notes: { type: 'text' },
465+
approved_at: { type: 'datetime' },
466+
// "while nothing has been approved, the amount is frozen" — the `== null`
467+
// spelling #4649 made the supported one (and the null-guard gate prescribes).
468+
amount: { type: 'currency', readonlyWhen: 'record.approved_at == null' },
469+
},
470+
};
471+
472+
/** A prior row from a driver that stores only the columns a write touched. */
473+
const sparsePrior = () => ({ id: 'r1', amount: 100 });
474+
/** The same row from a driver that returns every declared column. */
475+
const totalPrior = (approvedAt: unknown) => ({ id: 'r1', amount: 100, notes: null, approved_at: approvedAt });
476+
477+
describe('readonlyWhen binds a TOTAL record (#4953)', () => {
478+
it('evaluates `record.<declared> == null` on a SPARSE prior instead of faulting through', () => {
479+
// THE bug. Pre-#4953: `No such key: approved_at` ⇒ fail-open ⇒ amount written.
480+
const warnings: string[] = [];
481+
const out = stripReadonlyWhenFields(sparseLockFields, { amount: 999 }, sparsePrior(), {
482+
warn: (m: string) => warnings.push(m),
483+
} as never);
484+
expect(out).toEqual({});
485+
expect(warnings.some((w) => w.includes('failed to evaluate'))).toBe(false);
486+
expect(warnings.some((w) => w.includes('is read-only (readonlyWhen)'))).toBe(true);
487+
});
488+
489+
it('still KEEPS the change when the materialised value makes the predicate FALSE', () => {
490+
// Materialising is not "lock everything": the row HAS an approval date, so
491+
// the lock is off and the legitimate edit lands.
492+
expect(
493+
stripReadonlyWhenFields(sparseLockFields, { amount: 999 }, { id: 'r1', amount: 100, approved_at: '2026-01-01' }),
494+
).toEqual({ amount: 999 });
495+
});
496+
497+
it('reads the same verdict on a sparse prior as on a total one (the point)', () => {
498+
// One declaration, two drivers, one answer. This equality is the guarantee;
499+
// before #4953 the left side kept the change and the right side stripped it.
500+
const sparse = stripReadonlyWhenFields(sparseLockFields, { amount: 999 }, sparsePrior());
501+
const total = stripReadonlyWhenFields(sparseLockFields, { amount: 999 }, totalPrior(null));
502+
expect(sparse).toEqual(total);
503+
expect(sparse).toEqual({});
504+
});
505+
506+
it('materialises the `previous` root too, not just `record`', () => {
507+
const schema = { fields: { ...sparseLockFields.fields, amount: { type: 'currency', readonlyWhen: 'previous.approved_at == null' } } };
508+
expect(stripReadonlyWhenFields(schema, { amount: 999 }, sparsePrior())).toEqual({});
509+
expect(stripReadonlyWhenFields(schema, { amount: 999 }, { id: 'r1', amount: 100, approved_at: '2026-01-01' })).toEqual({ amount: 999 });
510+
});
511+
512+
it('applies on the BULK path identically — one payload, N sparse rows', () => {
513+
// A bulk write must not judge the same predicate by a different record
514+
// shape than a single-id write does.
515+
expect(stripReadonlyWhenFieldsMulti(sparseLockFields, { amount: 999 }, [sparsePrior()])).toEqual({});
516+
// ≥1 locked row still drops it for the batch; no locked row still writes.
517+
expect(stripReadonlyWhenFieldsMulti(sparseLockFields, { amount: 999 }, [
518+
{ id: 'r1', amount: 1, approved_at: '2026-01-01' },
519+
sparsePrior(),
520+
])).toEqual({});
521+
expect(stripReadonlyWhenFieldsMulti(sparseLockFields, { amount: 999 }, [
522+
{ id: 'r1', amount: 1, approved_at: '2026-01-01' },
523+
{ id: 'r2', amount: 2, approved_at: '2026-02-02' },
524+
])).toEqual({ amount: 999 });
525+
});
526+
527+
it('does NOT materialise when the prior row is not in hand (no fabrication)', () => {
528+
// `declared-fields.ts`'s standing rule: without the persisted state,
529+
// defaulting a declared field to null would FABRICATE a value that
530+
// contradicts the stored row. So this case keeps the historical fault →
531+
// fail-open exit, and the engine avoids it by fetching the prior row
532+
// whenever the object declares a readonlyWhen field (`needsPriorRecord`).
533+
const warnings: string[] = [];
534+
const out = stripReadonlyWhenFields(sparseLockFields, { amount: 999 }, null, {
535+
warn: (m: string) => warnings.push(m),
536+
} as never);
537+
expect(out).toEqual({ amount: 999 });
538+
expect(warnings.some((w) => w.includes('failed to evaluate — change allowed through'))).toBe(true);
539+
});
540+
541+
it('never mutates the caller\'s prior record (it is the engine\'s hookContext.previous)', () => {
542+
const prior = sparsePrior();
543+
stripReadonlyWhenFields(sparseLockFields, { amount: 999 }, prior);
544+
expect('approved_at' in prior).toBe(false);
545+
expect('notes' in prior).toBe(false);
546+
const rows = [sparsePrior()];
547+
stripReadonlyWhenFieldsMulti(sparseLockFields, { amount: 999 }, rows);
548+
expect('approved_at' in rows[0]!).toBe(false);
549+
});
550+
551+
it('leaves the fail-open branch ALIVE — an ordering comparison still faults over a total record', () => {
552+
// `null < null` is `no such overload`, so materialising does not make every
553+
// predicate evaluable. This is exactly why the null-guard gate exists.
554+
const warnings: string[] = [];
555+
const out = stripReadonlyWhenFields(
556+
{ fields: { ...sparseLockFields.fields, amount: { type: 'currency', readonlyWhen: 'record.notes < record.approved_at' } } },
557+
{ amount: 999 },
558+
sparsePrior(),
559+
{ warn: (m: string) => warnings.push(m) } as never,
560+
);
561+
expect(out).toEqual({ amount: 999 });
562+
expect(warnings.some((w) => w.includes('failed to evaluate — change allowed through'))).toBe(true);
563+
});
564+
565+
it('keeps fail-OPEN for an UNDECLARED key — materialising covers declared fields only', () => {
566+
// The #4649 line, unmoved: a typo must stay unevaluable so it is reported,
567+
// not silently read as null.
568+
const warnings: string[] = [];
569+
expect(stripReadonlyWhenFields(
570+
{ fields: { amount: { type: 'currency', readonlyWhen: 'record.stauts == null' } } },
571+
{ amount: 999 },
572+
{ id: 'r1', amount: 100 },
573+
{ warn: (m: string) => warnings.push(m) } as never,
574+
)).toEqual({ amount: 999 });
575+
expect(warnings.some((w) => w.includes('failed to evaluate — change allowed through'))).toBe(true);
576+
});
577+
578+
// ── the consequence that moves the OTHER way, pinned rather than discovered ──
579+
it('`has(record.<declared>)` is uniformly TRUE — so it locks even on a sparse prior', () => {
580+
// CEL's own rule: a materialised `null` is a PRESENT key holding null.
581+
// `has()` therefore guards against an UNDECLARED key, not an empty value.
582+
expect(stripReadonlyWhenFields(
583+
{ fields: { ...sparseLockFields.fields, amount: { type: 'currency', readonlyWhen: 'has(record.approved_at)' } } },
584+
{ amount: 999 },
585+
sparsePrior(),
586+
)).toEqual({});
587+
});
588+
589+
it('`!has(record.<declared>)` is uniformly FALSE — a lock spelled that way STOPS locking', () => {
590+
// The one verdict this change moves toward "allowed": pre-#4953 the sparse
591+
// binding made `!has(...)` true and the field was stripped. It was never a
592+
// guarantee — on a driver returning all columns the same declaration never
593+
// locked anything — so the flip replaces a storage-dependent verdict with a
594+
// deterministic one, and the deterministic answer is FALSE. An author who
595+
// means "while the field is empty" writes `== null` (the spelling
596+
// @objectstack/lint's null-guard gate prescribes).
597+
expect(stripReadonlyWhenFields(
598+
{ fields: { ...sparseLockFields.fields, amount: { type: 'currency', readonlyWhen: '!has(record.approved_at)' } } },
599+
{ amount: 999 },
600+
sparsePrior(),
601+
)).toEqual({ amount: 999 });
602+
});
603+
604+
// ── blast radius: nothing else at this write gate moves ─────────────────
605+
it('does not touch the object-level rules — `script` / `cross_field` stay fail-CLOSED (#4649)', () => {
606+
const withRules = {
607+
fields: { ...sparseLockFields.fields },
608+
validations: [
609+
{ type: 'script', name: 'typo_rule', message: 'nope', condition: 'record.stauts == null' },
610+
],
611+
};
612+
expect(() => evaluateValidationRules(withRules as never, { amount: 1 }, 'update', {
613+
previous: { id: 'r1', amount: 100 },
614+
} as never)).toThrow(/could not be evaluated/);
615+
const crossField = {
616+
fields: { ...sparseLockFields.fields },
617+
validations: [
618+
{ type: 'cross_field', name: 'typo_cross', message: 'nope', condition: 'record.stauts == null' },
619+
],
620+
};
621+
expect(() => evaluateValidationRules(crossField as never, { amount: 1 }, 'update', {
622+
previous: { id: 'r1', amount: 100 },
623+
} as never)).toThrow(/could not be evaluated/);
624+
});
625+
626+
it('does not disturb the #4889 parent binding: unbound root still LOCKS, parent stays unmaterialised', () => {
627+
// `parent` is a row of ANOTHER object — this function has no declared-field
628+
// list for it — and an ABSENT parent is the signal #4889 depends on.
629+
const warnings: string[] = [];
630+
expect(stripReadonlyWhenFields(invoiceLineFields, { quantity: 9999 }, { id: 'l1', invoice: 'inv1' }, {
631+
warn: (m: string) => warnings.push(m),
632+
} as never)).toEqual({});
633+
expect(warnings.some((w) => w.includes("reads 'parent'") && w.includes('LOCKED'))).toBe(true);
634+
// A parent that IS bound but does not carry the key stays a fault (no
635+
// materialisation of the header): fail-open, the change goes through.
636+
const warnings2: string[] = [];
637+
expect(stripReadonlyWhenFields(invoiceLineFields, { quantity: 9999 }, { id: 'l1', invoice: 'inv1' }, {
638+
warn: (m: string) => warnings2.push(m),
639+
} as never, { id: 'inv1' })).toEqual({ quantity: 9999 });
640+
expect(warnings2.some((w) => w.includes('failed to evaluate — change allowed through'))).toBe(true);
641+
});
642+
});
643+
455644
// #2948 — static `readonly:true` write enforcement (caller-supplied only).
456645
const stampedFields = {
457646
fields: {

0 commit comments

Comments
 (0)