From 70374639ed19f44dd5dd62661d85b43f575e5ab3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 13:59:56 +0000 Subject: [PATCH 1/2] fix(objectql): bind `previous` to null on insert so a rule that reads it is evaluable A validation rule's CEL scope left `previous` unbound on insert, so any reference to it -- the guard `previous == null` included -- faulted `Unknown variable: previous`, and a violating insert was refused as `unevaluable` instead of with the rule's own message (`sys_user_permission_set.validity_window_order` is the measured case). `previousBinding` decides the binding once per evaluation -- `null` on insert, the prior row made total on update, unbound when an update's row is not in hand -- and `ruleScope` builds every rule predicate's scope from it, so no seam can drop it with a `?? undefined` again. A column read off a null `previous` still refuses, now naming the read and the guard to add. Claude-Session: https://claude.ai/code/session_01TQLy3Yq6vF4SDpTXdr3Uwi Co-authored-by: Claude --- .../23910-validation-previous-insert.md | 24 +++ .../rule-previous-insert-binding.test.ts | 174 ++++++++++++++++ .../objectql/src/validation/rule-validator.ts | 187 ++++++++++++++---- ...s-user-permission-set-window-order.test.ts | 19 ++ 4 files changed, 362 insertions(+), 42 deletions(-) create mode 100644 .changeset/23910-validation-previous-insert.md create mode 100644 packages/objectql/src/validation/rule-previous-insert-binding.test.ts diff --git a/.changeset/23910-validation-previous-insert.md b/.changeset/23910-validation-previous-insert.md new file mode 100644 index 00000000000..4997afd3f4a --- /dev/null +++ b/.changeset/23910-validation-previous-insert.md @@ -0,0 +1,24 @@ +--- +'@objectstack/objectql': patch +--- + +Clause-②: no + +A validation rule that reads `previous` now answers an insert with its own message. On insert the +rule's CEL scope binds `previous` to `null` (there is no prior row), so the guard +`previous == null` decides the insert — the binding a flow's start condition already gets on its +create leg. Before, `previous` was unbound on insert: any reference to it, the guard included, +faulted `Unknown variable: previous`, and a violating insert was refused as `unevaluable` with that +fault instead of the rule's message. The platform's own `validity_window_order` on +`sys_user_permission_set` is the measured case: an inverted grant window now answers +`400 VALIDATION_FAILED` with "Valid Until must be later than Valid From …", not a CEL fault. + +The binding is the same for every predicate a write evaluates — a `script` / `cross_field` +condition, a `conditional`'s `when`, a field's `requiredWhen` and an option's `visibleWhen` — +and the validate-only preview judges an insert the same way. No shipped rule's verdict changes: +every rule this repository ships that reads `previous` refused and admitted the same inserts +before and after; only the refusal's reason moved. An update whose prior row is not in hand keeps +`previous` unbound and still fails closed, so `previous == null` never reads an update as an +insert. A rule that reads a column off `previous` with no guard (`previous.stage`) still has no +value on insert and still refuses it; the refusal now names that read and the guard to add — +`previous == null || …` to judge an insert, `previous != null && …` to skip one. diff --git a/packages/objectql/src/validation/rule-previous-insert-binding.test.ts b/packages/objectql/src/validation/rule-previous-insert-binding.test.ts new file mode 100644 index 00000000000..7f9ba8dc7c0 --- /dev/null +++ b/packages/objectql/src/validation/rule-previous-insert-binding.test.ts @@ -0,0 +1,174 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#23910] On INSERT a rule's CEL scope binds `previous` to `null` — there is + * no prior row, and that is a fact the rule can read. `previous == null` is how + * a rule says "this write creates the row", the same binding the flow engine + * gives a record-after-write start condition on its create leg. + * + * Before: the insert path handed the evaluator no prior record, and the scope + * builder omits an `undefined` root, so the identifier never existed. ANY + * reference to it — the guard `previous == null` included — faulted + * `Unknown variable: previous`. A rule written "on insert, or on an update + * that moves a bound" refused a violating insert as `unevaluable`, naming the + * fault, and its own message never reached the caller. + * + * Three things are pinned, because each alone leaves a hole: + * + * 1. every rule-evaluation seam that reads the scope binds it — a `script` / + * `cross_field` predicate, a `conditional`'s `when`, and a field's + * `requiredWhen` — so the guard decides an insert and the refusal is the + * rule's own; + * 2. the same rule keeps reading the real prior row on update; + * 3. an update whose prior row is NOT in hand keeps `previous` unbound: the + * row exists, so `null` there would answer "this is an insert" — a + * fabricated fact that would ADMIT a write the rule refuses. + * + * Refusals are read through the ADR-0112 envelope (`status` + `code`), then + * the field error the rule produced. + */ + +import { describe, it, expect } from 'vitest'; +import { resolveThrownHttpError } from '@objectstack/types'; +import { evaluateValidationRules } from './rule-validator'; + +type Mode = 'insert' | 'update'; + +function refusalOf(run: () => unknown): { status: number; code: string; fields: any[] } { + try { + run(); + } catch (e) { + const r = resolveThrownHttpError(e); + return { status: r.status, code: r.code, fields: (r.details?.fields ?? (e as any).fields ?? []) as any[] }; + } + throw new Error('expected the write to be refused, but it was admitted'); +} + +const FIELDS = { + starts: { name: 'starts', label: 'Starts', type: 'number' }, + ends: { name: 'ends', label: 'Ends', type: 'number' }, + code: { name: 'code', label: 'Code', type: 'text' }, +}; + +/** The guard shape the platform's own `validity_window_order` uses. */ +const ORDER_MESSAGE = 'Ends must be after Starts.'; +const orderSchema = { + fields: FIELDS, + validations: [ + { + type: 'cross_field' as const, + name: 'window_order', + fields: ['ends', 'starts'], + condition: { + dialect: 'cel', + source: + '(previous == null || record.starts != previous.starts || record.ends != previous.ends)' + + ' && record.starts != null && record.ends != null && record.ends <= record.starts', + }, + message: ORDER_MESSAGE, + }, + ], +}; + +const run = (schema: unknown, data: Record, mode: Mode, previous?: Record | null) => + () => evaluateValidationRules(schema as any, data, mode, previous === undefined ? {} : { previous }); + +describe('[#23910] insert binds `previous` to null — a predicate rule', () => { + it('NEGATIVE — a violating insert is refused with the RULE\'s message, not an `unevaluable` fault', () => { + const r = refusalOf(run(orderSchema, { starts: 5, ends: 3 }, 'insert')); + expect([r.status, r.code]).toEqual([400, 'VALIDATION_FAILED']); + expect(r.fields.map((f) => [f.field, f.code])).toEqual([['ends', 'rule_violation']]); + expect(r.fields[0].constraint?.reason).not.toBe('unevaluable'); + expect(r.fields[0].message).toBe(ORDER_MESSAGE); + }); + + it('a valid insert is admitted', () => { + expect(run(orderSchema, { starts: 3, ends: 5 }, 'insert')).not.toThrow(); + }); + + it('the same rule still reads the prior row on update: moving a bound is judged, an unrelated edit is not', () => { + const r = refusalOf(run(orderSchema, { ends: 1 }, 'update', { starts: 3, ends: 5 })); + expect([r.status, r.code]).toEqual([400, 'VALIDATION_FAILED']); + expect(r.fields[0].message).toBe(ORDER_MESSAGE); + // A row already stored inverted takes an edit that moves neither bound — + // `previous` is the stored row here, so the guard is false. + expect(run(orderSchema, { code: 'x' }, 'update', { starts: 5, ends: 3 })).not.toThrow(); + }); +}); + +describe('[#23910] insert binds `previous` to null — the other seams that read the scope', () => { + it('NEGATIVE — a `conditional` whose `when` is `previous == null` runs its branch on insert', () => { + const schema = { + fields: FIELDS, + validations: [ + { + type: 'conditional' as const, + name: 'new_rows_name_a_code', + when: { dialect: 'cel', source: 'previous == null' }, + then: { + type: 'script' as const, + name: 'new_row_code_required', + condition: { dialect: 'cel', source: 'record.code == null' }, + message: 'A new row names its code.', + }, + message: 'A new row names its code.', + }, + ], + }; + const r = refusalOf(run(schema, { starts: 1 }, 'insert')); + expect([r.status, r.code]).toEqual([400, 'VALIDATION_FAILED']); + expect(r.fields[0].constraint?.reason).not.toBe('unevaluable'); + expect(r.fields[0].message).toBe('A new row names its code.'); + // On update the `when` is false over the real prior row: the branch never runs. + expect(run(schema, { starts: 2 }, 'update', { starts: 1, code: null })).not.toThrow(); + }); + + it('NEGATIVE — a field `requiredWhen: previous == null` is required on insert, not refused as unevaluable', () => { + const schema = { fields: { ...FIELDS, code: { ...FIELDS.code, requiredWhen: 'previous == null' } } }; + const r = refusalOf(run(schema, { starts: 1 }, 'insert')); + expect([r.status, r.code]).toEqual([400, 'VALIDATION_FAILED']); + expect(r.fields.map((f) => [f.field, f.code])).toEqual([['code', 'required']]); + expect(run(schema, { starts: 1, code: 'c1' }, 'insert')).not.toThrow(); + expect(run(schema, { starts: 2 }, 'update', { starts: 1, code: null })).not.toThrow(); + }); + + it('NEGATIVE — an UNGUARDED `previous.` read still has no verdict on insert, and the refusal names that read', () => { + const schema = { + fields: FIELDS, + validations: [ + { + type: 'script' as const, + name: 'starts_frozen', + condition: { dialect: 'cel', source: 'record.starts != previous.starts' }, + message: 'Starts cannot change.', + }, + ], + }; + const r = refusalOf(run(schema, { starts: 1 }, 'insert')); + expect([r.status, r.code]).toEqual([400, 'VALIDATION_FAILED']); + expect(r.fields[0].constraint).toMatchObject({ rule: 'starts_frozen', reason: 'unevaluable' }); + expect(r.fields[0].message).toContain("'previous.starts'"); + }); +}); + +describe('[#23910] an update without its prior row keeps `previous` UNBOUND', () => { + it('NEGATIVE — `previous != null && …` is refused as unevaluable, never admitted as if it were an insert', () => { + const schema = { + fields: FIELDS, + validations: [ + { + type: 'script' as const, + name: 'updates_keep_starts_small', + condition: { dialect: 'cel', source: 'previous != null && record.starts > 10' }, + message: 'An update keeps Starts at 10 or below.', + }, + ], + }; + // The bulk seam's spelling: the row exists, its state is not in hand. + const r = refusalOf(run(schema, { starts: 11 }, 'update', null)); + expect([r.status, r.code]).toEqual([400, 'VALIDATION_FAILED']); + expect(r.fields[0].constraint).toMatchObject({ rule: 'updates_keep_starts_small', reason: 'unevaluable' }); + // …while an insert, where `null` is the truth, is decided: the guard is false. + expect(run(schema, { starts: 11 }, 'insert')).not.toThrow(); + }); +}); diff --git a/packages/objectql/src/validation/rule-validator.ts b/packages/objectql/src/validation/rule-validator.ts index 52a2caf219c..d0ae5ea1360 100644 --- a/packages/objectql/src/validation/rule-validator.ts +++ b/packages/objectql/src/validation/rule-validator.ts @@ -231,7 +231,7 @@ */ import { ExpressionEngine, collectCelRootIdentifiers, analyzeRelationshipTraversals, findTraversalConflicts } from '@objectstack/formula'; -import type { EvalPermissions, RelationshipTraversalAnalysis } from '@objectstack/formula'; +import type { EvalContext, EvalPermissions, RelationshipTraversalAnalysis } from '@objectstack/formula'; import type { Expression } from '@objectstack/spec'; import { AUDIT_PROVENANCE_FIELDS, RUNTIME_OWNED_FIELD_TYPES, referenceTargetOf, resolveInjectedSystemColumns } from '@objectstack/spec/data'; import type { ValidationAdvisoryEvent } from '@objectstack/spec/data'; @@ -323,9 +323,11 @@ interface RuleContext { /** Prior state overlaid with the PATCH, made TOTAL over the declared fields * (#4649) — see {@link materializeDeclaredFields}. */ merged: Record; - /** The prior record, likewise made total (a COPY — the engine's own - * `hookContext.previous` must not gain materialised nulls). */ - previous: Record | undefined; + /** The `previous` CEL binding (#23910): `null` on insert, the prior record + * made total on update (a COPY — the engine's own `hookContext.previous` + * must not gain materialised nulls), unbound when an update's prior row is + * not in hand. See {@link previousBinding}. */ + previous: PreviousBinding; mode: Mode; logger: EvaluateRulesOptions['logger']; /** Declared fields — the source of a violation's display label (#3957). */ @@ -1446,15 +1448,15 @@ function isReadonlyWhenLocked( parent?: ParentBinding, fields?: Record, ): boolean { - const res = ExpressionEngine.evaluate(toExpression(def.readonlyWhen!), { - record: merged, + const res = ExpressionEngine.evaluate(toExpression(def.readonlyWhen!), ruleScope( + merged, previous, // Bound ONLY when resolved. An absent binding is what makes the unbound-root // fault below reachable, and that fault is the signal — binding `null` here // would turn it into a `No such key`, which the arm after it REFUSES // (ADR-0137 D2) instead of holding the lock. - ...(parent != null ? { extra: { parent } } : {}), - }); + parent != null ? { parent } : undefined, + )); if (!res.ok) { const unbound = unknownVariableOf(res.error); if (unbound) { @@ -2994,7 +2996,7 @@ function evaluateOptionVisibility( fields: Record | undefined, data: Record, merged: Record, - previous: Record | undefined, + previous: PreviousBinding, currentUser: EvaluateRulesOptions['currentUser'], permissions: EvaluateRulesOptions['permissions'], errors: FieldValidationError[], @@ -3005,8 +3007,7 @@ function evaluateOptionVisibility( const user = (currentUser ?? undefined) as any; for (const { name, def, value, opt } of pickedGatedOptions(fields, data)) { const res = ExpressionEngine.evaluate(toExpression(opt.visibleWhen!), { - record: merged, - previous, + ...ruleScope(merged, previous), user, // [#18783] What `current_user.can(object, verb)` is answered from. Passed // through as the engine resolved it: `undefined` leaves `can` loudly @@ -3031,7 +3032,7 @@ function evaluateOptionVisibility( // [ADR-0137 D2, #22402 ruling A] Every other fault REFUSES the write, // naming the option, the field and the fault. Said in the log too: the // operator needs the broken gate there even though the caller is told. - const refusal = unevaluableOptionGateError(name, value, res.error, opt.visibleWhen!, fields); + const refusal = unevaluableOptionGateError(name, value, res.error, opt.visibleWhen!, fields, previous); logger?.warn?.( `option visibleWhen for '${name}=${String(value)}' failed to evaluate ` + `(${noActingUser ? 'system write' : 'authenticated caller'}: ${String(refusal.constraint?.fault)}) — write rejected`, @@ -3240,6 +3241,104 @@ function writesOnlyInjectedSystemColumns( return keys.every((key) => injected.has(key)); } +/** + * The `previous` root of a rule's CEL scope: a row, `null`, or unbound + * (`undefined`). See {@link previousBinding}. + */ +type PreviousBinding = Record | null | undefined; + +/** + * [#23910] What `previous` IS for one evaluation — decided here, once, by + * {@link evaluateValidationRules}, and handed unchanged to every predicate that + * evaluation runs (a `script` / `cross_field` condition, a `conditional`'s + * `when`, a field's `requiredWhen`, an option's `visibleWhen`) through + * {@link ruleScope}. One decision, so no seam can answer the question + * differently from its neighbour. + * + * - **INSERT → `null`.** There is no prior row, and that is a FACT the rule may + * read: `previous == null` is how a rule says "this write creates the row" — + * the binding the flow engine gives a record-after-write start condition on + * its create leg. Until #23910 an insert left it unbound, and the scope + * builder omits an `undefined` root (`buildScope`, `@objectstack/formula`), + * so ANY reference to it — the guard `previous == null` included — faulted + * `Unknown variable: previous`. A rule written "on insert, or on an update + * that moves a bound" then refused every violating insert as + * `unevaluable`, naming the fault, and its own message never reached the + * caller. A read of a COLUMN off it (`previous.x`) still has no value on an + * insert and still refuses the write — see {@link nullPreviousReadDetail}. + * - **UPDATE with the prior row in hand → that row**, made total over the + * declared fields (#4649: a declared column the driver did not return reads + * as `null` instead of faulting). Copied, never mutated in place — the same + * object is the engine's `hookContext.previous`, which after-hooks observe. + * - **UPDATE without it → unbound.** The row exists; its state is not in hand + * (the bulk seam's `previous: null`, an update-mode preview naming no row). + * `null` there would answer `previous == null` with "this is an insert" — a + * fabricated fact that would ADMIT a write a rule like + * `previous != null && …` exists to refuse. Unbound, such a predicate fails + * closed. `ruleNeedsPrior` keeps every rule that reads the prior state off + * the bulk seam, so this arm is a guard, not a path. + */ +function previousBinding( + mode: Mode, + priorRecord: Record | undefined, + fields: Record | undefined, +): PreviousBinding { + if (mode === 'insert') return null; + return priorRecord ? materializeDeclaredFields({ ...priorRecord }, fields) : undefined; +} + +/** + * [#23910] The CEL scope a rule predicate is evaluated in: `record`, the + * `previous` {@link previousBinding} decided, and the caller's `extra` roots + * (`parent`). Every rule-evaluation `ExpressionEngine.evaluate` in this file + * builds its scope here, so the `previous` binding cannot be dropped at one + * seam and kept at another — the `previous ?? undefined` each seam used to + * spell turned an explicit `null` back into "unbound". + * + * `null` rides `extra`, the bag `buildScope` merges onto the variable scope — + * the channel the flow engine binds its own `previous: null` through — + * because `EvalContext.previous` is typed for a row. `undefined` is simply + * omitted, which is what leaves the root unbound. + */ +function ruleScope( + record: Record, + previous: PreviousBinding, + extra?: Record, +): EvalContext { + if (previous === null) return { record, extra: { ...extra, previous: null } }; + return { record, ...(previous ? { previous } : {}), ...(extra ? { extra } : {}) }; +} + +/** + * [#23910] The trailing sentence for a predicate that read a column off + * `previous` where `previous` is `null` — an insert — or `undefined` when the + * fault is anything else. + * + * CEL answers `null.x` with `No such key: x`, the same fault an undeclared key + * raises, so the generic sentences would mislead twice over: "this object does + * not declare" is false for a declared column, and "its value was not + * supplied" sends the author to a payload that did supply it. On insert the + * record is total over the declared fields, so a declared key that faults can + * only have been read off `previous`. The repair is a guard, either way round. + */ +function nullPreviousReadDetail( + error: { kind: string; message: string }, + pred: string | Expression, + previous: PreviousBinding, + fields: Record | undefined, + what: string, +): string | undefined { + if (previous !== null || !fields) return undefined; + const key = missingKeyOf(error); + const source = celSourceOfPredicate(pred); + if (!key || !source || !Object.prototype.hasOwnProperty.call(fields, key)) return undefined; + const reads = analyzeRelationshipTraversals(source, 'previous'); + if (!reads || !(reads.bareFields.has(key) || reads.traversals.has(key))) return undefined; + return ` The ${what} reads 'previous.${key}', and an insert has no prior row: \`previous\` is null there.` + + ' Guard the read — `previous == null || …` to judge an insert, `previous != null && …` to skip one —' + + ' so the rule decides an insert without it.'; +} + /** * Evaluate an object's declared validation rules against an incoming write. * @@ -3282,14 +3381,10 @@ export function evaluateValidationRules( // it is and let the (rare) unevaluable predicate fail closed. `ruleNeedsPrior` // makes this path unreachable for every rule that reads the merged record. const groundTruth = mode === 'insert' || priorRecord !== undefined; - // The `previous` CEL binding is made total too (#4649): a predicate reading - // `previous.x` for a declared column the driver did not return would - // otherwise fault — and now that faults are rejections, that would 422 a - // perfectly good rule. Copied, never mutated in place: the same object is the - // engine's `hookContext.previous`, which after-hooks observe. - const previous = mode === 'update' && priorRecord - ? materializeDeclaredFields({ ...priorRecord }, fields) - : priorRecord; + // [#23910] The `previous` CEL binding — `null` on insert, the prior row made + // total on update (#4649), unbound when an update's row is not in hand — + // decided ONCE here for every predicate below. See {@link previousBinding}. + const previous = previousBinding(mode, priorRecord, fields); // Merged view used by predicate rules: prior state overlaid with the PATCH, // so a rule referencing an unchanged field still sees its persisted value. const merged: Record = { ...(previous ?? {}), ...data }; @@ -3327,15 +3422,15 @@ export function evaluateValidationRules( // and bound ONLY when the engine actually resolved one. An absent binding // is left absent rather than bound to `null`: `null.status` would fault as // `No such key` and lose the "which root was unbound" diagnostic below. - const parentScope = opts.parent != null ? { extra: { parent: opts.parent } } : {}; + const parentExtra = opts.parent != null ? { parent: opts.parent } : undefined; // The pre-check's header (see `EvaluateRulesOptions.previousParent`). Same // row as `parent` unless this write repoints the detail at another master. const prevParent = opts.previousParent !== undefined ? opts.previousParent : opts.parent; - const prevParentScope = prevParent != null ? { extra: { parent: prevParent } } : {}; + const prevParentExtra = prevParent != null ? { parent: prevParent } : undefined; for (const [name, def] of Object.entries(fields)) { const pred = def?.requiredWhen; if (!pred) continue; - const res = ExpressionEngine.evaluate(toExpression(pred), { record: merged, previous, ...parentScope }); + const res = ExpressionEngine.evaluate(toExpression(pred), ruleScope(merged, previous, parentExtra)); if (!res.ok) { // [ADR-0137 D2] A `requiredWhen` that cannot be evaluated REFUSES the // write and names the field and the rule; nothing is persisted (the @@ -3357,7 +3452,7 @@ export function evaluateValidationRules( opts.logger?.warn?.( `requiredWhen for '${name}' failed to evaluate (${res.error.kind}: ${String(res.error.message).split('\n')[0]}) — write rejected`, ); - errors.push(unevaluableFieldRuleError('requiredWhen', name, res.error, pred, fields)); + errors.push(unevaluableFieldRuleError('requiredWhen', name, res.error, pred, fields, previous)); continue; } if (res.value === true && isMissing(merged[name])) { @@ -3366,7 +3461,7 @@ export function evaluateValidationRules( // record, treat the pre state as COMPLIANT — enforcement stays on // unless a legacy violation is proven. if (mode === 'update' && previous) { - const pre = ExpressionEngine.evaluate(toExpression(pred), { record: previous, previous, ...prevParentScope }); + const pre = ExpressionEngine.evaluate(toExpression(pred), ruleScope(previous, previous, prevParentExtra)); const preViolated = pre.ok && pre.value === true && isMissing(previous[name]); if (preViolated) continue; // legacy rows rest } @@ -3605,7 +3700,7 @@ function checkStateMachine( rule: StateMachineRule, mode: Mode, data: Record, - previous: Record | undefined, + previous: PreviousBinding, ctx?: Pick, ): FieldValidationError | null { // An author-written `rule.message` wins over the built-in fallback, and is @@ -3768,12 +3863,17 @@ function unevaluableFieldRuleError( error: { kind: string; message: string }, pred: string | Expression, fields: Record | undefined, + previous?: PreviousBinding, ): FieldValidationError { const source = celSourceOfPredicate(pred); let detail: string | undefined; const missing = missingKeyOf(error); const through = missing && source ? referenceReadThrough(missing, source, fields) : undefined; - if (through) { + // [#23910] A column read off an insert's `null` previous — see {@link nullPreviousReadDetail}. + const nullRead = nullPreviousReadDetail(error, pred, previous, fields, 'predicate'); + if (nullRead !== undefined) { + detail = nullRead; + } else if (through) { detail = ` The predicate reads '${missing}' through '${through.ref}', a reference to '${through.target}'. A field-level` + ` \`${slot}\` is evaluated against this record alone and never reads the related record` @@ -3886,13 +3986,18 @@ function unevaluableOptionGateError( error: { kind: string; message: string }, pred: string | Expression, fields: Record | undefined, + previous?: PreviousBinding, ): FieldValidationError { const source = celSourceOfPredicate(pred); let detail: string | undefined; const missing = missingKeyOf(error); const unbound = missing ? undefined : unknownVariableOf(error); const through = missing && source ? referenceReadThrough(missing, source, fields) : undefined; - if (through) { + // [#23910] A column read off an insert's `null` previous — see {@link nullPreviousReadDetail}. + const nullRead = nullPreviousReadDetail(error, pred, previous, fields, 'predicate'); + if (nullRead !== undefined) { + detail = nullRead; + } else if (through) { detail = ` The predicate reads '${missing}' through '${through.ref}', a reference to '${through.target}'. An option's` + ' `visibleWhen` is evaluated against this record alone and never reads the related record' @@ -4150,14 +4255,14 @@ function referenceGuardRepair(field: string): string { function readsAnOwnColumnItLacks( source: string, record: Record, - previous: Record | undefined, + previous: PreviousBinding, fields: Record | undefined, ): boolean { const lacks = (holder: unknown, key: string): boolean => !(holder && typeof holder === 'object' && key in holder); const faultsOn = ( analysis: RelationshipTraversalAnalysis | null, - holder: Record | undefined, + holder: Record | null | undefined, skipReferences: boolean, ): boolean => { if (!analysis) return false; @@ -4202,7 +4307,7 @@ function referentialClearRefusal( expr: Expression, fault: FieldValidationError, record: Record, - previous: Record | undefined, + previous: PreviousBinding, related: RelatedRecordBinding | undefined, fields: Record | undefined, ): FieldValidationError | null { @@ -4255,7 +4360,7 @@ function referentialClearRefusal( function checkPredicate( rule: PredicateRule, record: Record, - previous: Record | undefined, + previous: PreviousBinding, logger: EvaluateRulesOptions['logger'], messages?: ValidationMessageContext, related?: RelatedRecordBinding, @@ -4285,10 +4390,7 @@ function checkPredicate( record = scope.record; } - const result = ExpressionEngine.evaluate(expr, { - record, - previous: previous ?? undefined, - }); + const result = ExpressionEngine.evaluate(expr, ruleScope(record, previous)); if (!result.ok) { // Still logged — the operator needs the fault in the log even though the @@ -4296,8 +4398,10 @@ function checkPredicate( logger?.warn?.( `Validation rule '${rule.name}' predicate failed to evaluate (${result.error.kind}: ${result.error.message}) — write rejected: a rule that cannot be evaluated fails closed, it is never skipped`, ); + const nullRead = nullPreviousReadDetail(result.error, rule.condition, previous, fields, 'predicate'); const unevaluable = unevaluableRuleError( - rule.name, field, result.error, 'predicate', undefined, + rule.name, field, result.error, 'predicate', + nullRead !== undefined ? { prose: `Validation rule '${rule.name}'`, detail: nullRead } : undefined, declaredColumnRead(fields, celSourceOfPredicate(rule.condition)), ); // [#20006] Same verdict; on a delete's reference cleanup, a text that names it. @@ -4454,17 +4558,16 @@ function checkConditional( rule: ConditionalRule, ctx: RuleContext, ): FieldValidationError | null { - const result = ExpressionEngine.evaluate(toExpression(rule.when), { - record: ctx.merged, - previous: ctx.previous ?? undefined, - }); + const result = ExpressionEngine.evaluate(toExpression(rule.when), ruleScope(ctx.merged, ctx.previous)); if (!result.ok) { ctx.logger?.warn?.( `Validation rule '${rule.name}' when-predicate failed to evaluate (${result.error.kind}: ${result.error.message}) — write rejected: a rule that cannot be evaluated fails closed, it is never skipped`, ); + const nullRead = nullPreviousReadDetail(result.error, rule.when, ctx.previous, ctx.fields, 'when-predicate'); return unevaluableRuleError( - rule.name, '_record', result.error, 'when-predicate', undefined, + rule.name, '_record', result.error, 'when-predicate', + nullRead !== undefined ? { prose: `Validation rule '${rule.name}'`, detail: nullRead } : undefined, declaredColumnRead(ctx.fields, celSourceOfPredicate(rule.when)), ); } diff --git a/packages/plugins/plugin-security/src/sys-user-permission-set-window-order.test.ts b/packages/plugins/plugin-security/src/sys-user-permission-set-window-order.test.ts index 442ceb2da42..dd39d862bc5 100644 --- a/packages/plugins/plugin-security/src/sys-user-permission-set-window-order.test.ts +++ b/packages/plugins/plugin-security/src/sys-user-permission-set-window-order.test.ts @@ -18,6 +18,10 @@ * existed does not block an unrelated edit — repairing it is the only write * that has to state a valid window. * + * An insert is judged with `previous` bound to `null` — the clause + * `previous == null` is how the rule says "this write makes the window" — so + * the refusal carries the rule's own message on insert exactly as on update. + * * Measured on a REAL `ObjectQL` engine over a real SQL driver with the REAL * `SecurityPlugin` started on it, the harness `grant-permission-set-name.test.ts` * uses: what is pinned is what the write door does, and a double would answer @@ -131,10 +135,25 @@ function envelopeOf(e: unknown) { return { status: r.status, code: r.code, fields: (r.details?.fields ?? []) as any[] }; } +/** The rule's own declared message — read off the object, never retyped here. */ +const WINDOW_ORDER_MESSAGE = (SysUserPermissionSet.validations as unknown as Array<{ name: string; message: string }>) + .find((r) => r.name === 'validity_window_order')!.message; + +/** + * The refusal is the RULE's verdict: its own message, and no `unevaluable` + * reason. `rule_violation` alone does not say that — a rule that could not be + * evaluated refuses under the same code, naming the CEL fault instead. On + * insert that is what this rule answered while the insert path left `previous` + * unbound: `previous == null` faulted `Unknown variable: previous`, and the + * administrator read a fault where the rule's sentence belongs. + */ function expectWindowRefusal(err: unknown): void { const env = envelopeOf(err); expect([env.status, env.code]).toEqual([400, 'VALIDATION_FAILED']); expect(env.fields.map((f) => [f.field, f.code])).toEqual([['valid_until', 'rule_violation']]); + expect(env.fields[0].constraint?.reason).not.toBe('unevaluable'); + // Its opening: a multi-row update appends the row it refused, `(record ID)`. + expect(String(env.fields[0].message).slice(0, WINDOW_ORDER_MESSAGE.length)).toBe(WINDOW_ORDER_MESSAGE); } async function grantsOf(h: Harness, userId: string): Promise { From ee0ed2834d4f6bf55357956acd1c03ce424f63b8 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 15:21:50 +0000 Subject: [PATCH 2/2] =?UTF-8?q?feat(objectql):=20hook=20conditions=20bind?= =?UTF-8?q?=20`previous`=20to=20null=20on=20insert=20too;=20Clause-?= =?UTF-8?q?=E2=91=A1=20yes=20(widening)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A declarative hook condition is now evaluated in the validation side's scope (`ruleScope`): `previous` is `null` on beforeInsert/afterInsert, the prior row on any other write that carries one, and unbound when an existing row's prior state is not in hand. Only the CEL binding changes; the engine's `ctx.previous` is untouched. A column read off the null `previous` still aborts the operation, now naming the read and the guard to add (`nullPreviousReadDetail`, shared with validation). The changeset becomes `minor` with `Clause-②: yes (widening)`: a rule or condition shaped `previous != null && ...` now admits inserts it refused with an `unevaluable` fault. The formulas page's `previous` row says `previous` is null on insert in validation, hook conditions and flows. Claude-Session: https://claude.ai/code/session_01TQLy3Yq6vF4SDpTXdr3Uwi Co-authored-by: Claude --- .../23910-validation-previous-insert.md | 40 +++++----- content/docs/data-modeling/formulas.mdx | 2 +- .../src/engine-after-hook-abort-unit.test.ts | 2 +- .../src/hook-condition-previous-scope.test.ts | 61 ++++++++++++---- packages/objectql/src/hook-wrappers.ts | 73 +++++++++++-------- .../objectql/src/validation/rule-validator.ts | 21 ++++-- 6 files changed, 130 insertions(+), 69 deletions(-) diff --git a/.changeset/23910-validation-previous-insert.md b/.changeset/23910-validation-previous-insert.md index 4997afd3f4a..6093555eb24 100644 --- a/.changeset/23910-validation-previous-insert.md +++ b/.changeset/23910-validation-previous-insert.md @@ -1,24 +1,30 @@ --- -'@objectstack/objectql': patch +'@objectstack/objectql': minor --- -Clause-②: no +Clause-②: yes (widening) -A validation rule that reads `previous` now answers an insert with its own message. On insert the -rule's CEL scope binds `previous` to `null` (there is no prior row), so the guard -`previous == null` decides the insert — the binding a flow's start condition already gets on its -create leg. Before, `previous` was unbound on insert: any reference to it, the guard included, -faulted `Unknown variable: previous`, and a violating insert was refused as `unevaluable` with that -fault instead of the rule's message. The platform's own `validity_window_order` on -`sys_user_permission_set` is the measured case: an inverted grant window now answers -`400 VALIDATION_FAILED` with "Valid Until must be later than Valid From …", not a CEL fault. +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 binding a flow's start +condition already gets on its create leg. Until now `previous` was unbound on insert: any +reference to it faulted `Unknown variable: previous`, and the write was refused (validation) or +aborted (hook) as `unevaluable`. So this widens what an insert can pass: a rule shaped +`previous != null && …` ("judge updates only") now ADMITS the inserts it used to refuse with that +fault, and a field's `requiredWhen: previous == null` now requires the field on insert instead of +refusing every insert. `previous == null || …` judges an insert, `previous != null && …` skips +one. A column read off `previous` on insert (`previous.stage`) still has no value and still +refuses the write; the refusal now names that read and the guard to add. + +A violating insert under a rule that reads `previous` now answers with the rule's own message. +The platform's own `validity_window_order` on `sys_user_permission_set` is the measured case: an +inverted grant window answers `400 VALIDATION_FAILED` with "Valid Until must be later than Valid +From …", not a CEL fault. The binding is the same for every predicate a write evaluates — a `script` / `cross_field` condition, a `conditional`'s `when`, a field's `requiredWhen` and an option's `visibleWhen` — -and the validate-only preview judges an insert the same way. No shipped rule's verdict changes: -every rule this repository ships that reads `previous` refused and admitted the same inserts -before and after; only the refusal's reason moved. An update whose prior row is not in hand keeps -`previous` unbound and still fails closed, so `previous == null` never reads an update as an -insert. A rule that reads a column off `previous` with no guard (`previous.stage`) still has no -value on insert and still refuses it; the refusal now names that read and the guard to add — -`previous == null || …` to judge an insert, `previous != null && …` to skip one. +and for a hook's `condition` on `beforeInsert` / `afterInsert`; the validate-only preview judges +an insert the same way. Only the CEL binding is `null`: a hook's `ctx.previous` is unchanged. No +shipped rule's verdict changes: every rule and hook this repository ships that reads `previous` +refused and admitted the same inserts before and after; only the refusal's reason moved. A write +to an existing row whose prior state is not in hand keeps `previous` unbound and still fails +closed, so `previous == null` never reads an update as an insert. diff --git a/content/docs/data-modeling/formulas.mdx b/content/docs/data-modeling/formulas.mdx index 4ecf2728193..b9de42b18e2 100644 --- a/content/docs/data-modeling/formulas.mdx +++ b/content/docs/data-modeling/formulas.mdx @@ -208,7 +208,7 @@ Keep them pure, dependency-free, and AI-readable. | Binding | Source | Available in | |:---|:---|:---| | `record` | the row being evaluated | formulas, validation, sharing, visibility | -| `previous` | row before update — on a `multi: true` write, that row's own pre-write state, in `before*` and `after*` hooks alike and in record-change triggers: since ADR-0058 Addendum II both phases dispatch once per matched row | hooks, validation on update | +| `previous` | the row before the update — on a `multi: true` write, that row's own pre-write state, in `before*` and `after*` hooks alike and in record-change triggers: since ADR-0058 Addendum II both phases dispatch once per matched row. `null` on insert, so `previous == null` judges an insert; a column read off it there (`previous.status`) has no value and faults | validation, hook conditions and flows — `null` on insert | | `input` | hook payload | hooks | | `current_user` | the authenticated subject — the canonical binding (ADR-0068). `user`, `ctx.user` and `os.user` are aliases of the **same** object | predicates with identity | | `os.user` | alias of `current_user` | seed, predicates with identity | diff --git a/packages/objectql/src/engine-after-hook-abort-unit.test.ts b/packages/objectql/src/engine-after-hook-abort-unit.test.ts index 8f046d6e95e..d5207acebcc 100644 --- a/packages/objectql/src/engine-after-hook-abort-unit.test.ts +++ b/packages/objectql/src/engine-after-hook-abort-unit.test.ts @@ -179,7 +179,7 @@ describe('#22782 — an aborting after* hook rolls its write back', () => { * condition can abort its write too, and the door's unit rolls the row back. */ describe('#22856 — an after* hook carrying a condition rolls its write back when the condition cannot be evaluated', () => { - /** `previous` is unbound on an insert, so this condition cannot be evaluated there. */ + /** `previous` is `null` on an insert, so a column read off it cannot be evaluated there. */ const unbindable = 'previous.name != record.name'; /** A key `case` does not declare: unevaluable on every event. */ const undeclared = 'record.stauts == "x"'; diff --git a/packages/objectql/src/hook-condition-previous-scope.test.ts b/packages/objectql/src/hook-condition-previous-scope.test.ts index 330e3cb89b3..08268fc405b 100644 --- a/packages/objectql/src/hook-condition-previous-scope.test.ts +++ b/packages/objectql/src/hook-condition-previous-scope.test.ts @@ -20,14 +20,17 @@ * - an UNDECLARED key stays unevaluable, so typos remain reportable; * - copied, never mutated in place — the after-hooks that run next observe * the engine's own `ctx.previous`; - * - absent (unbound identifier) whenever the prior state is not in hand: - * insert, and a predicate bulk update. + * - `null` on an insert event (#23910): there is no prior row, so + * `previous == null` judges the insert, and a column read off it + * (`previous.done`) still faults loudly; + * - absent (unbound identifier) on any other context whose prior state is + * not in hand — an unknown row is never bound as `null`. */ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { ObjectQL } from './engine.js'; import { bindHooksToEngine } from './hook-binder.js'; -import { wrapDeclarativeHook } from './hook-wrappers.js'; +import { wrapDeclarativeHook, HookConditionError } from './hook-wrappers.js'; import type { Hook, HookContext } from '@objectstack/spec/data'; /** `archived` is declared but never written by any test here — the @@ -150,27 +153,55 @@ describe('[#4784] hook condition binds `previous` alongside `record`', () => { expect((ctx.previous as any).archived).toBeUndefined(); }); - it('leaves `previous` UNBOUND on insert — verbatim the validation side', async () => { + it('binds `previous` to NULL on insert — the validation side\'s rule — so `previous == null` judges the insert', async () => { + const calls: string[] = []; + const { logger, conditionWarnings } = captureLogger(); + // [#23910] An insert has no prior row, and that is a fact the condition + // may read: `previous` is bound to `null` on an insert event, exactly as a + // validation rule's scope and a flow's create leg bind it. Until then it + // was unbound, so `previous == null` faulted `Unknown variable: previous` + // and #4775 aborted the insert. + const wrapped = wrapDeclarativeHook( + makeHook('previous == null && record.done == true', ['beforeInsert', 'afterInsert']), + (async () => { calls.push('ran'); }) as any, + { logger }, + ); + + const before = makeCtx({ event: 'beforeInsert', previous: undefined, input: { data: { done: true } } } as any); + await wrapped(before); + await wrapped(makeCtx({ event: 'afterInsert', previous: undefined, input: { data: { done: true } } } as any)); + expect(calls).toEqual(['ran', 'ran']); + expect(conditionWarnings()).toEqual([]); + // Only the CEL binding is null: the engine's own `ctx.previous`, which the + // after-hooks observe, is untouched. + expect(before.previous).toBeUndefined(); + }); + + it('NEGATIVE — a COLUMN read off the null `previous` on insert still faults loudly, naming the read', async () => { const calls: string[] = []; const { logger } = captureLogger(); - // `rule-validator.ts` binds `previous` only for `mode: 'update'` with a - // prior record in hand; on insert it passes `undefined`, which omits the - // identifier from the CEL scope. Referencing it on an insert event is - // therefore an author error, reported — not silently answered. const wrapped = wrapDeclarativeHook( makeHook(TRANSITION, ['beforeInsert']), (async () => { calls.push('ran'); }) as any, { logger }, ); - // #4775: an unbound `previous` is unevaluable, and unevaluable rejects the - // insert rather than being swallowed into `false`. - await expect(wrapped(makeCtx({ - event: 'beforeInsert', - previous: undefined, - input: { data: { done: true } }, - } as any))).rejects.toThrow(/Unknown variable: previous/); + // #4775: unevaluable rejects the insert rather than being swallowed into + // `false` — `previous` has no columns on an insert. + let err: any = null; + try { + await wrapped(makeCtx({ + event: 'beforeInsert', + previous: undefined, + input: { data: { done: true } }, + } as any)); + } catch (e) { + err = e; + } + expect(err).toBeInstanceOf(HookConditionError); + expect(err.reason).toBe('unevaluable'); + expect(String(err.message)).toContain("'previous.done'"); expect(calls).toEqual([]); }); diff --git a/packages/objectql/src/hook-wrappers.ts b/packages/objectql/src/hook-wrappers.ts index 3f1f3d6de2c..fb6056cc756 100644 --- a/packages/objectql/src/hook-wrappers.ts +++ b/packages/objectql/src/hook-wrappers.ts @@ -22,6 +22,7 @@ import { ExpressionEngine } from '@objectstack/formula'; import { noopHookMetricsRecorder, type HookMetricsRecorder, type HookMetricOutcome } from './hook-metrics.js'; import { materializeDeclaredFields } from './declared-fields.js'; import { describeCelFault, type CelFault } from './cel-fault.js'; +import { nullPreviousReadDetail, ruleScope, type PreviousBinding } from './validation/rule-validator.js'; import { hookRunAs, deriveHookApi, type HookRunAs } from './hook-run-as.js'; /** @@ -281,8 +282,9 @@ export function declaredHookCondition(meta: Pick): (Expressio * the platform can no longer paper over. * - `previous` — the record's pre-write state, built by * {@link pickPreviousPayload}: `ctx.previous`, made total over the same - * declared fields. Absent (an unbound CEL identifier) whenever the prior - * state is not in hand, exactly as on the validation side. + * declared fields; `null` on an insert event, which has no prior row; and + * absent (an unbound CEL identifier) whenever an existing row's prior + * state is not in hand — exactly as on the validation side (#23910). * * `previous` is what makes a TRANSITION expressible. Since #4770 `record` is * the record's STATE, so `record.done == true` is true on every update of an @@ -313,18 +315,17 @@ export function wrapDeclarativeHook( const check = ExpressionEngine.compile(expr); if (check.ok) { conditionFn = (ctx: HookContext) => { - // `previous` is passed through as-is: `undefined` means the binding - // is OMITTED from the CEL scope (see `buildScope` in - // @objectstack/formula), which is exactly what the validation side - // does when no prior record is in hand. Binding it to an empty - // object instead would answer `previous.x == null` with a - // fabricated "yes" for a record whose prior state is unknown. + // [#23910] The scope is the validation side's, built by the same + // `ruleScope`: `previous` is `null` on an insert event, the prior row + // on any other write that carries one, and OMITTED from the CEL scope + // (`undefined`, see `buildScope` in @objectstack/formula) when an + // existing row's prior state is not in hand — never invented. const record = pickRecordPayload(ctx); const previous = pickPreviousPayload(ctx); - const r = ExpressionEngine.evaluate(expr, { record: record ?? {}, previous }); + const r = ExpressionEngine.evaluate(expr, ruleScope(record ?? {}, previous)); if (!r.ok) { // [#4775] Fail LOUD. Not `false` — see `HookConditionError`. - throw unevaluableConditionError(meta, ctx, source, r.error); + throw unevaluableConditionError(meta, ctx, source, r.error, previous); } return Boolean(r.value); }; @@ -991,11 +992,20 @@ function unevaluableConditionError( ctx: HookContext, source: string, error: CelFault, + previous?: PreviousBinding, ): HookConditionError { - const { summary, missingKey, detail } = describeCelFault(error, { + const described = describeCelFault(error, { what: 'condition', undeclaredKeyFix: "fix the hook's condition, or declare the field", }); + const { summary } = described; + // [#23910] A column read off an insert's `null` previous gets the validation + // side's sentence (`nullPreviousReadDetail`): "this object does not declare" + // would be false for a declared column. Like the validation side, a sentence + // worded for the fault carries no generic `missingKey`. + const nullRead = nullPreviousReadDetail(error, source, previous, declaredFieldsFor(ctx), 'condition'); + const detail = nullRead ?? described.detail; + const missingKey = nullRead === undefined ? described.missingKey : undefined; const head = `Hook '${meta.name}' could not evaluate its condition (${summary}) — operation aborted.`; return new HookConditionError(`${head}${detail}`, { hook: meta.name, @@ -1182,26 +1192,31 @@ function pickRecordPayload(ctx: HookContext): any { * handlers columns the row never had (#4649 left the same note on the * validation side). * - * ## When it is ABSENT — verbatim the validation side's rule + * ## When it is `null`, and when it is ABSENT — verbatim the validation side's rule * - * `rule-validator.ts` binds `previous` only for `mode: 'update'` with a prior - * record actually fetched, and passes `undefined` otherwise, which omits the - * identifier from the CEL scope. Same here: - * - **insert** — there is no prior state, so `previous` is unbound and any - * reference to it is an author error, reported as such; - * - **any update-shaped context whose prior row is not in hand** — with no - * row there is no single prior record to bind, so `previous` stays unbound - * rather than being invented. A predicate (`multi: true`) bulk write is no - * longer one of these: since #5574 (ADR-0058 Addendum II, D1/D2) it - * dispatches BOTH phases per matched row — `after*` since #5038 — each - * context binding that row's own pre-image, so a transition condition - * reads exactly as it does on a single-record write. - * Binding `null`/`{}` instead would make `previous.x == null` answer "yes" - * for a record whose prior state is simply unknown — a fabricated fact, the - * one thing materialisation is careful never to do. + * `rule-validator.ts` decides the binding once per evaluation + * (`previousBinding`) and both surfaces build their scope with the same + * `ruleScope` (#23910). Same rule here: + * - **insert** (`beforeInsert` / `afterInsert`) — `null`. There is no prior + * row, and that is a FACT a condition may read: `previous == null` is how + * a condition says "this write creates the row", the binding a flow's + * create leg and a validation rule's insert already get. A COLUMN read off + * it (`previous.done`) still has no value, faults, and aborts the + * operation (#4775), with a sentence naming the read and the guard to add. + * Only the CEL binding is `null`: `ctx.previous`, the engine's pre-image, + * is not touched. + * - **any other context whose prior row is not in hand** — unbound. The row + * exists and its state is unknown, so `previous` stays unbound rather than + * being invented: `null` there would answer `previous == null` with "this + * is an insert", a fabricated fact, the one thing materialisation is + * careful never to do. A predicate (`multi: true`) bulk write is no longer + * one of these: since #5574 (ADR-0058 Addendum II, D1/D2) it dispatches + * BOTH phases per matched row — `after*` since #5038 — each context + * binding that row's own pre-image, so a transition condition reads + * exactly as it does on a single-record write. */ -function pickPreviousPayload(ctx: HookContext): Record | undefined { - if (isInsertEvent(ctx.event)) return undefined; +function pickPreviousPayload(ctx: HookContext): PreviousBinding { + if (isInsertEvent(ctx.event)) return null; const prior = ctx.previous; if (!prior || typeof prior !== 'object' || Array.isArray(prior)) return undefined; return materializeDeclaredFields({ ...(prior as Record) }, declaredFieldsFor(ctx)); diff --git a/packages/objectql/src/validation/rule-validator.ts b/packages/objectql/src/validation/rule-validator.ts index d0ae5ea1360..bbeae1c09c9 100644 --- a/packages/objectql/src/validation/rule-validator.ts +++ b/packages/objectql/src/validation/rule-validator.ts @@ -3243,9 +3243,11 @@ function writesOnlyInjectedSystemColumns( /** * The `previous` root of a rule's CEL scope: a row, `null`, or unbound - * (`undefined`). See {@link previousBinding}. + * (`undefined`). See {@link previousBinding}. Package-internal export: a + * declarative hook condition binds the same root (`pickPreviousPayload`, + * `hook-wrappers.ts`). */ -type PreviousBinding = Record | null | undefined; +export type PreviousBinding = Record | null | undefined; /** * [#23910] What `previous` IS for one evaluation — decided here, once, by @@ -3299,8 +3301,12 @@ function previousBinding( * the channel the flow engine binds its own `previous: null` through — * because `EvalContext.previous` is typed for a row. `undefined` is simply * omitted, which is what leaves the root unbound. + * + * Package-internal export: a declarative hook condition is evaluated in this + * same scope (`hook-wrappers.ts`), so the two surfaces cannot drift apart on + * what `previous` is. */ -function ruleScope( +export function ruleScope( record: Record, previous: PreviousBinding, extra?: Record, @@ -3320,12 +3326,15 @@ function ruleScope( * supplied" sends the author to a payload that did supply it. On insert the * record is total over the declared fields, so a declared key that faults can * only have been read off `previous`. The repair is a guard, either way round. + * + * Package-internal export: a declarative hook condition's refusal words the + * same read the same way (`unevaluableConditionError`, `hook-wrappers.ts`). */ -function nullPreviousReadDetail( +export function nullPreviousReadDetail( error: { kind: string; message: string }, pred: string | Expression, previous: PreviousBinding, - fields: Record | undefined, + fields: Record | undefined, what: string, ): string | undefined { if (previous !== null || !fields) return undefined; @@ -3336,7 +3345,7 @@ function nullPreviousReadDetail( if (!reads || !(reads.bareFields.has(key) || reads.traversals.has(key))) return undefined; return ` The ${what} reads 'previous.${key}', and an insert has no prior row: \`previous\` is null there.` + ' Guard the read — `previous == null || …` to judge an insert, `previous != null && …` to skip one —' - + ' so the rule decides an insert without it.'; + + ` so the ${what} decides an insert without it.`; } /**