Repository navigation
Commit b25d98a
fix(objectql): bind
Fixes #23910
Clause-②: yes (widening)
## What changed
On insert, `previous` is now bound to `null` in validation rules and in
declarative hook conditions. So a rule or condition that reads it
decides the insert. The flow engine already gives a start condition on
its create leg the same binding.
Before, the insert path left `previous` unbound. Any reference to it,
including the guard `previous == null`, faulted `Unknown variable:
previous`. A violating insert was then refused (validation) or aborted
(hook) as `unevaluable`, naming the CEL fault instead of the rule's
message. `sys_user_permission_set.validity_window_order` is the measured
case.
**This widens the published accept set**, hence `Clause-②: yes
(widening)` and a `minor` changeset:
- A rule shaped `previous != null && …` ("judge updates only") now
ADMITS inserts that it refused before with the `unevaluable` fault.
- A field's `requiredWhen: previous == null` now requires the field on
insert, instead of refusing every insert.
No shipped rule's or hook's verdict moves (census below).
### Validation (`packages/objectql/src/validation/rule-validator.ts`)
- **`previousBinding(mode, priorRecord, fields)`** decides the binding
once per evaluation. On insert it returns `null`. On an update with the
prior row in hand, it returns that row made total over the declared
fields, as before.
- **An update whose prior row is not in hand stays unbound.** Examples
are the bulk seam's `previous: null` and an update-mode preview that
names no row. `null` there would answer `previous == null` with "this is
an insert", which is false. Unbound, a rule that reads `previous` fails
closed.
- **`ruleScope(record, previous, extra)`** builds the scope for every
rule-evaluation `ExpressionEngine.evaluate` in the file. That covers:
- a `script` / `cross_field` condition;
- a `conditional`'s `when`;
- a field's `requiredWhen` and its ADR-0113 pre-check;
- an option's `visibleWhen`;
- `readonlyWhen`, which is update-only and unchanged.
The `previous ?? undefined` that turned an explicit `null` back into
"unbound" is gone. `null` rides `extra`, the bag `buildScope` merges
onto the variable scope, because `EvalContext.previous` is typed for a
row only.
- **`nullPreviousReadDetail`.** A column read off a `null` `previous`
(`record.stage != previous.stage`) still has no value on insert and
still refuses. The refusal now names `previous.KEY` and gives the
guards: `previous == null || …` judges an insert, `previous != null &&
…` skips one. Without this, the generic text would have said "a field
this object declares, but its value was not supplied". All four refusal
builders carry it.
### Hook conditions (`packages/objectql/src/hook-wrappers.ts`)
- **Same scope as validation.** A declarative hook `condition` is now
evaluated in `ruleScope`. `pickPreviousPayload` returns `null` on
`beforeInsert` / `afterInsert`. On any other write that carries a prior
row, it returns that row, copied and made total as before. When an
existing row's prior state is not in hand, `previous` stays unbound.
- **`ctx.previous` is untouched.** The engine's pre-image, which the
after-hooks observe, is not changed. Only the CEL binding is.
- **The refusal names the read.** A column read off the `null` on insert
still aborts the operation (#4775). `unevaluableConditionError` names
the read with the same `nullPreviousReadDetail` sentence.
- **Docblock rewritten.** The "When it is ABSENT" section of the
docblock is now "When it is `null`, and when it is ABSENT". It keeps the
update-without-row reasoning.
### Docs (`content/docs/data-modeling/formulas.mdx`)
The `previous` row of the scope table now says the following. `previous`
is the row before the update. It is `null` on insert in validation, hook
conditions and flows, so `previous == null` judges an insert. A column
read off it there has no value and faults. No other sentence on the page
says `previous` is unbound on insert.
`engine.ts` is not edited. The insert call site hands the evaluator
`mode: 'insert'`, and the binding derives from that. The validate-only
preview's insert mode judges the same way.
## Census (triage's instruction)
### Validation rules reading `previous`
Searched at `b0d12e2e9` across `packages/**` (platform objects, plugins,
services) and `examples/**`, non-test, TS and non-TS metadata.
| Rule | File | Condition | Insert verdict before | Insert verdict after
|
|---|---|---|---|---|
| `validity_window_order` (`cross_field`, error) |
`packages/plugins/plugin-security/src/objects/sys-user-permission-set.object.ts`
| `(previous == null` or a bound moved`)` and both bounds set and
`valid_until` at or before `valid_from` | inverted / equal window:
refused, 400, `reason: unevaluable`. Valid, one-bound or open window:
admitted | inverted / equal window: refused, 400, the rule's own
message. Valid, one-bound or open window: admitted |
That is the only shipped validation rule that reads `previous`. Its
verdict cannot move, for this reason:
- **Before:** the first clause was a CEL error, which `&&` absorbs
whenever another conjunct is `false`.
- **After:** the first clause is `true` on insert, and the rule answers
the remaining conjuncts.
So the same writes are refused and the same writes are admitted.
No field `requiredWhen` / `readonlyWhen` / `visibleWhen` in shipped
metadata reads `previous`. The other `*.object.ts` hits are prose:
comments, field descriptions and action copy in `sys-metadata-history`,
`sys-account`, `sys-member`, `sys-oauth-application`,
`sys-oauth-consent`, `sys-two-factor`, `sys-user-preference`,
`sys-setting-audit` and `sys-audit-log`.
### Declarative hook conditions reading `previous`
Searched at `a2863de08` across `packages/**` and `examples/**`.
| Hook | File | Events | Condition | Insert verdict before | Insert
verdict after |
|---|---|---|---|---|---|
| `showcase_audit_task_completion` |
`examples/app-showcase/src/data/hooks/index.ts` | `afterUpdate` |
`previous.done != true && record.done == true` | not dispatched on
insert | not dispatched on insert (unchanged) |
| `showcase_guard_task_reopen` |
`examples/app-showcase/src/data/hooks/index.ts` | `beforeUpdate` |
`previous.done == true && record.done != true` | not dispatched on
insert | not dispatched on insert (unchanged) |
No other shipped declarative hook condition names `previous`. Plugin and
service hooks read `ctx.previous` in JS, not in a CEL condition. The 16
flow start conditions that read `previous`
(`examples/app-showcase/src/automation/flows/**`,
`examples/app-todo/src/flows/task.flow.ts`) are the flow engine's, which
already binds `null` on insert.
## Verification
### CEL seam
Measured against the built `@objectstack/formula`.
| Expression | `previous` | Result |
|---|---|---|
| `previous == null` | unbound | `type: Unknown variable: previous` |
| `previous == null` | `null` | `true` |
| `previous != null && record.a > 5` | `null` | `false` |
| `previous.x` | `null` | `runtime: No such key: x` |
| `previous == null \|\| record.a != previous.a` | `null` | `true` |
| `has(previous.x)` | `null` | `false` |
`buildScope` (`packages/formula/src/stdlib.ts`) binds `previous` only
when it is not `undefined`.
### Pins
-
**`packages/objectql/src/validation/rule-previous-insert-binding.test.ts`**
(new) pins these cases:
- A guarded predicate refuses a violating insert with its own message
and no `unevaluable` reason. It admits a valid insert, and still reads
the prior row on update.
- A `conditional` with `when: previous == null` runs its branch on
insert.
- `requiredWhen: previous == null` is `required` on insert.
- An unguarded `previous.KEY` read refuses as `unevaluable`, naming
`previous.KEY`.
- An update with no prior row keeps `previous` unbound, so `previous !=
null && …` is refused, not admitted.
- **`packages/objectql/src/hook-condition-previous-scope.test.ts`.** The
insert case flipped from "leaves `previous` UNBOUND on insert" to two
cases:
- `previous == null && record.done == true` fires on `beforeInsert` and
`afterInsert`, and the engine's `ctx.previous` stays `undefined`.
- `previous.done != true && record.done == true` on `beforeInsert` still
aborts with a `HookConditionError`, `reason: 'unevaluable'`, and a
message naming `'previous.done'`.
-
**`packages/plugins/plugin-security/src/sys-user-permission-set-window-order.test.ts`**
asserts that every refusal carries `validity_window_order`'s own
declared message, read off the object, and no `unevaluable` reason. It
drives the real `ObjectQL` insert path over a real SQL driver with the
real `SecurityPlugin`.
### Red, then green
**Validation pins.**
- *Red* on `099cc9d69` (uncommitted pins): objectql pin 5 failed / 7;
window-order 4 insert refusals failed.
- *Green* at `70374639e`: 7 / 7 and 11 / 11. The objectql `local` suite:
435 files, 8220 tests passed.
- *Ablation* (`scripts/ablation-replace.mjs`): `if (mode === 'insert')
return null;` to `return undefined;` gave 5 / 7 and 4 / 11 failed. The
restore was proven: blob equal to HEAD, `git diff HEAD` empty.
**Hook pin.**
- *Red* on `b0d12e2e9` (flipped test, unchanged source): 2 failed / 25.
The message received was `… (type: Unknown variable: previous) …`.
- *Green* after the change: 25 / 25.
- *Ablation* at `ee0ed2834`: `if (isInsertEvent(ctx.event)) return
null;` to `return undefined;` gave 2 failed / 25. The restore was
proven: blob `fb6056cc7567` equal to HEAD, `git diff HEAD` empty.
No `dist/` is involved in either suite. objectql's tests import its
source, and plugin-security aliases `@objectstack/objectql` to
`src/index.ts`.
**Final head `a2863de08`**, through `scripts/pm/os-verify-lock.sh`:
- objectql `src/validation/`, `src/hook-*`,
`src/engine-after-hook-abort-unit.test.ts` and
`src/bulk-write-per-row-hooks.test.ts`: 41 files, 1300 passed.
- `plugin-security` window-order: 11 / 11.
- `pnpm --filter @objectstack/objectql typecheck`: `tsc --noEmit` plus
`check:test-typecheck` OK, with no new ledger signature.
### REST door
On a stock showcase boot of `b0d12e2e9`, `POST
/api/v1/data/sys_user_permission_set` with `valid_from:
2027-01-02T00:00:00Z` and `valid_until: 2027-01-01T00:00:00Z`:
- It answered **400** `VALIDATION_FAILED`. `fields[0]` = `{ field:
valid_until, code: rule_violation, message: "Valid Until must be later
than Valid From. …" }`, with no `constraint`.
- The valid window answered **201**.
### Gates at `a2863de08`
**Derived set.** `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 100 commands, and all 100 were run.
- **First pass:** 97 exited 0. Three refused on a prerequisite and were
not measurements:
- `check:skill-examples` (needs the `client-react` declarations);
- `check:dual-build-cjs-loads` (needs a workspace build);
- `check:i18n` (needs the built CLI closure).
- **After a workspace `turbo run build`:** all three exited 0.
- **Reconciliation:** `dispatch-gates --ran` reported "100 derived, 100
run, 0 NOT-MEASURED, 0 UNRUN".
- **Docs gate:** `pnpm check:doc-authoring` exited 0.
**Changeset gates.**
- `node scripts/check-changeset-no-major.mjs --base origin/main` exited
0: "This diff introduces no `major` bump". Its level axis reads the PR
body, and a local run has none.
- The level axis was also driven offline with `--event` and this body:
exit 0, "LEVEL AXIS: this PR declares clause-② `yes (widening)`, and no
package whose published source … it moves is graded `patch`".
- `node scripts/check-adr-0087-registration.mjs --base origin/main`
exited 0: "this PR adds no declared-breaking changeset (1 non-breaking
changeset(s) seen)". It reads `yes (widening)` as not breaking.
## Acceptance notes
- **Published skill text (`skills/**`).** It still describes `previous`
as unbound on insert:
- `skills/objectstack-data/references/data-hooks.md`: the "`previous` is
UNBOUND where there is no prior state" bullet, and the "a `previous`
reference on an insert" clause.
- `skills/objectstack-formula/SKILL.md`: the scope-table row "Insert
events …, validation rule on insert | **unbound**", plus the note under
the migration table.
That surface is governed (Tier H) and outside this card's surface, so it
is not edited here. The seat filed the correction as #23983
(`domain:skills`), to land after this PR. Its advice ("write
insert-event conditions over `record` alone"; a `previous.x` read on
insert fails the write) is still safe. Its "unbound" wording is now
false: `previous == null` evaluates on insert.
- **Dogfood comment.**
`packages/qa/dogfood/test/hook-after-condition-rollback.dogfood.test.ts`
comments that `previous` is unbound on an insert. Its condition,
`previous.name != record.name`, still faults on insert, because the
column read off `null` faults. So the test's verdict is unchanged; only
the comment's reason is stale. The objectql twin's comment
(`engine-after-hook-abort-unit.test.ts`) is corrected here.
- **`EvalContext.previous` type.** In `@objectstack/formula` it is typed
for a row only, while `buildScope` already binds `null`. Both surfaces
therefore route `null` through `extra`.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01TQLy3Yq6vF4SDpTXdr3Uwi)_
---------
Co-authored-by: Claude <noreply@anthropic.com>previous to null on insert so a rule that reads it answers with its own message (#23971)1 parent 9f12417 commit b25d98a
8 files changed
Lines changed: 469 additions & 88 deletions
File tree
- .changeset
- content/docs/data-modeling
- packages
- objectql/src
- validation
- plugins/plugin-security/src
| 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
208 | 208 | | |
209 | 209 | | |
210 | 210 | | |
211 | | - | |
| 211 | + | |
212 | 212 | | |
213 | 213 | | |
214 | 214 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
179 | 179 | | |
180 | 180 | | |
181 | 181 | | |
182 | | - | |
| 182 | + | |
183 | 183 | | |
184 | 184 | | |
185 | 185 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
20 | 20 | | |
21 | 21 | | |
22 | 22 | | |
23 | | - | |
24 | | - | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
25 | 28 | | |
26 | 29 | | |
27 | 30 | | |
28 | 31 | | |
29 | 32 | | |
30 | | - | |
| 33 | + | |
31 | 34 | | |
32 | 35 | | |
33 | 36 | | |
| |||
150 | 153 | | |
151 | 154 | | |
152 | 155 | | |
153 | | - | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
154 | 181 | | |
155 | 182 | | |
156 | | - | |
157 | | - | |
158 | | - | |
159 | | - | |
160 | 183 | | |
161 | 184 | | |
162 | 185 | | |
163 | 186 | | |
164 | 187 | | |
165 | 188 | | |
166 | | - | |
167 | | - | |
168 | | - | |
169 | | - | |
170 | | - | |
171 | | - | |
172 | | - | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
173 | 201 | | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
174 | 205 | | |
175 | 206 | | |
176 | 207 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
| 25 | + | |
25 | 26 | | |
26 | 27 | | |
27 | 28 | | |
| |||
281 | 282 | | |
282 | 283 | | |
283 | 284 | | |
284 | | - | |
285 | | - | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
286 | 288 | | |
287 | 289 | | |
288 | 290 | | |
| |||
313 | 315 | | |
314 | 316 | | |
315 | 317 | | |
316 | | - | |
317 | | - | |
318 | | - | |
319 | | - | |
320 | | - | |
321 | | - | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
322 | 323 | | |
323 | 324 | | |
324 | | - | |
| 325 | + | |
325 | 326 | | |
326 | 327 | | |
327 | | - | |
| 328 | + | |
328 | 329 | | |
329 | 330 | | |
330 | 331 | | |
| |||
991 | 992 | | |
992 | 993 | | |
993 | 994 | | |
| 995 | + | |
994 | 996 | | |
995 | | - | |
| 997 | + | |
996 | 998 | | |
997 | 999 | | |
998 | 1000 | | |
| 1001 | + | |
| 1002 | + | |
| 1003 | + | |
| 1004 | + | |
| 1005 | + | |
| 1006 | + | |
| 1007 | + | |
| 1008 | + | |
999 | 1009 | | |
1000 | 1010 | | |
1001 | 1011 | | |
| |||
1182 | 1192 | | |
1183 | 1193 | | |
1184 | 1194 | | |
1185 | | - | |
| 1195 | + | |
1186 | 1196 | | |
1187 | | - | |
1188 | | - | |
1189 | | - | |
1190 | | - | |
1191 | | - | |
1192 | | - | |
1193 | | - | |
1194 | | - | |
1195 | | - | |
1196 | | - | |
1197 | | - | |
1198 | | - | |
1199 | | - | |
1200 | | - | |
1201 | | - | |
| 1197 | + | |
| 1198 | + | |
| 1199 | + | |
| 1200 | + | |
| 1201 | + | |
| 1202 | + | |
| 1203 | + | |
| 1204 | + | |
| 1205 | + | |
| 1206 | + | |
| 1207 | + | |
| 1208 | + | |
| 1209 | + | |
| 1210 | + | |
| 1211 | + | |
| 1212 | + | |
| 1213 | + | |
| 1214 | + | |
| 1215 | + | |
| 1216 | + | |
1202 | 1217 | | |
1203 | | - | |
1204 | | - | |
| 1218 | + | |
| 1219 | + | |
1205 | 1220 | | |
1206 | 1221 | | |
1207 | 1222 | | |
| |||
0 commit comments