Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions .changeset/flow-trigger-record-materialize.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
---
"@objectstack/trigger-record-change": patch
---

fix(trigger-record-change): the seeded flow record is total over the object's declared fields — no more fault on an untouched field (#4953)

A record-change flow's `record` / `previous` CEL roots used to be **sparse**:
`record` was seeded as `{ ...(inputData ?? {}), ...after }` with no fallback to
the prior row, so a declared field this write's payload didn't mention — and the
driver's after-row didn't echo back either — was simply an ABSENT key. CEL is
strict about that: `record.x != null` on a record missing the key `x` doesn't
evaluate to `false`, it **faults** (`No such key: x`), while `has(record.x)`
silently answers `false` for the same reason — reading as "the field genuinely
has no value" when the truth is "this evaluation point never got told". A
`record-before-*` trigger's `record` was hit hardest: with no `after` row at all
(the write hasn't landed yet), it was literally just the incoming patch.

This is the services-lane half of the maintainer's 2026-08-06 ruling on #4953
item 1 ("server-side unified, cross-process deferred"). The engine-core half
(field `readonlyWhen`, PR #6454) already materializes; this closes the other
named server seam so both are now total, matching the sibling seams
(`rule-validator.ts`'s object validation / `requiredWhen`, `hook-wrappers.ts`'s
declarative hook `condition`s).

`record-change-trigger.ts`'s `buildContext` now:

- layers the prior row (`ctx.previous`, fetched unconditionally ahead of
dispatch for by-id writes since #7867) as the BASE of `record`, so a field
this write didn't touch keeps its real persisted value instead of vanishing —
this runs for every dispatch, before- and after-hooks alike, not just the
after-row merge #1872 already covered;
- then makes both `record` and `previous` total over the object's DECLARED
fields (a structural mirror of `@objectstack/objectql`'s
`materializeDeclaredFields`, keeping this package's zero build-time
dependency on objectql), filling whatever is STILL missing with an explicit
`null` — but only once the record's persisted state is actually in hand
(insert: always; update/delete: only when the prior row was fetched), so a
write whose prior row genuinely could not be read is left sparse rather than
fabricating a value that might contradict the stored row.

`has()` semantics are unchanged: once a declared field is present (materialized
or not), `has()` still answers whether the KEY is declared/present, not whether
the value is empty — `!= null` is still the way to test emptiness, same contract
`declared-fields.ts` has documented since #4649.
12 changes: 12 additions & 0 deletions .changeset/objectql-export-materialize-declared-fields.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
---
"@objectstack/objectql": patch
---

feat(objectql): publish `materializeDeclaredFields` from `@objectstack/objectql/core` (#4953)

Was an internal-only module (`declared-fields.ts`) shared by `rule-validator.ts` and
`hook-wrappers.ts` via relative import. Published from the `./core` entry so a package
that structurally mirrors the algorithm for its own reasons (`@objectstack/trigger-record-change`,
which keeps zero build-time dependency on `objectql`) has a test-time way to verify its
copy still agrees with the canonical one, instead of the two silently drifting behind a
doc comment's word. No behavior change to the function itself.
15 changes: 15 additions & 0 deletions content/docs/automation/flows.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -1249,6 +1249,21 @@ true`. Compare against the stored shape (`record.done == 1`) or normalize the
value before the condition.
</Callout>

<Callout type="info">
**`record` / `previous` are *total* over the object's declared fields** — the
same guarantee [validation predicates](/docs/data-modeling/validation)
carry (see the `has(x)` is not a null guard callout under "Basic Structure"). A field this write didn't touch (and, for `record`, a field the driver
didn't echo back) still reads as an explicit `null` rather than dropping out
of scope, so `record.discount != null` and `previous.stage != previous_stage`
evaluate cleanly instead of aborting with `No such key`. `has(record.x)` is
therefore uniformly `true` for a *declared* field the moment it's present at
all — including when its value is `null` — so it answers "is `x` declared on
this object", never "does `x` have a value". Guard emptiness with
`record.x != null` / `isBlank(record.x)`, same as everywhere else CEL runs
(see the [`has()` gotcha](/docs/data-modeling/formulas#cel-primer) above).
`has()` still earns its keep against a genuinely *undeclared* key.
</Callout>

## Run a flow via API

Flows of any type can be launched over HTTP — this is what an external system,
Expand Down
59 changes: 39 additions & 20 deletions packages/lint/src/validate-null-guards.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -71,7 +71,7 @@
* Surface ledger (each verdict traced to the code that decides it, so the next
* author does not have to re-derive it — #4811). `binding` is the measured
* shape TODAY; `verdict` is whether this gate runs there. Those are two
* questions, and since #6454 they have come apart on one row:
* questions, and since #6454 they have come apart on two rows:
*
* | surface | binding | evidence | verdict |
* |:-------------------------------|:--------|:------------------------------------------------------------|:--------|
Expand All@@ -80,7 +80,7 @@
* | field `requiredWhen` | TOTAL | same `merged` in `evaluateValidationRules` — fail-OPEN, so an unguarded predicate enforces NOTHING in silence | covered (#4811) |
* | field `readonlyWhen` | TOTAL | `rule-validator.ts` `readonlyWhenBindings` materialises BOTH roots (#4953 clause 1, landed in #6454) | excluded — NOT on totality; see below |
* | action `visible` / `disabled` | sparse | evaluated client-side; no materialization exists in `objectui` | excluded (decided — #4953 clause 2) |
* | flow / edge `condition` | sparse | `record-change-trigger.ts` seeds `{...(inputData ?? {}), ...after}` — #4953 clause 1's other half, not yet wired | excluded (not yet) |
* | flow / edge `condition` | TOTAL | `record-change-trigger.ts` `buildContext` layers `previous` under the payload/after-row then runs BOTH `record` and `previous` through a structural-mirror `materializeDeclaredFields` (#4953 clause 1's other half, services lane) | excluded — NOT on totality; see below |
* | sharing-rule `condition` | n/a | compiled to a SQL filter; `NULL > x` is three-valued, never faults | excluded |
* | field `expression` (`Field.formula`) | n/a | product judgement, not a wiring gap — see below | excluded |
*
Expand All@@ -98,10 +98,14 @@
* (`stripReadonlyWhenFields` merging `{...previous, ...data}` raw)
* describes code that no longer exists. What keeps the row excluded is
* clause 3 of the same ruling: the gate widens once BOTH server-side seams
* are total, and the other one — flow trigger-record seeding, services
* lane — is not wired yet. Widening this face alone would also mean the
* `binding` column had stopped being the thing that decides coverage,
* which is the property #4811 bought.
* are total. The other one — flow trigger-record seeding, services lane —
* is ALSO wired now (#4953 clause 1's other half; see the `flow / edge
* condition` row below), so both server-side seams the ruling named are
* total as of that landing. The actual gate widening clause 3 promises is
* tracked separately (#4811) rather than folded into either seam's own
* PR — this row (and the one below) stay excluded here on purpose, so the
* `binding` column keeps meaning what #4811 needs it to mean: a fact about
* the surface, not a verdict this module renders on itself.
*
* Two facts to carry into that widening; neither is bookkeeping:
*
Expand DownExpand Up@@ -142,20 +146,35 @@
* The ruling's replacement action for this face is the MIRROR of this gate
* — flag `!= null` on a sparse binding — and it is an evaluation owed by
* the devx / objectui lanes, never a widening of `checkNullGuards`.
* - **Flow / edge `condition`.** #4811 excluded these for flattened-scope
* ambiguity ("a bare identifier may be a flow variable"). That reason does
* not actually apply to this module — {@link findUnguardedNullableOperands}
* only ever resolves `record.<f>` / `previous.<f>` and never a bare
* identifier, and the engine binds `record` / `previous` unconditionally.
* The real blocker is totality: the trigger seeds the record as
* `{...(inputData ?? {}), ...after}` — spelled `inputDoc` here until #5671
* dropped that alias read — so a declared column the write never mentioned
* is an ABSENT key, and the `!= null` this gate prescribes would fault.
* Since #4953 that sparseness is a NOT-YET rather than a decision: clause 1
* puts this seam under the same server-side totality guarantee as
* `readonlyWhen`, and only the services-lane wiring is outstanding. When it
* lands, this row and the `readonlyWhen` row flip together — which is
* exactly what clause 3 asks for.
* - **Flow / edge `condition` — the row where `binding` and `verdict` came
* apart, same shape as `readonlyWhen` above.** #4811 originally excluded
* these for flattened-scope ambiguity ("a bare identifier may be a flow
* variable"). That reason does not actually apply to this module —
* {@link findUnguardedNullableOperands} only ever resolves `record.<f>` /
* `previous.<f>` and never a bare identifier, and the engine binds
* `record` / `previous` unconditionally. The real blocker was totality:
* the trigger used to seed the record as `{...(inputData ?? {}), ...after}`
* — spelled `inputDoc` here until #5671 dropped that alias read — so a
* declared column the write never mentioned was an ABSENT key, and the
* `!= null` this gate prescribes would fault.
*
* #4953 (services half) closed that gap: `record-change-trigger.ts`
* `buildContext` now layers `previous` under the payload/after-row (so an
* untouched field reads its REAL persisted value instead of going
* missing) and runs the result through a structural-mirror
* `materializeDeclaredFields` for whatever is still absent — gated on the
* same `groundTruth` rule `evaluateValidationRules` uses (insert always;
* update/delete only once the prior row was fetched), so a write whose
* prior row genuinely cannot be read is left sparse rather than
* fabricating a `null` over an unknown real value. Both `record` and
* `previous` are covered. The totality criterion above is therefore
* SATISFIED here too, and the evidence this row used to carry (the raw
* `{...(inputData ?? {}), ...after}` seed) describes code that no longer
* exists.
*
* What keeps the row excluded is the SAME clause-3 reason the
* `readonlyWhen` row states: the gate's actual widening is tracked
* separately (#4811), not folded into this seam's own PR.
* (The flattened-scope ambiguity is real for a *bare-identifier* checker —
* flow inputs shadow record fields, and a node's `outputVariable` can
* overwrite either — but that is a different, unbuilt pass.)
Expand Down
6 changes: 6 additions & 0 deletions packages/objectql/src/core.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -69,6 +69,12 @@ export { ValidationError, validateRecord } from './validation/record-validator.j
export type { FieldValidationError } from './validation/record-validator.js';
export { evaluateValidationRules, needsPriorRecord, legalNextStates } from './validation/rule-validator.js';
export type { EvaluateRulesOptions } from './validation/rule-validator.js';
// #4953 — published so a package that duplicates this algorithm for its own
// zero-build-dependency reasons (`@objectstack/trigger-record-change`'s
// structural mirror, `record-change-trigger.ts`) has a TEST-TIME way to
// verify its copy still agrees, instead of the two silently drifting behind
// one doc comment's word.
export { materializeDeclaredFields } from './declared-fields.js';
export {
InMemoryHookMetricsRecorder,
noopHookMetricsRecorder,
Expand Down
29 changes: 18 additions & 11 deletions packages/objectql/src/declared-fields.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,20 +6,27 @@
* Shared by the SERVER-side places that evaluate a CEL expression against "the
* record": object-level validation predicates + field `requiredWhen` +
* option `visibleWhen` (`validation/rule-validator.ts`, #1871 / #4649),
* declarative hook `condition`s (`hook-wrappers.ts`, #4770), and the field
* declarative hook `condition`s (`hook-wrappers.ts`, #4770), the field
* `readonlyWhen` strips on the write path (`validation/rule-validator.ts`
* `readonlyWhenBindings`, #4953). They used to disagree — a predicate saw a
* total record while a hook condition saw only the fields the current write
* happened to carry — which is precisely the drift this module exists to
* prevent: an author cannot be expected to know that the same `record.done ==
* true` means two different things depending on which surface reads it.
* `readonlyWhenBindings`, #4953, PR #6454), and — since #4953's services
* half — the flow-trigger record seeded in
* `packages/triggers/trigger-record-change/src/record-change-trigger.ts`.
* They used to disagree — a predicate saw a total record while a hook
* condition saw only the fields the current write happened to carry — which
* is precisely the drift this module exists to prevent: an author cannot be
* expected to know that the same `record.done == true` means two different
* things depending on which surface reads it.
*
* Two bindings are still sparse, and the difference between them matters:
* The flow-trigger seam is a STRUCTURAL MIRROR of this exact function, not an
* import of it: `trigger-record-change` keeps zero build-time dependency on
* `@objectstack/objectql` (the same reason it re-declares `FlowTriggerBinding`
* locally), so it carries its own copy with the identical algorithm and
* contract — see that file's own `materializeDeclaredFields` doc comment for
* the duplication rationale. This doc comment stays the canonical statement
* of the RULE; the copy defers to it rather than re-deriving.
*
* One binding is still sparse, and by decision rather than by gap:
*
* - The flow trigger record (`packages/triggers/trigger-record-change`) is a
* server seam the same ruling puts on this list; it is simply not wired yet
* (services lane, #4953 item 1's other half). Do not read its absence as a
* decision.
* - objectui's action `visible` / `disabled` binds whatever record the client
* already fetched. That one is a DECISION (#4953 item 2): making it total
* would mean every REST read padding out all declared columns, so it stays
Expand Down
1 change: 1 addition & 0 deletions packages/triggers/trigger-record-change/package.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,6 +23,7 @@
},
"devDependencies": {
"@objectstack/driver-sql": "workspace:*",
"@objectstack/formula": "workspace:*",
"@objectstack/objectql": "workspace:*",
"@objectstack/service-automation": "workspace:*",
"@types/node": "^26.1.2",
Expand Down
Loading
Loading