Skip to content

Commit c045b3f

Browse files
committed
feat(spec): builder forces required: true on a master_detail reference under controlled_by_parent (#9138)
Direction 2 of the #8772 maintainer ruling (2026-08-16, comment 5306089973): ObjectSchema.create() now forces required: true on every master_detail reference of a sharingModel: 'controlled_by_parent' object when the author omits it, and refuses an explicit required: false there with a located, prescriptive error. Raw .parse()/.safeParse() stay tolerant — metadata at rest keeps loading unrewritten; runtime (resolveCbpRelation) and lint severity are deliberately untouched (they are separate ruled slices). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y26DJEHSBhhAQ6wwfsHNza
1 parent 7a537ce commit c045b3f

2 files changed

Lines changed: 206 additions & 0 deletions

File tree

‎packages/spec/src/data/object.test.ts‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { describe, it, expect, vi, afterEach } from 'vitest';
88
// (#5286).
99
import { ObjectSchema, ObjectCapabilities, IndexSchema, ObjectFieldGroupSchema, ObjectExternalBindingSchema, ObjectAccessConfigSchema, LifecycleSchema, TenancyConfigSchema, isTenancyDisabled, resolveCrudAffordances, type ServiceObject } from './object.zod';
1010
import { resolveInjectedSystemColumns } from './injected-system-columns';
11+
import { Field } from './field.zod';
1112
import type { StateMachineValidation } from './validation.zod';
1213

1314
describe('ObjectCapabilities', () => {
@@ -1216,6 +1217,134 @@ describe('ObjectSchema.create()', () => {
12161217
});
12171218
});
12181219

1220+
// ============================================================================
1221+
// controlled_by_parent × master_detail — the builder forces `required: true`
1222+
// (#9138 — #8772 maintainer ruling, Direction 2 / ADR-0055)
1223+
// ============================================================================
1224+
1225+
describe('ObjectSchema.create() forces a required master_detail under controlled_by_parent (#9138)', () => {
1226+
it('forces required: true when `required` is omitted on the master reference', () => {
1227+
const obj = ObjectSchema.create({
1228+
name: 'cbp_line',
1229+
sharingModel: 'controlled_by_parent',
1230+
fields: {
1231+
parent: { type: 'master_detail', reference: 'cbp_header' },
1232+
note: { type: 'text' },
1233+
},
1234+
});
1235+
const fields = obj.fields as Record<string, { required?: boolean }>;
1236+
expect(fields.parent.required).toBe(true);
1237+
// Scope: only the master reference is forced — sibling fields keep the
1238+
// ordinary default (required: false).
1239+
expect(fields.note.required).toBe(false);
1240+
});
1241+
1242+
it('preserves an explicit required: true and the rest of the field config', () => {
1243+
const obj = ObjectSchema.create({
1244+
name: 'cbp_line_explicit',
1245+
sharingModel: 'controlled_by_parent',
1246+
fields: {
1247+
parent: Field.masterDetail('cbp_header', {
1248+
label: 'Header',
1249+
required: true,
1250+
deleteBehavior: 'cascade',
1251+
}),
1252+
},
1253+
});
1254+
const parent = (obj.fields as Record<string, Record<string, unknown>>).parent;
1255+
expect(parent.required).toBe(true);
1256+
expect(parent.deleteBehavior).toBe('cascade');
1257+
expect(parent.label).toBe('Header');
1258+
});
1259+
1260+
it('forces the Field.masterDetail helper shape too (the helper omits required)', () => {
1261+
const obj = ObjectSchema.create({
1262+
name: 'cbp_line_helper',
1263+
sharingModel: 'controlled_by_parent',
1264+
fields: { parent: Field.masterDetail('cbp_header', { label: 'Header' }) },
1265+
});
1266+
expect((obj.fields as Record<string, { required?: boolean }>).parent.required).toBe(true);
1267+
});
1268+
1269+
it('REFUSES an explicit required: false, loudly, naming object + field + the fix', () => {
1270+
let message = '';
1271+
try {
1272+
ObjectSchema.create({
1273+
name: 'cbp_bad',
1274+
sharingModel: 'controlled_by_parent',
1275+
fields: {
1276+
parent: { type: 'master_detail', reference: 'cbp_header', required: false },
1277+
},
1278+
});
1279+
throw new Error('expected ObjectSchema.create to refuse required: false under controlled_by_parent');
1280+
} catch (e) {
1281+
message = (e as Error).message;
1282+
}
1283+
expect(message).not.toContain('expected ObjectSchema.create to refuse');
1284+
expect(message).toContain("ObjectSchema.create('cbp_bad')");
1285+
expect(message).toContain('`parent`');
1286+
expect(message).toContain('required: false');
1287+
expect(message).toContain('controlled_by_parent');
1288+
// The message carries the prescription, not just the verdict.
1289+
expect(message).toContain('Remove `required: false`');
1290+
expect(message).toContain('change its `sharingModel`');
1291+
});
1292+
1293+
it('forces EVERY master_detail reference under the object, not just the first', () => {
1294+
// The prose contract is "exactly one required master_detail", but nothing
1295+
// enforces the count today (#7474 owns the zero-reference case at publish);
1296+
// forcing each declared reference keeps every candidate safe rather than
1297+
// silently blessing only the first.
1298+
const obj = ObjectSchema.create({
1299+
name: 'cbp_multi',
1300+
sharingModel: 'controlled_by_parent',
1301+
fields: {
1302+
a: { type: 'master_detail', reference: 'master_a' },
1303+
b: { type: 'master_detail', reference: 'master_b' },
1304+
},
1305+
});
1306+
const fields = obj.fields as Record<string, { required?: boolean }>;
1307+
expect(fields.a.required).toBe(true);
1308+
expect(fields.b.required).toBe(true);
1309+
});
1310+
1311+
it('leaves master_detail on a NON-controlled_by_parent object alone (scope pin)', () => {
1312+
const omitted = ObjectSchema.create({
1313+
name: 'plain_line',
1314+
fields: { parent: { type: 'master_detail', reference: 'plain_header' } },
1315+
});
1316+
expect((omitted.fields as Record<string, { required?: boolean }>).parent.required).toBe(false);
1317+
1318+
const explicit = ObjectSchema.create({
1319+
name: 'plain_line_explicit',
1320+
sharingModel: 'private',
1321+
fields: { parent: { type: 'master_detail', reference: 'plain_header', required: false } },
1322+
});
1323+
expect((explicit.fields as Record<string, { required?: boolean }>).parent.required).toBe(false);
1324+
});
1325+
1326+
it('raw .parse()/.safeParse() stay TOLERANT of the old shape — metadata at rest keeps loading', () => {
1327+
// The other half of the #8772 ruling: the narrowing is authoring-time
1328+
// only. Stored metadata rehydrated through the schema (never through the
1329+
// builder) must keep loading, UNREWRITTEN — runtime tolerance for existing
1330+
// installs stays with the security gate, and the lint rule stays `warning`
1331+
// until the v18 card (#9139) promotes it.
1332+
const atRest = {
1333+
name: 'cbp_stored',
1334+
sharingModel: 'controlled_by_parent',
1335+
fields: {
1336+
parent: { type: 'master_detail', reference: 'cbp_header', required: false },
1337+
},
1338+
};
1339+
const result = ObjectSchema.safeParse(atRest);
1340+
expect(result.success).toBe(true);
1341+
if (result.success) {
1342+
const fields = (result.data as { fields: Record<string, { required?: boolean }> }).fields;
1343+
expect(fields.parent.required).toBe(false);
1344+
}
1345+
});
1346+
});
1347+
12191348
// ============================================================================
12201349
// Namespace removal (D4) — Object identity is single-sourced on `name`.
12211350
// ============================================================================

‎packages/spec/src/data/object.zod.ts‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2249,6 +2249,76 @@ function assertSystemDataIsWritable(
22492249
);
22502250
}
22512251

2252+
/**
2253+
* [#9138 — #8772 maintainer ruling, Direction 2 / ADR-0055] Under
2254+
* `sharingModel: 'controlled_by_parent'` the builder FORCES `required: true`
2255+
* on every `master_detail` reference, and REFUSES an explicit
2256+
* `required: false` there, loudly.
2257+
*
2258+
* Why: a `controlled_by_parent` detail's access is *derived* from its master
2259+
* through that reference (the `sharingModel` docblock above already states
2260+
* "exactly one required `master_detail` field"). A non-required master
2261+
* reference arms the worst measured failure shape: an insert may omit the
2262+
* master FK, the row lands with a null FK, the derived read filter
2263+
* `masterFK IN (accessible master ids)` can never match null — the row is
2264+
* invisible to everyone — and every later by-id write answers
2265+
* `422 MISSING_REQUIRED_FIELD`. Today only the security gate
2266+
* (`assertControlledByParentWrite`) closes that shape, and #8772 measured that
2267+
* the declaration and the enforcement disagree. This makes the unsafe shape
2268+
* impossible to NEWLY declare:
2269+
*
2270+
* - omitted `required` → forced to `true` (the lint rule
2271+
* `relationship/master-detail-required` already computes exactly this fix);
2272+
* - explicit `required: false` → refused with a located error — the author
2273+
* wrote a contradiction, and silently flipping an explicitly authored value
2274+
* would hide it (ADR-0032 "no silent failure").
2275+
*
2276+
* Lives at `create()` — the authoring surface (ADR-0077) — beside
2277+
* {@link assertSystemDataIsWritable}, and deliberately NOT in raw
2278+
* `.parse()`/`.safeParse()`: metadata already at rest must keep loading.
2279+
* Runtime tolerance is the other half of the #8772 ruling — the security
2280+
* gate's fallbacks stay, and the lint rule stays `warning` until v18 — so
2281+
* publish-time refuses new declarations while runtime tolerates old ones.
2282+
*
2283+
* Alias spellings (`isRequired` / `mandatory` / `notNull`) need no handling
2284+
* here: `FieldSchema` is a `strictObject`, so those keys are refused at parse
2285+
* with a rename suggestion — they can never land a `required: false` this
2286+
* check would miss. A non-boolean `required` is left for Zod to report as the
2287+
* real type error.
2288+
*/
2289+
function forceCbpMasterDetailRequired(
2290+
objectName: unknown,
2291+
sharingModel: unknown,
2292+
fields: unknown,
2293+
): unknown {
2294+
if (sharingModel !== 'controlled_by_parent') return fields;
2295+
if (!fields || typeof fields !== 'object' || Array.isArray(fields)) return fields;
2296+
let out: Record<string, unknown> | undefined;
2297+
for (const [fieldName, def] of Object.entries(fields as Record<string, unknown>)) {
2298+
if (!def || typeof def !== 'object' || Array.isArray(def)) continue;
2299+
const field = def as Record<string, unknown>;
2300+
if (field.type !== 'master_detail') continue;
2301+
if (field.required === true) continue;
2302+
if (field.required === false) {
2303+
const name = typeof objectName === 'string' && objectName.length > 0 ? objectName : '<unnamed>';
2304+
throw new Error(
2305+
`ObjectSchema.create('${name}'): field \`${fieldName}\` declares \`required: false\` on a `
2306+
+ "`master_detail` reference under `sharingModel: 'controlled_by_parent'` — a contradiction "
2307+
+ 'with no honest reading (#8772). A controlled-by-parent detail derives ALL of its record '
2308+
+ 'access from the master this field names (ADR-0055); a row allowed to omit the master FK '
2309+
+ 'is unreadable by everyone (the derived filter `masterFK IN (accessible master ids)` '
2310+
+ 'never matches null) and unwritable thereafter. Remove `required: false` (the builder '
2311+
+ 'declares the reference required by construction), or if this object is not a '
2312+
+ 'master-detail child, change its `sharingModel`.',
2313+
);
2314+
}
2315+
if (field.required !== undefined) continue;
2316+
out ??= { ...(fields as Record<string, unknown>) };
2317+
out[fieldName] = { ...field, required: true };
2318+
}
2319+
return out ?? fields;
2320+
}
2321+
22522322
/**
22532323
* [ADR-0079] Back-compat alias normalization: an object authored with the
22542324
* deprecated `displayNameField` key still parses by mapping it onto the
@@ -2368,8 +2438,15 @@ export const ObjectSchema = lazySchema(() => {
23682438
// contradiction with no honest reading — refuse it here, where it is cheap
23692439
// to fix, rather than shipping a bucket whose name lies again.
23702440
assertSystemDataIsWritable(cfg.name, cfg.managedBy, cfg.userActions);
2441+
// [#9138 — #8772 ruling, Direction 2] A `controlled_by_parent` object's
2442+
// `master_detail` reference is forced `required: true` (an explicit
2443+
// `required: false` throws, loudly) so the unsafe shape cannot be newly
2444+
// declared. Raw `.parse()`/`.safeParse()` stay tolerant for metadata at
2445+
// rest — see the function's docblock.
2446+
const forcedFields = forceCbpMasterDetailRequired(cfg.name, cfg.sharingModel, cfg.fields);
23712447
const withDefaults = {
23722448
...cfg,
2449+
...(forcedFields === cfg.fields ? {} : { fields: forcedFields }),
23732450
label: cfg.label ?? snakeCaseToLabel(cfg.name as string),
23742451
};
23752452
// [ADR-0079] `ObjectSchemaBase.parse` here is the alias-normalizing override

0 commit comments

Comments
 (0)