fix(objectql): materialise the master-detail header a parent predicate reads (#6457) (#6788)

`resolveMasterDetailParent(s)` handed the driver-read header row through as-is,
so a `parent.<field>` predicate faulted with `No such key` on any driver that
did not echo back the column it reads. That fault is NOT `Unknown variable:
parent` — `parent` IS bound — so it missed #4889's fail-closed carve-out and
took the ordinary fail-OPEN exit: a `readonlyWhen` lock was let through and a
`requiredWhen` requirement was not enforced.

The header is now made TOTAL over the MASTER object's declared fields inside
the two resolvers, which are the only place holding both the master's schema
and the just-read row. One change serves both consumers; no strip or validator
signature moves; the same `materializeDeclaredFields` helper as every other
server seam (#1871/#4649/#4953); no extra query.

The fail-closed line is preserved exactly: materialisation only ever applies to
a header row that EXISTS, so an unresolvable header still leaves `parent`
unbound, still faults as `Unknown variable: parent`, and is still read as
LOCKED. Both paths pinned, and told apart by fault channel rather than by the
write's outcome.


Claude-Session: https://claude.ai/code/session_01GNUt6cLqqcaLVbiDbin27R

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-zhuang

.changeset/parent-header-materialize.md

@@ -0,0 +1,66 @@
+---
+"@objectstack/objectql": patch
+---
+
+fix(objectql): materialise the master-detail header a `parent`-scoped predicate reads (#6457)
+
+`readonlyWhen: parent.status == 'paid'` and `requiredWhen: parent.status == 'sent'`
+are documented **server** guarantees (#4889 / #4977 bound the `parent` root at the
+write path so they are enforced by the engine and not only by the inline grid).
+What was still storage-dependent is what that bound header CONTAINS. The engine
+resolved it with a plain driver read and passed the row through as-is, so a driver
+that returns only the columns it stored handed the predicate a header missing the
+very key it reads.
+
+CEL is strict about missing keys, and the resulting fault is `No such key: status`
+— **not** `Unknown variable: parent`, because `parent` IS bound. That misses
+#4889's fail-closed carve-out and takes the ordinary fail-OPEN exit, so:
+
+- a `readonlyWhen` lock was **let through** and the frozen field was written;
+- a `requiredWhen` requirement was **not enforced** and the record was accepted
+  with the field empty.
+
+Whether a declared lock or requirement enforced anything therefore depended on
+which columns the driver happened to echo back — something the author who wrote
+the predicate cannot see or control. This is #4953's trap on a different root:
+the 2026-08-06 ruling made `record` / `previous` total at every server seam and
+deliberately left `parent` out, as an ABSENT `parent` is the fail-closed signal.
+
+**The header is now made TOTAL over the MASTER object's declared fields** inside
+`ObjectQL.resolveMasterDetailParent` and `resolveMasterDetailParents` — the only
+place holding both the master's schema and the just-read row, so one change
+serves both consumers and no strip/validator signature moves. It reuses the same
+`materializeDeclaredFields` helper as every other server seam (#1871/#4649/#4953),
+covers the single-id, bulk and insert paths, adds no query (the declared-field
+table is a registry lookup, read once per batch), and copies each header before
+materialising so the stored row never gains materialised nulls.
+
+**Verdicts move, in both directions, and only for a header that RESOLVED:**
+
+| header state | before | now |
+|---|---|---|
+| carries the key | evaluates per verdict | unchanged |
+| resolved, key absent | fault ⇒ fail-OPEN (lock let through / requirement skipped) | evaluates — the key reads `null`, so locks lock and requirements enforce |
+| unresolvable (`null`) | `readonlyWhen` LOCKED (#4889) / `requiredWhen` fail-OPEN (#4977) | **unchanged** |
+
+The bottom row is the one thing this change does not touch. Materialisation is
+only ever applied to a header row that exists, so an unresolvable header still
+leaves `parent` unbound, still faults as `Unknown variable: parent`, and is still
+read as LOCKED for `readonlyWhen` — and still fail-OPEN for `requiredWhen`, the
+deliberate #4977 asymmetry. The two cases stay distinguishable by fault channel,
+and both are pinned.
+
+**Consequences worth knowing before writing a `parent`-scoped predicate**, the
+same two `declared-fields.ts` states for `record`:
+
+- `has(parent.<declared field>)` is now uniformly TRUE — a materialised `null` is
+  a PRESENT key holding null (CEL's own rule). `has()` guards against an
+  UNDECLARED key on the header, not against an empty value; test emptiness with
+  `parent.x != null`.
+- Scope is the master's DECLARED fields only. A typo (`parent.stauts`) stays
+  unevaluable and therefore reportable rather than silently reading as `null`.
+
+If you have a `parent`-scoped `readonlyWhen` that was quietly failing open on a
+sparse-returning driver, it starts locking; a `requiredWhen` in the same position
+starts rejecting writes that leave the field empty. That is the declaration being
+enforced as written.

packages/objectql/src/engine-readonly-when-parent.test.ts

@@ -16,6 +16,14 @@
 // The record-scoped contrast the issue drew — `showcase_invoice.tax_rate` with
 // `readonlyWhen: record.status == 'paid'`, which worked all along — is pinned in
 // the same file so a future change cannot fix one by breaking the other.
+//
+// #6457 extends this suite rather than starting its own, because what it changes
+// is the same binding this file already owns: the header the engine resolves is
+// now TOTAL over the MASTER object's declared fields, so a `parent.<field>`
+// predicate no longer depends on which columns the driver echoed back. Only the
+// MIDDLE row of the issue's verdict table moves (resolved-but-sparse: fail-OPEN
+// ⇒ evaluated); the fail-CLOSED line above (unresolvable ⇒ LOCKED) is asserted
+// unchanged, on the same fixtures, in the same block.
 
 import { describe, it, expect, beforeEach } from 'vitest';
 import { ObjectQL } from './engine.js';
@@ -126,18 +134,55 @@ describe('parent-scoped readonlyWhen is enforced server-side (#4889)', () => {
         unit_price: { type: 'currency', readonlyWhen: "parent.status == 'paid'" },
         // No lock at all — a line is still editable in the ways the author left open.
         description: { type: 'text' },
+        // [#6457] The issue's own predicate — a lock that reads a header key the
+        // driver may or may not have echoed back. TRUE on a header carrying no
+        // `status`, which is what makes the middle-row flip observable.
+        locked_until_status: { type: 'text', readonlyWhen: 'parent.status == null' },
+        // [#6457] The recorded CONSEQUENCE (#4953's, one root over): a
+        // materialised `null` is a PRESENT key, so `has()` over a DECLARED
+        // master field is uniformly TRUE.
+        has_guard: { type: 'text', readonlyWhen: 'has(parent.status)' },
+        // [#6457] The #4649 line, unmoved: materialisation covers the master's
+        // DECLARED fields only, so an author typo on the header stays
+        // unevaluable — and therefore fail-OPEN — instead of reading as null.
+        typo_guard: { type: 'text', readonlyWhen: 'parent.stauts == null' },
       },
     } as any);
 
     storeFor('showcase_invoice').set('INV-1003', { id: 'INV-1003', invoice_number: 'INV-1003', status: 'paid', tax_rate: 8 });
     storeFor('showcase_invoice').set('INV-1004', { id: 'INV-1004', invoice_number: 'INV-1004', status: 'draft', tax_rate: 8 });
+    // [#6457] A header row the driver returned WITHOUT its `status` column — the
+    // MIDDLE row of the issue's verdict table. `status` is declared on the
+    // master; this row simply does not carry the key.
+    storeFor('showcase_invoice').set('INV-SPARSE', { id: 'INV-SPARSE', invoice_number: 'INV-SPARSE' });
     storeFor('showcase_invoice_line').set('line_paid', { id: 'line_paid', invoice: 'INV-1003', quantity: 6, unit_price: 49.99, description: 'seat' });
     storeFor('showcase_invoice_line').set('line_draft', { id: 'line_draft', invoice: 'INV-1004', quantity: 3, unit_price: 10, description: 'seat' });
+    storeFor('showcase_invoice_line').set('line_sparse', {
+      id: 'line_sparse', invoice: 'INV-SPARSE', quantity: 1, unit_price: 5,
+      // A description OUTSIDE the 'seat' match set the bulk tests above use.
+      description: 'sparse', locked_until_status: 'kept', has_guard: 'kept', typo_guard: 'kept',
+    });
   });
 
   const line = (id: string) => storeFor('showcase_invoice_line').get(id);
   const invoice = (id: string) => storeFor('showcase_invoice').get(id);
 
+  /** Everything the engine warned during one write — the strip's own channel,
+   *  which is how the fail-OPEN exit and the LOCKED exits are told apart. */
+  async function warningsDuring(run: () => Promise<unknown>): Promise<string[]> {
+    const warns: string[] = [];
+    const base = (engine as any).logger;
+    (engine as any).logger = new Proxy(base, {
+      get: (t: any, k: string) => (k === 'warn' ? (m: string) => warns.push(String(m)) : t[k]),
+    });
+    try {
+      await run();
+    } finally {
+      (engine as any).logger = base;
+    }
+    return warns;
+  }
+
   it('THE REGRESSION: a paid invoice\'s frozen line survives the PATCH that used to rewrite it', async () => {
     // Verbatim from the issue: PATCH {"quantity":9999,"unit_price":0.01} on a
     // line of the PAID invoice INV-1003 returned 200 and PERSISTED.
@@ -219,4 +264,141 @@ describe('parent-scoped readonlyWhen is enforced server-side (#4889)', () => {
     await engine.update('showcase_invoice_line', { id: 'line_paid', description: 'note' });
     expect(reads.filter((r) => r === 'showcase_invoice')).toHaveLength(0);
   });
+
+  // ── #6457 — the resolved header is TOTAL over the MASTER's declared fields ──
+  //
+  // The issue's three-row verdict table, pinned end-to-end. Exactly one row
+  // moves. The other two — including #4889's fail-CLOSED line — are asserted on
+  // the same fixtures precisely so the move cannot quietly take them with it,
+  // and they are told apart by FAULT CHANNEL, not by the write's outcome: rows 2
+  // and 3 can both end in "the field was not written", and only the warning says
+  // whether that was an evaluated verdict or a refusal to guess.
+
+  it('ROW 1 (header carries the key): evaluates as it always did, verdict unchanged', async () => {
+    // INV-1003 carries `status: 'paid'`, so `parent.status == null` is FALSE and
+    // the field is writable. No fault ⇒ nothing on the fail-open channel.
+    const warns = await warningsDuring(() =>
+      engine.update('showcase_invoice_line', { id: 'line_paid', locked_until_status: 'written' }));
+    expect(line('line_paid')).toMatchObject({ locked_until_status: 'written' });
+    expect(warns.some((w) => w.includes('failed to evaluate — change allowed through'))).toBe(false);
+  });
+
+  it('ROW 2 — THE FIX: a header missing the key now LOCKS instead of failing open', async () => {
+    // Before #6457 this was `No such key: status` on a BOUND `parent`, so
+    // `unknownVariableOf` did not match, the ordinary fail-OPEN exit ran, and
+    // the declared lock was let through. The header is now total over the
+    // master's declared fields, so `status` reads `null`, the predicate is TRUE,
+    // and the field is stripped.
+    const warns = await warningsDuring(() =>
+      engine.update('showcase_invoice_line', { id: 'line_sparse', locked_until_status: 'forged' }));
+    expect(line('line_sparse')).toMatchObject({ locked_until_status: 'kept' });
+    // The verdict came from an EVALUATION: the fail-open exit is not on the
+    // channel at all…
+    expect(warns.some((w) => w.includes('failed to evaluate — change allowed through'))).toBe(false);
+    expect(warns.some((w) => w.includes("Field 'locked_until_status' is read-only (readonlyWhen)"))).toBe(true);
+    // …and it is NOT #4889's unbound-root exit either — `parent` IS bound here.
+    // This is the assertion that keeps ROW 2 and ROW 3 distinguishable.
+    expect(warns.some((w) => w.includes("reads 'parent'"))).toBe(false);
+  });
+
+  it('ROW 2: the strip is reported to the caller as `readonly_when` (#3407)', async () => {
+    const events: any[] = [];
+    await engine.update(
+      'showcase_invoice_line',
+      { id: 'line_sparse', locked_until_status: 'forged' },
+      { onFieldsDropped: (e: any) => events.push(e) } as any,
+    );
+    expect(events).toEqual([
+      { object: 'showcase_invoice_line', fields: ['locked_until_status'], reason: 'readonly_when' },
+    ]);
+  });
+
+  it('ROW 3 (unresolvable header): still LOCKED, and still by the UNBOUND-ROOT exit (#4889)', async () => {
+    // The fail-closed line, unmoved and byte-identical: materialisation is only
+    // ever applied to a row that EXISTS, so a header that resolves to nothing
+    // still leaves `parent` unbound and still faults as `Unknown variable`.
+    storeFor('showcase_invoice_line').set('orphan', { id: 'orphan', invoice: 'GONE', locked_until_status: 'kept' });
+    const warns = await warningsDuring(() =>
+      engine.update('showcase_invoice_line', { id: 'orphan', locked_until_status: 'forged' }));
+    expect(line('orphan')).toMatchObject({ locked_until_status: 'kept' });
+    expect(warns.some((w) => w.includes("reads 'parent'") && w.includes('LOCKED'))).toBe(true);
+  });
+
+  it('a sparse header that answers FALSE allows the change — by a VERDICT, not by a fault', async () => {
+    // `parent.status == 'paid'` over a header carrying no status now evaluates
+    // to FALSE. The write lands either way; what changed is why, and the why is
+    // what every other predicate on that header depends on.
+    const warns = await warningsDuring(() =>
+      engine.update('showcase_invoice_line', { id: 'line_sparse', quantity: 42 }));
+    expect(line('line_sparse')).toMatchObject({ quantity: 42 });
+    expect(warns.some((w) => w.includes('failed to evaluate — change allowed through'))).toBe(false);
+  });
+
+  it('CONSEQUENCE: `has(parent.<declared>)` is uniformly TRUE — it locks even on a sparse header', async () => {
+    // CEL's own rule, the same one #4953 recorded for `record`: a materialised
+    // `null` is a PRESENT key. `has()` guards against an UNDECLARED key on the
+    // header, not against an empty value — test emptiness with `!= null`.
+    await engine.update('showcase_invoice_line', { id: 'line_sparse', has_guard: 'forged' });
+    expect(line('line_sparse')).toMatchObject({ has_guard: 'kept' });
+  });
+
+  it('BOUNDARY: an UNDECLARED key on the header stays unevaluable — fail-OPEN (#4649 unmoved)', async () => {
+    // `parent.stauts` is a typo, not a sparse column. Materialising it would
+    // paper over the bug; it must stay reportable.
+    const warns = await warningsDuring(() =>
+      engine.update('showcase_invoice_line', { id: 'line_sparse', typo_guard: 'written' }));
+    expect(line('line_sparse')).toMatchObject({ typo_guard: 'written' });
+    expect(warns.some((w) => w.includes('failed to evaluate — change allowed through'))).toBe(true);
+  });
+
+  it('does NOT mutate the stored header row — the materialised copy stays local', async () => {
+    // The in-memory driver hands back the stored object BY REFERENCE, which is
+    // exactly how a materialisation leaks: the header would silently gain a
+    // `status: null` column that every later reader (and after-hooks) sees.
+    await engine.update('showcase_invoice_line', { id: 'line_sparse', locked_until_status: 'forged' });
+    expect('status' in invoice('INV-SPARSE')).toBe(false);
+    expect(invoice('INV-SPARSE')).toEqual({ id: 'INV-SPARSE', invoice_number: 'INV-SPARSE' });
+  });
+
+  it('BULK: the batch path materialises its headers too, per matched row', async () => {
+    await engine.update(
+      'showcase_invoice_line',
+      { locked_until_status: 'forged' },
+      { where: { description: 'sparse' }, multi: true } as any,
+    );
+    expect(line('line_sparse')).toMatchObject({ locked_until_status: 'kept' });
+  });
+
+  it('BULK: a batch under a header that DOES carry the key still writes', async () => {
+    await engine.update(
+      'showcase_invoice_line',
+      { locked_until_status: 'written' },
+      { where: { description: 'seat' }, multi: true } as any,
+    );
+    expect(line('line_paid')).toMatchObject({ locked_until_status: 'written' });
+    expect(line('line_draft')).toMatchObject({ locked_until_status: 'written' });
+  });
+
+  it('BULK: an unresolvable header still LOCKS the batch (fail-CLOSED, bulk twin)', async () => {
+    storeFor('showcase_invoice_line').set('orphan_b', {
+      id: 'orphan_b', invoice: 'GONE', description: 'orphaned', locked_until_status: 'kept',
+    });
+    await engine.update(
+      'showcase_invoice_line',
+      { locked_until_status: 'forged' },
+      { where: { description: 'orphaned' }, multi: true } as any,
+    );
+    expect(line('orphan_b')).toMatchObject({ locked_until_status: 'kept' });
+  });
+
+  it('costs no extra header read — materialisation is a registry lookup, not a query', async () => {
+    const reads: string[] = [];
+    const original = (engine as any).findOne.bind(engine);
+    (engine as any).findOne = async (name: string, q: any, o?: any) => {
+      reads.push(name);
+      return original(name, q, o);
+    };
+    await engine.update('showcase_invoice_line', { id: 'line_sparse', locked_until_status: 'forged' });
+    expect(reads.filter((r) => r === 'showcase_invoice')).toHaveLength(1);
+  });
 });