Repository navigation
fix(objectql): a record the same cascade deletes never refuses its sibling's delete - #22377
Conversation
…n base) Claude-Session: https://claude.ai/code/session_01EUBvqtauTDmHi2ZgY759p2 Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EUBvqtauTDmHi2ZgY759p2 Co-authored-by: Claude <noreply@anthropic.com>
…(WIP) Claude-Session: https://claude.ai/code/session_01EUBvqtauTDmHi2ZgY759p2 Co-authored-by: Claude <noreply@anthropic.com>
…ecting the set; document the rule on the DELETE door Claude-Session: https://claude.ai/code/session_01EUBvqtauTDmHi2ZgY759p2 Co-authored-by: Claude <noreply@anthropic.com>
…scade-sibling-restrict
…g the cascade's two phases share Claude-Session: https://claude.ai/code/session_01EUBvqtauTDmHi2ZgY759p2 Co-authored-by: Claude <noreply@anthropic.com>
…caller's limit Claude-Session: https://claude.ai/code/session_01EUBvqtauTDmHi2ZgY759p2 Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 6 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 2 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 17 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 7347cb9633e8163f71719af24341194afdb3e120 && git checkout 7347cb9633e8163f71719af24341194afdb3e120
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 16096e8d7be20a584bc1ada8a437617f4ea7e997 faa4ac206aac62e12cb5259b9d7dc4c95d31d386 && git checkout -B drift-repro 16096e8d7be20a584bc1ada8a437617f4ea7e997 && git merge --no-ff faa4ac206aac62e12cb5259b9d7dc4c95d31d386
node scripts/docs-audit/affected-docs.mjs --json 16096e8d7be20a584bc1ada8a437617f4ea7e997
|
Contract reviewServed-tier: Inputs: card #22305 (body; triage 6062588548; claim 6068498126; os-dev-report 6071613831); ① Derived judgmentsThe refusal removed, against the cited text:
The rule as built (
Pins and reverse verification. ② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS |
Fixes #22305
Clause-②: no
What was wrong
A by-id delete runs
ObjectQL.cascadeDeleteRelations. It walks_registry.getAllObjects()in registration order and recurses depth-first through the publicdelete(). Each level judged its ownrestrictrefusals without knowing about the cascade that called it.The measured shape is hotcrm's account. An account cascades (
master_detail) to its contacts and to its contracts. Each contract holds a required lookup to a contact, and that lookup's defaultset_nullescalates torestrict. With the contacts registered first, the walk deleted a contact first, and the contact's own walk refused on a contract that the same account delete was about to remove. With the contracts registered first, the identical delete succeeded.The rule (triage ruling 6062588548, verbatim)
The fix
cascadeDeleteRelationsnow runs in two phases. Both phases read relations and rows one way:cascadeRelationBehaviordecides which relations point at an object, andprobeReferencingRowsreads the referencing rows (system identity, the same filter, the same missing-table and multi-value handling).collectCascadeDeleteSetis read-only. It walks breadth-first across every relation whose resolved behaviour iscascade, starting from the root, and collects every record the delete will remove. The root is in the set. A visited check stops it on a cycle.walkReferencingRelationsis the existing walk, in the existing order. Three things change:restrictandset_nullconsider only rows outside the set. That covers an authoredrestrictand both halves of the requiredset_nullescalation.set_nullwrites nothing to a row in the set.The set is carried by an
AsyncLocalStorageon the engine (cascadeDeleteSets), the same kind of carrier astxStore. A nesteddelete()for a member joins the set. Any other delete, such as one a hook makes, is the root of its own cascade, as before. The set is not stored on the request context, for two reasons: a caller that passed no context still passes none, and no request input can add a record to the set.Base vs head, measured
Rig: a real
ObjectQLover a realSqlDriver(better-sqlite3), called throughObjectStackProtocolImplementation.deleteData, the methodDELETE /api/v1/data/:object/:idserves. Base ise87070ed49. Head is4e096ce935, whoseengine.tsis byte-identical to the final head's. The objectqldist/was rebuilt for each leg.ablation-dist-preflightproves which build each leg read: the markercascadeDeleteSetsis absent from base'sdist/and present in head's.409 DELETE_RESTRICTED,dependentObjectzz_contract, count 2. Nothing deleted.zz_invoiceto a contact), contacts first409namingzz_contract. The defect hid the real refusal.409 DELETE_RESTRICTED,dependentObjectzz_invoice, count 1. Rolled back; all 6 rows remain.409namingzz_invoice, count 1. Rolled back.set_nullinside the set (zz_memoto a contact), contacts firstset_nullinside the set, memos firstbeforeDeletedispatches. Nothing deleted.beforeDeletebeforeDelete404 RECORD_NOT_FOUNDon the second pathThe REST door was measured once on the real HTTP stack (
bootStack, admin,DELETE /api/v1/data/zzr_account/:id):409, body"code":"DELETE_RESTRICTED", namingzzr_contract.200.200, and every row is gone.Atomicity and side effects
planCascadeAtomicityanswers'atomic'), and it stays one: the set is collected inside that transaction. In the outside-restrict control, contracts first, head deletes 4 rows before the refusal, and the rollback restores every one of them.'split') is unchanged and stays non-atomic, aswarnCascadeNotAtomicdeclares. On that path, a later refusal can also leave behind a member whose restrict was skipped. The code comment states this.beforeDeleteandafterDeleteexactly once, in both orders (pinned).set_nullon a member. Base issued the extra UPDATE only with the contacts registered first: update hooks fired and an audit update row was written, then the record was deleted. Head never issues it. Both orders now leave the trail that base left when the memos were registered first.Cost
Phase 1 adds one extra probe per cascading relation per member.
restrictandset_nullrelations are not read in phase 1.In phase 1, the relation scan runs once per object, not once per record.
Pins and reverse verification (at
faa4ac206a)packages/objectql/src/engine-cascade-delete-sibling-restrict.test.tshas 21 cases, all green at head.engine.ts(hash-proven swap and restore)set_nullmember; authored restrict between members; root membershipBoth ablations went through
scripts/ablation-replace.mjsin wrap mode. Each restore is proven by blob hash:bd05e02c8654after restore equals the HEAD blob.Clause-② evidence
The built declarations were compared, base vs head:
index.d.tsandcore.d.tsare identical apart from the shared chunk's file hash.privatemember names and their doc comments:cascadeDeleteSets,collectCascadeDeleteSet,cascadeRelationBehavior,fileReferenceCheckElevation,probeReferencingRows,walkReferencingRelations.Also in this PR
packages/objectql/src/federated-injected-column-readers.test.ts: this ledger names the readers of injected columns by function. Its twocascadeDeleteRelationsrows now namecascadeRelationBehavior, where those calls moved.content/docs/api/data-api.mdx: theDELETE /data/:object/:idsection said relations honour theirdeleteBehavior"with one substitution". That sentence would be false after this change, so the section now states the cascade-set rule. This file is outside the dispatched file surface..changeset/22305-cascade-sibling-restrict.md: apatchfor@objectstack/objectql. Changesets pre mode is on (.changeset/pre.json).Tests
All runs used the shared verify lock.
@objectstack/objectqlatfaa4ac206a:pnpm --filter @objectstack/objectql test): 388 files, 7628 tests passed;typecheck: green, includingcheck:test-typecheck(the new test file adds no debt).4e096ce935. The two later commits touch only objectql test files, which none of these suites reads.@objectstack/rest: 263 files, 4951 passed, 326 skipped.@objectstack/runtime: 340 files, 4776 passed, 19 skipped.node scripts/pm/dispatch-gates.mjs --commandswas derived atfaa4ac206a(97 commands). All 97 were run on that head and each exited 0, after the dists thatcheck:skill-examplesandcheck:dual-build-cjs-loadsread were built.--ranreconciles 97 derived, 97 run, 0 NOT-MEASURED, 0 UNRUN.faa4ac206a:eslint --no-inline-config --format jsonreports 3 files, 0 errors and 0 warnings. The narrowing is a measurement, not a skip:eslint --print-configresolves a config (5 rules) for each file, so they are in the linted population. The config enables no type-aware linting (parserOptions.projectis null), so this diff cannot change the result for any untouched file. The repo-widepnpm lintis left to CI.Acceptance notes
content/docs/data-modeling/field-types.mdxsayscascadeandrestrictare "the values honored as written". That sentence describes how a field's behaviour resolves, and it is still true per field. The cascade-set rule is a delete-time rule and is documented indata-api.mdx(above). No other doc page states a refusal this change removes.ObjectQL.MAX_CASCADE_DEPTH(10) is not threaded through the recursion:delete()callscascadeDeleteRelationswith the default depth 0, so the bound never fires. That was true before this PR, and it is why a data cycle recursed without end. The entered-row skip now ends a cycle within one cascade. The dead bound is left as it was.Generated by Claude Code