diff --git a/.changeset/nine-parents-keep-details.md b/.changeset/nine-parents-keep-details.md new file mode 100644 index 0000000000..ea4eaadb69 --- /dev/null +++ b/.changeset/nine-parents-keep-details.md @@ -0,0 +1,12 @@ +--- +'@objectstack/spec': minor +'@objectstack/objectql': minor +--- + +`FieldSchema` now rejects an authored `deleteBehavior: 'set_null'` on a `master_detail` field at parse time (#9689). The engine has always resolved every value except `restrict` on that type to `cascade`, so the declaration asked for the child rows to be kept and got them deleted — silently, at the moment the parent went away. The rejection names the outcome and both legal re-declarations (`restrict` refuses the parent delete while children exist — no data loss; `cascade`, or omitting the key, accepts the cascade deliberately; a `lookup` is the type to use when children must survive the parent). + +Mechanism (the #7918 Option A shape, plus the 2026-08-24 idempotent-materialization ruling): the property-level `.default('set_null')` moved off `deleteBehavior` into a post-check `.overwrite()`, so the schema can tell an authored `set_null` from a defaulted one — and the `.overwrite()` never materializes a default the schema itself would refuse as authored. A bare `master_detail` now parses to output that OMITS `deleteBehavior` (previously the baked `set_null` was indistinguishable from an authored one by design, so parse output rejected itself on the mainline `ObjectSchema.create()` → `defineStack` re-parse — every app build with a bare `master_detail` failed). Built app artifacts stop carrying a value the schema itself refuses; the engine treats absent exactly as it treated the baked value (both cascade — measured, behavior unchanged). Every other field type keeps byte-identical output — non-reference types still carry the default at its shape position, and `set_null` on `lookup` stays legal. The inferred `Field` output type now declares `deleteBehavior` as optional (the same accepted cost as the currency `precision` relocation); at runtime a parsed field carries it on every type except `master_detail`, where absence is the honest spelling. + +There is deliberately no automatic conversion (`field-master-detail-set-null-refused` in the migration registry): only the author knows whether they meant `restrict` (keep-my-children, as a refusal) or `cascade`. Stored rows carrying the refused combination keep loading and serving — registry validation is a diagnostic, not a gate — and are refused on their next authoring-path save. + +`@objectstack/objectql`: the engine behavior is unchanged (an authored `set_null` on `master_detail` still cascades — the #9625 pin holds), but the coercion site now logs loudly (`error`, falling back to `warn`) when the combination reaches it via a raw registration or a pre-tightening stored row — the two populations parse-time rejection cannot catch. diff --git a/content/docs/data-modeling/field-types.mdx b/content/docs/data-modeling/field-types.mdx index a0b9c3c18a..91699ba91c 100644 --- a/content/docs/data-modeling/field-types.mdx +++ b/content/docs/data-modeling/field-types.mdx @@ -337,7 +337,7 @@ Parent-child relationship (cascading delete by default). |:---|:---|:---|:---| | `reference` | `string` | **required** | Target (master) object name | | `referenceFilters` | `string[]` | — | **Removed** (#2377, ADR-0049) — no longer a recognized field property. `FieldSchema` is a strict object, so an unknown key is **rejected with guidance**, not silently stripped (ADR-0078): the error echoes the offending key and prescribes the replacement. Use structured `lookupFilters` + `dependsOn` instead; see [Relationships](/docs/data-modeling/relationships) | -| `deleteBehavior` | `'restrict' \| 'cascade' \| 'set_null'` | `'cascade'` | Behavior when parent is deleted. `restrict` is the only value that deviates: master-detail cascades on everything else, so an explicit `set_null` here is **not** honored — the child is deleted with the parent | +| `deleteBehavior` | `'restrict' \| 'cascade'` | `'cascade'` | Behavior when parent is deleted. `restrict` refuses the parent delete while children exist; `cascade` (and omitting the key) deletes the children with the parent. An explicit `set_null` is a **parse-time rejection** on this type (#9689) — a detail row cannot outlive its master; use a `lookup` if children must survive the parent | | `inlineEdit` | `boolean \| 'grid' \| 'form'` | — | Edit child records inline on the parent create/edit form (`true` = auto-pick, `'grid'`, or `'form'`) | | `inlineColumns` | `array` | — | Optional explicit inline grid columns | | `inlineAmountField` | `string` | — | Numeric child field used for the inline running total | diff --git a/content/docs/protocol/objectql/types.mdx b/content/docs/protocol/objectql/types.mdx index 22aa7e2491..9a58524b20 100644 --- a/content/docs/protocol/objectql/types.mdx +++ b/content/docs/protocol/objectql/types.mdx @@ -698,9 +698,13 @@ const opportunities = await engine.find('opportunity', { > been emptied. > > On `master_detail` the same reading applies from the other side: `restrict` -> is the only value that deviates from `cascade`, so an explicit -> `deleteBehavior: set_null` on a master-detail reference is *not* honored — -> the child is cascaded away. +> is the only value that deviates from `cascade`. An explicit +> `deleteBehavior: set_null` on a master-detail reference is a **parse-time +> rejection** (#9689) — a detail row cannot outlive its master, so the spec +> refuses the declaration instead of silently cascading the children it asked +> to keep. Metadata that bypasses the parse (a raw registration, or a row +> stored before the tightening) still resolves to `cascade`, now with a loud +> engine log at the coercion site. > > The refusal carries **two** messages, for two audiences. `error` is written for > the person who clicked delete: it is rendered in the caller's locale from the diff --git a/packages/objectql/src/engine-cascade-delete.test.ts b/packages/objectql/src/engine-cascade-delete.test.ts index eb17aa08f1..c4cdd4139d 100644 --- a/packages/objectql/src/engine-cascade-delete.test.ts +++ b/packages/objectql/src/engine-cascade-delete.test.ts @@ -29,9 +29,13 @@ * `multiple: true` case (see below) and a `master_detail` declaring an explicit * `set_null`, which is silently resolved to `cascade`. * - * Whether the spec should reject `set_null` on a `master_detail` at publish - * time rather than dropping it at delete time is still an open question, - * carded separately — not decided by this suite. + * [#9689] That question is now answered (maintainer ruling 2026-08-19): + * `FieldSchema` REJECTS an authored `set_null` on a `master_detail` at parse + * time, and this engine logs loudly when the combination still reaches the + * coercion site (raw registrations — like this suite's — and metadata stored + * before the tightening; the engine registers raw objects and never + * re-parses, so the spec-layer rejection alone measurably does not change + * anything here). The COERCION itself is unchanged and stays pinned below. * * ## [#9688] The multi-value refusal now judges EMPTINESS, per row * @@ -54,7 +58,7 @@ * An authored `deleteBehavior: 'restrict'` is untouched by any of it. */ -import { describe, it, expect, beforeEach } from 'vitest'; +import { describe, it, expect, beforeEach, vi } from 'vitest'; import { ObjectQL } from './engine.js'; const acct = { @@ -468,9 +472,13 @@ describe('cascadeDeleteRelations — required FK escalates set_null → restrict it('[#9625] a master_detail declaring an explicit deleteBehavior:set_null still cascades', async () => { // The neighbouring resolution with the same blind spot: `restrict` is - // the only value that deviates, so `set_null` is accepted by - // `FieldSchema` on this type and then dropped here. Pinned so the - // silent coercion is a documented fact rather than an absence. + // the only value that deviates, so every other value is dropped here. + // Pinned so the coercion is a documented fact rather than an absence. + // [#9689] `FieldSchema` now rejects this combination at parse time, + // but THIS registration is raw (the engine never re-parses), so the + // combination still reaches the engine and the coercion still applies + // — this pin stays TRUE by ruling; the delete-time change is the loud + // log, pinned in its own describe below. const a = await engine.insert('acct', { name: 'Acme' }); const l = await engine.insert('line', { parent: a.id }); @@ -515,3 +523,113 @@ describe('cascadeDeleteRelations — required FK escalates set_null → restrict expect((await engine.findOne('note', { where: { id: n.id } }) as any).account).toBeNull(); }); }); + +// [#9689] (maintainer ruling 2026-08-19, Q3 = B): the coercion above stays, +// and it now LOGS. `FieldSchema` rejects an authored `set_null` on a +// `master_detail` at parse time, so a value that still reaches the engine came +// in around the parse seam (raw registration / pre-tightening stored row) — +// the population parse-time rejection measurably cannot catch. The log fires +// on exactly the authored combination: not on a bare master_detail, not on an +// authored cascade, and not on restrict (which never coerces). +describe('cascadeDeleteRelations — [#9689] authored set_null on master_detail logs loudly at the coercion site', () => { + // A bare master_detail — the overwhelmingly common spelling; the engine + // resolves it to cascade identically, and it must NOT log. + const stanzaBare = { + name: 'stanza', + label: 'Stanza', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + parent: { name: 'parent', type: 'master_detail' as const, reference: 'acct' }, + }, + }; + // An authored cascade — same resolved behavior, deliberate; must NOT log. + const verseCascade = { + name: 'verse', + label: 'Verse', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + parent: { + name: 'parent', type: 'master_detail' as const, reference: 'acct', + deleteBehavior: 'cascade', + }, + }, + }; + + function makeSpyLogger(withError = true) { + const spy = { + info: vi.fn(), warn: vi.fn(), debug: vi.fn(), + ...(withError ? { error: vi.fn() } : {}), + }; + return spy as Record<'info' | 'warn' | 'debug' | 'error', ReturnType>; + } + + // NOTE the registration set is per test: the log fires at the COERCION + // SITE — whenever the parent delete computes the child field's behavior — + // not only when that child holds rows. A misdeclared child object in the + // registry therefore logs on every parent delete (deliberate: the + // declaration is wrong whether or not rows exist today), so the negative + // control below must not register `line` at all. + async function makeEngine(logger: Record, objects: unknown[]) { + const engine = new ObjectQL({ logger }); + const { driver } = makeStubDriver(); + engine.registerDriver(driver, true); + await engine.init(); + // Two-arg spelling (packageId is the signature's required 2nd arg — + // the house pattern of batch-row-authoring-feedback.test.ts): the + // 1-arg call this helper first shipped with added a TS2554 to the + // frozen TEST_DEBT ledger (354 -> 355), and the ratchet only shrinks. + for (const o of objects) engine.registry.registerObject(o as any, 'com.objectstack.test.9689'); + return engine; + } + + it('logs via logger.error when the parent delete coerces an authored set_null to cascade', async () => { + const logger = makeSpyLogger(); + const engine = await makeEngine(logger, [acct, lineExplicitSetNull, stanzaBare, verseCascade]); + const a = await engine.insert('acct', { name: 'Acme' }); + const l = await engine.insert('line', { parent: a.id }); + + await engine.delete('acct', { where: { id: a.id } } as any); + // The pinned behavior is unchanged: the child cascaded away. + expect(await engine.findOne('line', { where: { id: l.id } })).toBeNull(); + + const hits = logger.error.mock.calls.filter((c) => String(c[0]).includes("deleteBehavior: 'set_null'")); + expect(hits).toHaveLength(1); + const msg = String(hits[0][0]); + // Attribution: which declaration, on which relation, and the outcome. + expect(msg).toContain('line.parent'); + expect(msg).toContain('master_detail'); + expect(msg).toContain('NOT honored'); + expect(msg).toContain('CASCADES'); + // Actionability: both legal re-declarations are named. + expect(msg).toContain("'restrict'"); + expect(msg).toContain("'cascade'"); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it('falls back to logger.warn when the sink has no error method (#9750 sanctioned shape — never an optional call)', async () => { + const logger = makeSpyLogger(false); + const engine = await makeEngine(logger, [acct, lineExplicitSetNull]); + const a = await engine.insert('acct', { name: 'Acme' }); + await engine.insert('line', { parent: a.id }); + + await engine.delete('acct', { where: { id: a.id } } as any); + expect(logger.warn.mock.calls.some((c) => String(c[0]).includes("deleteBehavior: 'set_null'"))).toBe(true); + }); + + it('does NOT log for a bare master_detail or an authored cascade (same resolved behavior, no divergence)', async () => { + const logger = makeSpyLogger(); + const engine = await makeEngine(logger, [acct, stanzaBare, verseCascade]); + const a = await engine.insert('acct', { name: 'Acme' }); + const s = await engine.insert('stanza', { parent: a.id }); + const v = await engine.insert('verse', { parent: a.id }); + + await engine.delete('acct', { where: { id: a.id } } as any); + // Both cascaded (resolved behavior identical to the logging case) … + expect(await engine.findOne('stanza', { where: { id: s.id } })).toBeNull(); + expect(await engine.findOne('verse', { where: { id: v.id } })).toBeNull(); + // … and neither logged: the divergence between declared and delivered + // exists only for the authored set_null. + const all = [...logger.error.mock.calls, ...logger.warn.mock.calls].map((c) => String(c[0])); + expect(all.filter((m) => m.includes("deleteBehavior: 'set_null'"))).toHaveLength(0); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index f2e9bbbc80..f02cf6cff3 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -10823,16 +10823,45 @@ export class ObjectQL implements IObjectQLEngine { // // [#9625] "Only an explicit `restrict` deviates" is the whole of it: // every other value a master_detail can declare — including an - // explicit `deleteBehavior: 'set_null'`, which `FieldSchema` accepts - // on this type — resolves to `cascade` here, silently. Measured and - // pinned (`engine-cascade-delete.test.ts`); whether the spec should - // reject the combination at publish time instead of the engine - // dropping it at delete time is a judgement, carded separately. + // explicit `deleteBehavior: 'set_null'` — resolves to `cascade` here, + // silently. Measured and pinned (`engine-cascade-delete.test.ts`). let behavior: string = fdef.type === 'master_detail' ? (fdef.deleteBehavior === 'restrict' ? 'restrict' : 'cascade') : (fdef.deleteBehavior || 'set_null'); + // [#9689] (maintainer ruling 2026-08-19, Q3 = B): the judgement the + // #9625 comment above deferred is now taken — `FieldSchema` REJECTS an + // authored `deleteBehavior: 'set_null'` on a `master_detail` at parse + // time, so the value is meaningful again: one that still reaches this + // site came in around the parse seam (a raw `registerObject`, or a + // stored/artifact row written before the tightening — the two + // populations parse-time rejection measurably cannot catch, since the + // engine registers raw objects and never re-parses). The coercion + // itself stays: this delete is about to CASCADE children whose + // declaration asked for them to be kept, and that divergence must be + // loud and attributable, not silent. Sanctioned logger shape per + // PR #9750: reach for `error`, fall back to `warn` — NEVER an optional + // call like `logger.error?.()`, which emits nothing against a sink + // with no `error`. Caveat, measured (#4447): a built app artifact + // materializes FieldSchema defaults, so an artifact-loaded BARE + // master_detail also carries `set_null` and logs here — that residual + // imprecision is the materialized-default defect tracked as #9784, not + // a reason to soften this log. + if (fdef.type === 'master_detail' && fdef.deleteBehavior === 'set_null') { + const msg = + `[cascade-delete] ${childName}.${fieldName} declares deleteBehavior: 'set_null' on a ` + + `master_detail referencing '${object}' — that value is NOT honored on master_detail: the ` + + `delete of ${object}/${String(id)} CASCADES its referencing child rows, the opposite of what ` + + `the declaration asks (children kept). FieldSchema now rejects this combination at parse time; ` + + `this row reached the engine around the parse seam (raw registration, or metadata stored ` + + `before the tightening). Re-declare the field: 'restrict' refuses the parent delete while ` + + `children exist (no data loss), 'cascade' (or omitting the key) accepts the cascade ` + + `deliberately, or make it a lookup if children must survive the parent.`; + if (typeof this.logger.error === 'function') this.logger.error(msg); + else this.logger.warn(msg); + } + // A REQUIRED foreign key cannot be nulled — set_null would issue an // UPDATE clearing the FK, which the child's required-field validator // rejects with a misleading " is required" 400 (the field isn't diff --git a/packages/spec/src/data/field.test.ts b/packages/spec/src/data/field.test.ts index 82f6d1751c..9121d683a8 100644 --- a/packages/spec/src/data/field.test.ts +++ b/packages/spec/src/data/field.test.ts @@ -481,6 +481,120 @@ describe('FieldSchema', () => { expect(result.deleteBehavior).toBe('set_null'); }); + // [#9689] (maintainer ruling 2026-08-19, Q1 = A): an AUTHORED + // `deleteBehavior: 'set_null'` on a `master_detail` is a named parse-time + // rejection — the engine resolves it to `cascade` (measured and pinned in + // `engine-cascade-delete.test.ts`), the opposite of what the declaration + // asks for. The default relocated off the property (`.optional()` + + // `.meta({default})` + `.overwrite()`, the #7918 Option A shape) so this + // check can tell authored from defaulted. Second ruling (2026-08-24, + // idempotent materialization): the `.overwrite()` never materializes a + // default the schema itself would refuse as authored — a bare + // `master_detail` parses to output that OMITS `deleteBehavior`, so + // `parse(parse(x))` holds on the mainline `create()` → `defineStack` + // path; every OTHER type keeps byte-identity with the `.default()` era, + // which is what the rest of this block pins. + describe('[#9689] deleteBehavior: set_null on master_detail is a parse-time rejection', () => { + const md = (extra: Record = {}) => ({ + name: 'parent_id', + label: 'Parent Record', + type: 'master_detail', + reference: 'parent_object', + ...extra, + }); + + it('rejects an AUTHORED set_null on a master_detail, at path deleteBehavior, with the named message', () => { + const result = FieldSchema.safeParse(md({ deleteBehavior: 'set_null' })); + expect(result.success).toBe(false); + if (result.success) return; + const issue = result.error.issues.find((i) => i.path.join('.') === 'deleteBehavior'); + expect(issue).toBeDefined(); + // The message head is contract: it must say the declaration is NOT + // honored and that the children would be DELETED, so an AI author + // reading the rejection learns the actual outcome, not just "invalid". + expect(issue?.message).toContain("`deleteBehavior: 'set_null'` is not honored on a `master_detail` field"); + expect(issue?.message).toContain('DELETED'); + // And it must name both legal ways out. + expect(issue?.message).toContain("'restrict'"); + expect(issue?.message).toContain("'cascade'"); + }); + + it('still parses a BARE master_detail — and no longer materializes set_null (2026-08-24 ruling: idempotent materialization)', () => { + // The schema refuses an AUTHORED `set_null` on this type, and the + // materialized spelling is indistinguishable from the authored one BY + // DESIGN — so baking it made parse output self-rejecting on re-parse + // (measured: 4 bare `master_detail` fields red the showcase build via + // `create()` → `defineStack`). Absent is what the output must say; the + // engine treats absent and `set_null` identically on this type (both + // cascade — `engine-cascade-delete.test.ts`). + const result = FieldSchema.parse(md()); + expect(result.deleteBehavior).toBeUndefined(); + expect('deleteBehavior' in result).toBe(false); + }); + + it('parse is IDEMPOTENT on a bare master_detail — the mainline create() → defineStack chain re-parses parse output', () => { + // This is the exact chain that carried the pre-ruling defect: parse #1 + // baked `set_null`, parse #2 rejected it. Both layers pinned green. + const once = FieldSchema.parse(md()); + expect(FieldSchema.safeParse(once).success).toBe(true); + const obj = ObjectSchema.create({ + name: 'child_thing', + label: 'Child Thing', + fields: { parent: { label: 'Parent', type: 'master_detail', reference: 'parent_thing' } }, + }); + expect(ObjectSchema.safeParse(obj).success).toBe(true); + }); + + it('materializes the default at its SHAPE position on every other type, not appended at the tail (byte-identity with the .default() era)', () => { + // Serialized parse output is what built app artifacts ship (#4447), so + // key ORDER is part of the byte-identity contract for the types that + // still materialize the default. `deleteBehavior` sits between + // `reference` and `hidden` in the shape; a naive + // `{ ...field, deleteBehavior }` re-materialization would emit it last. + const json = JSON.stringify(FieldSchema.parse({ + name: 'account_id', label: 'Account', type: 'lookup', reference: 'account', + })); + expect(json).toContain('"reference":"account","deleteBehavior":"set_null","hidden":false'); + }); + + it('leaves an authored cascade and restrict on master_detail untouched', () => { + expect(FieldSchema.parse(md({ deleteBehavior: 'cascade' })).deleteBehavior).toBe('cascade'); + expect(FieldSchema.parse(md({ deleteBehavior: 'restrict' })).deleteBehavior).toBe('restrict'); + }); + + it('keeps an authored set_null legal on lookup — required or not', () => { + const lookup = { + name: 'account_id', label: 'Account', type: 'lookup', + reference: 'account', deleteBehavior: 'set_null', + }; + expect(FieldSchema.parse(lookup).deleteBehavior).toBe('set_null'); + expect(FieldSchema.parse({ ...lookup, required: true }).deleteBehavior).toBe('set_null'); + }); + + it('keeps non-reference types accepting and defaulting the key (installed-base artifact shape, #4447)', () => { + // Verbatim shape from examples/app-showcase/dist/objectstack.json — a + // materialized datetime carrying only FieldSchema defaults. Built + // artifacts ship this on EVERY field type; it must stay legal. + const showcaseVerbatim = { + label: 'Created At', type: 'datetime', required: false, + searchable: false, multiple: false, unique: false, + deleteBehavior: 'set_null', hidden: false, + readonly: false, sortable: true, externalId: false, + }; + expect(() => FieldSchema.parse(showcaseVerbatim)).not.toThrow(); + // And a bare text field still gets the materialized default. + expect(FieldSchema.parse({ name: 'title', label: 'Title', type: 'text' }).deleteBehavior).toBe('set_null'); + }); + + it('keeps FieldSchema.shape enumerable (no pipe degradation from the relocation)', () => { + // `.overwrite()` rather than `.transform()` is load-bearing: a pipe + // answers shape introspection with an empty set, and form generators + // enumerate this shape. + expect(Object.keys(FieldSchema.shape).length).toBeGreaterThan(50); + expect(Object.keys(FieldSchema.shape)).toContain('deleteBehavior'); + }); + }); + it('should accept the relatedList prominence tri-state (false | true | primary)', () => { for (const relatedList of [false, true, 'primary'] as const) { const field: Field = { diff --git a/packages/spec/src/data/field.zod.ts b/packages/spec/src/data/field.zod.ts index e2fef5bf26..505149e183 100644 --- a/packages/spec/src/data/field.zod.ts +++ b/packages/spec/src/data/field.zod.ts @@ -685,7 +685,8 @@ export const InlineGridColumnSchema = lazySchema(() => strictObject({ requiredWhen: ExpressionInputSchema.optional().describe('Predicate (CEL) — the cell is required when TRUE. Same `record` + `parent` scope as `readonlyWhen`.'), })); -export const FieldSchema = lazySchema(() => strictObject({ +export const FieldSchema = lazySchema(() => { + const base = strictObject({ surface: 'this field', history: FIELD_HISTORY, aliases: { @@ -955,7 +956,28 @@ export const FieldSchema = lazySchema(() => strictObject({ // `referenceFilters` (string[]) removed in the 16.x line (#2377, ADR-0049): // the lookup picker reads the structured `lookupFilters` ({field,operator,value}), // never this string[] form — as authored it filtered nothing. Use `lookupFilters`. - deleteBehavior: z.enum(['set_null', 'cascade', 'restrict']).optional().default('set_null').describe('What happens if referenced record is deleted'), + /** + * #9689 (maintainer ruling 2026-08-19, Q1 = A — the #7918 Option A shape) — + * `.default('set_null')` moved off this property and into the `.overwrite()` + * below, and this placement is load-bearing. A property-level default + * materializes AT PARSE, so a refinement over the parsed object cannot tell + * an authored `deleteBehavior: 'set_null'` from an untouched one — measured: + * a bare `master_detail` and one explicitly declaring `set_null` parsed to + * BYTE-IDENTICAL output, so a naive per-type refinement would refuse every + * bare `master_detail` ever parsed (the permanently-noisy shape the #7918 + * ruling forbids). Declared `.optional()`, the authored-vs-absent + * distinction survives to the `.superRefine` below — where an AUTHORED + * `set_null` on a `master_detail` is a named parse-time rejection — and the + * `.overwrite` then materializes the same `'set_null'` AFTER the check, at + * its shape position, so parse OUTPUT is byte-identical to the + * `.default('set_null')` era. The `default` annotation states the contract + * default to schema consumers without touching parse order — the + * `autonumberFormat` pattern below. + */ + deleteBehavior: z.enum(['set_null', 'cascade', 'restrict']).optional().meta({ + description: 'What happens if referenced record is deleted', + default: 'set_null', + }), /** * Master-detail INLINE EDITING. On a child's `master_detail`/`lookup` field * (whose `reference` is the parent object), declare that "this child is @@ -1464,7 +1486,13 @@ export const FieldSchema = lazySchema(() => strictObject({ // (see `metadata-type-schemas.test.ts` for how the other 24 took an early // return) — so it has been a known gap longer than any of its siblings. ...MetadataProtectionFields, -}).superRefine((field, ctx) => { + }); + // #9689 — the shape's declaration order, captured so the `.overwrite()` below + // can re-insert the materialized `deleteBehavior` at its shape POSITION. + // Zod builds parse output in shape order; a plain spread would append the + // key at the tail and break the byte-identity contract above. + const shapeOrder = Object.keys(base.shape); + return base.superRefine((field, ctx) => { // [#11339] `referenceVia` declares the id half of a polymorphic pointer // pair (ADR-0052 §5) — semantics only a plain `text` column carries. On a // relationship type it contradicts the type's own single static target, and @@ -1563,6 +1591,32 @@ export const FieldSchema = lazySchema(() => strictObject({ } } + // #9689 (maintainer ruling 2026-08-19, Q1 = A): an AUTHORED + // `deleteBehavior: 'set_null'` on a `master_detail` is a publish-time error. + // The engine resolves every value except `restrict` on this type to + // `cascade` (`cascadeDeleteRelations` — measured and pinned in + // `engine-cascade-delete.test.ts`), so this declaration asks for the child + // rows to be KEPT and gets them DELETED — data loss relative to the declared + // intent, silently, at the moment the parent goes away. Honoring it is ruled + // out (a detail row whose master reference is nulled becomes an unreachable + // orphan — the outcome #8772/#9138 exist to prevent). `field.deleteBehavior` + // here is pre-`.overwrite`, so `undefined` means "not authored" — a bare + // `master_detail` (the overwhelmingly common spelling) never fires this. + if (field.type === 'master_detail' && field.deleteBehavior === 'set_null') { + ctx.addIssue({ + code: 'custom', + path: ['deleteBehavior'], + message: + "`deleteBehavior: 'set_null'` is not honored on a `master_detail` field: a detail row " + + 'cannot outlive its master (a nulled master reference would orphan it), so the engine ' + + "resolves every value except 'restrict' to 'cascade' — the children this declaration " + + "asks to keep would be DELETED. Declare 'restrict' to refuse deleting a master that " + + "still has details (no data loss — the closest reading of \"keep my children\"), " + + "declare 'cascade' (or omit the key) to accept the cascade deliberately, or use a " + + '`lookup` field if the children must survive the parent.', + }); + } + // #7127: an authored `defaultValue` must be one of the key's three legal // shapes — CEL envelope / runtime token / literal — and legal for THIS // field. The shapes are told apart FIRST (`default-value-shape.ts`, the @@ -1616,7 +1670,51 @@ export const FieldSchema = lazySchema(() => strictObject({ + `({ dialect: 'cel', source: '…' }).${suggestionText}`, }); } -})); + }).overwrite((field) => { + // #9689 — the relocated `.default('set_null')`, applied AFTER the checks + // above. `.overwrite()` rather than `.transform()` per the measured #6926 + // precedent (`CurrencyConfigSchema` in this file is the sibling): it keeps + // this schema a `ZodObject` (a pipe has no `.extend` and answers shape + // introspection with an empty set), and checks run in attachment order, so + // the superRefine above always sees the pre-materialized value. The key is + // re-inserted at its SHAPE position (Zod emits parse output in shape + // order), so output is byte-identical to the `.default('set_null')` era on + // every field type EXCEPT `master_detail` — see the ruling below. The one + // accepted cost, same as the currency precedent's: the INFERRED output + // type now declares `deleteBehavior?` even though a parsed non- + // `master_detail` field always carries it (ADR-0122 forbids hand-narrowing + // the inferred type); the runtime contract is the enforced one. + if (field.deleteBehavior !== undefined) return field; + // #9689 (maintainer ruling 2026-08-24, idempotent materialization — + // 「四维分析一致的，接手你的建议。」): NEVER materialize a default the + // schema itself would refuse as authored. The superRefine above rejects an + // AUTHORED `set_null` on a `master_detail`, and the two spellings are + // indistinguishable to any later parse BY DESIGN — so baking `set_null` + // onto a bare `master_detail` made parse output self-rejecting on + // re-parse, and `ObjectSchema.create()` → `defineStack` re-parses on the + // MAINLINE app-build path (measured: 4 bare `master_detail` fields red the + // showcase build; `parse(parse(x))` threw for accepted x). A bare + // `master_detail` therefore parses to output that OMITS `deleteBehavior`: + // the engine treats absent exactly as it treated the baked `set_null` + // (both resolve to `cascade` — measured in the #9689 exhaustion matrix, + // pinned in `engine-cascade-delete.test.ts`), and built artifacts stop + // carrying a value the schema itself refuses. Every other type keeps + // byte-identity, and the #7918 currency `precision` twin of this landmine + // is #11423 — same principle, its own card. + if (field.type === 'master_detail') return field; + const withDefault: Record = { ...field, deleteBehavior: 'set_null' }; + const out: Record = {}; + for (const key of shapeOrder) { + if (key in withDefault) out[key] = withDefault[key]; + } + // A strict object emits no unknown keys; this tail loop is belt-and-braces + // so a future passthrough key could never be silently dropped here. + for (const key of Object.keys(withDefault)) { + if (!(key in out)) out[key] = withDefault[key]; + } + return out as typeof field; + }); +}); /** * Author-facing shape of a field — what `FieldSchema.parse(...)` accepts. Since diff --git a/packages/spec/src/migrations/entries/semantic/18.field-master-detail-set-null-refused.ts b/packages/spec/src/migrations/entries/semantic/18.field-master-detail-set-null-refused.ts new file mode 100644 index 0000000000..3f00c53282 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.field-master-detail-set-null-refused.ts @@ -0,0 +1,37 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +export const entry: SemanticMigration = { + id: 'field-master-detail-set-null-refused', + surface: "object field `deleteBehavior: 'set_null'` authored on a `master_detail` field", + replacement: "an explicit `deleteBehavior: 'restrict'` or `'cascade'` (or no declaration, " + + 'which is the cascade default) — re-declared deliberately, because only the author knows ' + + "which they meant. There is deliberately NO automatic conversion: `'set_null'` here asked " + + 'for the child rows to be KEPT, and both mechanical rewrites betray that intent in a ' + + "different direction — stripping the key silently ratifies the cascade the author did not " + + "ask for (the same collapse of intent that produced the defect), while `'restrict'` is the " + + 'only rewrite that cannot lose data (the parent delete is refused while children exist — ' + + 'the closest honest reading of "keep my children") but turns a delete that silently ' + + 'succeeded into a loud refusal. If the children genuinely must survive the parent, the ' + + 'field wants to be a `lookup`, not a `master_detail`', + reason: + "`FieldSchema` accepted `deleteBehavior: 'set_null'` on a `master_detail` while the engine's " + + '`cascadeDeleteRelations` resolves every value except `restrict` on that type to `cascade` ' + + '— so the declaration asked for the children to be kept and the engine DELETED them, ' + + 'silently, at the moment the parent went away: data loss relative to the declared intent, ' + + 'the ADR-0049 declared-but-unenforced shape on a delete path. Honoring the value is ruled ' + + 'out (maintainer, 2026-08-19): a detail row whose master reference is nulled becomes an ' + + 'unreachable orphan, which is precisely what the orphan-detail work exists to prevent. The ' + + 'schema now refuses the authored combination at parse time (declared = enforced), and the ' + + 'engine logs loudly if a raw registration or a pre-tightening stored row still carries it ' + + 'to the coercion site. A BARE `master_detail` is untouched: the default still materializes ' + + "as `'set_null'` in parse output (byte-identical to before) and still resolves to cascade.", + acceptanceCriteria: + "No `master_detail` field declares `deleteBehavior: 'set_null'`. Bare `master_detail` " + + 'declarations, authored `cascade`/`restrict`, and `set_null` on `lookup` parse ' + + 'byte-identically to before. Stored `sys_metadata` rows carrying the refused combination ' + + 'keep loading and serving (registry validation is a diagnostic, not a gate) but flag ' + + '`metadata_spec_invalid` and are refused on their next authoring-path save — re-declare ' + + 'the field deliberately when that happens.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 3daee72644..0bbaea7309 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -5756,6 +5756,39 @@ const step18: MigrationStep = { + '(`{"address.city": …}`) needs NO action — it is deliberately not judged. Reads complete ' + 'with no `INVALID_FIELD` naming a dotted filter key, at either door.', }, + { + id: 'field-master-detail-set-null-refused', + surface: "object field `deleteBehavior: 'set_null'` authored on a `master_detail` field", + replacement: "an explicit `deleteBehavior: 'restrict'` or `'cascade'` (or no declaration, " + + 'which is the cascade default) — re-declared deliberately, because only the author knows ' + + "which they meant. There is deliberately NO automatic conversion: `'set_null'` here asked " + + 'for the child rows to be KEPT, and both mechanical rewrites betray that intent in a ' + + "different direction — stripping the key silently ratifies the cascade the author did not " + + "ask for (the same collapse of intent that produced the defect), while `'restrict'` is the " + + 'only rewrite that cannot lose data (the parent delete is refused while children exist — ' + + 'the closest honest reading of "keep my children") but turns a delete that silently ' + + 'succeeded into a loud refusal. If the children genuinely must survive the parent, the ' + + 'field wants to be a `lookup`, not a `master_detail`', + reason: + "`FieldSchema` accepted `deleteBehavior: 'set_null'` on a `master_detail` while the engine's " + + '`cascadeDeleteRelations` resolves every value except `restrict` on that type to `cascade` ' + + '— so the declaration asked for the children to be kept and the engine DELETED them, ' + + 'silently, at the moment the parent went away: data loss relative to the declared intent, ' + + 'the ADR-0049 declared-but-unenforced shape on a delete path. Honoring the value is ruled ' + + 'out (maintainer, 2026-08-19): a detail row whose master reference is nulled becomes an ' + + 'unreachable orphan, which is precisely what the orphan-detail work exists to prevent. The ' + + 'schema now refuses the authored combination at parse time (declared = enforced), and the ' + + 'engine logs loudly if a raw registration or a pre-tightening stored row still carries it ' + + 'to the coercion site. A BARE `master_detail` is untouched: the default still materializes ' + + "as `'set_null'` in parse output (byte-identical to before) and still resolves to cascade.", + acceptanceCriteria: + "No `master_detail` field declares `deleteBehavior: 'set_null'`. Bare `master_detail` " + + 'declarations, authored `cascade`/`restrict`, and `set_null` on `lookup` parse ' + + 'byte-identically to before. Stored `sys_metadata` rows carrying the refused combination ' + + 'keep loading and serving (registry validation is a diagnostic, not a gate) but flag ' + + '`metadata_spec_invalid` and are refused on their next authoring-path save — re-declare ' + + 'the field deliberately when that happens.', + }, { id: 'field-scale-precision-integer-refused', surface: 'object field `scale` / `precision` declarations (`Field.number` and friends) — '