feat(lint)!: relationship/master-detail-required refuses the three unsafe master-reference shapes at error on a controlled_by_parent object

.changeset/9139-cbp-master-detail-required-error.md

@@ -0,0 +1,33 @@
+---
+"@objectstack/lint": minor
+"@objectstack/spec": minor
+---
+
+feat(lint)!: `relationship/master-detail-required` refuses the three unsafe master-reference shapes at `error` on a `controlled_by_parent` object (#9139)
+
+Clause-②: no (narrowing)
+
+A `controlled_by_parent` detail derives all of its record access from the master its `master_detail` reference names (ADR-0055). Three declarable shapes of that reference leave the security gate as the only thing refusing a detail record saved without its master, because record validation never checks a field that is not `required` and skips `readonly` and `system` fields before its required check:
+
+1. `required` absent, or `required: false`;
+2. `required: true` with `readonly: true`;
+3. `required: true` with `system: true`.
+
+A record that lands without its master anyway is readable by nobody, and every later write to it by id is refused. Until now `relationship/master-detail-required` was a `warning` with the predicate "`required` is not `true`", on every object, so shapes 2 and 3 drew no finding at any severity. The maintainer ruling of 2026-08-16 (Direction 1) scheduled the promotion for the v18 boundary, scoped to `controlled_by_parent`.
+
+**BREAKING — what moves for consumers.**
+
+- `os lint` reports each of the three shapes at `error` when the object declares `sharingModel: 'controlled_by_parent'`, located at the defect (`…fields.FIELD.required`, `.readonly` or `.system`). It covers every `master_detail` field of such an object, the same scope the builder's `required: true` force already applies. `os lint` therefore exits non-zero on such a stack, and the metadata-generation rubric (`scoreMetadata`) weighs the finding as an error and marks the stack `valid: false`.
+- `@objectstack/spec` gains the step-18 semantic migration entry `cbp-master-detail-required-lint-error`, so `os migrate meta` across protocol 18 prints the prescription below.
+
+**Remedy — the v18 upgrade-checklist line.** On every object with `sharingModel: 'controlled_by_parent'`, give each `master_detail` reference `required: true` and remove any `readonly: true` or `system: true` from it. `os lint` now refuses the missing-`required`, `required` + `readonly` and `required` + `system` shapes there at `error` (`relationship/master-detail-required`). An object authored through `ObjectSchema.create` already gets `required: true` when the key is omitted, so the edit there is dropping the flag.
+
+**Unchanged.**
+
+- On every object that is not `controlled_by_parent` the rule is exactly as before: a `warning` for a `master_detail` without `required: true`, the same message and fix, and no finding for the two flagged shapes.
+- The rule is not in the authoring-rule registry. `os build`, `os validate` and the metadata save door do not run it, so a stack carrying one of the shapes still builds and publishes. Only `os lint`'s exit code and the generation rubric move.
+- Runtime is untouched. The security gate keeps refusing an insert that omits the master FK on these shapes and keeps resolving the master for metadata already at rest, and stored metadata is neither rewritten nor refused on load.
+- No export or signature moves in either package.
+- Measured before crossing, at `b04a5295f`: 129 authored objects across the example apps, the platform, plugin and service objects and the CLI's golden eval corpus. 7 of them are `controlled_by_parent`, and 0 draw the new `error`.
+
+<!-- adr-0087: registered cbp-master-detail-required-lint-error -->

content/docs/protocol/kernel/error-handling.mdx

@@ -515,11 +515,12 @@ measured to create a detail record whose master reference is null — a record t
 readable by nobody and answers 422 on every later write by id. The refusal is correct; only
 its status departs from the rule above.
 
-**These shapes are authorable today.** Publish-time lint reports a `master_detail` without
-`required` as a *warning* (`relationship/master-detail-required`), and does not report the
-`readonly`, `system`, or fallback-`lookup` shapes at all — so a stack can publish clean and
-still reach the 422. Branch on `code`, and read the status off the response rather than
-deriving it from this page.
+**These shapes are authorable today.** `os lint` refuses the three `master_detail` shapes at
+*error* (`relationship/master-detail-required`), but that rule is not part of the publish
+gate — `os build`, `os validate` and the metadata save door do not run it — and it does not
+report the fallback-`lookup` shape at all. So a stack can publish clean and still reach the
+422. Branch on `code`, and read the status off the response rather than deriving it from this
+page.
 
 #### `INVALID_FIELD`
 **HTTP Status:** 400  

packages/cli/test/score.test.ts

@@ -87,15 +87,21 @@ describe('scoreMetadata', () => {
         { name: 'invoice_line', label: 'Line', sharingModel: 'controlled_by_parent', fields: { invoice: { type: 'master_detail', label: 'Invoice', reference: 'invoice', required: true, inlineEdit: true } } },
       ],
     });
-    // A warning: master_detail not required.
+    // A warning: master_detail not required — on an object that is NOT
+    // `controlled_by_parent`. Under `controlled_by_parent` the same shape is an
+    // error (`relationship/master-detail-required`'s v18 tier), so a fixture
+    // kept there would still pass the comparison below while measuring the
+    // error weight, not the warning one.
     const withWarning = scoreMetadata({
       objects: [
         { name: 'invoice', label: 'Invoice', sharingModel: 'private', fields: { name: { type: 'text', label: 'Name', required: true } } },
-        { name: 'invoice_line', label: 'Line', sharingModel: 'controlled_by_parent', fields: { invoice: { type: 'master_detail', label: 'Invoice', reference: 'invoice', deleteBehavior: 'cascade' } } },
+        { name: 'invoice_line', label: 'Line', sharingModel: 'private', fields: { invoice: { type: 'master_detail', label: 'Invoice', reference: 'invoice', deleteBehavior: 'cascade' } } },
       ],
     });
     expect(onlySuggestions.score).toBeGreaterThan(withWarning.score);
     expect(onlySuggestions.counts.errors).toBe(0);
+    expect(withWarning.counts.errors).toBe(0);
+    expect(withWarning.counts.warnings).toBeGreaterThan(0);
   });
 
   it('reports the schema error messages', () => {

packages/lint/src/data-model-rules.master-detail-required.test.ts

@@ -0,0 +1,160 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * R2 `relationship/master-detail-required` — two tiers under one rule id.
+ *
+ * On a `sharingModel: 'controlled_by_parent'` object the master reference is
+ * what the object's record access is derived through, and three declarable
+ * shapes leave the security gate as the only thing refusing a detail record
+ * saved without its master: `required` absent (or `false`), `required: true` +
+ * `readonly: true`, and `required: true` + `system: true` (record validation
+ * never checks a non-required field and skips readonly/system fields before its
+ * required check). All three are refused here at `error` — the v18 narrowing of
+ * the authoring contract (maintainer ruling of 2026-08-16, Direction 1).
+ *
+ * Before the narrowing the predicate was `required !== true` at `warning` on
+ * every object, so the two flagged shapes drew no finding at ANY severity. The
+ * cases below pin each shape separately for that reason: a one-line severity
+ * flip would have turned the first red and left the other two silent.
+ *
+ * The other half is what keeps this from being a blanket escalation: outside
+ * `controlled_by_parent` nothing at runtime refuses a non-required
+ * `master_detail`, so there the verdict is unchanged — a `warning` on a missing
+ * `required`, and silence on the two flagged shapes.
+ */
+
+import { describe, expect, it } from 'vitest';
+import { Field, ObjectSchema } from '@objectstack/spec/data';
+import { lintDataModel } from './data-model-rules.js';
+
+const R2 = 'relationship/master-detail-required';
+
+/** The master, plus one detail object carrying exactly the fields under test. */
+const stack = (sharingModel: string | undefined, fields: unknown): unknown[] => [
+  { name: 'work_order', sharingModel: 'private', fields: { name: { type: 'text' } } },
+  { name: 'work_order_item', ...(sharingModel ? { sharingModel } : {}), fields },
+];
+
+const r2 = (objects: unknown[]) => lintDataModel(objects as any[]).filter((issue) => issue.rule === R2);
+
+/** A `master_detail` reference to `work_order`, plus the shape under test. */
+const masterRef = (shape: Record<string, unknown>) => ({
+  order: { type: 'master_detail', reference: 'work_order', deleteBehavior: 'cascade', ...shape },
+});
+
+const UNSAFE_SHAPES: Array<[label: string, shape: Record<string, unknown>, path: string]> = [
+  ['`required` absent', {}, 'objects[1].fields.order.required'],
+  ['`required: false`', { required: false }, 'objects[1].fields.order.required'],
+  ['`required: true` + `readonly: true`', { required: true, readonly: true }, 'objects[1].fields.order.readonly'],
+  ['`required: true` + `system: true`', { required: true, system: true }, 'objects[1].fields.order.system'],
+];
+
+describe('R2 on a controlled_by_parent object — the three unsafe shapes are refused at error', () => {
+  it.each(UNSAFE_SHAPES)('%s → one error, located at the defect', (_label, shape, path) => {
+    const issues = r2(stack('controlled_by_parent', masterRef(shape)));
+    expect(issues).toHaveLength(1);
+    expect(issues[0]).toMatchObject({ severity: 'error', rule: R2, path });
+  });
+
+  // ── CONTROLS — the refusal must be able to stay silent, or the reds above
+  // prove nothing about the shapes they name.
+  it('CONTROL: `required: true` with neither flag is clean', () => {
+    expect(r2(stack('controlled_by_parent', masterRef({ required: true })))).toEqual([]);
+  });
+
+  it('CONTROL: an explicit `readonly: false` / `system: false` is clean — the flag, not the key, is the defect', () => {
+    expect(
+      r2(stack('controlled_by_parent', masterRef({ required: true, readonly: false, system: false }))),
+    ).toEqual([]);
+  });
+
+  it('a field carrying several defects is ONE finding whose fix names every edit', () => {
+    const issues = r2(stack('controlled_by_parent', masterRef({ readonly: true, system: true })));
+    expect(issues).toHaveLength(1);
+    expect(issues[0]).toMatchObject({ severity: 'error', path: 'objects[1].fields.order.required' });
+    expect(issues[0].fix).toContain('required: true');
+    expect(issues[0].fix).toContain('readonly');
+    expect(issues[0].fix).toContain('system');
+  });
+
+  // Scope is every `master_detail` of the object, as the builder half of the
+  // same ruling enforces it — not only the one the runtime resolves as master.
+  it('reports every master_detail field of the object, each on its own path', () => {
+    const issues = r2(
+      stack('controlled_by_parent', {
+        order: { type: 'master_detail', reference: 'work_order', required: true },
+        batch: { type: 'master_detail', reference: 'work_order', required: true, readonly: true },
+        legacy: { type: 'master_detail', reference: 'work_order' },
+      }),
+    );
+    expect(issues.map((issue) => [issue.severity, issue.path])).toEqual([
+      ['error', 'objects[1].fields.batch.readonly'],
+      ['error', 'objects[1].fields.legacy.required'],
+    ]);
+  });
+
+  it('reads the array field form too', () => {
+    const issues = r2(
+      stack('controlled_by_parent', [{ name: 'order', type: 'master_detail', reference: 'work_order', required: true, system: true }]),
+    );
+    expect(issues).toHaveLength(1);
+    expect(issues[0]).toMatchObject({ severity: 'error', path: 'objects[1].fields.order.system' });
+  });
+
+  it('a lookup is not R2’s subject, whatever the sharing model', () => {
+    expect(
+      r2(stack('controlled_by_parent', { order: { type: 'lookup', reference: 'work_order', readonly: true } })),
+    ).toEqual([]);
+  });
+});
+
+describe('R2 outside controlled_by_parent — the verdict is unchanged', () => {
+  it.each([['private'], ['public_read'], ['public_read_write'], [undefined]])(
+    'sharingModel %s: a missing `required` is still a warning, at `.required`',
+    (sharingModel) => {
+      const issues = r2(stack(sharingModel, masterRef({})));
+      expect(issues).toHaveLength(1);
+      expect(issues[0]).toMatchObject({
+        severity: 'warning',
+        path: 'objects[1].fields.order.required',
+        fix: 'required: true',
+      });
+    },
+  );
+
+  it.each([
+    ['`required: true` + `readonly: true`', { required: true, readonly: true }],
+    ['`required: true` + `system: true`', { required: true, system: true }],
+  ])('sharingModel private: %s draws nothing, as before', (_label, shape) => {
+    expect(r2(stack('private', masterRef(shape)))).toEqual([]);
+  });
+});
+
+// What actually reaches the rule from the authoring builder. Under
+// `controlled_by_parent`, `ObjectSchema.create()` forces an omitted `required`
+// to `true` and refuses an explicit `false`, but never inspects `readonly` or
+// `system` — so the two flagged shapes leave the builder intact, and this rule
+// is the authoring-time refusal they meet.
+describe('R2 over objects built by ObjectSchema.create()', () => {
+  const build = (field: ReturnType<typeof Field.masterDetail>) =>
+    ObjectSchema.create({
+      name: 'work_order_item',
+      sharingModel: 'controlled_by_parent',
+      fields: { order: field },
+    });
+
+  it.each([
+    ['readonly', { readonly: true }],
+    ['system', { system: true }],
+  ])('a %s master reference passes the builder (forced required) and is refused here', (flag, extra) => {
+    const detail = build({ ...Field.masterDetail('work_order', { label: 'Work Order' }), ...extra });
+    const issues = r2([{ name: 'work_order', sharingModel: 'private', fields: {} }, detail]);
+    expect(issues).toHaveLength(1);
+    expect(issues[0]).toMatchObject({ severity: 'error', path: `objects[1].fields.order.${flag}` });
+  });
+
+  it('CONTROL: the builder’s forced `required: true` on a plain master reference lints clean', () => {
+    const detail = build(Field.masterDetail('work_order', { label: 'Work Order' }));
+    expect(r2([{ name: 'work_order', sharingModel: 'private', fields: {} }, detail])).toEqual([]);
+  });
+});