Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .changeset/25092-cbp-master-writable.md
Original file line number Diff line number Diff line change
@@ -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

<!-- adr-0087: not-required (already-registered cbp-master-relation-tiers-retired) registered before this change, amended in this diff: the parse's one-master entry now carries this narrowing, and the stored-row listing reports it under that id -->

**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.
26 changes: 19 additions & 7 deletions packages/lint/src/data-model-rules.master-detail-required.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof Field.masterDetail>) =>
ObjectSchema.create({
Expand All @@ -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}` });
});
Expand Down
15 changes: 9 additions & 6 deletions packages/lint/src/data-model-rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
9 changes: 5 additions & 4 deletions packages/lint/src/rule-explanations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
],
};
Expand Down
Original file line number Diff line number Diff line change
@@ -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<string, Record<string, unknown>>();
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<string, unknown>) {
nextId += 1;
if (table === 'sys_metadata') rows.set(`${data.type}|${data.name}`, data);
return { id: `r_${nextId}` };
},
async update(_t: string, data: Record<string, unknown>, opts?: Record<string, unknown>) {
assertEngineUpdateDispatch(data, opts);
return { id: null };
},
async delete(_t: string, opts?: Record<string, unknown>) {
assertEngineDeleteDispatch(opts);
return { deleted: 0 };
},
registry: { registerItem: () => {}, registerObject: () => {} },
};
const protocol: any = new ObjectStackProtocolImplementation(engine, () => new Map());
return { protocol, rows };
}

async function rejection(promise: Promise<unknown>): Promise<any> {
try {
await promise;
} catch (err) {
return err;
}
throw new Error('expected the save to be rejected, but it resolved');
}

const detailWith = (flags: Record<string, unknown>) => ({
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);
});
});
21 changes: 21 additions & 0 deletions packages/spec/src/conversions/refused-shapes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
30 changes: 27 additions & 3 deletions packages/spec/src/conversions/refused-shapes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 = {
Expand All @@ -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;
Expand Down
Loading
Loading