Repository navigation
cascadeDeleteRelations' set_null limb nulls the WHOLE multi-value array, dropping every other live reference #9438
Description
Activity
Triage: lands in
packages/objectql/src/engine.ts⇒domain:engine-core, type Bug,pm:queue+target:v17(board class ①: the moment #9437 lands, this limb executes for the first time and silently destroys live references on a published surface — an RC carrying #9437 without this is shipping data loss).Triage-adjudicated interim = the card's option 2, inherited from the pinned sibling one branch over: a defaulted
set_nullon a REQUIRED lookup already escalates torestricton the reasoning that a write the FK cannot honestly perform must refuse rather than corrupt — the same reasoning covers a set-valued FK whose "null" would discard unrelated members. This shrinks acceptance (409 where silent loss was), mints no new semantics, and is a one-line revert when the real fix lands. Sequencing is the point: land it in the same round as PR #9437 so the lossy write is never live onmain— the dispatching seat should treat this as coupled to #9437's landing window, and the claim must name PR #9437 in its serial constraints. Veto window: this round's report on #6015.The question blocking option 1 (member removal — the real fix) is split to #9447 (
needs-user-decision, residual shape[]vsnull, four facets + recommendation on the card); when it is ruled, option 1 + the interim's revert are one PR here.⚠️ Clause-② is YES: accept/reject behavior changes (some deletes now refuse 409). Dispatch at the contract-review tier, claim comment carriesClause-②: yes.Triage seat routine — session
session_pm_triage_20260818T0104Z.
Generated by Claude Code
Maintainer ruling — this card stays open and gets done on its own; #9437 ships a holding position instead
Recording the decision so this card's scope is unambiguous.
Ruled (maintainer, 2026-08-18): Option B in the same round as #9437, then A — this card — as its own round.
What #9437 will now carry
A one-line-revertible escalation: a
multiple: truereference field that would take theset_nulllimb escalates torestrictand refuses the delete loudly, mirroring the required-FK escalation already pinned three lines above it in the same block. It mints no semantics. It is explicitly a holding position, and #9437's changeset will say so and name this card.⛔ #9437 does not close this card and carries no closing keyword naming it.
What this card still owes
The question is unchanged and is the reason B exists:
What does
deleteBehavior: 'set_null'MEAN on amultiple: truereference field?The direction is not really in doubt — remove the deleted member, keep the rest. The undecided part is the residual shape when the last member is removed:
[]ornull? That is observable on the read path and to any required multi-value validator, which is why it was not worth guessing inside a P0 fix.Why it was sequenced this way, for whoever picks this up
The four-dimension analysis that produced the ruling, compressed:
- Real need — both harms are measured, not speculative. But the 400 is visible and the array-nulling is invisible: a 200 with data silently gone. Per unit, the invisible one is worse.
- Long-term soundness — shipping the lossy write would have converted an undecided semantic into published behaviour. That is the most expensive thing to undo in a platform: once real data has been nulled, "remove the member" stops being a bug fix and becomes a breaking change plus a migration. B costs one line today; C would have cost a major.
- AI-authored metadata —
deleteBehaviordefaults toset_null, so an AI-authored multi-value lookup that never mentions it lands on the lossy path by default. Nobody chose it. B surfaces the undecided semantics at the moment they matter; A, once landed, makes it declared and enforced. - Reversibility — B reverts in one line and is not a regression against today: today these deletes all return 400, so
restrictandcascaderelationships get strictly better, and only theset_nulllimb still refuses — now with a meaningful error instead ofINVALID_FILTER. No scenario is worse than the status quo. C, by contrast, is the only option whose harm is unrecoverable — once the sibling references are gone, they are gone.
Two implementation sub-calls made by PM, both reversible
Flagged to the maintainer as open; the ruling covered sequencing, so these were decided to keep a P0 moving. Either can be vetoed.
- Escalate both the defaulted and the explicitly-authored
set_null, not only the defaulted one. B's purpose is to destroy no data and mint no semantics; letting an explicitly-authoredset_nullstill run leaves exactly the harm B exists to prevent, for a smaller population. No one authoringset_nullon a set-valued field can have meant "clear the set and drop the other references" — that semantic has never existed to be chosen. - A distinct error code, not a bare
DELETE_RESTRICTED. Per ADR-0110 D3, "refused because the semantics are undecided" and "refused because you configuredrestrict" are different facts; conflating them makes the temporary state indistinguishable from permanent policy and un-greppable when this card lands. The implementer has been told to stop and report if a new ledger entry looks like a contract widening that needs its own decision.
When this card is done
The escalation added by #9437 is the thing to remove — it was built to be removed, and the distinct error code is what makes it findable.
Related: #9437 (the P0 fix + holding position), #9362 (the P0), #8895 (the tightening that made the fail-open loud), #9390 (duplicate report of #9362, triage's to dedup).
Generated by Claude Code
Ruling relay (triage seat): #9447 is now ruled — maintainer, 2026-08-18, verbatim 「同意」 on the triage recommendation: the emptied multi-value lookup reads back as
[], andrequiredon a multi-value lookup means non-empty array. Consequence for this card: option 1 (member removal) is unblocked —next = current.filter(v => String(v) !== String(id)), writenext(empty ⇒[]), and the interimrestrictescalation reverts in the same PR. If #9437's landing window allows, interim + real fix may collapse into one motion; the constraint that the lossy write is never live onmainis unchanged. TheFieldSchemadoc-block sentence is the spec seat's half (#9447); the engine change consumes it and must not paraphrase it.
Generated by Claude Code
Unlock notice from the
domain:specseat (#6017): the ruling your option-1 fix consumes is now landed onorigin/main— #9447 closed via PR #9493 (squasheddf0c12de7).FieldSchema's doc blocks now pin, verbatim contract: an emptied multi-value lookup reads back as[], nevernull(binding for EVERY writer — cascade repair, form clears, API writes), andrequiredon a multi-value lookup means NON-EMPTY array. Citepackages/spec/src/data/field.zod.ts(therequiredandmultipledoc blocks, ruling 2026-08-18) from the member-removal PR, and revert the interimrestrictescalation in the same stroke per the ruling comment on #9447.One measured heads-up for that PR: the current validator PASSES
[]on a required+multiple: truelookup (record-validator.ts:171-173isMissingcovers undefined/null/blank only — read sites:477/:1011), so the ruledrequired-means-non-empty semantics are documented but NOT yet enforced; the enforcement gap is filed as #9476. Sequence as your lane sees fit — the doc contract does not block your member-removal limb either way.
Generated by Claude Code
Claiming this issue.
- Session:
session_01NTKPDRoynY8i3HmdSFUxFj - Branch:
claude/issue-9438-set-null-member-removal - Clause-②: yes
Scope: implement member removal for the
set_nulllimb on multi-value lookups (filter+ write, emptied array written as[]per the landed ruling inpackages/spec/src/data/field.zod.ts), and revert the interimrestrictescalation from #9437 in the same PR. Not touching #9476 (validator non-empty enforcement) or the #9437 probe repair.
Generated by Claude Code
- Session:
{ "issue": 9438, "status": "done", "branch": "claude/issue-9438-set-null-member-removal", "pr": "https://github.com/objectstack-ai/objectstack/pull/9520", "premise_still_valid": true, "summary": "Implemented member removal for the set_null limb on multiple:true reference fields in cascadeDeleteRelations (packages/objectql/src/engine.ts): the deleted id is filtered out of the stored array by whole-member String comparison and the remainder written back; an emptied set is written as [], never null, consuming the landed FieldSchema contract (packages/spec/src/data/field.zod.ts, multiple/required doc blocks, #9447 ruling 2026-08-18) which the code cites rather than paraphrases. The #9437 interim restrict escalation is reverted in the same PR: grep -c for multiValueHold / TEMPORARY / 9438 in engine.ts are all 0. The probe repair, required-FK escalation, cascade, explicit restrict, and single-valued set_null are untouched; #9476 is not addressed. No conflicts between the dispatch prompt and the standing contract were hit, except the completion-check requiring zero '9438' occurrences, which ruled out the house-style issue-id tag on the new write's comment — the citation there is #9447 plus the spec path instead.", "tests": "Union run at branch head 18c20564a5 (clean tree). objectql unit suite engine-cascade-delete-multivalue-probe.test.ts 17/17; full package 'pnpm --filter @objectstack/objectql test' 3828/3828 (216 files). runtime integration cascade-delete-multivalue-lookup-real-driver.integration.test.ts 7/7, asserting on the database via the driver's own connection (['acc_a','acc_b'] -> ['acc_b'] with acc_b still resolving; ['acc_a'] -> [] asserted not null); full package 2510/2510 (168 files). Typecheck clean both packages. Reverse verification, three legs, each ablation REBUILT into objectql dist and proved live with scripts/ablation-dist-preflight.mjs before trusting the colour (restore leg proved all three markers absent): leg 1 (member-removal write ablated alone) -> 5 unit + 2 integration pins RED, direction as expected ('expected null to deeply equal [acc_b]' / 'expected null to deeply equal []'); leg 2 (hold re-added alone) -> the same 7 success pins RED on DELETE_RESTRICTED 409; leg 3 (WIDEN direction, multiValued forced true, unit layer) -> single-valued set_null control RED ('expected [] to be null'), proving the does-not-fire control non-vacuous. Gates: derived with scripts/pm/dispatch-gates.mjs over changed paths; all 16 derived/convention families green locally, incl. type-check-debt --re-measure after the full packages closure build; ratchet families re-run at 18c20564a5. Nothing derived was skipped.", "open_questions": [], "out_of_scope_findings": [] }
Generated by Claude Code
- added a commit that references this issue
on Aug 23, 2026
Found while implementing #9362 (PR #9437). Filed unassigned; not fixed there, because the repair needs one decision this seat should not pick alone.
Reachable-after: #9362. Until that PR lands, this limb has never executed for a multi-value relationship in the history of the codebase — pre-#8895 the dependents probe swallowed its own failure and skipped the relation, post-#8895 it raised
INVALID_FILTER/ 400 and aborted the whole delete. Repairing the probe is what makes it run for the first time.The defect
cascadeDeleteRelations(packages/objectql/src/engine.ts) appliesdeleteBehavior: 'set_null'— the DEFAULT for a plainlookup— by writing the field tonull:On a
multiple: truefield that slot holds an ARRAY of references. Nulling it discards every OTHER member, none of which has anything to do with the record being deleted.Measured, on the real stack
Real
ObjectQLover a realSqlDriveron better-sqlite3, with the #9362 probe fix applied (without it the call is a 400 and nothing runs):zz_field_zoo#z1holdsrefs: ["acc_a","acc_b"], whererefsis{ type: 'lookup', reference: 'zz_account', multiple: true }— deleteBehavior defaulted, i.e.set_null.DELETE zz_account/acc_areturns 200.zz_field_zoo#z1re-reads asrefs: null.The live reference to
acc_bis gone, silently. On the stock showcase the same shape isshowcase_field_zoo.f_lookupsatshowcase_account.Why it was not repaired inside #9362
The DIRECTION is not really in doubt — "set null" on a set-valued foreign key should drop the broken member, not the set. What is undecided is the residual shape when the array empties:
[]ornull. That difference is observable to clients on the read path and to arequiredmulti-value validator,FieldSchemasays nothing about it, and no sibling declaration or landed ruling pins it. Guessing it inside a P0 hotfix would be minting metadata semantics by accident.Note there is a pinned sibling one branch over in the same block: a defaulted
set_nullon a REQUIRED lookup already escalates torestrict, on the reasoning that a FK which cannot be nulled must refuse rather than issue a contradictory write. An escalation on the same grounds is one of the options below, and is deliberately NOT taken unilaterally for the same reason.Options
next = current.filter(v => String(v) !== String(id)), then writenext. Needs the residual-shape answer.set_nulltorestrictwhen the field ismultiple: true, mirroring the required-FK escalation exactly. Refuses loudly instead of writing anything, decides no new semantics, and is a one-line revert once option 1's question is answered. Costs: a delete that "succeeded" before ObjectQL.cascadeDeleteRelations fails OPEN: a failed dependents probe skips therestrictguard entirely, so a delete that should be refused succeeds silently #8895 (by skipping the guard) now returns 409.Recommendation: answer the residual shape and take option 1; take option 2 in the same round as #9437 if the answer is not immediate, so the lossy write is never live on
main.Reproduction
packages/runtime/src/cascade-delete-multivalue-lookup-real-driver.integration.test.tson branchclaude/issue-9362-cascade-probe-multiple-lookupcarries the rig; add azz_field_zoofixture holding two ids and delete one of them.Generated by Claude Code