Skip to content

Commit da989bb

Browse files
fix(objectql): a validate() preview binds the master-detail header the write binds (#22474) (#22518)
Fixes #22474 Clause-②: yes ## What was wrong `ObjectQL.validate()` (the preview behind `validateData` and the import dry run) made one `evaluateValidationRules` call and bound no master-detail header. Every write binds it: the insert passes `parent: insertParentForRow?.(rows[i])`, the by-id update passes `parent: roWhenParent` and `previousParent: roWhenPreviousParent`, and the bulk update passes `parent: parentForRow?.(row)` and `previousParent: previousParentForRow?.(row)`. So a field `requiredWhen: "parent.status == 'sent'"` faulted in the preview (`Unknown variable: parent`) and the preview refused the row as `rule_violation` / `unevaluable`, in both modes, while the write admitted it under a draft header. **Measured before the change:** this PR's test file, run with `engine.ts` put back to its `e148ca9842` blob (`5ca77a148de1`), is 17 red of 21. The 4 green are the no-header-read control, the cannot-read pin (it holds trivially when the preview refuses every such row), and the two parity floor cases. The preview answered `description: rule_violation` for an insert under a draft header and under a sent header, for an update with a stored line's `id` under either header, and for the import dry run (through `ObjectStackProtocolImplementation` and `runImport`) of a created and of a matched line. The real insert, update and import admitted the draft-header rows and refused the sent-header rows with `required`. The parity case listed exactly the two missing inputs: `parent` (3 write calls) and `previousParent` (2 write calls). ## The change (`packages/objectql/src/engine.ts`) - **Bound as the write binds it.** `validate()` resolves the header per row from the judged view, the write's own payload-FK-first reading (`masterIdOf`): the row's FK on an insert, and on an update the patch's FK, else the FK of the stored row that #22445's change already reads. A repointing update also binds the stored row's own header as `previousParent`, for the ADR-0113 pre-check, exactly when the write resolves it (`repointsMaster`). - **Under the write's gate.** Only an object that declares a field `requiredWhen` reading `parent` (`hasParentScopedRequiredWhen`, the insert's gate and the `requiredWhen` half of the update's gate) reads a header. Every other object's preview reads none. - **Read under the caller's access** (new private `resolvePreviewParents`). The write reads the header elevated (`resolveMasterDetailParent(s)` under `referenceCheckContext`). The preview runs none of the write's gates, so it reads as #22445's stored-row read does: 1. The read door, under the caller's own context, one `find` for the batch, decides whether the header exists for this caller. A header it does not return binds as a missing header binds (`null`), so the rule refuses as unevaluable whatever the header holds. A failed read binds every header as missing, as the write's own failed header read does, and logs at `warn`. 2. The write's own header read (`resolveMasterDetailParents`) runs only for the headers read 1 returned. 3. A header column is kept only where the read door served this caller that very value (`servedAsStored`). The header is then materialised over the master's declared fields, so a hidden or masked column reads `null`: judged as empty. - The orphaned one-line docblock above `servedAsStored` (named in #22445's ACCEPT as riding the next PR that touches this spot) is moved back onto `mergeReadContext`; `servedAsStored`'s docblock now names its second caller. `packages/core/src/utils/import-runner.ts` is not touched: the dry run already sends the row's FK (create) and the matched record's `id` (update), which is everything the preview needs. ## Pins (`packages/objectql/src/validate-preview-parent.test.ts`, 21 cases) - (a) Insert, and update with a stored line's `id`: admitted under a draft header, refused with `required` under a sent header; the real insert and by-id update give the same verdict. - (b) A repoint from a draft onto a sent header is refused, as the write refuses it (the stored row complied under the header it leaves); a legacy line under a sent header rests through a repoint onto another sent header, as on the write. - (c) Control: an object with a master-detail relation and only a row-scoped `requiredWhen` reads no header in either mode (an engine middleware counts every header read under any context); positive control: the parent-scoped line does read one. - (d) Security. A caller whose read scope excludes the header gets one verdict whatever the header holds, equal to the verdict for a header id that names no row (insert and update). A caller served `status` hidden, and one served it masked, get one verdict whatever the header holds, and it is the empty-column verdict. Control: a caller who reads the header in full gets the write's verdict in both modes. - (e) The import dry run through the protocol, created line and matched line (an edit, and a repoint onto a sent header), answers what the real import does. - (f) Parity, read from `engine.ts` itself (below). ### How the parity pin finds the call sites It parses `engine.ts` with the TypeScript compiler API, finds every call whose callee is the identifier `evaluateValidationRules`, and attributes it to its enclosing class method: the one in `validate` is the preview, every other is a write (today four: `insert`'s insert call, `update`'s by-id call and its two bulk calls). A call's inputs are the keys of its options object literal, including the keys of an object literal spread into it (`...(c ? { k } : {})`). An options argument it cannot see into fails the test rather than passing it. It asserts: exactly one preview call; at least four write calls covering both modes; every key a write call passes is passed by the preview; the preview passes no key no write passes. ## Reverse verification and ablations Each leg through `scripts/ablation-replace.mjs` (anchor must hit once, blob change and restore proven against `HEAD`), inside a script with its own EXIT/INT/TERM restore trap. Run on head `57c3ce9f57` and again on the merged head `0958e192ac` (`engine.ts` blob `df2300664b3c` in both), with identical red sets. The subject is imported as source (`./engine.js`), so no `dist/` is in the resolution path. | leg | mutation | red | green | |---|---|---|---| | A1 | preview binds `parent: undefined`, `previousParent: undefined` | 15: all of (a), (b), (e), the hide/mask pins and the full-reader control in (d) | (c) both, the cannot-read pin, the 3 parity cases | | A1b | `previousParent: undefined` only | 2: the repoint pin in (b), the import repoint in (e) | 19 | | S1 | the visibility read under `referenceCheckContext` (elevated) | 3: the cannot-read, hide and mask pins | 18, the full-reader control included | | S2 | `servedAsStored(...)` replaced by `true` | 1: the mask pin | 20, the hide pin included | | S3 | the whole column filter replaced by `true` | 2: the hide and mask pins | 19 | | G1 | the `hasParentScopedRequiredWhen` gate replaced by `true` | 1: the no-header-read control | 20 | | P1 | a new key added to the insert's write call only | 1: "every key a write call passes, the preview passes", naming the key and `engine.ts` line | 20 | | P2 | `parent` / `previousParent` keys removed from the preview call | 16: A1's 15 plus that parity case | 5 | | P3 | a key added to the preview call only | 1: "the preview passes no input that no write passes" | 20 | A first P3 attempt used an anchor its own replacement contained; the tool refused it before any run (anchor count did not drop), and it was re-run with a distinct anchor. An earlier G1 leg ran while a sibling gate's full build had removed `packages/metadata-protocol/dist`, so its suite never loaded; it is discarded, and all nine legs above were re-run together on both heads. ## Verification (head `0958e192ac`, `origin/main` `446c8b2a64` merged in, no conflict, no file of this PR touched by it) - `pnpm --filter @objectstack/objectql exec vitest run --project local --maxWorkers=2`: 392 files, 7737 tests passed (also on `57c3ce9f57` before the merge, same counts). - `pnpm --filter @objectstack/objectql typecheck`: exit 0 on `0958e192ac`; the new test file is in the `tsconfig.test.json` program (`--listFiles`) and adds no error to the pinned test-typecheck debt. - Gates from `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands` (68 commands): all 68 exit 0, each exit captured before any pipe; `--ran` reconciliation: 68 derived, 68 run, 0 NOT-MEASURED. On the pre-merge head two gates were red and are fixed here: `check:objectql-double-limit` (the test double's `find` carried a read counter its probe could not drive; the control now counts reads with an engine middleware) and `check:query-options-erasure` (an `as any` on the header read's options, now `EngineQueryOptions`). - Lint, narrowed: `eslint --no-inline-config --format json` over the two changed source files, 2 files, 0 errors, 0 warnings. Population read from the config (`calculateConfigForFile`: both `.ts` files linted, the changeset ignored); the config enables no type-aware linting (no `parserOptions.project`), so the diff cannot move the verdict of an untouched file. ## Acceptance notes - **The write reads the header elevated (finding, reported per the dispatch).** Measured with a throwaway test on this branch: a caller whose read scope excludes header `hx` inserts a line under it; the write admits it while `hx` is draft and refuses it with `required` while `hx` is sent, so the write's verdict depends on a header that caller cannot read. This is the documented design of `resolveMasterDetailParent` (the caller's right to write the detail is settled before the header is read; the tenant wall is kept). The preview keeps the stricter rule, so for such a caller the preview and the write can answer differently, which the changeset states. - **Named limit kept:** the preview still runs no `readonlyWhen` strip, so the update's judgement of the FK's own `readonlyWhen` lock (`settleMasterDetailLanding` step 1) is not run; a repoint that lock would take back out is judged against the header the patch names. - An option `visibleWhen` that reads `parent` (the card's "likely sibling") is unchanged: no write binds `parent` for option gates either (`evaluateOptionVisibility` takes no header), so, read from source and not measured here, preview and write already agree there (both refuse the faulting pick). - The parity pin compares option keys, not values; the behavioural pins hold the values of `parent` and `previousParent`. --- _Generated by [Claude Code](https://claude.ai/code/session_01Bw3y2DWhT9RPnrmDsNqEVG)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9c93ce6 commit da989bb

3 files changed

Lines changed: 701 additions & 11 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@objectstack/objectql': minor
3+
---
4+
5+
A `validate()` preview binds the master-detail header the write binds, so an import dry run of a detail row whose field rule reads `parent` admits the rows the import admits (#22474)
6+
7+
Clause-②: yes
8+
9+
The preview's accept set widens: rows it refused, and the write admits, are now admitted. No key, export or error code is added, removed or renamed.
10+
11+
- **Before.** A preview (`engine.validate(object, rows, { mode })`, `validateData`, and the import dry run) bound no master-detail header. A field `requiredWhen` that reads the header, such as `parent.status == 'sent'` on an invoice line, faulted there and the preview refused the row with `rule_violation` / `unevaluable`, in both modes, although the insert and the update resolve the header and admit it under a draft header.
12+
- **Now.** The preview binds `parent` as the write binds it: from the row's master-detail reference on an insert, and on an update from the reference the patch carries, else from the stored row the row's `id` names. An update that moves the row to another header also binds the header the stored row hangs off, which the write reads to decide whether the stored row already violated the rule. So a row under a draft header is admitted and a row that leaves the field empty under a sent header is refused with `required`, as the write refuses it. Only an object that declares a field `requiredWhen` reading `parent` reads a header, as on the write.
13+
- **Read under the caller's access.** The write reads the header with system access, kept to the caller's organization. The preview first reads it through the engine's read door, under the caller's own context: a header the caller cannot read gets the verdict a missing header gets. A header column is judged only where the read door served this caller that very value. A column the read door hid, or served transformed (a partially masked field, a masked secret), is judged as empty, so the verdict never depends on a header value the caller could not read in full. For such a caller the preview can therefore answer differently from the write, which judges the stored header.
14+
- **Unchanged.** An object with no field `requiredWhen` reading `parent` reads no header and gets the verdict it got before. The preview still runs no `readonlyWhen` strip, so an update whose `readonlyWhen` would keep the row on its stored header is judged against the header the patch names.

‎packages/objectql/src/engine.ts‎

Lines changed: 160 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3207,18 +3207,20 @@ export interface EngineReadOptions {
32073207
context?: ExecutionContext;
32083208
}
32093209

3210-
/** Merge read-path execution context from the query and the trailing options. */
32113210
/**
32123211
* [#22445] Did the read door serve this value AS STORED — can the caller read
32133212
* the stored value in full? The `update`-mode preview keeps a stored column
3214-
* only when this holds for it (see `storedRows` in `ObjectQL.validate`).
3213+
* only when this holds for it (see `storedRows` in `ObjectQL.validate`), and
3214+
* [#22474] the preview keeps a master-detail header column only when this
3215+
* holds for it (see `ObjectQL.resolvePreviewParents`).
32153216
*
32163217
* Both values come off the same driver read shape (`driver.findOne` under the
3217-
* same `buildDriverOptions`), so a column nothing transformed compares equal
3218-
* by construction; what differs is what the read path REWROTE for this caller
3219-
* (a partial mask, a masked secret, a read hook's rewrite). Structural, not
3220-
* identity: the two reads are two driver calls, so a `Date`, an array or a
3221-
* JSON object arrives as two equal objects.
3218+
* same `buildDriverOptions`; for a header, the read door's `find` under the
3219+
* caller's context and under the write's elevated one), so a column nothing
3220+
* transformed compares equal by construction; what differs is what the read
3221+
* path REWROTE for this caller (a partial mask, a masked secret, a read hook's
3222+
* rewrite). Structural, not identity: the two reads are two driver calls, so a
3223+
* `Date`, an array or a JSON object arrives as two equal objects.
32223224
*
32233225
* ⛔ Fails CLOSED: a pair it cannot prove equal — another object kind (a
32243226
* buffer, a class instance), a nesting past the depth bound — answers `false`,
@@ -3247,6 +3249,7 @@ function servedAsStored(served: unknown, stored: unknown, depth = 0): boolean {
32473249
&& servedKeys.every((k) => Object.prototype.hasOwnProperty.call(stored, k) && servedAsStored(served[k], stored[k], depth + 1));
32483250
}
32493251

3252+
/** Merge read-path execution context from the query and the trailing options. */
32503253
function mergeReadContext(
32513254
fromQuery?: ExecutionContext,
32523255
fromOptions?: ExecutionContext,
@@ -8718,6 +8721,106 @@ export class ObjectQL implements IObjectQLEngine {
87188721
};
87198722
}
87208723

8724+
/**
8725+
* [#22474] The master-detail headers a `validate()` preview binds, per row:
8726+
* `parents[i]` is the header row `i` is judged under (`parent`), and
8727+
* `previousParents[i]` the header its stored row hangs off when the row
8728+
* REPOINTS it (`previousParent`, for the ADR-0113 pre-check), else
8729+
* `undefined`, which the evaluator reads as "the same header", as the
8730+
* write's own `undefined` is read.
8731+
*
8732+
* The header ids are read the way the write reads them ({@link masterIdOf}):
8733+
* `views[i]` is the judged view, the row on an insert and the stored row
8734+
* merged with the patch on an update, so the patch's FK wins over the stored
8735+
* one; `storedRows[i]` names the header a repoint leaves.
8736+
*
8737+
* ## Read under the caller's access, not elevated
8738+
*
8739+
* The write reads the header ELEVATED ({@link resolveMasterDetailParents}):
8740+
* the caller's right to write the detail was settled by the write's gates
8741+
* before the header is read. The preview runs none of those gates, so a
8742+
* header read elevated here would answer a predicate over a row the caller
8743+
* may not read. Two reads, each answering one question, as the preview's
8744+
* stored-row read does:
8745+
*
8746+
* 1. ⛔ MAY THIS CALLER SEE THE HEADER — the READ DOOR, under the caller's
8747+
* own context, one `find` for the whole batch. A header it does not
8748+
* return binds as a missing header binds: `null`, so a predicate that
8749+
* reads `parent` refuses as unevaluable, whatever the header holds. A
8750+
* failed read binds every header as missing, as the write's own failed
8751+
* header read does.
8752+
* 2. WHAT THE WRITE WOULD JUDGE — the write's own header read
8753+
* ({@link resolveMasterDetailParents}), for the headers read 1 returned
8754+
* only.
8755+
*
8756+
* ⛔ A header column is kept only where the read door served this caller
8757+
* that very value ({@link servedAsStored}); every other column, hidden or
8758+
* served transformed (a partial mask keeps the key and replaces the value),
8759+
* is judged as empty: the header is materialised over the master's declared
8760+
* fields after the columns are kept, so such a column reads `null`. So the
8761+
* verdict never depends on a header value the caller could not have read in
8762+
* full.
8763+
*/
8764+
private async resolvePreviewParents(
8765+
schema: unknown,
8766+
views: ReadonlyArray<Record<string, unknown>>,
8767+
storedRows: ReadonlyArray<Record<string, unknown> | undefined>,
8768+
context: ExecutionContext | undefined,
8769+
): Promise<{
8770+
parents: Array<Record<string, unknown> | null>;
8771+
previousParents: Array<Record<string, unknown> | null | undefined>;
8772+
}> {
8773+
const rel = resolveMasterDetailRelation(schema as any);
8774+
const landingIds = views.map((view) => (rel ? masterIdOf(rel.fk, null, view) : undefined));
8775+
const priorIds = storedRows.map((stored) => (rel && stored ? masterIdOf(rel.fk, null, stored) : undefined));
8776+
// The write resolves a second header only for a repoint (`repointsMaster`).
8777+
const repoints = priorIds.map((prior, i) => prior != null && landingIds[i] !== prior);
8778+
const ids = new Set<string>();
8779+
landingIds.forEach((id) => { if (id != null) ids.add(String(id)); });
8780+
priorIds.forEach((id, i) => { if (repoints[i]) ids.add(String(id)); });
8781+
8782+
const headers = new Map<string, Record<string, unknown>>();
8783+
if (rel && ids.size > 0) {
8784+
// 1. The read door, under the caller's own context.
8785+
const served = new Map<string, Record<string, unknown>>();
8786+
try {
8787+
const rows = await this.find(rel.master, {
8788+
where: { id: { $in: [...ids] } },
8789+
context,
8790+
} as EngineQueryOptions) as Array<Record<string, unknown>>;
8791+
for (const row of Array.isArray(rows) ? rows : []) {
8792+
if (row?.id != null && ids.has(String(row.id))) served.set(String(row.id), row);
8793+
}
8794+
} catch (err) {
8795+
this.logger?.warn?.('validate preview: the caller\'s read of the master-detail header failed — parent stays unbound', {
8796+
object: rel.master, error: err,
8797+
});
8798+
}
8799+
// 2. The write's own header read, for the headers the caller can read.
8800+
if (served.size > 0) {
8801+
const asWritten = await this.resolveMasterDetailParents(
8802+
schema, null, [...served.keys()].map((id) => ({ [rel.fk]: id })), context,
8803+
);
8804+
for (const [id, servedRow] of served) {
8805+
const header = asWritten({ [rel.fk]: id });
8806+
if (!header) continue;
8807+
const kept: Record<string, unknown> = {};
8808+
for (const [key, value] of Object.entries(header)) {
8809+
if (Object.prototype.hasOwnProperty.call(servedRow, key) && servedAsStored(servedRow[key], value)) {
8810+
kept[key] = value;
8811+
}
8812+
}
8813+
headers.set(id, this.materializeParentHeader(rel.master, kept));
8814+
}
8815+
}
8816+
}
8817+
const headerOf = (id: string | number | undefined) => (id == null ? null : (headers.get(String(id)) ?? null));
8818+
return {
8819+
parents: landingIds.map(headerOf),
8820+
previousParents: priorIds.map((id, i) => (repoints[i] ? headerOf(id) : undefined)),
8821+
};
8822+
}
8823+
87218824
/**
87228825
* [#18682] Resolve the related rows this object's PREDICATE rules read one
87238826
* hop through a reference field, for a whole batch of rows at once.
@@ -13046,10 +13149,24 @@ export class ObjectQL implements IObjectQLEngine {
1304613149
* refusal it is. A row with no address, or whose id names no row this
1304713150
* caller can read, is judged on the patch alone, as before.
1304813151
*
13152+
* ## The master-detail header is bound as the write binds it (#22474)
13153+
*
13154+
* Every write binds the detail row's master-detail header for a field
13155+
* `requiredWhen` that reads `parent`, and an update that repoints the row
13156+
* binds the header it leaves as `previousParent` for the ADR-0113
13157+
* pre-check. A preview that bound neither refused, as unevaluable, rows the
13158+
* write admits. So the preview binds both, from the same view and under the
13159+
* same gate as the write (an object with no parent-scoped `requiredWhen`
13160+
* reads no header). ⛔ Unlike the write, it reads the header under the
13161+
* caller's own access ({@link resolvePreviewParents}): a header the caller
13162+
* cannot read binds as a missing one, and a header column the read door
13163+
* served transformed or not at all is judged as empty.
13164+
*
1304913165
* Nothing is written and no sequence is consumed. Reads do happen: the
13050-
* stored row above, and the related rows a traversing rule names (see the
13051-
* `previewRelatedForRow` block below). Validation itself is in-process,
13052-
* which is what makes row-by-row dry run of a large import affordable.
13166+
* stored row above, the master-detail headers, and the related rows a
13167+
* traversing rule names (see the `previewRelatedForRow` block below).
13168+
* Validation itself is in-process, which is what makes row-by-row dry run
13169+
* of a large import affordable.
1305313170
*/
1305413171
async validate(
1305513172
object: string,
@@ -13090,7 +13207,10 @@ export class ObjectQL implements IObjectQLEngine {
1309013207
// (see the `previewRelatedForRow` block below), and [#22445] an
1309113208
// `update`-mode row that carries its address issues two READS of the
1309213209
// stored row — the read door under the caller's own context, then the
13093-
// write's own prior-row read (the `storedRows` block below). Nothing is
13210+
// write's own prior-row read (the `storedRows` block below), and [#22474]
13211+
// an object with a parent-scoped `requiredWhen` issues two READS of the
13212+
// master-detail headers its rows name, the same two doors in the same
13213+
// order (the `previewParents` block below). Nothing is
1309413214
// written, no write hook runs (a read through the read door fires its read
1309513215
// hooks, as the related read always has), and the
1309613216
// RELATED read happens only for a caller who passes the arms the
@@ -13310,6 +13430,30 @@ export class ObjectQL implements IObjectQLEngine {
1331013430
// stored row.
1331113431
const judgedViews = rows.map((row, i) => (storedRows[i] ? { ...storedRows[i], ...row } : row));
1331213432

13433+
// [#22474] THE MASTER-DETAIL HEADER a field `requiredWhen` reads as
13434+
// `parent`, bound as the write binds it and under the write's own gate:
13435+
// only an object that declares a parent-scoped `requiredWhen` reads a
13436+
// header (`hasParentScopedRequiredWhen`, the insert's gate and the
13437+
// update's `requiredWhen` half), so every other object's preview reads
13438+
// none. The header is named by the judged view, the write's
13439+
// payload-FK-first reading (`masterIdOf`): the row's own FK on an insert,
13440+
// and on an update the patch's FK, else the stored row's. A repointing
13441+
// update also binds the header the stored row hangs off as
13442+
// `previousParent`, the header the write's ADR-0113 pre-check reads.
13443+
//
13444+
// ⛔ Read under the caller's access, unlike the write, which reads the
13445+
// header elevated: see {@link resolvePreviewParents}. A header the caller
13446+
// cannot read binds as a missing header binds, and a header column the
13447+
// read door did not serve this caller as stored is judged as empty.
13448+
//
13449+
// Named limit: the update's `readonlyWhen` judgement of the FK itself
13450+
// (`settleMasterDetailLanding` ①) is not run, the limit `readonlyWhen`
13451+
// already holds in this preview, so a repoint that lock would take back
13452+
// out is judged against the header it names.
13453+
const previewParents = hasParentScopedRequiredWhen(schemaForValidation as any)
13454+
? await this.resolvePreviewParents(schemaForValidation, judgedViews, storedRows, options?.context)
13455+
: undefined;
13456+
1331313457
// [#18682] The preview owes the SAME relationship resolution the real write
1331413458
// does. Without it a rule that reads one hop through a reference field
1331513459
// reports `valid: false` (unevaluable) against a row `insert()` happily
@@ -13391,6 +13535,11 @@ export class ObjectQL implements IObjectQLEngine {
1339113535
// update's `previous: priorRecord`. Absent, the record is the patch.
1339213536
...(storedRows[i] ? { previous: storedRows[i] } : {}),
1339313537
logger: this.logger, currentUser, skipStateMachine, messages,
13538+
// [#22474] The header the write binds: the insert's
13539+
// `insertParentForRow`, the update's landing header and, for a
13540+
// repoint, `roWhenPreviousParent`.
13541+
parent: previewParents?.parents[i],
13542+
previousParent: previewParents?.previousParents[i],
1339413543
related: previewRelatedForRow(judgedViews[i]),
1339513544
permissions: previewPermissionsFor(row),
1339613545
});

0 commit comments

Comments
 (0)