Repository navigation
Commit 5dba7f3
fix(objectql)!: a field-level requiredWhen / readonlyWhen that cannot be evaluated refuses the write (ADR-0137 D2) (#20028)
Fixes #19727
Clause-②: no (narrowing)
## What this does
ADR-0137 D2 (Accepted), verbatim: "At submit time, a field-rule
predicate that cannot be evaluated refuses the write and names the field
and the rule. Nothing is persisted." Triage's execution reading on the
card (comment 5788580082) puts the server-side `requiredWhen` /
`readonlyWhen` under D2, and this PR is the server half. Its scope is
the two arms ADR-0137's own Context measured as fail-open:
- **`requiredWhen`** (`rule-validator.ts:3026-3044` at base
`61609edf81`): the arm logged `requiredWhen for 'X' failed to evaluate —
skipped` and ran `continue`, so the record saved with the field empty.
**It now refuses.** Every fault takes this arm, including an unbound
`parent` (a master-detail header that could not be resolved for the
write). The refusal goes into the call's `ValidationError`, which is
thrown before any driver call.
- **`readonlyWhen`** (`rule-validator.ts:1350-1351` at base, inside
`isReadonlyWhenLocked`): the non-root arm logged `change allowed
through` and returned `false`, so the field was written. **It now throws
a `ValidationError`** from the strip, before anything is written. The
unbound-root carve-out is unchanged and still LOCKS.
Both arms go through the refusal builder that `checkPredicate` has used
since the #4649 work. `unevaluableRuleError` gained a `subject`
parameter, so the envelope stays the same: `VALIDATION_FAILED` (served
as 400), one entry with `field` set to the field, `code:
'rule_violation'`, and `constraint: { rule: 'requiredWhen' |
'readonlyWhen', reason: 'unevaluable', fault, missingKey? , hint? }`.
The message reads `Field 'X' requiredWhen could not be evaluated (...) —
write rejected.` The operator also gets a `warn` line saying the write
was rejected.
Details:
- **Bulk `readonlyWhen` checks every matched row.** Before, the check
stopped at the first row that locked (`some`). Now a row that faults
refuses the write no matter where it sits among the matched rows, so the
verdict no longer depends on the order the driver returns rows in. The
refusal names the row (`(record ID)`), the same way the bulk validation
refusal does. Warnings are said once per key.
- **The refusal names a lookup read correctly.** A predicate that reads
through a lookup (`record.account.tier`) faults as `No such key: tier`,
because field-level predicates are never hydrated. The generic sentence
would tell the author to declare `tier` on the wrong object. The refusal
instead says the column is read through `account`, a reference to
`fp_account`, and points the author at a `validations[]` `script` rule.
An unbound `parent` also gets its own sentence.
- **`@objectstack/lint` build-time messages.** Three author-facing
strings said the server skips a faulting `requiredWhen`, and this change
makes them false: the unbound-root slot consequence, the
`parent`-without-a-master consequence, and the null-guard outcome, which
switches from `'fail-open'` to `'fail-closed'`. All three now say the
write is refused. The `doc-authoring` prose-id baseline shrank by the
two tracker ids those strings dropped. That is the gate's own prescribed
remedy.
Not changed here, on purpose:
- option-level `visibleWhen` (`evaluateOptionVisibility`), which D2 does
not name and which stays fail-open;
- the render side (ADR-0137 D3);
- the readonlyWhen unbound-root carve-out, which LOCKS;
- a predicate that evaluates, which is judged exactly as before in both
directions;
- the ADR-0113 legacy-row rule for an evaluated `requiredWhen`.
`packages/spec` is untouched.
## Census: stored predicates that could start refusing
I checked every `requiredWhen` / `readonlyWhen` under `packages/**`,
`examples/**` and `apps/**`, excluding tests, at `7a6e91f5fd`. I
evaluated each predicate with the real `ExpressionEngine` over normal
and null bindings.
| where | predicate | faults on a write it judges? |
|---|---|---|
| `examples/app-showcase` invoice `issued_on` | `requiredWhen:
record.status in ['sent', 'paid']` | no (a null status evaluates to
`false`) |
| invoice `tax_rate` | `readonlyWhen: record.status == 'paid'` | no |
| invoice `paid_on` | `requiredWhen: record.status == 'paid'` | no |
| invoice_line `description` | `requiredWhen: record.quantity >= 100` |
faults only on a null `quantity`. `quantity` is `required` with
`defaultValue: 1`. Measured on the engine: an insert without it gets the
default and is accepted, and an update that nulls it is refused by
`required` first. So no normal write reaches the fault |
| invoice_line `product` / `quantity` / `unit_price` | `readonlyWhen:
parent.status == 'paid'` | only when unbound (unresolvable header),
which takes the unchanged LOCKED arm |
| `plugin-security` `sys_permission_set.name` | `readonlyWhen: record.id
!= null && record.id != ''` | no |
Result: none of the in-repo predicates start refusing. `driver-sql`'s
`builtin-column-collision.ts` names the keys as a classification map and
holds no predicate. How many stored predicates in deployments fault is
unmeasured. ADR-0137's Consequences says the loud state is what reveals
them, and the changeset says so.
## Pins, and the ablation that turns each red
The new file is
`packages/objectql/src/engine-field-predicate-fault.test.ts`. It uses
the real engine and an in-memory driver, and reads "nothing persisted"
off the store. It has 13 cases.
| pin | ablation (via `scripts/ablation-replace.mjs`, WRAP mode) | red |
|---|---|---|
| (a) requiredWhen fault refuses insert, update, supplied-field, lookup
and bulk | A1: fault arm back to `continue` | 5 of 5 (a) cases |
| (b) readonlyWhen non-root fault refuses single-id and bulk | A2:
`return false` before the throw | 3 of 3 (b) cases |
| (b) bulk refusal holds whatever the row order | A3: stop at the first
locking row | the ok-row-first case only |
| (a) lookup refusal names the reference | A4: traversal sentence off |
the lookup case only |
| (c) CONTROL requiredWhen FALSE accepted | A5a: FALSE read as TRUE |
that case only |
| (c) CONTROL readonlyWhen FALSE written | A5b: every lock reads LOCKED
| that case only |
| (c) CONTROL requiredWhen TRUE refused as `required` | A5c: TRUE routed
to the fault builder | that case only |
| (c) CONTROL readonlyWhen TRUE dropped, no refusal | A5d: every lock
reads UNLOCKED | that case only |
| (d) CONTROL option `visibleWhen` stays fail-open | A6: the option
fault arm pushes an error | that case only |
For every leg, the anchor hit x1 and went to x0, the blob changed, and
the restore was proven: blob `5bfc176263` equals HEAD and `git diff
HEAD` is empty. The script also carried an EXIT/INT/TERM trap that
restores by absolute path. No build was needed: the subject resolves
through relative source imports (`./engine.js` →
`./validation/rule-validator.js`), not through `dist/`.
Existing pins that locked in the retired fail-open branch were flipped.
Each still pins its original subject:
- `rule-validator.test.ts` (7 cases);
- `rule-fail-closed.test.ts` (the requiredWhen "neighbour");
- `engine-required-when-parent.test.ts` (orphan header, 3 cases);
- `engine-readonly-when-parent.test.ts` (undeclared header key; the
three `not change allowed through` absence checks were retargeted to the
refusal channel so they are not vacuous);
- `engine-readonly-when-interdependent-locks.test.ts` (a faulting FK
lock);
- `engine-reference-tenant-scope.test.ts` (4 cases). The invariant these
pin, that a locked, an open and a missing header give one answer, holds.
The answer is now `note: rule_violation` for all of them.
One fixture was re-spelled: `rule-validator.test.ts` "accepts the write
once the field is supplied" now supplies `quantity`. Its unrelated
`record.quantity >= 100` rule had been faulting silently.
## Test readings
- `@objectstack/objectql` at `7a6e91f5fd`: 313 files, 5256 tests passed.
`typecheck` exits 0, and `check:test-typecheck` holds its ledger.
- `@objectstack/lint` at `7a6e91f5fd`: 108 files, 4138 tests passed.
`typecheck` exits 0.
- `@objectstack/runtime` at `7a6e91f5fd`: 276 files, 3893 passed, 1
skipped.
- `@objectstack/plugin-security` at `b25e969e1d`: 128 files, 2501
passed. It reads `@objectstack/objectql` through a `dist/` built from
the same `rule-validator.ts`.
- `@objectstack/rest` at `b25e969e1d`: 194 files, 3265 passed, 1
skipped. Same `dist/` as above.
## Gates
`node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` at `7a6e91f5fd` derived 72 commands. I ran
all 72: 71 exit 0. `pnpm check:dual-build-cjs-loads` exits 3
(PREREQUISITE NOT MET, needs a full `pnpm build`), so it is NOT MEASURED
locally. `--ran` reconciliation: 72 derived, 71 run, 1 NOT-MEASURED
(derived from the recorded exit 3), 0 UNRUN.
- `check:adr-0087-registration` is green. The changeset reads as
`BREAKING+bang+clause-②-narrowing`, and its disposition is not-required
(no-migration-prescription): nothing authored moves and no stored shape
is refused.
- `check:objectql-double-limit` went red once, on the new test's `find`
double, and is green now that the double honours `limit`.
- `check:doc-authoring` went red once, on a stale over-pin, and is green
after the prescribed baseline shrink.
Narrowed lint, instead of the CI-owned `pnpm lint`: I ran `eslint
--no-inline-config --format json` on the 13 touched `.ts` files. Result:
13 files, 0 errors, 0 warnings, none ignored. All 13 fall under the
`**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}` config object
(`eslint.config.mjs:971`). That config never enables type-aware linting
(no `parserOptions.project`, per its own note at
`eslint.config.mjs:327`), so this diff cannot move the verdict on an
untouched file.
## Acceptance notes
- **D2 is applied literally: the submit is refused whatever the write
does to the field.** A `requiredWhen` whose predicate faults refuses
even a write that supplies the field, and a `readonlyWhen` fault refuses
even an echoed, unchanged value. There is a measured alternative. CEL
absorbs an error in `&&` (`record.statsu == 'closed' && record.reason ==
null` evaluates to `false` when `reason` is set). So the same rule
written as a `validations[]` script refuses only when the field is
empty. The report puts this to the seat as an open question and
recommends staying literal.
- **Error precedence moved for an unresolvable header.**
`evaluateValidationRules` runs before `assertReferencesResolve`. A
detail whose header FK does not resolve, and that carries a
`parent`-scoped `requiredWhen`, now meets the requiredWhen refusal ("no
header could be resolved") before `header: reference_not_found`. Both
are loud, and the tenant-scope no-leak invariant holds.
- **A field-level predicate that reads through a lookup always
refuses.** The field level is never hydrated; only validation conditions
are. Neither the schema nor lint refuses such a predicate at authoring
(`validate-expressions.ts` passes `traversalHydration` for validation
rules only). The census found none in this repository.
- After this change, `NullGuardOutcome`'s `'fail-open'` member has no
caller, and its clause still describes the old `requiredWhen` runtime.
Its doc comment says so. It is not removed here.
- The diff goes beyond the claim's declared file surface
(`rule-validator.ts`, its tests, `.changeset/`). It adds
`packages/lint/**`, `packages/objectql/src/{cel-fault,master-detail}.ts`
(docblocks this change made false), five more
`packages/objectql/src/*.test.ts` pin files, and
`scripts/doc-authoring-prose-id.baseline.json`.
---
_Generated by [Claude
Code](https://claude.ai/code/session_019c3Hi6ZMU1p6m6aA6Bz45d)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent b76aad5 commit 5dba7f3
15 files changed
Lines changed: 979 additions & 273 deletions
File tree
- .changeset
- packages
- lint/src
- objectql/src
- validation
- scripts
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
985 | 985 | | |
986 | 986 | | |
987 | 987 | | |
988 | | - | |
989 | | - | |
990 | | - | |
991 | | - | |
| 988 | + | |
| 989 | + | |
| 990 | + | |
| 991 | + | |
| 992 | + | |
992 | 993 | | |
993 | 994 | | |
994 | 995 | | |
| |||
1006 | 1007 | | |
1007 | 1008 | | |
1008 | 1009 | | |
1009 | | - | |
1010 | | - | |
| 1010 | + | |
| 1011 | + | |
| 1012 | + | |
1011 | 1013 | | |
1012 | 1014 | | |
1013 | 1015 | | |
| |||
1883 | 1885 | | |
1884 | 1886 | | |
1885 | 1887 | | |
1886 | | - | |
| 1888 | + | |
1887 | 1889 | | |
1888 | | - | |
1889 | | - | |
| 1890 | + | |
| 1891 | + | |
| 1892 | + | |
1890 | 1893 | | |
1891 | 1894 | | |
1892 | 1895 | | |
| |||
2517 | 2520 | | |
2518 | 2521 | | |
2519 | 2522 | | |
2520 | | - | |
2521 | | - | |
2522 | | - | |
| 2523 | + | |
| 2524 | + | |
| 2525 | + | |
2523 | 2526 | | |
2524 | 2527 | | |
2525 | 2528 | | |
| |||
2552 | 2555 | | |
2553 | 2556 | | |
2554 | 2557 | | |
2555 | | - | |
2556 | | - | |
2557 | | - | |
2558 | | - | |
| 2558 | + | |
| 2559 | + | |
| 2560 | + | |
| 2561 | + | |
| 2562 | + | |
2559 | 2563 | | |
2560 | | - | |
2561 | | - | |
2562 | | - | |
| 2564 | + | |
| 2565 | + | |
| 2566 | + | |
2563 | 2567 | | |
2564 | 2568 | | |
2565 | 2569 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
527 | 527 | | |
528 | 528 | | |
529 | 529 | | |
530 | | - | |
531 | | - | |
532 | | - | |
533 | | - | |
534 | | - | |
535 | | - | |
536 | | - | |
| 530 | + | |
| 531 | + | |
| 532 | + | |
| 533 | + | |
| 534 | + | |
| 535 | + | |
| 536 | + | |
| 537 | + | |
537 | 538 | | |
538 | 539 | | |
539 | 540 | | |
| |||
631 | 632 | | |
632 | 633 | | |
633 | 634 | | |
634 | | - | |
635 | | - | |
636 | | - | |
637 | | - | |
638 | | - | |
639 | | - | |
640 | | - | |
641 | | - | |
| 635 | + | |
| 636 | + | |
| 637 | + | |
| 638 | + | |
| 639 | + | |
| 640 | + | |
| 641 | + | |
| 642 | + | |
| 643 | + | |
642 | 644 | | |
643 | 645 | | |
644 | 646 | | |
| |||
803 | 805 | | |
804 | 806 | | |
805 | 807 | | |
806 | | - | |
807 | | - | |
808 | | - | |
| 808 | + | |
| 809 | + | |
| 810 | + | |
| 811 | + | |
809 | 812 | | |
810 | 813 | | |
811 | 814 | | |
| |||
1640 | 1643 | | |
1641 | 1644 | | |
1642 | 1645 | | |
1643 | | - | |
1644 | | - | |
1645 | | - | |
1646 | | - | |
1647 | | - | |
1648 | | - | |
1649 | | - | |
| 1646 | + | |
| 1647 | + | |
| 1648 | + | |
| 1649 | + | |
| 1650 | + | |
| 1651 | + | |
| 1652 | + | |
1650 | 1653 | | |
1651 | 1654 | | |
1652 | 1655 | | |
| |||
1658 | 1661 | | |
1659 | 1662 | | |
1660 | 1663 | | |
1661 | | - | |
| 1664 | + | |
1662 | 1665 | | |
1663 | 1666 | | |
1664 | 1667 | | |
| |||
1679 | 1682 | | |
1680 | 1683 | | |
1681 | 1684 | | |
1682 | | - | |
1683 | | - | |
1684 | | - | |
1685 | | - | |
1686 | | - | |
| 1685 | + | |
| 1686 | + | |
| 1687 | + | |
| 1688 | + | |
| 1689 | + | |
| 1690 | + | |
1687 | 1691 | | |
1688 | 1692 | | |
1689 | 1693 | | |
1690 | 1694 | | |
1691 | 1695 | | |
1692 | 1696 | | |
1693 | 1697 | | |
1694 | | - | |
1695 | | - | |
1696 | | - | |
| 1698 | + | |
| 1699 | + | |
| 1700 | + | |
1697 | 1701 | | |
1698 | 1702 | | |
1699 | 1703 | | |
1700 | 1704 | | |
1701 | 1705 | | |
1702 | 1706 | | |
1703 | 1707 | | |
1704 | | - | |
| 1708 | + | |
1705 | 1709 | | |
1706 | 1710 | | |
1707 | 1711 | | |
| |||
0 commit comments