fix(objectql,lint): 服务端补上 parent 作用域 readonlyWhen 的绑定与执行 (#4889)

.changeset/parent-scoped-readonly-when-server-enforced.md

@@ -0,0 +1,47 @@
+---
+"@objectstack/objectql": minor
+"@objectstack/lint": minor
+---
+
+fix(objectql,lint): enforce parent-scoped `readonlyWhen` on the server (#4889)
+
+`readonlyWhen: P\`parent.status == 'paid'\`` — the documented "once the header
+invoice is Paid, its lines are frozen" lock — was enforced **only in the client
+grid**. The server-side strip bound `record` and `previous` and had no `parent`
+at all, so every parent-scoped predicate faulted, took the fail-open branch, and
+the write landed anyway. On the reference app that meant one `PATCH` rewrote the
+quantity and unit price of a settled invoice's line: HTTP 200, value persisted,
+the grid still drawing the cell read-only. ADR-0057 D10 puts enforcement on the
+server and makes the client courtesy; here only the courtesy layer enforced.
+
+**`parent` is now bound on the write path.** For a detail object — one declaring
+exactly one `master_detail` relationship — the engine resolves the master record
+and binds it as `parent` before the strip runs, on both the single-id and the
+bulk (`multi: true`) update paths. A repointing write is judged against the
+master it *lands on*, not the one it leaves. The read is gated on the payload
+actually touching a parent-scoped predicate (decided from the parsed CEL AST, so
+a field named `parent_id` costs nothing), and the bulk path batch-reads the
+distinct headers in one query rather than one per row.
+
+**An unbindable scope no longer waives the lock.** A `readonlyWhen` that names a
+root the operation could not bind now resolves to **locked** — the field is
+stripped — instead of "not locked". "The platform could not check this" must not
+mean "allowed" on a field the author declared frozen. This is deliberately the
+narrowest possible carve-out from the fail-open policy the strip has always had:
+a predicate that is merely *broken* on the record (undeclared key, `null`
+ordering overload, parse error, engine throw) still fails open exactly as
+before, and `requiredWhen` / option `visibleWhen` are untouched. Recorded as an
+addendum to ADR-0058's D5 fail-policy matrix, alongside the same narrowing
+already made for validation predicates (#4649) and hook conditions (#4775).
+
+**And the runtime branch is a backstop, not the plan.** `objectstack compile`
+now **rejects** a `parent`-scoped `readonlyWhen` on an object that declares no
+`master_detail` relationship, or two of them (where the metadata does not say
+which one is "the parent" and picking by declaration order would make a
+data-integrity lock depend on field ordering). The common authoring mistake is
+caught where it is cheap to fix, so it never reaches a runtime that has to judge
+it — declared, not guessed.
+
+No metadata changes are required: an app whose parent-scoped locks were already
+correct simply starts having them enforced. If you authored one on an object
+with no single master, the build now names it.

content/docs/data-modeling/fields.mdx

@@ -366,6 +366,36 @@ protocol 17 (#3855): authoring it is rejected with an error naming the
 replacement, not silently stripped, and `os migrate meta --from 16` rewrites
 existing sources automatically.
 
+#### Locking a detail from its master: `parent`
+
+On a **detail** object — one that declares a `master_detail` relationship — a
+`readonlyWhen` predicate may read the header record as `parent`. The canonical
+case is "once the invoice is Paid, its lines are frozen":
+
+```typescript
+quantity: Field.number({
+  label: 'Qty',
+  required: true,
+  // `parent` = the record on the other end of this object's master_detail field.
+  readonlyWhen: P`parent.status == 'paid'`,
+}),
+```
+
+The server binds `parent` on the update path by reading the master the row
+points at (a repointing write is judged against the master it lands on), so the
+lock holds against a direct API call, not only in the grid.
+
+Two rules make this predictable:
+
+- **Exactly one master.** `parent` resolves only when the object declares
+  exactly one `master_detail` relationship. With none — or with two, where the
+  metadata does not say which one is "the parent" — `objectstack compile`
+  rejects the predicate with an error naming the object.
+- **An unresolvable `parent` means locked, not open.** If the header cannot be
+  read at write time, the field is treated as read-only rather than written. A
+  lock the platform could not evaluate is never waived: that is what makes the
+  declaration a guarantee instead of a hint.
+
 ## Naming Conventions
 
 - Field names use `snake_case`: `first_name`, `annual_revenue`, `is_active`

docs/adr/0058-expression-and-predicate-surface.md

@@ -22,6 +22,40 @@
 
 ---
 
+> **Addendum (2026-08, #4889) — D5's "non-security ⇒ fail soft" line is NARROWED
+> at the write-path gates: an UNEVALUABLE gate now fails CLOSED.**
+> D5 sorts fail policy by *security-relevance*, and it put validation, hook and
+> field predicates on the fail-soft side. Three findings since have shown that
+> the sorting axis was one notch off for a specific subset: the predicates that
+> **gate a write**. Fail-soft there does not mean "the rule was advisory"; it
+> means "the guarantee the author declared did not happen, and nothing told
+> anyone". The write returns 200, the client still renders the guard, and only a
+> log line disagrees.
+>
+> Three instances, one direction:
+>
+> | # | Gate | Old | New |
+> | :-- | :-- | :-- | :-- |
+> | #4649 | object validation predicate (`script`/`cross_field`/`when`) | skip rule + log | **reject the write**, naming the rule and key |
+> | #4775 | declarative hook `condition` | → `false` + log | **abort the operation** (`HookConditionError`) |
+> | #4889 | field `readonlyWhen` whose predicate names an **unbound scope root** (e.g. `parent` with no master-detail header resolved) | → "not locked", write lands | **treat the field as LOCKED** (strip it) |
+>
+> The narrowing is deliberately *not* "every CEL fault is now fail-closed". A
+> predicate that is simply BROKEN on this record — an undeclared key, a `null`
+> ordering overload, a parse error, an engine throw — keeps D5's fail-soft
+> policy at the field-predicate surfaces (`requiredWhen`, option `visibleWhen`,
+> and every non-scope `readonlyWhen` fault), because the author has no remedy for
+> an engine fault and bricking CRUD over one is the cure being worse. What
+> changed is the case where the expression is **well-formed and supported** and
+> the site simply could not bind what it names: there, "I could not check" must
+> not resolve to "allowed", because the *declaration itself* says otherwise.
+>
+> Read the Pass-2 evidence table below as a snapshot of 2026-06, not as current
+> behaviour, for those three rows. D5's tiering stands everywhere else — formula
+> → `null` + log, flow → throw, security predicates fail closed.
+
+---
+
 ## TL;DR
 
 ObjectStack exposes **~50 authorable declarations** that hold an expression — formulas, visibility/required/readonly predicates, validation rules, hook conditions, flow/edge conditions, sharing-rule conditions, RLS `using`/`check`, action/view/app visibility, notification/ETL/export/sync/connector conditions — and they all funnel through **one authoring primitive** (`ExpressionInputSchema` → `{ dialect: 'cel', source }`, helpers `cel`/`F`/`P`). The authoring surface is already unified and clean.

docs/adr/0072-reference-scope-and-resolvability.md

@@ -53,7 +53,7 @@ for (const [k, v] of Object.entries(context.record))
 | Flow / edge / decision | `service-automation/src/engine.ts:946-964` | `record`,`previous`, **flattened record fields**, flow `variables`, node outputs, `$runId`/`$flowName` | bare **and** `record.x` |
 | Formula field (`Field.expression`) | `objectql/src/engine.ts:119` | `{ now, timezone, user, org, record }` | `record.x` only |
 | Validation (`script`/`cross_field`/`when`) | `objectql/src/validation/rule-validator.ts:289` | `{ record: {...previous,...patch}, previous }` | `record.x` only |
-| Field `visibleWhen`/`requiredWhen`/`readonlyWhen` | `objectql/src/validation/rule-validator.ts:178-190` | merged `record`, `previous`, (`parent` for master-detail) | `record.x` only |
+| Field `visibleWhen`/`requiredWhen`/`readonlyWhen` | `objectql/src/validation/rule-validator.ts:178-190` | merged `record`, `previous`, (`parent` for master-detail — server-bound for `readonlyWhen` since #4889; the client grid binds it for all three) | `record.x` only |
 | Hook lifecycle `condition` | `objectql/src/hook-wrappers.ts:84` | `{ record }` | `record.x` only |
 | RLS `using`/`check` (compile→filter) | `plugin-security/src/rls-compiler.ts:259` | `current_user.*` (+ pre-resolved membership), record field names | field operands; pushdown subset only |
 | Sharing-rule `condition` (compile→filter) | `plugin-sharing/src/bootstrap-declared-sharing-rules.ts:61` | record fields only | field operands; pushdown subset only |

packages/lint/src/validate-expressions.test.ts

@@ -475,6 +475,10 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
         objects: [{
           name: 'inv_line',
           fields: {
+            // #4889 — `parent` is bound from THIS relationship at write time, so
+            // the fixture declares it: a detail object without one has no header
+            // for the server to read (see the gate's own cases below).
+            inv: { type: 'master_detail', reference: 'inv' },
             qty: { type: 'number', required: true, defaultValue: 1, readonlyWhen: "parent.status == 'paid'" },
             note: { type: 'text', requiredWhen: 'record.qty >= 100' },
           },
@@ -483,6 +487,62 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
       expect(issues).toHaveLength(0);
     });
 
+    // #4889 — `parent`-scoped `readonlyWhen` is enforced by the SERVER binding
+    // the object's master-detail header. No single master ⇒ no binding ⇒ the
+    // write path holds the field locked forever. The metadata says so at build
+    // time, so the build says so.
+    describe('parent-scoped `readonlyWhen` needs a resolvable master (#4889)', () => {
+      const parentScopeIssues = (obj: Record<string, unknown>) =>
+        validateStackExpressions({ objects: [obj] }).filter((i) => /reads `parent`/.test(i.message));
+
+      it('rejects it on an object that declares NO master_detail relationship', () => {
+        const issues = parentScopeIssues({
+          name: 'orphan_line',
+          fields: {
+            inv: { type: 'lookup', reference: 'inv' }, // a lookup is not a master
+            qty: { type: 'number', readonlyWhen: "parent.status == 'paid'" },
+          },
+        });
+        expect(issues).toHaveLength(1);
+        expect(issues[0]!.severity).toBe('error');
+        expect(issues[0]!.message).toMatch(/declares no `master_detail` relationships/);
+      });
+
+      it('rejects it when TWO masters leave "the parent" unstated', () => {
+        const issues = parentScopeIssues({
+          name: 'junction',
+          fields: {
+            left: { type: 'master_detail', reference: 'a' },
+            right: { type: 'master_detail', reference: 'b' },
+            qty: { type: 'number', readonlyWhen: "parent.status == 'paid'" },
+          },
+        });
+        expect(issues).toHaveLength(1);
+        expect(issues[0]!.message).toMatch(/declares 2 `master_detail` relationships/);
+      });
+
+      it('does not fire on a field literally named `parent_id`, or a `parent` string literal', () => {
+        expect(parentScopeIssues({
+          name: 'node',
+          fields: {
+            parent_id: { type: 'text' },
+            kind: { type: 'text' },
+            a: { type: 'text', readonlyWhen: "record.parent_id != ''" },
+            b: { type: 'text', readonlyWhen: "record.kind == 'parent'" },
+          },
+        })).toHaveLength(0);
+      });
+
+      it('is scoped to `readonlyWhen` — `requiredWhen`/`visibleWhen` verdicts are unchanged', () => {
+        expect(parentScopeIssues({
+          name: 'orphan_line',
+          fields: {
+            qty: { type: 'number', requiredWhen: "parent.status == 'paid'", visibleWhen: "parent.status == 'paid'" },
+          },
+        })).toHaveLength(0);
+      });
+    });
+
     it('flags a bare-field sharing-rule condition', () => {
       const issues = validateStackExpressions({
         objects: [{ name: 'crm_account', fields: { region: { type: 'text' } } }],

packages/lint/src/validate-expressions.ts

@@ -23,7 +23,7 @@
  * See `validate-null-guards.ts` for the decision procedure and its scope.
  */
 
-import { validateExpression } from '@objectstack/formula';
+import { validateExpression, collectCelRootIdentifiers } from '@objectstack/formula';
 import { collectFlowGraphs, resolveFlowNodeExpressions } from '@objectstack/spec/automation';
 import type { FlowNodeParsed } from '@objectstack/spec/automation';
 
@@ -148,6 +148,39 @@ function buildNullableFieldIndex(objects: AnyRec[]): Map<string, Set<string>> {
   return idx;
 }
 
+/**
+ * [#4889] Does this CEL source read the `parent` root — the master-detail
+ * header the write path binds for a detail object's field predicates?
+ *
+ * Decided from the parsed AST (the same `collectCelRootIdentifiers` the runtime
+ * gate uses, so build and runtime can never disagree about what "reads
+ * `parent`" means), never a substring scan: a field named `parent_id`, or the
+ * string literal `'parent'`, is not a reference to the binding. A source that
+ * does not parse answers `false` — the ordinary syntax pass already reports it,
+ * and this gate must not report the same defect twice under a worse name.
+ */
+function readsParentRoot(source: string): boolean {
+  const roots = collectCelRootIdentifiers(source);
+  return roots.ok && roots.roots.includes('parent');
+}
+
+/**
+ * The number of `master_detail` relationships an object declares — what decides
+ * whether `parent` is a fact the metadata states (#4889). Exactly one ⇒ the
+ * write path binds that master as `parent`. Zero ⇒ nothing to bind. Two ⇒ no
+ * single "the parent", and picking one by declaration order would make a
+ * data-integrity lock depend on field ordering.
+ */
+function masterDetailCount(obj: AnyRec): number {
+  let n = 0;
+  for (const [, def] of fieldEntries(obj)) {
+    if (def.type !== 'master_detail') continue;
+    const ref = def.reference ?? def.referenceTo;
+    if (typeof ref === 'string' && ref.trim() !== '') n += 1;
+  }
+  return n;
+}
+
 /** The raw CEL source behind a predicate slot (string or `{ dialect, source }`). */
 function celSourceOf(raw: unknown): string | undefined {
   if (typeof raw === 'string') return raw;
@@ -419,13 +452,47 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] {
     // with `field.columnName` itself in #2377: the field no longer exists, so there
     // is no dual-source ambiguity to guard — external column mapping is `external.columnMap`.)
 
+    // [#4889] How many masters this object has, for the `parent`-scope gate
+    // below. Computed once per object, not per field.
+    const masters = masterDetailCount(obj);
+
     for (const [fname, f] of fieldList) {
       // Field-level conditional rules are server-enforced (rule-validator) and
       // record-scoped — a bare ref silently fails the rule (required/readonly
       // not enforced = data-integrity hole). #1928 class, same as actions.
       for (const key of ['requiredWhen', 'readonlyWhen', 'conditionalRequired', 'visibleWhen'] as const) {
         check(`object '${objectName}' · field '${fname}' ${key}`, (f as AnyRec)[key], objectName, 'record');
       }
+      // [#4889] A `parent`-scoped `readonlyWhen` is a SERVER-enforced lock:
+      // the write path resolves the object's master-detail header and binds it
+      // as `parent`. That binding exists only when the object declares exactly
+      // ONE `master_detail` relationship. With none — or with two, where the
+      // metadata does not say which one "the parent" is — the predicate can
+      // never evaluate, and the runtime then holds the field LOCKED forever (it
+      // will not wave a declared lock through just because it could not be
+      // checked). The metadata already contains everything needed to see that
+      // at build time, so it is decided here rather than discovered as an
+      // unwritable field in production — PD #12, declared rather than guessed.
+      //
+      // Scoped to `readonlyWhen` on purpose: it is the one field predicate the
+      // server enforces as a write-path LOCK, so it is the one whose unbindable
+      // scope changes what lands in the database. `requiredWhen` /
+      // `visibleWhen` keep their existing verdicts untouched.
+      const roWhenSource = celSourceOf(f.readonlyWhen);
+      if (masters !== 1 && roWhenSource && readsParentRoot(roWhenSource)) {
+        issues.push({
+          where: `object '${objectName}' · field '${fname}' readonlyWhen`,
+          message:
+            `\`readonlyWhen\` reads \`parent\`, but object '${objectName}' declares ` +
+            `${masters === 0 ? 'no' : `${masters}`} \`master_detail\` relationship${masters === 1 ? '' : 's'} — ` +
+            `so the server has no header record to bind as \`parent\` and the field would be locked on every write. ` +
+            (masters === 0
+              ? `Declare the owning relationship as \`Field.masterDetail('<master>')\`, or rewrite the predicate against \`record\`.`
+              : `\`parent\` needs exactly one master; name the header explicitly through \`record.<fk>\` state instead, or model the extra relationship as a \`lookup\`.`),
+          source: roWhenSource,
+          severity: 'error',
+        });
+      }
       // #4811 — `requiredWhen` is the one field-level slot that meets the
       // null-guard gate's totality criterion: `evaluateValidationRules`
       // evaluates it against the SAME `materializeDeclaredFields`-merged