From 2030d882acd9c3023529604fefee55686ca111c1 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 13:02:48 +0000 Subject: [PATCH 1/2] fix(objectql): the referential delete walk honours a tree field's deleteBehavior One family predicate (statesDeleteSemantics: master_detail, lookup, tree) shared by cascadeRelationBehavior, the collection and dependents walks that read it, and planCascadeAtomicity. Claude-Session: https://claude.ai/code/session_01ADzJtzYTLUfgrRZHxkagkX Co-authored-by: Claude --- .../src/engine-cascade-delete-tree.test.ts | 381 ++++++++++++++++++ packages/objectql/src/engine.ts | 72 +++- 2 files changed, 442 insertions(+), 11 deletions(-) create mode 100644 packages/objectql/src/engine-cascade-delete-tree.test.ts diff --git a/packages/objectql/src/engine-cascade-delete-tree.test.ts b/packages/objectql/src/engine-cascade-delete-tree.test.ts new file mode 100644 index 00000000000..2a88dd422e6 --- /dev/null +++ b/packages/objectql/src/engine-cascade-delete-tree.test.ts @@ -0,0 +1,381 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#22957] A `tree` field's `deleteBehavior` is what a delete of the record it + * references does: `set_null` (the default the spec materializes) clears the + * child's parent, `cascade` deletes the subtree, `restrict` refuses the delete. + * + * ## The defect + * + * `FieldSchema` materializes `deleteBehavior: 'set_null'` on a `tree` beside + * `lookup`, because the type "is in the relational family, where the key + * states delete semantics". The referential delete walk admitted + * `master_detail` and `lookup` only, so it never read a `tree` relation: + * deleting a parent answered success and left every child holding the + * deleted id, and an authored `cascade` or `restrict` was inert the same way. + * Measured at the public doors on `examples/app-showcase` + * (`showcase_field_zoo.f_tree`): `os verify --delete-behaviors` reported it + * contradicted through REST and the system door. + * + * ## What is pinned here + * + * - `set_null`, materialized by the spec's own parse and authored, clears the + * child's parent and keeps the child; + * - `cascade` deletes the whole subtree, past `MAX_CASCADE_DEPTH`, and + * nothing outside it; + * - `restrict` refuses with the ADR-0112 envelope naming the tree's own object + * as the dependent, and keeps the row; a leaf still deletes; + * - a cycle in the data terminates: a corrupt row pointing at its own + * descendant, and a row pointing at itself; + * - a subtree cascade is one unit of work: a refusal from a node deep in it + * rolls back the nodes already deleted (`planCascadeAtomicity` reads the + * same family); + * - `multiple: true` is refused on a `tree` by the parse, and a raw + * registration around it is read single-valued, as the walk's multi-value + * arbiter (`isMultiValueField`) reads it for every type; + * - CONTROLS: `lookup` (default `set_null`, `cascade`, `restrict`) and + * `master_detail` (default `cascade`, `restrict`) answer exactly as before. + * + * The driver has REAL snapshot rollback (the `engine-cascade-delete-atomic` + * shape): a stub whose rollback is a no-op cannot tell "refused and rolled + * back" from "refused halfway", so the atomicity pin would pass vacuously. + */ + +import { describe, it, expect } from 'vitest'; +import { FieldSchema } from '@objectstack/spec/data'; +import { ObjectQL } from './engine.js'; + +type Row = Record; + +function makeDriver() { + const stores = new Map>(); + const storeFor = (o: string): Map => { + let s = stores.get(o); + if (!s) { s = new Map(); stores.set(o, s); } + return s; + }; + const committed: unknown[] = []; + const rolledBack: unknown[] = []; + const snapshots = new Map>>(); + let nextId = 0; + const matches = (row: Row, where: unknown): boolean => { + if (!where || typeof where !== 'object') return true; + for (const [k, v] of Object.entries(where as Row)) { + if (k.startsWith('$')) continue; + const exp = v && typeof v === 'object' && '$eq' in (v as Row) ? (v as Row).$eq : v; + if ((row[k] ?? null) !== (exp ?? null)) return false; + } + return true; + }; + const driver: any = { + name: 'primary', version: '0.0.0', supports: {}, + committed, rolledBack, + rowsOf: (o: string): Row[] => Array.from(storeFor(o).values()), + rowOf: (o: string, id: unknown): Row | undefined => storeFor(o).get(String(id)), + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async syncSchema() {}, + async find(o: string, ast: { where?: unknown; limit?: unknown } | undefined) { + const rows = Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)); + return typeof ast?.limit === 'number' ? rows.slice(0, ast.limit) : rows; + }, + async findOne(o: string, ast: { where?: unknown } | undefined) { + for (const r of storeFor(o).values()) if (matches(r, ast?.where)) return r; + return null; + }, + async create(o: string, data: Row) { + nextId += 1; + const id = (data.id as string | undefined) ?? `r_${nextId}`; + const row = { ...data, id }; + storeFor(o).set(id, row); + return row; + }, + async update(o: string, id: string, data: Row) { + const s = storeFor(o); + const cur = s.get(String(id)); + if (!cur) throw new Error(`not found ${o}/${id}`); + const up = { ...cur, ...data, id }; + s.set(String(id), up); + return up; + }, + async upsert(o: string, data: Row) { + const id = data.id as string | undefined; + return id && storeFor(o).has(id) ? this.update(o, id, data) : this.create(o, data); + }, + async delete(o: string, id: string) { return storeFor(o).delete(String(id)); }, + async count(o: string, ast: { where?: unknown } | undefined) { return (await this.find(o, ast)).length; }, + async bulkCreate(o: string, rows: Row[]) { return Promise.all(rows.map((r) => this.create(o, r))); }, + async bulkUpdate() { return []; }, + async bulkDelete() {}, + async beginTransaction() { + const handle = { __trx: snapshots.size + committed.length + rolledBack.length + 1 }; + const snap = new Map>(); + for (const [o, s] of stores) snap.set(o, new Map(Array.from(s, ([k, v]) => [k, { ...v }]))); + snapshots.set(handle, snap); + return handle; + }, + async commit(handle: unknown) { snapshots.delete(handle); committed.push(handle); }, + async rollback(handle: unknown) { + const snap = snapshots.get(handle); + if (snap) { stores.clear(); for (const [o, s] of snap) stores.set(o, s); } + snapshots.delete(handle); + rolledBack.push(handle); + }, + }; + return driver; +} + +async function makeEngine(objects: unknown[]) { + const quiet = () => {}; + const engine = new ObjectQL({ logger: { debug: quiet, info: quiet, warn: quiet, error: quiet } } as any); + const driver = makeDriver(); + engine.registerDriver(driver, true); + await engine.init(); + for (const o of objects) engine.registry.registerObject(o as any, '__test__'); + return { engine, driver }; +} + +const id = { name: 'id', type: 'text' as const, primaryKey: true }; +const name = { name: 'name', type: 'text' as const }; + +/** An object whose hierarchy is the `tree` field `parent`, declared as `parentDef`. */ +const treeObject = (objectName: string, parentDef: Record) => ({ + name: objectName, + label: objectName, + fields: { id, name, parent: { name: 'parent', ...parentDef } }, +}); + +/** The tree field as the spec's parse produces it, which is what a built artifact carries. */ +const parsedTree = (reference: string, extra: Record = {}) => + FieldSchema.parse({ type: 'tree', reference, ...extra }) as Record; + +const namesOf = (rows: Row[]) => rows.map((r) => r.name).sort(); + +async function refusalOf(promise: Promise): Promise { + try { await promise; } catch (err) { return err; } + throw new Error('expected the delete to be refused'); +} + +describe('[#22957] a tree field\'s deleteBehavior takes effect on a parent delete', () => { + describe('set_null — the materialized default and the authored value', () => { + it('the spec\'s parse materializes set_null on a tree that authors nothing', () => { + // The premise the engine pins below read: the value a built artifact carries. + expect(parsedTree('zt_node').deleteBehavior).toBe('set_null'); + }); + + it.each([ + ['materialized by the parse', parsedTree('zt_node')], + ['authored', { type: 'tree', reference: 'zt_node', deleteBehavior: 'set_null' }], + ['absent (a raw registration around the parse)', { type: 'tree', reference: 'zt_node' }], + ])('%s: deleting the parent clears the child\'s parent and keeps the child', async (_label, def) => { + const { engine, driver } = await makeEngine([treeObject('zt_node', def)]); + const p = await engine.insert('zt_node', { name: 'P' }); + const c = await engine.insert('zt_node', { name: 'C', parent: p.id }); + const bystander = await engine.insert('zt_node', { name: 'B', parent: c.id }); + + await engine.delete('zt_node', { where: { id: p.id } } as any); + + expect(driver.rowOf('zt_node', p.id)).toBeUndefined(); + expect(driver.rowOf('zt_node', c.id)).toMatchObject({ name: 'C', parent: null }); + // The other direction: a row that never referenced the deleted record is untouched. + expect(driver.rowOf('zt_node', bystander.id)).toMatchObject({ name: 'B', parent: c.id }); + }); + }); + + describe('cascade — a subtree delete', () => { + const node = treeObject('zt_branch', { type: 'tree', reference: 'zt_branch', deleteBehavior: 'cascade' }); + + it('deletes the node and every descendant, and nothing outside the subtree', async () => { + const { engine, driver } = await makeEngine([node]); + const root = await engine.insert('zt_branch', { name: 'root' }); + const a = await engine.insert('zt_branch', { name: 'a', parent: root.id }); + const b = await engine.insert('zt_branch', { name: 'b', parent: a.id }); + await engine.insert('zt_branch', { name: 'c', parent: b.id }); + await engine.insert('zt_branch', { name: 'b2', parent: a.id }); + await engine.insert('zt_branch', { name: 'a2', parent: root.id }); + const other = await engine.insert('zt_branch', { name: 'other' }); + await engine.insert('zt_branch', { name: 'other_kid', parent: other.id }); + + await engine.delete('zt_branch', { where: { id: a.id } } as any); + + expect(namesOf(driver.rowsOf('zt_branch'))).toEqual(['a2', 'other', 'other_kid', 'root']); + // One unit of work: the hierarchy makes the object reference itself. + expect(driver.committed).toHaveLength(1); + expect(driver.rolledBack).toHaveLength(0); + }); + + it('reaches past MAX_CASCADE_DEPTH (10): a 15-level chain is deleted whole', async () => { + // The cap bounds `planCascadeAtomicity`'s schema walk. The row cascade + // is bounded by its set, so a deep subtree is never cut off part-way, + // which would strand nodes whose parent is gone. + const { engine, driver } = await makeEngine([node]); + const top = await engine.insert('zt_branch', { name: 'L0' }); + let prev = top; + for (let i = 1; i < 15; i++) prev = await engine.insert('zt_branch', { name: `L${i}`, parent: prev.id }); + const outside = await engine.insert('zt_branch', { name: 'outside' }); + + await engine.delete('zt_branch', { where: { id: top.id } } as any); + + expect(namesOf(driver.rowsOf('zt_branch'))).toEqual(['outside']); + expect(driver.rowOf('zt_branch', outside.id)).toBeDefined(); + }); + + it('terminates on a corrupt cycle: a row pointing at its own descendant', async () => { + const { engine, driver } = await makeEngine([node]); + const r = await engine.insert('zt_branch', { name: 'R' }); + const k = await engine.insert('zt_branch', { name: 'K', parent: r.id }); + const g = await engine.insert('zt_branch', { name: 'G', parent: k.id }); + const outside = await engine.insert('zt_branch', { name: 'outside' }); + // Written at the driver, the way corrupt data arrives: R's parent is its grandchild. + await driver.update('zt_branch', r.id, { parent: g.id }); + + await engine.delete('zt_branch', { where: { id: k.id } } as any); + + // K's subtree is G, whose subtree is R, whose subtree is K: each once. + expect(namesOf(driver.rowsOf('zt_branch'))).toEqual(['outside']); + expect(driver.rowOf('zt_branch', outside.id)).toBeDefined(); + }); + + it('terminates on a row that is its own parent', async () => { + const { engine, driver } = await makeEngine([node]); + const s = await engine.insert('zt_branch', { name: 'S' }); + const kid = await engine.insert('zt_branch', { name: 'kid', parent: s.id }); + await driver.update('zt_branch', s.id, { parent: s.id }); + + await engine.delete('zt_branch', { where: { id: s.id } } as any); + + expect(driver.rowOf('zt_branch', s.id)).toBeUndefined(); + expect(driver.rowOf('zt_branch', kid.id)).toBeUndefined(); + }); + + it('is one unit of work: a refusal deep in the subtree rolls back the nodes already deleted', async () => { + const { engine, driver } = await makeEngine([node]); + const root = await engine.insert('zt_branch', { name: 'root' }); + await engine.insert('zt_branch', { name: 'k1', parent: root.id }); + const k2 = await engine.insert('zt_branch', { name: 'k2', parent: root.id }); + const vetoed = await engine.insert('zt_branch', { name: 'vetoed', parent: k2.id }); + engine.registerHook( + 'beforeDelete', + async (ctx: any) => { + if (String(ctx.input?.id) === String(vetoed.id)) { + throw Object.assign(new Error('this node is held'), { status: 409 }); + } + }, + { object: 'zt_branch' }, + ); + + const err = await refusalOf(engine.delete('zt_branch', { where: { id: root.id } } as any)); + + expect(err).toMatchObject({ status: 409 }); + // k1 was cascaded away before the hook on `vetoed` refused: it is back. + expect(namesOf(driver.rowsOf('zt_branch'))).toEqual(['k1', 'k2', 'root', 'vetoed']); + expect(driver.rolledBack).toHaveLength(1); + expect(driver.committed).toHaveLength(0); + }); + }); + + describe('restrict', () => { + const node = treeObject('zt_held', { type: 'tree', reference: 'zt_held', deleteBehavior: 'restrict' }); + + it('refuses a parent delete while a child references it, and keeps both rows', async () => { + const { engine, driver } = await makeEngine([node]); + const p = await engine.insert('zt_held', { name: 'P' }); + const c = await engine.insert('zt_held', { name: 'C', parent: p.id }); + + const err = await refusalOf(engine.delete('zt_held', { where: { id: p.id } } as any)); + + expect(err).toMatchObject({ code: 'DELETE_RESTRICTED', status: 409, dependentObject: 'zt_held' }); + expect(driver.rowOf('zt_held', p.id)).toMatchObject({ name: 'P' }); + expect(driver.rowOf('zt_held', c.id)).toMatchObject({ name: 'C', parent: p.id }); + }); + + it('lets a leaf go: nothing references it', async () => { + const { engine, driver } = await makeEngine([node]); + const p = await engine.insert('zt_held', { name: 'P' }); + const c = await engine.insert('zt_held', { name: 'C', parent: p.id }); + + await engine.delete('zt_held', { where: { id: c.id } } as any); + + expect(driver.rowOf('zt_held', c.id)).toBeUndefined(); + expect(driver.rowOf('zt_held', p.id)).toMatchObject({ name: 'P' }); + }); + }); + + describe('multiple: true', () => { + it('is refused on a tree by the spec\'s parse, so no multi-valued tree reaches the walk through it', () => { + const result = FieldSchema.safeParse({ type: 'tree', reference: 'zt_multi', multiple: true }); + expect(result.success).toBe(false); + expect(result.error?.issues.some((i) => i.path.join('.') === 'multiple')).toBe(true); + }); + + it('a raw registration around the parse is read single-valued, as the walk reads every type', async () => { + // `isMultiValueField` is the walk's one multi-value arbiter, and a + // `tree` is not a multi-capable type: the probe asks scalar equality + // and `set_null` writes `null`, never a member-removed array. + const { engine, driver } = await makeEngine([ + treeObject('zt_multi', { type: 'tree', reference: 'zt_multi', multiple: true }), + ]); + const p = await engine.insert('zt_multi', { name: 'P' }); + const c = await engine.insert('zt_multi', { name: 'C', parent: p.id }); + + await engine.delete('zt_multi', { where: { id: p.id } } as any); + + expect(driver.rowOf('zt_multi', p.id)).toBeUndefined(); + expect(driver.rowOf('zt_multi', c.id)).toMatchObject({ name: 'C', parent: null }); + }); + }); + + describe('CONTROLS — lookup and master_detail are unchanged', () => { + const account = { name: 'zc_account', label: 'Account', fields: { id, name } }; + const ref = (fieldName: string, def: Record) => ({ [fieldName]: { name: fieldName, ...def } }); + + it('lookup: the default clears, cascade deletes; master_detail: the default cascades', async () => { + const { engine, driver } = await makeEngine([ + account, + { name: 'zc_cleared', label: 'Cleared', fields: { id, name, ...ref('account', FieldSchema.parse({ type: 'lookup', reference: 'zc_account' }) as Row) } }, + { name: 'zc_cascaded', label: 'Cascaded', fields: { id, name, ...ref('account', { type: 'lookup', reference: 'zc_account', deleteBehavior: 'cascade' }) } }, + { name: 'zc_detail', label: 'Detail', fields: { id, name, ...ref('account', { type: 'master_detail', reference: 'zc_account' }) } }, + ]); + const a = await engine.insert('zc_account', { name: 'A' }); + const cleared = await engine.insert('zc_cleared', { name: 'n', account: a.id }); + await engine.insert('zc_cascaded', { name: 'c', account: a.id }); + await engine.insert('zc_detail', { name: 'd', account: a.id }); + + await engine.delete('zc_account', { where: { id: a.id } } as any); + + expect(driver.rowsOf('zc_account')).toHaveLength(0); + expect(driver.rowOf('zc_cleared', cleared.id)).toMatchObject({ name: 'n', account: null }); + expect(driver.rowsOf('zc_cascaded')).toHaveLength(0); + expect(driver.rowsOf('zc_detail')).toHaveLength(0); + }); + + it.each([ + ['lookup', 'zc_lookup_held'], + ['master_detail', 'zc_detail_held'], + ])('%s + restrict still refuses with the envelope and keeps the row', async (type, holder) => { + const { engine, driver } = await makeEngine([ + account, + { name: holder, label: holder, fields: { id, name, ...ref('account', { type, reference: 'zc_account', deleteBehavior: 'restrict' }) } }, + ]); + const a = await engine.insert('zc_account', { name: 'A' }); + await engine.insert(holder, { name: 'h', account: a.id }); + + const err = await refusalOf(engine.delete('zc_account', { where: { id: a.id } } as any)); + + expect(err).toMatchObject({ code: 'DELETE_RESTRICTED', status: 409, dependentObject: holder }); + expect(driver.rowOf('zc_account', a.id)).toMatchObject({ name: 'A' }); + }); + + it('an object nothing references opens no transaction for its delete', async () => { + // The `'none'` verdict of `planCascadeAtomicity`, unchanged: the plain + // delete is the CONTROL the atomicity pin above is read against. + const { engine, driver } = await makeEngine([account]); + const a = await engine.insert('zc_account', { name: 'A' }); + + await engine.delete('zc_account', { where: { id: a.id } } as any); + + expect(driver.rowsOf('zc_account')).toHaveLength(0); + expect(driver.committed).toHaveLength(0); + expect(driver.rolledBack).toHaveLength(0); + }); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index fb3321056ac..3371784e6b9 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -1592,6 +1592,39 @@ function declaredMultiValued(field: { type?: unknown; multiple?: unknown } | nul }); } +/** + * [#22957] The reference types whose `deleteBehavior` states DELETE SEMANTICS: + * the relational family the referential delete walk follows. + * + * `tree` is a member because the spec says so. `FieldSchema`'s `.overwrite` + * (`packages/spec/src/data/field.zod.ts`) materializes `deleteBehavior: + * 'set_null'` on a `tree` beside `lookup`, because the type "is in the + * relational family, where the key states delete semantics". Its reference is + * self-referential by contract (`refuseForeignTreeReference`, `object.zod.ts`), + * so its `cascade` is a subtree delete. The walk used to admit only + * `master_detail` and `lookup`, so every behaviour a `tree` declared, the + * materialized default included, was inert: a parent delete left its children + * holding the deleted id. + * + * ⛔ `user` stays out. It stores like a `lookup`, but the spec's parse puts it + * on the other side of the line and materializes no `deleteBehavior` on it, so + * no `user` field declares a delete outcome by default. Moving it is a change + * to the declaration, made in the spec, never here. + */ +const DELETE_SEMANTICS_REFERENCE_TYPES: ReadonlySet = new Set(['master_detail', 'lookup', 'tree']); + +/** + * [#22957] Does `fdef` belong to {@link DELETE_SEMANTICS_REFERENCE_TYPES}? This + * is the ONE family test every reader of the referential delete walk asks: + * {@link ObjectQL.cascadeRelationBehavior}, through it the collection walk and + * the dependents walk, and {@link ObjectQL.planCascadeAtomicity}. So the walk + * that runs and the verdict on whether it runs as one unit of work cannot + * disagree about which relations take part. ⛔ Never a type literal at a site. + */ +function statesDeleteSemantics(fdef: { type?: unknown } | null | undefined): boolean { + return typeof fdef?.type === 'string' && DELETE_SEMANTICS_REFERENCE_TYPES.has(fdef.type); +} + /** * Plan the formula pass of a read: which formula fields to evaluate, and — * when the caller named a projection — the projection the DRIVER is asked for. @@ -17179,8 +17212,9 @@ export class ObjectQL implements IObjectQLEngine { * Three verdicts, because "wrap it in a transaction" is only unconditionally * right in one of them: * - * - **`'none'`** — no registered object declares a `master_detail`/`lookup` - * pointing at `object`, so `cascadeDeleteRelations` cannot write anything + * - **`'none'`** — no registered object declares a relation of the delete + * family ({@link statesDeleteSemantics}: `master_detail` / `lookup` / + * `tree`) pointing at `object`, so `cascadeDeleteRelations` cannot write anything * and there is no multi-write unit to make atomic. The delete runs exactly * as it did before this card: one `driver.delete`, no transaction opened, * no degrade warning. A plain non-cascade delete is the CONTROL for this @@ -17230,10 +17264,17 @@ export class ObjectQL implements IObjectQLEngine { const objects: ServiceObject[] = this._registry.getAllObjects(); // Which objects reference `name` via a relation the cascade would follow. - // The `master_detail`/`lookup` + `reference` test is `cascadeDeleteRelations`'s - // own, so the two cannot disagree about who participates. `restrict` fields - // are INCLUDED deliberately: a restrict refusal is the card's core repro, - // and it must roll back the siblings already cascaded before it. + // The family test ({@link statesDeleteSemantics}) + `reference` test is + // `cascadeDeleteRelations`'s own, so the two cannot disagree about who + // participates. `restrict` fields are INCLUDED deliberately: a restrict + // refusal is the card's core repro, and it must roll back the siblings + // already cascaded before it. + // + // [#22957] A `tree` takes part since it joined the family. Its reference + // names the object that declares it, so an object whose only relation is + // its own hierarchy now plans `'atomic'` rather than `'none'`: a subtree + // cascade is several deletes, and a refusal from a node deep in it must + // roll back the nodes deleted before it. const referencing = (name: string): string[] => { const out: string[] = []; for (const child of objects) { @@ -17241,7 +17282,7 @@ export class ObjectQL implements IObjectQLEngine { const fields = (child as any)?.fields as Record | undefined; if (!childName || !fields) continue; for (const [fieldName, fdef] of Object.entries(fields)) { - if (!fdef || (fdef.type !== 'master_detail' && fdef.type !== 'lookup')) continue; + if (!statesDeleteSemantics(fdef)) continue; // [#18550] The carrier is read through the ONE arbiter, so a // `reference` no reader can read REFUSES here instead of reading as // "this child does not reference `name`". The two answers stay @@ -17662,8 +17703,9 @@ export class ObjectQL implements IObjectQLEngine { /** * Apply referential delete behavior for relations pointing AT this record, - * before it is removed. For every registered object with a `master_detail` - * or `lookup` field referencing `object`, honor the field's `deleteBehavior`: + * before it is removed. For every registered object with a `master_detail`, + * `lookup` or `tree` field ({@link statesDeleteSemantics}) referencing + * `object`, honor the field's `deleteBehavior`: * - `cascade` → delete the dependent rows (recursively, so grandchildren * are handled by each child's own delete), * - `set_null` → clear the foreign key. On a MULTI-VALUED field @@ -17676,7 +17718,7 @@ export class ObjectQL implements IObjectQLEngine { * doc block, #9447 maintainer ruling 2026-08-18), * - `restrict` → refuse the delete when dependents exist. * `master_detail` defaults to `cascade` (the parent owns the child - * lifecycle); `lookup` defaults to `set_null` — except a `set_null` default + * lifecycle); `lookup` and `tree` default to `set_null` — except a `set_null` default * on a REQUIRED lookup escalates to `restrict` (you can't null a NOT NULL * FK; restricting with a clear dependent-count message beats a misleading * " is required" 400 from the child) — and on a `multiple: true` @@ -17835,7 +17877,9 @@ export class ObjectQL implements IObjectQLEngine { fieldName: string, fdef: any, ): string | undefined { - if (!fdef || (fdef.type !== 'master_detail' && fdef.type !== 'lookup')) return undefined; + // [#22957] The family test is the shared predicate, so this reader, the + // two walks that read it and `planCascadeAtomicity` admit one family. + if (!statesDeleteSemantics(fdef)) return undefined; // [#18550] Same arbiter, same absence-vs-unreadability split as // {@link ObjectQL.planCascadeAtomicity} states above — and this is the // seam where the silence was measurable end to end: an unreadable @@ -17880,6 +17924,12 @@ export class ObjectQL implements IObjectQLEngine { // an explicit `restrict` deviates. A plain lookup honors its // configured deleteBehavior (default set_null). // + // [#22957] A `tree` reads like a `lookup`: the spec materializes the same + // `set_null` default on it, and an authored `cascade` / `restrict` is + // honoured as written. Its `cascade` is a subtree delete, and a cycle in + // the data (a corrupt row pointing at its own descendant) terminates on + // the cascade set, the same membership test any self-reference has. + // // [#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'` — resolves to `cascade` here, From 5ce34e0670577a4b36380dc3d3d6a111b7a1e65c Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 13:19:44 +0000 Subject: [PATCH 2/2] test(objectql): pin a required tree's set_null escalation; changeset Claude-Session: https://claude.ai/code/session_01ADzJtzYTLUfgrRZHxkagkX Co-authored-by: Claude --- .changeset/22957-tree-delete-behavior.md | 18 ++++++++++++++ .../src/engine-cascade-delete-tree.test.ts | 24 +++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 .changeset/22957-tree-delete-behavior.md diff --git a/.changeset/22957-tree-delete-behavior.md b/.changeset/22957-tree-delete-behavior.md new file mode 100644 index 00000000000..6555443f7b2 --- /dev/null +++ b/.changeset/22957-tree-delete-behavior.md @@ -0,0 +1,18 @@ +--- +'@objectstack/objectql': patch +--- + +fix(objectql): a `tree` field's `deleteBehavior` now takes effect when its parent record is deleted, and the default clears the reference + +Clause-②: no + +The spec gives a `tree` field (a record's parent in a hierarchy on its own object) a `deleteBehavior`, like a `lookup`. When the field declares none, the parse fills in `set_null`. The engine's referential delete pass only followed `master_detail` and `lookup` fields. So deleting a parent node returned success and left every child pointing at the deleted id. An authored `cascade` or `restrict` on a `tree` did nothing either. + +A by-id delete now applies a `tree` field's behaviour the way it applies a `lookup`'s: + +- `set_null` (the default): the children stay, and their parent field is cleared. +- `cascade`: the whole subtree is deleted, at any depth, and nothing outside it. A cycle in the data (a row whose parent is its own descendant, or itself) ends: each row is deleted once. +- `restrict`: deleting a node that still has children is refused with `DELETE_RESTRICTED` (409), and the dependent named is the tree's own object. A leaf node still deletes. +- On a required `tree` field, `set_null` cannot clear the parent, so deleting a node that has children is refused with `DELETE_RESTRICTED`, as it is for a required `lookup`. + +Deleting a record of an object that has a `tree` field now runs as one transaction when the object is on the default datasource, as every delete that can cascade already does. If a node deep in the subtree refuses its delete (a `beforeDelete` hook, say), the nodes already deleted are restored and the call is refused. `lookup` and `master_detail` behave exactly as before. diff --git a/packages/objectql/src/engine-cascade-delete-tree.test.ts b/packages/objectql/src/engine-cascade-delete-tree.test.ts index 2a88dd422e6..4bdba434b26 100644 --- a/packages/objectql/src/engine-cascade-delete-tree.test.ts +++ b/packages/objectql/src/engine-cascade-delete-tree.test.ts @@ -25,6 +25,8 @@ * nothing outside it; * - `restrict` refuses with the ADR-0112 envelope naming the tree's own object * as the dependent, and keeps the row; a leaf still deletes; + * - a REQUIRED tree's `set_null` escalates to that refusal, as a required + * `lookup`'s does; * - a cycle in the data terminates: a corrupt row pointing at its own * descendant, and a row pointing at itself; * - a subtree cascade is one unit of work: a refusal from a node deep in it @@ -300,6 +302,28 @@ describe('[#22957] a tree field\'s deleteBehavior takes effect on a parent delet }); }); + describe('required', () => { + it('a required tree\'s set_null cannot clear the parent, so the parent delete is refused', async () => { + // The escalation a required `lookup` already takes: a parent that must + // be present cannot be cleared, so the delete is refused rather than + // answered with the child's own `required` refusal. + const { engine, driver } = await makeEngine([ + treeObject('zt_rooted', parsedTree('zt_rooted', { required: true })), + ]); + // A required hierarchy's top points at itself; the engine refuses a + // reference to a row that does not exist yet, so it is written at the + // driver. + const root = await driver.create('zt_rooted', { id: 'root', name: 'root', parent: 'root' }); + const c = await engine.insert('zt_rooted', { name: 'C', parent: root.id }); + + const err = await refusalOf(engine.delete('zt_rooted', { where: { id: root.id } } as any)); + + expect(err).toMatchObject({ code: 'DELETE_RESTRICTED', status: 409, dependentObject: 'zt_rooted' }); + expect(driver.rowOf('zt_rooted', root.id)).toMatchObject({ name: 'root' }); + expect(driver.rowOf('zt_rooted', c.id)).toMatchObject({ name: 'C', parent: root.id }); + }); + }); + describe('multiple: true', () => { it('is refused on a tree by the spec\'s parse, so no multi-valued tree reaches the walk through it', () => { const result = FieldSchema.safeParse({ type: 'tree', reference: 'zt_multi', multiple: true });