Skip to content

Commit 5bd7cbb

Browse files
committed
docs(lint,objectql): correct the null-guard seam ledger after #6454 — readonlyWhen is TOTAL, excluded by ruling, not by binding
The surface ledger in validate-null-guards.ts still said field readonlyWhen binds a sparse record ("stripReadonlyWhenFields merges {...previous, ...data} and never materializes"). That stopped being true when PR #6454 landed the engine-core share of the #4953 ruling: readonlyWhenBindings now runs both roots through materializeDeclaredFields. The row now records the real state — binding TOTAL, verdict still excluded, but by clause 3 of the ruling (both server-side seams must be total before the gate widens; the flow trigger-record half is not wired yet), not by the totality criterion. The rewritten row also records the one verdict #6454 moved the OTHER way: on a total record !has(record.b) over a declared field is uniformly FALSE, so a lock spelled that way STOPPED locking — plus the fail-open policy (isReadonlyWhenLocked allows the change through on a fault, except the #4889 unbound-root branch), which the widening must carry into the evidence column. Neighbouring rows re-verified against origin/main in the same pass: - flow / edge condition: still sparse (record-change-trigger.ts:309), but the reason is now "not yet wired" (#4953 clause 1, services lane), not an open question; also renames the seed's inputDoc spelling to inputData (#5671). - action visible/disabled: still sparse, now by DECISION (#4953 clause 2). - validation rules / hook condition / requiredWhen / sharing rules / field expression rows: verified accurate, unchanged. - cel-fault.ts header: "both evaluators" is three seams since #4953 (the rider the issue named). Comment-only change: no runtime behaviour moves, and no gate or test asserts the ledger's text — the deliverable is that the decision input the next author reads is true again. Fixes #6458 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F8q5J1MQyocgtNspb15fSn
1 parent 64d764e commit 5bd7cbb

4 files changed

Lines changed: 94 additions & 26 deletions

File tree

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1559,7 +1559,8 @@ describe('null-guard gate (#4763)', () => {
15591559
// flattened scope (this gate never resolves a bare identifier — only
15601560
// `record.<f>`/`previous.<f>`, and the engine binds both roots
15611561
// unconditionally). It is that `record-change-trigger.ts` seeds the flow's
1562-
// record as `{ ...inputDoc, ...after }` with no `materializeDeclaredFields`,
1562+
// record as `{ ...(inputData ?? {}), ...after }` (spelled `inputDoc` until
1563+
// #5671 dropped that alias read) with no `materializeDeclaredFields`,
15631564
// so a declared column the write never mentioned is an ABSENT key — and on
15641565
// an absent key the `!= null` this gate prescribes faults exactly like the
15651566
// comparison it was meant to guard.

packages/lint/src/validate-expressions.ts

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -340,14 +340,19 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] {
340340
* (`rule-validator.ts`, fail-closed since #4761), lifecycle hook `condition`s,
341341
* and field `requiredWhen` (#4811).
342342
*
343-
* Totality is the whole criterion, not a detail: on a total binding `has()` is
344-
* uniformly true and `!= null` is the fix, while on a SPARSE one `has()` is a
345-
* genuine guard and `!= null` faults with `No such key` — so pointing this gate
346-
* at a sparse-bound surface would reject correct metadata and prescribe a fix
347-
* that breaks it. `validate-null-guards.ts` carries the measured evidence table
348-
* and the per-surface ledger (action predicates, flow conditions, field
349-
* `readonlyWhen`, sharing rules and `Field.formula` are each excluded there with
350-
* a traced reason). Read it before extending this call.
343+
* Totality is the criterion that can DISQUALIFY a surface: on a total binding
344+
* `has()` is uniformly true and `!= null` is the fix, while on a SPARSE one
345+
* `has()` is a genuine guard and `!= null` faults with `No such key` — so
346+
* pointing this gate at a sparse-bound surface would reject correct metadata
347+
* and prescribe a fix that breaks it. It is not by itself a licence to cover:
348+
* since #6454 field `readonlyWhen` is total and still excluded, by clause 3 of
349+
* the #4953 ruling (the gate widens only once BOTH server-side seams are
350+
* total, and the flow trigger-record half is not wired yet).
351+
* `validate-null-guards.ts` carries the measured evidence table and the
352+
* per-surface ledger (action predicates, flow conditions, field
353+
* `readonlyWhen`, sharing rules and `Field.formula` are each excluded there
354+
* with a traced reason, and the reasons are no longer all the same one). Read
355+
* it before extending this call.
351356
*/
352357
const checkNullGuards = (
353358
where: string,
@@ -609,7 +614,8 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] {
609614
// unconditionally, so it is immune to that ambiguity.
610615
//
611616
// The real blocker is totality. `record-change-trigger.ts` seeds the flow's
612-
// record as `{ ...inputDoc, ...after }` with no `materializeDeclaredFields`,
617+
// record as `{ ...(inputData ?? {}), ...after }` (spelled `inputDoc` until
618+
// #5671 dropped that alias read) with no `materializeDeclaredFields`,
613619
// so a declared column the write never mentioned is an ABSENT key, not a
614620
// null one — and there `record.x != null` faults (`No such key`) exactly
615621
// like the comparison it was meant to guard. The gate's prescription is
@@ -775,12 +781,16 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] {
775781
// required and the write sails through. Validation rules at least
776782
// reject fail-closed since #4761.
777783
//
778-
// `readonlyWhen` is deliberately NOT included even though it sits on
779-
// the same field: it is evaluated by `stripReadonlyWhenFields`, which
780-
// builds `{ ...previous, ...data }` and never materializes, so its
781-
// binding is sparse and `!= null` would be the wrong prescription
782-
// there. Same for `conditionalRequired` / `visibleWhen`, which have no
783-
// record-scoped total binding of their own. See the surface ledger in
784+
// `readonlyWhen` is still NOT included even though it sits on the same
785+
// field — but no longer because its binding is sparse. Since #6454
786+
// `readonlyWhenBindings` runs both roots through
787+
// `materializeDeclaredFields`, so `!= null` IS the right prescription
788+
// there now; what holds the wiring back is clause 3 of the #4953 ruling
789+
// (both server-side seams first — the flow trigger-record half is
790+
// outstanding). When it is wired it needs `'fail-open'`, like
791+
// `requiredWhen` above and unlike the validation-rule surface. Same for
792+
// `conditionalRequired` / `visibleWhen`, which have no record-scoped
793+
// total binding of their own. See the surface ledger in
784794
// `validate-null-guards.ts`.
785795
checkNullGuards(
786796
`object '${objectName}' · field '${fname}' requiredWhen`,

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

Lines changed: 61 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -65,25 +65,65 @@
6565
* breaks it — strictly worse than not covering the surface at all. Hence:
6666
*
6767
* **This module may only be wired to a surface whose record binding is total.**
68+
* Necessary, not sufficient — see the `readonlyWhen` row below, where a total
69+
* binding is still excluded for a reason totality has nothing to say about.
6870
*
6971
* Surface ledger (each verdict traced to the code that decides it, so the next
70-
* author does not have to re-derive it — #4811):
72+
* author does not have to re-derive it — #4811). `binding` is the measured
73+
* shape TODAY; `verdict` is whether this gate runs there. Those are two
74+
* questions, and since #6454 they have come apart on one row:
7175
*
7276
* | surface | binding | evidence | verdict |
7377
* |:-------------------------------|:--------|:------------------------------------------------------------|:--------|
7478
* | object validation rules | TOTAL | `rule-validator.ts` `materializeDeclaredFields(merged, …)` | covered |
7579
* | lifecycle hook `condition` | TOTAL | `hook-wrappers.ts` `materializeDeclaredFields(…)` | covered |
7680
* | field `requiredWhen` | TOTAL | same `merged` in `evaluateValidationRules` — fail-OPEN, so an unguarded predicate enforces NOTHING in silence | covered (#4811) |
77-
* | field `readonlyWhen` | sparse | `stripReadonlyWhenFields` merges `{...previous, ...data}` and never materializes | excluded |
78-
* | action `visible` / `disabled` | sparse | evaluated client-side; no materialization exists in `objectui` | excluded |
79-
* | flow / edge `condition` | sparse | `record-change-trigger.ts` seeds `{...inputDoc, ...after}` | excluded |
81+
* | field `readonlyWhen` | TOTAL | `rule-validator.ts` `readonlyWhenBindings` materialises BOTH roots (#4953 clause 1, landed in #6454) | excluded — NOT on totality; see below |
82+
* | action `visible` / `disabled` | sparse | evaluated client-side; no materialization exists in `objectui` | excluded (decided — #4953 clause 2) |
83+
* | flow / edge `condition` | sparse | `record-change-trigger.ts` seeds `{...(inputData ?? {}), ...after}` — #4953 clause 1's other half, not yet wired | excluded (not yet) |
8084
* | sharing-rule `condition` | n/a | compiled to a SQL filter; `NULL > x` is three-valued, never faults | excluded |
8185
* | field `expression` (`Field.formula`) | n/a | product judgement, not a wiring gap — see below | excluded |
8286
*
83-
* The three exclusions that are *not* self-evident, spelled out because a
87+
* The four exclusions that are *not* self-evident, spelled out because a
8488
* surface excluded without a reason is indistinguishable from one nobody
8589
* looked at — the failure mode this whole family of issues is about:
8690
*
91+
* - **Field `readonlyWhen` — the row where `binding` and `verdict` came
92+
* apart.** Its binding is no longer sparse. Since #6454 (the engine-core
93+
* share of the maintainer's #4953 ruling, clause 1) `rule-validator.ts`
94+
* builds the two roots in `readonlyWhenBindings` and runs BOTH — `record`
95+
* (the prior row overlaid with the PATCH) and `previous` — through the same
96+
* `materializeDeclaredFields`. The totality criterion above is therefore
97+
* SATISFIED here, and the evidence this row used to carry
98+
* (`stripReadonlyWhenFields` merging `{...previous, ...data}` raw)
99+
* describes code that no longer exists. What keeps the row excluded is
100+
* clause 3 of the same ruling: the gate widens once BOTH server-side seams
101+
* are total, and the other one — flow trigger-record seeding, services
102+
* lane — is not wired yet. Widening this face alone would also mean the
103+
* `binding` column had stopped being the thing that decides coverage,
104+
* which is the property #4811 bought.
105+
*
106+
* Two facts to carry into that widening; neither is bookkeeping:
107+
*
108+
* 1. **The fail policy is the OPPOSITE of the two surfaces #4763 wired.**
109+
* Validation rules and hook `condition`s are fail-CLOSED. A faulting
110+
* `readonlyWhen` is fail-OPEN: `isReadonlyWhenLocked` logs
111+
* `failed to evaluate — change allowed through`, and the field the
112+
* author declared frozen is WRITTEN. (One exception, #4889 — a fault
113+
* naming an UNBOUND ROOT resolves to LOCKED.) So this face wants
114+
* {@link nullGuardMessage}'s `'fail-open'` outcome for the same reason
115+
* the `requiredWhen` row already carries it: the damage is a declared
116+
* lock that silently enforces nothing, not a rejected write.
117+
* 2. **Making the binding total moved one verdict the OTHER way.** On a
118+
* total record `has(record.<declared>)` is uniformly TRUE and
119+
* `!has(record.<declared>)` uniformly FALSE, so a lock spelled
120+
* `readonlyWhen: !has(record.b)` STOPPED locking when #6454 landed.
121+
* That is the `declared-fields.ts` contract since #4649 — `has()`
122+
* guards an UNDECLARED key, never an empty value; test emptiness with
123+
* `!= null` — and #6454 measured the cell and pinned both spellings in
124+
* `rule-validator.test.ts`. It is why "this face is materialized now"
125+
* is not, on its own, an accurate summary of what changed here.
126+
*
87127
* - **Action `visible` / `disabled`.** #4811 asked whether the ActionEngine
88128
* materializes declared fields before evaluating. It does not: the record
89129
* is whatever the client already fetched (a record-detail read, or a LIST
@@ -93,17 +133,29 @@
93133
* `ExpressionInputSchema`, which the renderers preserve) and a fault IS
94134
* fail-closed — the action silently vanishes — so the *trap* is real here.
95135
* But with a sparse binding the prescription inverts (see the table), so the
96-
* gate cannot be the thing that catches it. Covering this surface requires
97-
* first deciding whether the action-predicate binding should be made total,
98-
* which is a platform contract change, not a lint change.
136+
* gate cannot be the thing that catches it. That question is now DECIDED
137+
* rather than open: #4953 clause 2 defers making this binding total —
138+
* it would mean every REST read padding out all declared columns — so the
139+
* face stays sparse, is documented as sparse, and an author there guards
140+
* with `has()` (`declared-fields.ts` says the same from the engine's side).
141+
* The exclusion is permanent under the current decision, not pending one.
142+
* The ruling's replacement action for this face is the MIRROR of this gate
143+
* — flag `!= null` on a sparse binding — and it is an evaluation owed by
144+
* the devx / objectui lanes, never a widening of `checkNullGuards`.
99145
* - **Flow / edge `condition`.** #4811 excluded these for flattened-scope
100146
* ambiguity ("a bare identifier may be a flow variable"). That reason does
101147
* not actually apply to this module — {@link findUnguardedNullableOperands}
102148
* only ever resolves `record.<f>` / `previous.<f>` and never a bare
103149
* identifier, and the engine binds `record` / `previous` unconditionally.
104150
* The real blocker is totality: the trigger seeds the record as
105-
* `{...inputDoc, ...after}`, so a declared column the write never mentioned
151+
* `{...(inputData ?? {}), ...after}` — spelled `inputDoc` here until #5671
152+
* dropped that alias read — so a declared column the write never mentioned
106153
* is an ABSENT key, and the `!= null` this gate prescribes would fault.
154+
* Since #4953 that sparseness is a NOT-YET rather than a decision: clause 1
155+
* puts this seam under the same server-side totality guarantee as
156+
* `readonlyWhen`, and only the services-lane wiring is outstanding. When it
157+
* lands, this row and the `readonlyWhen` row flip together — which is
158+
* exactly what clause 3 asks for.
107159
* (The flattened-scope ambiguity is real for a *bare-identifier* checker —
108160
* flow inputs shadow record fields, and a node's `outputVariable` can
109161
* overwrite either — but that is a different, unbuilt pass.)

packages/objectql/src/cel-fault.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,12 @@
99
* `condition`s (`hook-wrappers.ts`, #4775). Both were told to reuse ONE error
1010
* shape, and the only durable way to keep two messages worded alike is to stop
1111
* writing them twice — the same argument that put {@link
12-
* ./declared-fields.js#materializeDeclaredFields} in front of both evaluators.
12+
* ./declared-fields.js#materializeDeclaredFields} in front of every server-side
13+
* evaluator: two when this module was written, THREE since #4953 added the
14+
* field `readonlyWhen` strips. That third seam is fail-open rather than
15+
* rejecting, so it reads only {@link unknownVariableOf} (the #4889
16+
* unbound-root branch) and never {@link describeCelFault}'s rejection
17+
* sentences.
1318
*
1419
* This module has no opinion about what a caller does with a fault. It answers
1520
* three questions and hands back a sentence:

0 commit comments

Comments
 (0)