Skip to content

Commit fe5ed7c

Browse files
fix(objectql)!: a user field's declared deleteBehavior takes effect when the user is deleted (#29253) (#29368)
domain:server#5 · os-steve · session_01ADHkmQKwKFCp1m6Pb6Q8c1 <!-- claude-projects-attribution: {"github_login":"os-steve"} --> _Requested by **Steve** · [project thread](https://claude.ai/code/project/chan_01FdL1XeUyvVNxLreh3F4Qi9?thread=cmsg_01FdL1XeUyvVNxLreh3F4Qi9C3uenhvETo3EhaCvXV7GjQ)_ Fixes #29253 Clause-②: no (narrowing) Metadata-protocol: semantic Skills-docs: follow-up #29400 — skills/objectstack-data/rules/field-types.md (the `user` row lists no `deleteBehavior`), skills/objectstack-data/rules/relationships.md; `content/docs/data-modeling/fields.mdx` is updated here **BREAKING** for an app that declares `deleteBehavior` on a `user` field: the declared value now takes effect when the user is deleted. A delete of a user held by a `restrict` field (or a `required` field declaring `set_null`) is now refused with `409 DELETE_RESTRICTED` where it succeeded; a declared `cascade` now deletes the holding rows; a declared `set_null` now clears them. **The fix:** if you declared `deleteBehavior` on a `user` field only as documentation, remove it or change it to what you want enforced. ## 维护者速读 - 改了什么:user 字段上作者写明的 deleteBehavior 现在真的生效——删除用户时,restrict 拒绝删除(409 DELETE_RESTRICTED),cascade 删掉持有该用户的记录,set_null 清空该字段(多值字段只移除该成员);没写 deleteBehavior 的 user 字段行为不变。 - 为什么改:spec 接受并保留 user 字段上的 deleteBehavior,但引擎的删除联动只认 master_detail / lookup / tree,user 字段上的声明一直是摆设,被删用户的 id 残留在记录里。 - 风险与回滚:席位裁定为收紧(narrowing,BREAKING,objectql minor):写了 restrict(或 required + set_null)的应用,删除被引用的用户从 200 变成 409;写了 cascade 的会连带删记录。#9784 之前存下的对象可能在 user 字段上带着自动写入的 set_null,现在也会生效。仓内平台对象和示例里没有这样的字段。回滚即 revert 本 PR。 - 先看:packages/objectql/src/engine.ts(statesDeleteSemantics 与 deleteWalkTargetOf),packages/objectql/src/engine-cascade-delete-user-field.test.ts,packages/objectql/src/federated-injected-column-readers.test.ts(台账两行)。 - 两分钟验证:pnpm --filter @objectstack/objectql exec vitest run src/engine-cascade-delete-user-field.test.ts,预期 16 条全绿。 - 需要你做的:无,席位落地。 ## What changes **Measured first: which doors delete a user** (at `origin/main` `b1779200`): | Door | Reaches `ObjectQL.delete` (so the walk runs)? | Code path | |---|---|---| | better-auth admin remove-user, self delete-user (and its callback) | yes, by id, as the system | `internalAdapter.deleteUser` deletes the `user` model through the plugin-auth adapter: `objectql-adapter.ts` `delete` / `deleteMany` call `dataEngine.delete(objectName, { where: { id } })`, and `dataEngine.delete` is `e.delete(m, asAttributedSystem(q))` (`isSystem: true`). The request runs inside `runSubjectErasureAtomically` (`SESSION_ERASURE_PATHS`), so a refusal rolls the credential rows back. | | Generic data doors (REST `DELETE /data/sys_user/:id`, MCP / batch data tools) | refused before the delete | `identity-write-guard.ts` registers a `beforeDelete` that refuses a caller-context write to any `managedBy: 'better-auth'` object (ADR-0092 D2); `sys_user` declares no generic delete (`userActions: { edit: true }` only). | | Ban / soft delete | not a delete | `admin-ban-endpoints.ts` updates the user and deletes its sessions; the `sys_user` row stays. | Every door that removes the `sys_user` row reaches the walk, so this is (B) enforce, and no spec change. **The change** (`packages/objectql/src/engine.ts` only): - `statesDeleteSemantics` (the one family test every reader of the walk asks) admits a `user` field when, and only when, it declares a `deleteBehavior` of the enum's three values. A `user` field that declares none stays out: the spec's parse materializes no value on it, and the engine invents no default. - `deleteWalkTargetOf` (new) answers the object a relation points at for `cascadeRelationBehavior` and `planCascadeAtomicity`. It still reads the `reference` carrier through `referenceCarrierOf`, so an unreadable carrier still throws; for a `user` field it answers `referenceTargetOf` (`sys_user`), so a `{ type: 'user' }` with no `reference` still points at the user. - Nothing else moves: an admitted `user` field then goes through the walk exactly as a `lookup` with the same authored value, including the multi-value `$contains` probe and member removal, the required `set_null` → `restrict` escalation, the two-phase cascade set and the one-transaction plan. `DELETE_SEMANTICS_REFERENCE_TYPES` keeps `master_detail`, `lookup`, `tree`; its docblock's old "`user` stays out … made in the spec, never here" paragraph is replaced by the declared-only rule above. Docs: one paragraph in `content/docs/data-modeling/fields.mdx` (Referential integrity). Changeset: `.changeset/29253-user-field-delete-behavior.md`, `@objectstack/objectql` **minor** (the level a semantic metadata-protocol kind takes), with the BREAKING banner, the one-line fix, and the ADR-0087 disposition `not-required (no-migration-prescription)`: no schema key, export or type moves, and there is no stored value for a conversion to rewrite. ## Tests New `packages/objectql/src/engine-cascade-delete-user-field.test.ts` (16 tests; the real `ObjectQL` over a stub driver with snapshot rollback, whose WHERE matcher answers `$or` / `$and` / `$in` / `$contains` and throws on any other operator). Deletes go through the engine with and without the adapter's system context: - no `deleteBehavior` (parsed single, parsed multi, raw, required): the user is deleted and the holder keeps the id; - `restrict`, single and multi-valued: `DELETE_RESTRICTED` / 409 naming the holder's object, both rows kept; the same code and status a `lookup` restrict to `sys_user` answers; a user nobody holds still deletes; a `user` field with no `reference` still refuses; - `cascade`, single and multi-valued: the holders are deleted, a row holding another user stays; a later `restrict` rolls the cascade back; - `set_null`: single cleared to `null`; multi loses only the member, an emptied set is `[]`; required `set_null` refuses. **Forced test edit:** `packages/objectql/src/federated-injected-column-readers.test.ts` (the #21918 reader ledger). The two `referenceCarrierOf()` seam uses moved from `cascadeRelationBehavior` / `planCascadeAtomicity` into `deleteWalkTargetOf`, so the two old `skips` rows are replaced by two `not-a-read` rows for the helper. The two callers' own `isFederatedUnprovisionedInjectedColumn()` rows stay and still bind the skip. ## Acceptance notes - Behaviour that changes for an app: a `user` field that declares `restrict`, or `set_null` with `required: true`, now refuses deleting the user it holds (409, was 200); `cascade` now deletes the holders; `set_null` now clears them. A repo-wide scan of `Field.user(` / `type: 'user'` declarations under `packages/` and `examples/` found none that declares `deleteBehavior`. - Rows stored before #9784: the spec's parse then wrote `deleteBehavior: 'set_null'` on every field type, `user` included, so an object stored in that era can carry a `set_null` on a `user` field no author wrote. It now takes effect (cleared on user delete; refused on a `required` field). Stated in the changeset; the seat accepted it with no spec card. - The two declaration lines above were ruled by seat 5 after round 1 (a narrowing, semantic kind); the dispatch claim carried no arm and kind none. - Spec prose left as it is (no `packages/spec/**` edit on this card): the `FieldSchema` `.overwrite` comment in `packages/spec/src/data/field.zod.ts` still says a `user` field sits outside the walk's family; true for a bare one, no longer for an authored one. - `os verify --delete-behaviors` reports an authored `user` field `not-derivable` (its target `sys_user` is not an app-declared object), so that family does not start probing these fields. ## Verification <details><summary>Red first: the pins against the walk as it was (ablation 1)</summary> `return false; // ABLATION-29253` in place of the `user` arm of `statesDeleteSemantics` (the walk on `main`): `Tests 10 failed | 6 passed (16)`. Every restrict / cascade / set_null pin fails (five with "expected the delete to be refused"); the four no-`deleteBehavior` controls stay green. </details> <details><summary>Ablations (scripts/ablation-replace.mjs; restore by git checkout HEAD, git diff HEAD empty, disk blob == HEAD blob 5defa62)</summary> 1. `user` arm answers `false` (user out of the walk, as on `main`): `Tests 10 failed | 6 passed (16)`. 2. `deleteWalkTargetOf` answers the carrier only: `Tests 1 failed | 15 passed (16)`, only "a user field naming no reference still references sys_user". 3. `user` arm answers `true` (every user field joins, a bare one defaulting to `set_null`): `Tests 4 failed | 12 passed (16)`, the four no-`deleteBehavior` controls (e.g. `expected { person: null } to match { person: 'r_1' }`, multi `expected { person: [] }`). </details> <details><summary>Code head 7675ff7: tests and gates (origin/main merged once, after #29245 landed)</summary> - `pnpm --filter @objectstack/objectql test`: `Test Files 531 passed (531)`, `Tests 10292 passed (10292)`; then `pnpm --filter @objectstack/objectql typecheck` (incl. `check:test-typecheck`): exit 0 (lock VERDICT command-exit 0). The first full run on the merge head 039625f was `1 failed`: the reader-ledger test above, fixed in 7675ff7. - New pins `src/engine-cascade-delete-user-field.test.ts`: `Tests 16 passed (16)`; `src/federated-injected-column-readers.test.ts` + `src/engine-cascade-federated-tenant-anchor.test.ts`: `Tests 17 passed (17)`; #29245's `src/engine-lookup-referential-integrity.test.ts` runs in the full suite above. - `node scripts/pm/dispatch-gates.mjs --commands` (no paths): 93 families derived; all 93 run, each exit 0; `--ran`: 93 run, 0 NOT-MEASURED, 0 UNRUN (a DERIVED zero). Includes `check:where-matcher`, `check:objectql-double-limit`, `check:engine-double-contract` (none new). - Lint: root eslint over `packages/objectql` with `--no-inline-config`: exit 0. Repo-wide `pnpm lint` is CI's. </details> <details><summary>Round 2 head 4a0ed13 (changeset and declaration only; code unchanged from 7675ff7)</summary> - Dogfood battery, closure built first (`pnpm --filter '@objectstack/dogfood^...' build`, lock VERDICT command-exit 0), then `pnpm --filter @objectstack/dogfood test` (no shard, no path): lock VERDICT command-exit 0 · `Test Files 392 passed | 3 skipped (395)` · `Tests 3227 passed | 28 skipped (3255)` · no failures. - `node scripts/check-adr-0087-registration.mjs --base origin/main`: exit 0, 1 declared-breaking changeset (banner, bang and narrowing arm all read), disposition `not-required (no-migration-prescription)`. - `node scripts/check-changeset-no-major.mjs --base origin/main --event` (this body as the pull_request payload): exit 0, level axis reads the narrowing arm and the semantic kind, no moved package graded patch. - `node scripts/pm/dispatch-gates.mjs --commands` (no paths): the same 93 families; all 93 run, each exit 0; `--ran`: 93 run, 0 NOT-MEASURED, 0 UNRUN (a DERIVED zero). </details> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent d93acbb commit fe5ed7c

5 files changed

Lines changed: 461 additions & 17 deletions

File tree

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
'@objectstack/objectql': minor
3+
---
4+
5+
fix(objectql)!: a `user` field's declared `deleteBehavior` takes effect when the user is deleted (#29253)
6+
7+
Clause-②: no (narrowing) — deleting a user that a `user` field declaring `restrict` (or `set_null` with `required: true`) holds is refused with `409 DELETE_RESTRICTED`, where it succeeded before; a declared `cascade` now deletes the rows holding the user, and a declared `set_null` now clears them. Nothing is widened.
8+
Metadata-protocol: semantic
9+
10+
<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no export or type changes: a `deleteBehavior` the spec already accepts on a `user` field is now enforced by the delete walk the way it is on a `lookup`. The refusal is the walk's own DELETE_RESTRICTED, so there is no stored value for a conversion to rewrite; an author who did not mean the declaration removes it or changes it. -->
11+
12+
**BREAKING** for an app that declares `deleteBehavior` on a `user` field: the declared value now takes effect when the user is deleted.
13+
14+
A `user` field accepts `deleteBehavior`, but the referential delete walk only read `master_detail`, `lookup` and `tree` fields, so the value did nothing: deleting a user that a `restrict` user field held was not refused, a `cascade` user field did not delete the rows holding that user, and every holder kept the deleted user's id.
15+
16+
- **Now.** A `user` field that declares a `deleteBehavior` joins the walk and reads it as a `lookup` reads its own. `cascade` deletes the rows holding the deleted user. `set_null` clears the field; a `multiple: true` field loses only that member, and an emptied set is `[]`. `restrict` refuses the delete with `409 DELETE_RESTRICTED`, naming the holder's object. A `set_null` on a `required` user field is refused the same way, as on a required `lookup`. A field that names no `reference` still points at `sys_user`, because the type fixes the target.
17+
- **Unchanged.** A `user` field that declares no `deleteBehavior` (the spec's parse adds none) stays out of the walk: deleting the user leaves the field as it was. `lookup`, `master_detail` and `tree` behave as before.
18+
- **Which deletes.** Every user delete: better-auth deletes the user (admin remove-user, self delete-user) through the engine by id, and the generic data routes refuse a caller's own `sys_user` delete. A `restrict` refusal reaches the auth caller as `409 DELETE_RESTRICTED`, and the credential rows deleted before it are rolled back.
19+
20+
- **Rows stored before #9784.** Until then the spec's parse wrote `deleteBehavior: 'set_null'` on every field type, `user` included, so an object stored in that era can carry a `set_null` on a `user` field that no author wrote. That value now takes effect: the field is cleared when the user is deleted, and on a `required` field the delete is refused.
21+
22+
**The fix.** If you declared `deleteBehavior` on a `user` field only as documentation, remove it or change it to what you want enforced. Nothing to do for a `user` field that declares no `deleteBehavior`.

‎content/docs/data-modeling/fields.mdx‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,13 @@ record is deleted; the integrity check above governs what may be **written**
302302
here in the first place. They are two halves of the same relationship contract.
303303
</Callout>
304304
305+
A `user` field takes part only when it declares a `deleteBehavior`. Declared, it
306+
works as it does on a `lookup` when the user is deleted: `cascade` deletes the
307+
records holding that user, `set_null` clears the field (a `multiple: true` field
308+
loses only that member), and `restrict` refuses the user delete with
309+
`409 DELETE_RESTRICTED`; a `set_null` on a `required` user field refuses it too. A
310+
`user` field that declares none keeps the deleted user's id, as it always has.
311+
305312
### File & Media Types
306313
307314
| Type | Factory | Description |

0 commit comments

Comments
 (0)