diff --git a/.changeset/25092-cbp-master-writable.md b/.changeset/25092-cbp-master-writable.md new file mode 100644 index 00000000000..0c1387fb419 --- /dev/null +++ b/.changeset/25092-cbp-master-writable.md @@ -0,0 +1,27 @@ +--- +'@objectstack/spec': minor +'@objectstack/lint': patch +--- + +feat(spec)!: a `controlled_by_parent` object whose one required `master_detail` is marked `readonly: true` or `system: true` is refused at parse + +Clause-②: no (narrowing) + +Metadata-protocol: narrowed + + + +**BREAKING** (a declaration the parse accepted is now refused), shipped as `minor` under the 18.x convention for breaking changes, with no retirement stage. + +ADR-0055 §1: a `controlled_by_parent` object derives its record access from exactly one required `master_detail` field. That one master must be a reference every write of a detail supplies, and a field declared `readonly: true` or `system: true` is not one an author's write supplies, so it cannot be that master. `os lint` already reported it at `error` under `relationship/master-detail-required`, but `os lint` is not a publishing door: `os validate`, `os build`, `ObjectSchema.create()`, `defineStack` and the metadata save door accepted it. + +- **FROM:** `sharingModel: 'controlled_by_parent'` with one required `master_detail` declared `readonly: true` and/or `system: true` parses, builds and saves. +- **TO:** the parse refuses it at `fields`, naming the object, the field and each flag (the message opens with "Object 'OBJECT' declares sharingModel 'controlled_by_parent' with its one required master_detail, FIELD, marked readonly: true", with each name and flag in a code span). The save door answers `422 INVALID_METADATA` with that issue and stores nothing. `ObjectSchema.create()` refuses it too, because the parse it ends in is the same rule. An objectExtensions entry that leaves the merged master flagged is refused at the extension, as before for the other shapes. +- **The fix:** remove `readonly: true` and `system: true` from the master reference and declare it as an ordinary required field. If the rows must outlive their parent, the object is not a detail: give it its own `sharingModel` (`private`, `public_read` or `public_read_write`). +- **Unchanged:** the runtime. The flags do not change which field the runtime resolves as the master. A flag on a second, non-required `master_detail` beside the master is still reported by `os lint` alone. + +Data at rest still loads. A stored object in this shape registers under its `[metadata_spec_invalid]` diagnostic, and `os migrate meta --stored` lists it as a TODO under `cbp-master-relation-tiers-retired`. No conversion rewrites it: dropping the flag changes who may write the reference, which is the owner's decision. + +`@objectstack/spec/data`: `controlledByParentMasterRefusal` returns the new shape `master-not-writable` for this object, so a producer that asks it before writing `controlled_by_parent` gets the parse's verdict. `ControlledByParentMasterShape` gains that member. `isControlledByParentMasterField` is unchanged: a flagged required `master_detail` still counts as the master, so beside a second required one the object is still refused as having two. + +`@objectstack/lint`: the `relationship/master-detail-required` explanation (`os explain`) now says that the parse refuses every shape that leaves the object without exactly one writable required master, and that a shape on a second, non-required `master_detail` is this rule's report alone. diff --git a/packages/lint/src/data-model-rules.master-detail-required.test.ts b/packages/lint/src/data-model-rules.master-detail-required.test.ts index 0df5641f6c1..86f32ed75c9 100644 --- a/packages/lint/src/data-model-rules.master-detail-required.test.ts +++ b/packages/lint/src/data-model-rules.master-detail-required.test.ts @@ -148,11 +148,12 @@ describe('R2 outside controlled_by_parent — the verdict is unchanged', () => { }); }); -// What actually reaches the rule from the authoring builder. Under +// What 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. +// to `true` and refuses an explicit `false`; the parse it ends in refuses a one +// master marked `readonly` or `system` (spec shape `master-not-writable`), so +// the two flagged shapes no longer leave the builder, and this rule reports +// them on a stack that never went through it. describe('R2 over objects built by ObjectSchema.create()', () => { const build = (field: ReturnType) => ObjectSchema.create({ @@ -161,12 +162,23 @@ describe('R2 over objects built by ObjectSchema.create()', () => { fields: { order: field }, }); + // [#25092] The builder no longer lets these two through: the parse it ends in + // refuses a one master marked `readonly` / `system` (spec shape + // `master-not-writable`). This rule still reports the same field on the + // object as the builder would have emitted it (`required` forced to `true`), + // which is what `os lint` reads from a stack that never went through it. 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]); + ])('a %s master reference is refused by the builder itself, and reported here on the literal', (flag, extra) => { + const field = { ...Field.masterDetail('work_order', { label: 'Work Order' }), ...extra }; + expect(() => build(field)).toThrow(/with its one required `master_detail`, `order`, marked/); + const literal = { + name: 'work_order_item', + sharingModel: 'controlled_by_parent', + fields: { order: { ...field, required: true } }, + }; + const issues = r2([{ name: 'work_order', sharingModel: 'private', fields: {} }, literal]); expect(issues).toHaveLength(1); expect(issues[0]).toMatchObject({ severity: 'error', path: `objects[1].fields.order.${flag}` }); }); diff --git a/packages/lint/src/data-model-rules.ts b/packages/lint/src/data-model-rules.ts index 167736fcf9b..d1f3e083323 100644 --- a/packages/lint/src/data-model-rules.ts +++ b/packages/lint/src/data-model-rules.ts @@ -310,16 +310,19 @@ const CBP_ORPHAN_CONSEQUENCE = * forces `required: true` on every `master_detail` reference under * `controlled_by_parent` and refuses an explicit `required: false` — * `forceCbpMasterDetailRequired` in `packages/spec/src/data/object.zod.ts`). - * The builder never inspects `readonly` / `system`, so shapes 2 and 3 pass - * through `create()` untouched, and shape 1 survives every path that does not - * go through `create()` (a plain object literal, raw `.parse()` of stored - * metadata); this rule is where all three meet a refusal at authoring time. + * The spec parse refuses, at every door that parses, every shape that leaves + * the object without exactly one writable required master (`ObjectSchema`'s + * `controlledByParentMasterRefusal`: a missing `required` since ruling B, + * `readonly` / `system` on the one master since #25092, ruling 6328204856 + * route A, shape `master-not-writable`). What only this rule reports is a + * shape on a second, non-required `master_detail` beside the master. * * ⚠ Reach: `lintDataModel` is `os lint`'s sweep and the metadata-generation * rubric (`score.ts`), and nothing else — it is not an `AUTHORING_RULES` entry * (`authoring-rule-wiring.test.ts` → `DIRECT_CALL_RATCHET`), so `os build`, - * `os validate` and the metadata save door do not run it. The `error` moves - * `os lint`'s exit code and the rubric's `valid`, not a publish verdict. + * `os validate` and the metadata save door do not run it; they meet the spec + * parse's refusal above instead. The `error` moves `os lint`'s exit code and + * the rubric's `valid`, not a publish verdict. * * One finding per field, located at the first defect it names; the `fix` * names every edit the field needs. diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts index 9527198ce3b..0712a52fdb5 100644 --- a/packages/lint/src/rule-explanations.ts +++ b/packages/lint/src/rule-explanations.ts @@ -2779,10 +2779,11 @@ const RELATIONSHIP_MASTER_DETAIL_REQUIRED_EXPLANATION: RuleExplanation = { 'security gate (`assertControlledByParentWrite`) is the only thing refusing such an insert.', 'Every `master_detail` field of such an object is judged, not only the one the runtime resolves ' + 'as the master: that is the scope `ObjectSchema.create()` enforces when it forces ' + - '`required: true` and refuses an explicit `required: false`. The builder never inspects ' + - '`readonly` or `system`, and a plain object literal or a raw `.parse()` of stored metadata ' + - 'never runs it, so this rule is where all three shapes meet a refusal at authoring time. One ' + - 'finding per field, located at its first defect.', + '`required: true` and refuses an explicit `required: false`. The spec parse also ' + + 'refuses every shape that leaves the object without exactly one writable required master, so ' + + '`os validate`, `os build` and the save door stop there; a shape on a second, non-required ' + + '`master_detail` beside the master is this rule\'s report alone. One finding per field, located ' + + 'at its first defect.', DATA_MODEL_SWEEP_REACH, ], }; diff --git a/packages/metadata-protocol/src/protocol.cbp-master-not-writable-refusal.test.ts b/packages/metadata-protocol/src/protocol.cbp-master-not-writable-refusal.test.ts new file mode 100644 index 00000000000..3a297873742 --- /dev/null +++ b/packages/metadata-protocol/src/protocol.cbp-master-not-writable-refusal.test.ts @@ -0,0 +1,95 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #25092 — the object save door refuses a `controlled_by_parent` detail whose + * one required `master_detail` is marked `readonly: true` or `system: true`. + * + * ADR-0055 §1 derives such an object's master from exactly one required + * `master_detail` that every write of a detail supplies. The rule lives in the + * spec parse (`controlledByParentMasterRefusal`, shape `master-not-writable`), + * and `saveMetaItem` parses with `ObjectSchema`, so the door answers + * `422 INVALID_METADATA` at `fields` and stores nothing — the same refusal + * `os validate` and `os build` meet, from the same rule. + */ +import { describe, expect, it } from 'vitest'; +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core'; +import { ObjectStackProtocolImplementation } from './protocol.js'; + +/** The engine surface the repository write path touches (as the container-issue-descent harness). */ +function makeProtocol() { + const rows = new Map>(); + let nextId = 0; + const engine: any = { + async findOne(object: string, query?: EngineFindOneQueryInput) { + assertEngineFindOnePredicate(object, query); return null; }, + async find() { return []; }, + async insert(table: string, data: Record) { + nextId += 1; + if (table === 'sys_metadata') rows.set(`${data.type}|${data.name}`, data); + return { id: `r_${nextId}` }; + }, + async update(_t: string, data: Record, opts?: Record) { + assertEngineUpdateDispatch(data, opts); + return { id: null }; + }, + async delete(_t: string, opts?: Record) { + assertEngineDeleteDispatch(opts); + return { deleted: 0 }; + }, + registry: { registerItem: () => {}, registerObject: () => {} }, + }; + const protocol: any = new ObjectStackProtocolImplementation(engine, () => new Map()); + return { protocol, rows }; +} + +async function rejection(promise: Promise): Promise { + try { + await promise; + } catch (err) { + return err; + } + throw new Error('expected the save to be rejected, but it resolved'); +} + +const detailWith = (flags: Record) => ({ + name: 'work_order_item', + label: 'Work Order Item', + sharingModel: 'controlled_by_parent', + fields: { + order: { name: 'order', type: 'master_detail', label: 'Order', reference: 'work_order', required: true, ...flags }, + note: { name: 'note', type: 'text', label: 'Note' }, + }, +}); + +describe('#25092 the save door refuses a controlled_by_parent detail whose one master is not writable', () => { + it.each([ + ['readonly: true', { readonly: true }, '`readonly: true`'], + ['system: true', { system: true }, '`system: true`'], + ])('a one master marked %s answers 422 INVALID_METADATA at `fields`, and nothing is stored', async (_label, flags, named) => { + const { protocol, rows } = makeProtocol(); + + const err = await rejection(protocol.saveMetaItem({ + type: 'object', name: 'work_order_item', item: detailWith(flags), + })); + + expect(err.code).toBe('INVALID_METADATA'); + expect(err.status).toBe(422); + expect(err.issues).toEqual([expect.objectContaining({ path: 'fields', code: 'custom' })]); + expect(err.issues[0].message.startsWith( + "Object 'work_order_item' declares sharingModel 'controlled_by_parent' with its one required " + + `\`master_detail\`, \`order\`, marked ${named}.`, + )).toBe(true); + expect(rows.size).toBe(0); + }); + + it('control: the same detail with a writable master saves', async () => { + const { protocol, rows } = makeProtocol(); + + const result = await protocol.saveMetaItem({ + type: 'object', name: 'work_order_item', item: detailWith({}), + }); + + expect(result.success).toBe(true); + expect(rows.size).toBe(1); + }); +}); diff --git a/packages/spec/src/conversions/refused-shapes.test.ts b/packages/spec/src/conversions/refused-shapes.test.ts index 94698679698..f02d47f1cb4 100644 --- a/packages/spec/src/conversions/refused-shapes.test.ts +++ b/packages/spec/src/conversions/refused-shapes.test.ts @@ -151,6 +151,27 @@ describe('#23270 — a stored controlled_by_parent object without exactly one re expect(JSON.parse(todo.from).fields.title).toBeUndefined(); }); + it.each([ + ['readonly: true', md('crm_opportunity', { required: true, readonly: true })], + ['system: true', md('crm_opportunity', { required: true, system: true })], + // The Studio shape plus the flag: D2 gives `required: true`, and the flag is still listed. + ['readonly: true, `required` omitted', md('crm_opportunity', { readonly: true })], + ])('#25092 — a one master marked %s: one TODO under the same entry, and the TODO does not claim the runtime denies it', (_label, master) => { + const row = cbpRow('crm_line', { opportunity: master }); + const { item, todos } = storedTodos('object', row); + expect(item.fields.opportunity.readonly ?? item.fields.opportunity.system).toBe(true); + expect(todos).toHaveLength(1); + const todo = todos[0]!; + expect(todo.conversionId).toBe('cbp-master-relation-tiers-retired'); + expect(todo.path).toBe('objects[0].fields'); + expect(todo.reason).toMatch(/Its one required `master_detail`, `opportunity`, is marked `readonly` or `system`\. Dropping the flag changes who may write the reference/); + expect(todo.reason).toContain('the metadata save door refuses it with 422 INVALID_METADATA until it is corrected'); + expect(todo.reason).not.toContain('every read of its rows is denied'); + expect(todo.reason).toContain("Object 'crm_line' declares sharingModel 'controlled_by_parent' with its one required `master_detail`"); + // The listing and the parse refuse ONE set: the stored row still fails the parse, at the same key. + expect(cbpIssue(item)?.path).toEqual(['fields']); + }); + it('the Studio shape — one master_detail, `required` omitted — converts and is NOT listed', () => { const row = cbpRow('crm_line', { opportunity: md('crm_opportunity') }); const { item, todos, notices } = storedTodos('object', row); diff --git a/packages/spec/src/conversions/refused-shapes.ts b/packages/spec/src/conversions/refused-shapes.ts index 91f11d9e613..dcff14fec90 100644 --- a/packages/spec/src/conversions/refused-shapes.ts +++ b/packages/spec/src/conversions/refused-shapes.ts @@ -50,6 +50,7 @@ import { flowNodeValueTemplateRefusals } from '../automation/flow-value-slot-template.js'; import { controlledByParentMasterRefusal, + isControlledByParentMasterField, type ControlledByParentMasterRefusal, } from '../data/controlled-by-parent-master.js'; import { ComponentPropsMap } from '../ui/component.zod.js'; @@ -123,7 +124,31 @@ function controlledByParentFate(refusal: ControlledByParentMasterRefusal, fields return 'It declares no `master_detail` and no required `lookup`, so there is no master to derive ' + 'and nothing to convert. Add a `master_detail` field with `required: true` referencing the master ' + `object, ${ownSharingModel}.`; + case 'master-not-writable': { + const master = refusal.masterDetails.find((n) => isControlledByParentMasterField(fields[n]))!; + return `Its one required \`master_detail\`, \`${master}\`, ` + + 'is marked `readonly` or `system`. Dropping the flag changes who may write the reference, so the ' + + 'upgrade does not write it. Remove the flag and declare the master reference as an ordinary required ' + + `field, ${ownSharingModel}.`; + } + } +} + +/** + * What a stored row in a refused shape does once it is left as stored. Every + * shape but one leaves the runtime with no master to derive. A + * `master-not-writable` object still resolves its one required `master_detail` + * as the master at runtime (ADR-0055 §1 refuses it at the parse, not at the + * runtime), so what it loses is the save door: it is re-saved only once + * corrected. + */ +function controlledByParentLeftAsStored(refusal: ControlledByParentMasterRefusal): string { + if (refusal.shape === 'master-not-writable') { + return 'Left as stored, it registers under `[metadata_spec_invalid]` and the metadata save door refuses ' + + 'it with 422 INVALID_METADATA until it is corrected.'; } + return 'Left as stored, it registers under `[metadata_spec_invalid]` and the platform derives no master: ' + + 'every read of its rows is denied and every write refused with 422 INVALID_METADATA.'; } const controlledByParentWithoutOneMaster: RefusedStoredShape = { @@ -141,9 +166,8 @@ const controlledByParentWithoutOneMaster: RefusedStoredShape = { path: `${path}.${refusal.path}`, from: JSON.stringify({ sharingModel: obj.sharingModel, fields: relation }), reason: 'No lossless rewrite: protocol 18 derives a controlled-by-parent object\'s master from exactly ' - + 'one `master_detail` declared `required: true`, and this object does not declare exactly one. Left as stored, ' - + 'it registers under `[metadata_spec_invalid]` and the platform derives no master: every read of ' - + 'its rows is denied and every write refused with 422 INVALID_METADATA. ' + + 'one writable `master_detail` declared `required: true`, and this object does not declare exactly one. ' + + `${controlledByParentLeftAsStored(refusal)} ` + `${controlledByParentFate(refusal, fields)} ${refusal.message}`, }); return obj; diff --git a/packages/spec/src/data/controlled-by-parent-master.test.ts b/packages/spec/src/data/controlled-by-parent-master.test.ts index b7503510461..b2c9d7d765b 100644 --- a/packages/spec/src/data/controlled-by-parent-master.test.ts +++ b/packages/spec/src/data/controlled-by-parent-master.test.ts @@ -1,6 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { describe, expect, it } from 'vitest'; +import { ZodError } from 'zod'; // Read through the `@objectstack/spec/data` entry, not the module: the pin is // on the PUBLISHED predicate a producer imports (#23272). @@ -58,6 +59,25 @@ const SHAPES: Array<[string, Record, ControlledByParentMasterSh ['one master_detail, required omitted', { order: md('work_order') }, 'master-not-required'], ['a required lookup only', { order: lookup('work_order', { required: true }) }, 'required-lookup-only'], ['no relation', { note: { type: 'text', label: 'Note' } }, 'no-master'], + // [#25092] The one master is writable: `readonly` / `system` on it are refused. + ['one required master_detail, readonly: true', { order: md('work_order', { required: true, readonly: true }) }, 'master-not-writable'], + ['one required master_detail, system: true', { order: md('work_order', { required: true, system: true }) }, 'master-not-writable'], + [ + 'one required master_detail, readonly: true and system: true', + { order: md('work_order', { required: true, readonly: true, system: true }) }, + 'master-not-writable', + ], + [ + 'one required master_detail, readonly: true, beside a required lookup', + { order: md('work_order', { required: true, readonly: true }), owner_ref: lookup('account', { required: true }) }, + 'master-not-writable', + ], + ['one required master_detail, readonly: false and system: false', { order: md('work_order', { required: true, readonly: false, system: false }) }, undefined], + [ + 'a readonly required master_detail beside a second required one is still two masters', + { a: md('work_order', { required: true, readonly: true }), b: md('account', { required: true }) }, + 'several-required-masters', + ], ]; describe('controlledByParentMasterRefusal — the export and ObjectSchema agree (#23272)', () => { @@ -88,6 +108,9 @@ describe('controlledByParentMasterRefusal — the export and ObjectSchema agree describe('isControlledByParentMasterField — the field half the refusal counts (#23272)', () => { it.each([ ['a required master_detail', md('x', { required: true }), true], + // The flags are judged on the object's one master, not here: the field is still the master. + ['a required master_detail, readonly: true', md('x', { required: true, readonly: true }), true], + ['a required master_detail, system: true', md('x', { required: true, system: true }), true], ['a master_detail, required: false', md('x', { required: false }), false], ['a master_detail, required omitted', md('x'), false], ['a master_detail, required "true" (not a boolean)', md('x', { required: 'true' }), false], @@ -100,11 +123,54 @@ describe('isControlledByParentMasterField — the field half the refusal counts expect(isControlledByParentMasterField(field)).toBe(expected); }); - it('is the count the refusal makes: exactly one counting field satisfies the rule', () => { + it('is the count the refusal makes: exactly one counting field, and a writable one, satisfies the rule', () => { for (const [, fields] of SHAPES) { - const counted = Object.values(fields).filter(isControlledByParentMasterField).length; + const counted = Object.values(fields).filter(isControlledByParentMasterField) as Array>; + const writable = counted.length === 1 && counted[0]!.readonly !== true && counted[0]!.system !== true; const refusal = controlledByParentMasterRefusal('o', 'controlled_by_parent', fields); - expect(refusal === undefined).toBe(counted === 1); + expect(refusal === undefined).toBe(writable); } }); }); + +describe('#25092 a one master marked readonly or system is refused at authoring, through the one rule', () => { + it('names the object, the field and every flag, and prescribes dropping them', () => { + const refusal = controlledByParentMasterRefusal('work_order_item', 'controlled_by_parent', { + order: md('work_order', { required: true, readonly: true, system: true }), + }); + expect(refusal).toMatchObject({ path: 'fields', shape: 'master-not-writable', masterDetails: ['order'] }); + expect(refusal!.message).toMatch( + /^Object 'work_order_item' declares sharingModel 'controlled_by_parent' with its one required `master_detail`, `order`, marked `readonly: true` and `system: true`\./, + ); + expect(refusal!.message).toContain('Remove `readonly: true` and `system: true` from `order`'); + }); + + it('ObjectSchema.create() refuses it too: the builder forces `required` and the parse it ends in refuses the flag', () => { + const build = () => + ObjectSchema.create({ + name: 'work_order_item', + sharingModel: 'controlled_by_parent', + fields: { order: { type: 'master_detail', label: 'Order', reference: 'work_order', readonly: true } }, + }); + let thrown: unknown; + try { + build(); + } catch (err) { + thrown = err; + } + expect(thrown).toBeInstanceOf(ZodError); + const issues = (thrown as ZodError).issues.filter((i) => /declares sharingModel 'controlled_by_parent'/.test(i.message)); + expect(issues).toHaveLength(1); + expect(issues[0]!.path).toEqual(['fields']); + expect(issues[0]!.message).toContain('with its one required `master_detail`, `order`, marked `readonly: true`'); + }); + + it('control: the same builder call without the flag builds', () => { + const obj = ObjectSchema.create({ + name: 'work_order_item', + sharingModel: 'controlled_by_parent', + fields: { order: { type: 'master_detail', label: 'Order', reference: 'work_order' } }, + }); + expect((obj.fields.order as { required?: unknown }).required).toBe(true); + }); +}); diff --git a/packages/spec/src/data/controlled-by-parent-master.ts b/packages/spec/src/data/controlled-by-parent-master.ts index 1500e7c092d..98db836bcf7 100644 --- a/packages/spec/src/data/controlled-by-parent-master.ts +++ b/packages/spec/src/data/controlled-by-parent-master.ts @@ -35,6 +35,8 @@ * `z.boolean().default(false)`, so on the parsed map an omitted `required` is * `false`, and on a stored body it is `undefined`. The predicate counts a * master only where `required === true`, so the two give the same verdict. + * `readonly` (`z.boolean().default(false)`) and `system` (optional) are read + * the same way: only `=== true` marks the one master unwritable. */ /** @@ -52,18 +54,28 @@ * (or a non-boolean the field parse refuses on its own); * - `required-lookup-only` — no `master_detail`, one or more required `lookup` * fields (the retired third tier); - * - `no-master` — no `master_detail` and no required `lookup`. + * - `no-master` — no `master_detail` and no required `lookup`; + * - `master-not-writable` — exactly one required `master_detail`, declared + * `readonly: true` or `system: true`. ADR-0055 §1 derives the master from a + * reference every write of a detail supplies, and those two flags mark a + * field an author's write does not supply (one the platform fills, or one a + * write may not set), so it cannot be the one master. Dropping the flag is + * the owner's call: it changes who may write the reference. */ export type ControlledByParentMasterShape = | 'several-required-masters' | 'several-masters' | 'master-not-required' | 'required-lookup-only' - | 'no-master'; + | 'no-master' + | 'master-not-writable'; /** Why an object fails the one-required-master rule — see {@link controlledByParentMasterRefusal}. */ export interface ControlledByParentMasterRefusal { - /** Where the parse reports it: `fields` (more than one candidate) or `sharingModel` (none). */ + /** + * Where the parse reports it: `fields` (more than one candidate, or the one + * master is not writable) or `sharingModel` (none). + */ readonly path: 'fields' | 'sharingModel'; readonly shape: ControlledByParentMasterShape; /** The `master_detail` fields the object declares, in field order. */ @@ -79,7 +91,11 @@ export interface ControlledByParentMasterRefusal { * A `master_detail` field declared `required: true`, and nothing else: a * non-required `master_detail` and a required `lookup` are the two tiers * ruling B retired (#23100). An object satisfies the rule when EXACTLY ONE of - * its fields counts — {@link controlledByParentMasterRefusal} is that count. + * its fields counts, and that one is writable (neither `readonly: true` nor + * `system: true`) — {@link controlledByParentMasterRefusal} is that judgement. + * The flags are judged on the one master, not here: a flagged required + * `master_detail` is still the field the runtime resolves as the master, so a + * second one beside it is still two masters. * * Reads `required === true` on either the parsed field or a stored body (see * the module header), and accepts any value: a non-object counts as no master. @@ -90,6 +106,12 @@ export function isControlledByParentMasterField(field: unknown): boolean { return f.type === 'master_detail' && f.required === true; } +/** + * The writability flags a `controlled_by_parent` master may not carry, in the + * order a refusal names them. See the `master-not-writable` shape. + */ +const UNWRITABLE_MASTER_FLAGS = ['readonly', 'system'] as const; + /** * Judge one object against ADR-0055 §1's one-required-master rule. Returns * `undefined` when the rule does not apply (another sharing model, no field @@ -109,7 +131,27 @@ export function controlledByParentMasterRefusal( .filter(([, f]) => f?.type === 'lookup' && f.required === true) .map(([key]) => key); const masters = entries.filter(([, f]) => isControlledByParentMasterField(f)).map(([key]) => key); - if (masters.length === 1) return undefined; + if (masters.length === 1) { + const master = masters[0]!; + const def = (fields as Record | undefined>)[master]; + const flags = UNWRITABLE_MASTER_FLAGS.filter((flag) => def?.[flag] === true); + if (flags.length === 0) return undefined; + return { + path: 'fields', + shape: 'master-not-writable', + masterDetails, + requiredLookups, + message: + `Object '${name}' declares sharingModel 'controlled_by_parent' with its one required \`master_detail\`, ` + + `\`${master}\`, marked ${flags.map((f) => `\`${f}: true\``).join(' and ')}. ADR-0055 §1: since ` + + '@objectstack/spec 18, a controlled-by-parent object derives its master from exactly one required ' + + '`master_detail` field that every write of a detail supplies, and a field marked `readonly` or ' + + '`system` is not one an author\'s write supplies, so this reference cannot be that master. ' + + `Remove ${flags.map((f) => `\`${f}: true\``).join(' and ')} from \`${master}\` and declare the ` + + 'master reference as an ordinary required field; if the rows must outlive their parent, this object ' + + "is not a detail: give it its own sharingModel ('private', 'public_read' or 'public_read_write').", + }; + } if (masters.length > 1) { return { path: 'fields', diff --git a/packages/spec/src/data/object.zod.ts b/packages/spec/src/data/object.zod.ts index a80dcd22824..7ba859f2f65 100644 --- a/packages/spec/src/data/object.zod.ts +++ b/packages/spec/src/data/object.zod.ts @@ -2183,6 +2183,18 @@ function refuseAttachedBlockConflicts(attachedOnRead: unknown, fields: unknown, * object this rule refuses is denied every read and refused every write by the * runtime too. The parse says so where an author is present to act. * + * [#25092, ruling 6328204856 route A] The one master must also be WRITABLE: + * a required `master_detail` declared `readonly: true` or `system: true` is + * refused at `['fields']` (shape `master-not-writable`). ADR-0055 §1 derives + * the master from a reference every write of a detail supplies, and a field + * carrying either flag is not one an author's write supplies. Unlike the shapes above, the runtime still + * resolves this one (the flags do not change which field is the master), so + * this half of the rule lives at the parse alone, the authoring contract, and + * the runtime is not changed by it. `os lint`'s + * `relationship/master-detail-required` reports the same flags at `error` on + * every `master_detail` of the object; it is the advisory sweep, this is the + * refusal every door that parses meets. + * * Read on the PARSED field map: `required` is `z.boolean().default(false)`, so * an omitted `required` here is `false` — and is refused. That is the builder's * own contract seen from the other side: `ObjectSchema.create()` forces an @@ -3545,6 +3557,12 @@ function assertReferenceViaSiblingDeclared(objectName: unknown, fields: unknown) * wrote a contradiction, and silently flipping an explicitly authored value * would hide it (ADR-0032 "no silent failure"). * + * `readonly` / `system` are not read here: the builder forces nothing about + * them, and the parse it ends in refuses a one master carrying either + * ({@link refuseControlledByParentWithoutOneMaster}, shape + * `master-not-writable`), so `create()` refuses them through that one rule + * rather than a second copy of it. + * * Lives at `create()` — the authoring surface (ADR-0077) — beside * {@link assertSystemDataIsWritable}, and deliberately NOT in raw * `.parse()`/`.safeParse()`: a parse must not rewrite what it reads. The force diff --git a/packages/spec/src/migrations/entries/semantic/18.cbp-master-detail-required-lint-error.ts b/packages/spec/src/migrations/entries/semantic/18.cbp-master-detail-required-lint-error.ts index 6cc40799da9..65ed84b2142 100644 --- a/packages/spec/src/migrations/entries/semantic/18.cbp-master-detail-required-lint-error.ts +++ b/packages/spec/src/migrations/entries/semantic/18.cbp-master-detail-required-lint-error.ts @@ -33,7 +33,9 @@ export const entry: SemanticMigration = { + 'still reaches it from any object not authored through the builder. The security gate keeps ' + 'refusing these inserts. (The later step-18 entry `cbp-master-relation-tiers-retired` goes ' + 'further for the first shape: a `master_detail` without `required: true` is refused at parse, ' - + 'and the runtime no longer resolves a master from it.)', + + 'and the runtime no longer resolves a master from it. The same entry now refuses the other two ' + + 'at parse on the object\'s one required master, so `ObjectSchema.create` no longer lets them ' + + 'through either.)', acceptanceCriteria: '`os lint` reports no `relationship/master-detail-required` finding at `error`: on every ' + 'object declaring `sharingModel: \'controlled_by_parent\'`, each `master_detail` reference ' @@ -41,10 +43,12 @@ export const entry: SemanticMigration = { + 'which emits `required: true`) and carries neither `readonly: true` nor `system: true`. ' + '⚠️ WHICH DOOR: this rule\'s refusal is `os lint`\'s and the metadata-generation rubric\'s ' + 'only. The rule is not in the authoring-rule registry, so `os build`, `os validate` and the ' - + 'metadata save door do not run it, and a stack carrying the `readonly` or `system` shape still ' - + 'builds and publishes — read `os lint`\'s exit code, not a green build. (A missing `required` ' - + 'on an object\'s only `master_detail` is refused at parse by every one of those doors since ' - + '`cbp-master-relation-tiers-retired`.) Stored metadata is not rewritten and keeps loading, and ' + + 'metadata save door do not run it. They refuse at parse, under `cbp-master-relation-tiers-retired`, ' + + 'the shapes ADR-0055 §1 decides on the object\'s one required master: a missing `required` on its only ' + + '`master_detail`, and `readonly: true` or `system: true` on the one required `master_detail`. A flag ' + + 'or a missing `required` on any other, non-required `master_detail` of the object is this rule\'s ' + + 'alone — read ' + + '`os lint`\'s exit code there, not a green build. Stored metadata is not rewritten and keeps loading, and ' + 'the security gate still refuses an insert that omits the master FK on these shapes. Repo ' + 'census at the time of the change: 129 authored objects across the example ' + 'apps, the platform and plugin objects and the CLI\'s golden eval corpus, 7 of them ' diff --git a/packages/spec/src/migrations/entries/semantic/18.cbp-master-relation-tiers-retired.ts b/packages/spec/src/migrations/entries/semantic/18.cbp-master-relation-tiers-retired.ts index 6758179f226..4fde6e3ad88 100644 --- a/packages/spec/src/migrations/entries/semantic/18.cbp-master-relation-tiers-retired.ts +++ b/packages/spec/src/migrations/entries/semantic/18.cbp-master-relation-tiers-retired.ts @@ -9,19 +9,25 @@ import type { SemanticMigration } from '../../types.js'; // builder's own `true`. That is the D2 conversion `cbp-master-detail-required-defaulted`, // and objectui's Studio is its producer. Every other shape is this entry's // prescription: a `lookup` turned `master_detail` changes delete behaviour, and -// leaving `controlled_by_parent` is an author's call. +// leaving `controlled_by_parent` is an author's call. The one master is also +// writable (#25092, ruling 6328204856 route A): `readonly` / `system` on it are +// refused by the same parse rule, not converted, and the runtime is unchanged. export const entry: SemanticMigration = { id: 'cbp-master-relation-tiers-retired', surface: 'object.sharingModel = \'controlled_by_parent\' together with object.fields — the master ' + 'relation the object derives its record access from', replacement: 'Exactly one `master_detail` field declared `required: true`, with a `reference` to the master ' - + 'object; every other parent of the object is a `lookup`. Nothing else derives a master any more: a ' + + 'object and neither `readonly: true` nor `system: true` on it; every other parent of the object is a ' + + '`lookup`. Nothing else derives a master any more: a ' + '`master_detail` without `required: true` and a required `lookup` were the security runtime\'s second ' + 'and third fallbacks and both retired, and two required `master_detail` fields are no longer resolved ' + 'by field order. `ObjectSchema` refuses each of those shapes at parse, naming the object — so ' + '`ObjectSchema.create()`, `defineStack`, `os validate` and the metadata save door refuse them — and ' + 'the runtime derives no master from them: every read of such an object is denied and every write ' - + 'refused with 422 INVALID_METADATA. Data at rest whose one `master_detail` merely omits `required` ' + + 'refused with 422 INVALID_METADATA. A one master marked `readonly` or `system` is refused at parse too, ' + + 'by the same doors, naming the field and the flag; the runtime still resolves that field as the master, ' + + 'so the object is not denied, and the fix is to drop the flag and declare the master reference as an ' + + 'ordinary required field. Data at rest whose one `master_detail` merely omits `required` ' + 'is converted, not refused: the D2 conversion `cbp-master-detail-required-defaulted` gives it the ' + '`required: true` that `ObjectSchema.create()` would have given it. It runs on stored rows, built ' + 'artifacts and `os migrate meta`, never on a live authoring parse. A required `lookup` to the master becomes `type: \'master_detail\'` ' @@ -44,20 +50,30 @@ export const entry: SemanticMigration = { + 'it for data at rest. Nothing else converts. An explicit `required: false` is a contradiction the ' + 'builder refuses. Two masters would need a pick. Turning a `lookup` into a `master_detail` changes ' + 'what deleting the master does to every row, and whether an object should instead stop being ' - + 'controlled by its parent is a product decision only its author can make.', + + 'controlled by its parent is a product decision only its author can make. The same rule refuses a one ' + + 'master marked `readonly: true` or `system: true`: ADR-0055 §1 derives the ' + + 'master from a reference every write of a detail supplies, a field carrying either flag is not one an ' + + 'author\'s write supplies, and `os lint` alone reported them, so `os validate`, `os build` and the save ' + + 'door accepted a master that could not be the one master. It is not converted: dropping the flag changes who may ' + + 'write the reference.', acceptanceCriteria: 'Every object declaring `sharingModel: \'controlled_by_parent\'` declares exactly one `master_detail` ' + 'field with `required: true` (through `ObjectSchema.create()`, an omitted `required` on its one ' + '`master_detail` is emitted as `true`), and `ObjectSchema.safeParse` of it succeeds: no ' - + '"declares sharingModel \'controlled_by_parent\' but no required `master_detail` field" or ' - + '"with N required `master_detail` fields" issue. `os lint` reports neither ' + + '"declares sharingModel \'controlled_by_parent\' but no required `master_detail` field", ' + + '"with N required `master_detail` fields" or "with its one required `master_detail`, … marked" issue. `os lint` reports neither ' + '`security-controlled-by-parent-no-relation` nor `security-controlled-by-parent-ambiguous-relation`. At ' + 'runtime the security plugin logs no `[security/controlled-by-parent]` "resolves no master relation" ' + 'line for the object, and a member who can read its master reads its rows. A stored row whose one ' + '`master_detail` omitted `required` is served with `required: true` (`cbp-master-detail-required-defaulted`); ' - + 'a stored row in any other retired shape still registers (with its `[metadata_spec_invalid]` ' - + 'diagnostic) and is denied until it is corrected and saved. `os migrate meta --stored` lists each such ' + + 'a stored row in any other retired shape (two masters, none, a required `lookup` only, an explicit ' + + '`required: false`) still registers (with its `[metadata_spec_invalid]` diagnostic) and is denied until ' + + 'it is corrected and saved. A stored row whose one required `master_detail` is marked `readonly` or ' + + '`system` also registers under that diagnostic, and the runtime still resolves its master, so it is not ' + + 'denied; the metadata save door refuses it with 422 INVALID_METADATA until it is corrected. ' + + '`os migrate meta --stored` lists each such ' + 'row as a TODO under this entry\'s id, before the upgrade and after it, naming the object, its shape (two ' - + 'masters, none, a required `lookup` only, an explicit `required: false`) and the owner\'s choice, and ' + + 'masters, none, a required `lookup` only, an explicit `required: false`, a master marked `readonly` or ' + + '`system`) and the owner\'s choice, and ' + 'rewrites none of them.', }; diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 3c66782a1fb..ed4a1358e61 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -466,6 +466,21 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/metadata-protocol/src/protocol.cbp-master-not-writable-refusal.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/metadata-protocol/src/protocol.cbp-master-not-writable-refusal.test.ts", + "verb": "findOne", + "pinned": 1 + }, + { + "file": "packages/metadata-protocol/src/protocol.cbp-master-not-writable-refusal.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/metadata-protocol/src/protocol.cluster-mutation-fanout.test.ts", "verb": "delete",