Skip to content

fix(objectql): a tree field's deleteBehavior takes effect on a parent delete - #23055

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-22957-tree-delete-behavior
Oct 12, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-22957-tree-delete-behavior

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #22957
Clause-②: no

The engine's referential delete walk now follows tree fields. The deleteBehavior the spec declares on a tree (materialized as set_null when the author writes none) takes effect on a by-id delete of the parent. This follows triage's ruling (comment 6119365636): the runtime honours the declared behaviour, and the spec's materialization stays as it is.

What changed

  • packages/objectql/src/engine.ts: one family predicate, statesDeleteSemantics over DELETE_SEMANTICS_REFERENCE_TYPES (master_detail, lookup, tree), replaces the master_detail / lookup literal at both readers of the delete walk that tested the family. A tree resolves like a lookup: an authored value is used as written, and an absent one resolves to set_null.
  • packages/objectql/src/engine-cascade-delete-tree.test.ts: 18 pins, listed below.
  • .changeset/22957-tree-delete-behavior.md: patch for @objectstack/objectql.

Readers of the walk (read on origin/main 9f4b652e3a)

reader family test before now
cascadeRelationBehavior master_detail / lookup literal the predicate
collectCascadeDeleteSet (phase 1) reads cascadeRelationBehavior code unchanged, widened through it
walkReferencingRelations (phase 2, the dependents walk) reads cascadeRelationBehavior code unchanged, widened through it
planCascadeAtomicity and its referenceCarrierOf probe master_detail / lookup literal the predicate, so a subtree cascade is one unit of work
the #9625 resolved-behaviour reads: the master_detail coercion to cascade, the master_detail + set_null warning, and the required set_null escalation to restrict read the resolved behaviour unchanged. A tree takes the lookup branch, so a required tree's set_null escalates like a required lookup's (pinned)
the referenceCarrierOf call in buildSummaryIndex master_detail / lookup literal left as is. It resolves a roll-up summary's foreign key and is not part of the delete walk

Measured before and after

os verify --app examples/app-showcase/objectstack.config.ts --delete-behaviors --json

Before: 9f4b652e3a. After: objectql built from 2030d882ac. engine.ts is byte-identical from there through this head, 85abb2ed96.

before after
exit 1 0
hardFailures 1 0
showcase_field_zoo.f_tree (declared set_null) contradicted. REST answered 200 and the system door succeeded, and both left the holder on the deleted id proven. Both doors cleared it (holderValue: null)
the other 23 rows 8 proven, 1 inconclusive (showcase_invoice.contact, refused by an app validation rule), 11 excluded, 3 not-derivable identical, row by row

Real driver: ObjectQL + SqlDriver (better-sqlite3), a scratch probe (not committed)

case before after
tree, set_null materialized by FieldSchema.parse child keeps the deleted id child's parent is null
tree registered raw, no key child keeps the deleted id child's parent is null
tree cascade: delete a from root, a, b, c (a chain) plus a2 and an unrelated pair only a deleted a, b, c deleted; root, a2 and the pair kept
tree restrict parent deleted, child left dangling DELETE_RESTRICTED 409 naming the tree's own object; both rows kept; the leaf then deletes
tree cascade, a 15-level chain 1 of 15 deleted 15 of 15 deleted
tree cascade, a corrupt cycle (root's parent is its grandchild) and a row that is its own parent only the deleted row went terminates (7 ms); every row of the cycle deleted once
tree registered raw with multiple: true child keeps the deleted id child's parent is null (read single-valued)
tree cascade with a beforeDelete veto on a deep node root deleted, children kept (nothing walked) refused 409; all four rows kept (rolled back)
CONTROL: lookup default set_null, lookup cascade, master_detail default cleared / deleted / deleted identical
CONTROL: lookup and master_detail restrict both DELETE_RESTRICTED 409 identical

The four questions the dispatch asked

  • multiple: true. FieldSchema refuses multiple: true on a tree at parse: it is declarable only on select, lookup, user, file and image. lookup has a multi-valued path (the $contains probe and member removal). A tree cannot reach that path through the contract. The walk's multi-value arbiter declaredMultiValued asks the spec's isMultiValueField, which reads a raw-registered multi tree as single-valued, as every other engine site does. Nothing new was added for it.
  • The default. The spec's parse puts set_null on a bare tree, and the test pins that as its premise. The engine reads the field raw, and an absent key also resolves to set_null. Both are pinned.
  • Depth and cycles. MAX_CASCADE_DEPTH (10) bounds only planCascadeAtomicity's schema walk. cascadeDeleteRelations has a depth parameter, but the nested deletes never advance it (the one call site passes the default), so the cascade over rows is bounded by its set. collectCascadeDeleteSet expands each record once, and the deleting walk skips a record it has already entered. Pinned: a 15-level chain is deleted whole, and a corrupt cycle and a self-parent both terminate.
  • Atomicity. An object whose only relation is its own hierarchy used to plan 'none' and opened no transaction. It now plans 'atomic', so a refusal deep in a subtree rolls back the nodes deleted before it.

Pins (engine-cascade-delete-tree.test.ts, in-file driver with snapshot rollback)

  • set_null: the parse premise; then materialized, authored, and absent: the child's parent is cleared, the child is kept, and a bystander is untouched (3 cases).
  • cascade: a subtree with a single commit; 15 levels deleted whole; a corrupt cycle terminates; a self-parent terminates; a deep veto rolls everything back.
  • restrict: the parent delete is refused with code + status + dependentObject, and both rows are kept; a leaf deletes.
  • required: a required tree's set_null escalates to DELETE_RESTRICTED.
  • multiple: the parse refuses multiple: true on a tree; a raw registration is read single-valued.
  • CONTROLS: lookup default and cascade plus the master_detail default; lookup and master_detail restrict (2 cases); an object nothing references opens no transaction.

Ablation (from the committed head 5ce34e0670, through scripts/ablation-replace.mjs)

Each leg's anchor hit 1 to 0 and the blob changed. After each leg the restore read blob == HEAD and git diff HEAD was empty.

  • Leg A: tree dropped from DELETE_SEMANTICS_REFERENCE_TYPES gave 11 failed and 7 passed. Red: all 11 tree-behaviour pins. Green: the 2 parse premises, the restrict leaf delete, and the 4 CONTROL tests.
  • Leg B: only planCascadeAtomicity's reader put back to the old literal gave 2 failed and 16 passed: the subtree's single-commit assertion and the deep-veto rollback. So the atomicity widening is pinned on its own.

Tests

  • pnpm --filter @objectstack/objectql exec vitest run --maxWorkers=2 src/engine-cascade-delete-tree.test.ts: 18 passed, at 5ce34e0670.
  • The full package suite (vitest run --project local --maxWorkers=2) ran at 2030d882ac plus the then-uncommitted required pin: 406 files and 7953 tests passed, 1 failed. The failure was the required pin as first written: its fixture referenced a row that did not exist, and the engine's write-time reference check refused it. The fixture was corrected and passes in the single-file run above.
  • pnpm --filter @objectstack/objectql typecheck: exit 0, covering tsc and check:test-typecheck. The new file is in tsconfig.test.json's program (--listFiles count 1).
  • @objectstack/objectql's public surface is unchanged (no export, no type), so no importing package owes a test.

Gates

dispatch-gates --commands on this head 85abb2ed96 derives 68 commands: the dispatched 57 plus 11 that the changeset brings in. All 68 exit 0. The --ran reconciliation reads 68 derived, 68 run, 0 NOT-MEASURED, and that zero is derived from the recorded exit codes. check:dual-build-cjs-loads first answered PREREQUISITE NOT MET because 8 packages outside this diff had no dist/. They were built (all turbo cache hits), and the gate re-ran with exit 0.

Deviations

  • The pins use an in-file driver with snapshot rollback, the same shape as the five sibling engine-cascade-delete-* suites, not a real driver. packages/objectql lists no driver package among its dependencies, and adding one (to package.json, pnpm-lock.yaml and the test-source-alias ledger) is outside the claim's file surface. The real-driver evidence is the showcase verify (sqlite-wasm, through REST and the system door) and the scratch SqlDriver probe above.
  • origin/main gained two docs-only commits (ADR text and anchor JSON) after the base. They were merged into this branch as 85abb2ed96; no package changed.

Acceptance notes

  • A tree that omits reference is still not walked. ObjectSchema accepts it as a self-reference (the refuseForeignTreeReference rule: the reference stays optional), and the parse materializes set_null on it. But referenceCarrierOf reads no carrier, and referenceTargetOf calls a referenceless tree targetless. Measured on this head through REST: DELETE answered 200 and the child kept the deleted id. Repairing it is a spec decision: which of the two readings is the contract, and whether a referenceless tree's target should resolve to its owning object. That is not a consumer-side fallback to add here. Reported to the seat.
  • Two comments in packages/spec/src/data/field.zod.ts (the deleteBehavior TSDoc and the .overwrite block) still say that the engine reads the key only behind a master_detail / lookup guard. That now includes tree. These are comments only. Carrier: none. Noted, not filed.
  • An authored deleteBehavior on a user field still parses and is still not walked. That is the spec's existing line for user, and this PR leaves it untouched.

Generated by Claude Code

…eteBehavior

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 <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 12, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql, touching 5 documentable anchor(s).

7 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/data-api.mdx (via set_null (literal, a string literal in a comment on a changed line))
  • content/docs/data-modeling/field-types.mdx (via set_null (literal, a string literal in a comment on a changed line))
  • content/docs/data-modeling/fields.mdx (via set_null (literal, a string literal in a comment on a changed line))
  • content/docs/data-modeling/validation-rules.mdx (via set_null (literal, a string literal in a comment on a changed line))
  • content/docs/deployment/cli.mdx (via set_null (literal, a string literal in a comment on a changed line))
  • content/docs/deployment/troubleshooting.mdx (via set_null (literal, a string literal in a comment on a changed line))
  • content/docs/protocol/objectql/types.mdx (via set_null (literal, a string literal in a comment on a changed line))

⛔ 2 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v15.mdx (via set_null (literal, a string literal in a comment on a changed line))
  • content/docs/releases/v17/17-1.mdx (via set_null (literal, a string literal in a comment on a changed line))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 2 anchor(s) matched too much of the corpus to be a work list: ObjectQL (symbol, 72 pages), master_detail (literal, 33 pages)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 17 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json df7a137923be503bf6fea15a6fc944d3ba06df6f → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 6f3877adac2979bd0c06b471d2b781d49fb64027 — the merge of head 85abb2ed96cc910f5b29d5757f32fa933bd1ceb6 into base df7a137923be503bf6fea15a6fc944d3ba06df6f, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 6f3877adac2979bd0c06b471d2b781d49fb64027 && git checkout 6f3877adac2979bd0c06b471d2b781d49fb64027
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin df7a137923be503bf6fea15a6fc944d3ba06df6f 85abb2ed96cc910f5b29d5757f32fa933bd1ceb6 && git checkout -B drift-repro df7a137923be503bf6fea15a6fc944d3ba06df6f && git merge --no-ff 85abb2ed96cc910f5b29d5757f32fa933bd1ceb6

node scripts/docs-audit/affected-docs.mjs --json df7a137923be503bf6fea15a6fc944d3ba06df6f

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs df7a137923be503bf6fea15a6fc944d3ba06df6f → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 85abb2ed96cc910f5b29d5757f32fa933bd1ceb6
Local-runs: none

Inputs read, and nothing else: card #22957 (body; triage 6119365636; claim 6124337059; report 6125920806; seat review 6126018973), PR #23055 (body, the 3-file list, the net diff against main from merge base df7a137923), and the 40 check-runs on this head. Every required context reads success (Lint & Repo Gates, TypeScript Type Check, Test Core, Build Core, Governed Surface Queue Guard); Check Changeset and Check PR Size read success; no governed path. The spec texts cited were read on origin/main ec7f7f83b6 (packages/spec/src/data/field.zod.ts, field-value.zod.ts, object.zod.ts); engine.ts's readers were enumerated on origin/main and on this head.

① Derived judgments

  1. tree joins the delete family — right. The spec declares it: FieldSchema's .overwrite materializes deleteBehavior: 'set_null' on tree beside lookup because the type "is in the relational family, where the key states delete semantics"; REFERENCE_VALUE_TYPES lists tree; the key's own description is "What happens if referenced record is deleted". Triage ruled repair (A) and refused (B). The engine was the layer falling short; the diff fixes the engine and leaves packages/spec untouched, as the claim's surface requires.
  2. A tree resolves through the lookup branch (an authored value as written; absent is set_null) — right. The master_detail branch's coercion rests on "a detail cannot outlive its master", which a hierarchy does not assert, and the spec materializes the same default on tree as on lookup. Against the declared semantics, each change:
    • an authored restrict on a tree now refuses the parent's delete with DELETE_RESTRICTED/409 and dependentObject the tree's own object — right: that refusal IS what the author wrote, and a leaf still deletes (pinned);
    • a required tree's set_null escalates to restrict — right by the walk's unchanged rule (a cleared scalar foreign key trips the child's own required validator with a misleading 400; the escalation is a property of the resolved behaviour plus required, not of the type), the reading a required lookup has carried since Docs and engine disagree on deleteBehavior: 'set_null' written EXPLICITLY on a required lookup — the escalation to restrict cannot see the difference #9625 (pinned);
    • cascade deletes the subtree — right: a tree's reference must name the declaring object (refuseForeignTreeReference), so cascade has no other meaning. Termination on a corrupt cycle rests on membership, not depth: collectCascadeDeleteSet enqueues a record only when cascadeSetAdd first admits it, and walkReferencingRelations records entered before the first nested delete — read on main and unchanged by the diff; pinned for 15 levels, a cycle and a self-parent.
  3. planCascadeAtomicity reads the same predicate — right and necessary. A subtree cascade is several deletes, and the atomicity rule ("atomic honoured for real, or refused") would be contradicted by the one relation the walk now follows; the dev's ablation leg B shows the pin bites on its own. Consequence named, because the changeset states it and an operator will see it: an object with a tree field references itself, so its by-id delete never plans 'none' again — on the default datasource every such delete opens a transaction (children or not); on another datasource warnCascadeNotAtomic fires once per object; on a driver without beginTransaction the transaction() degrade warns once per driver. Correct, and the CONTROL (an object nothing references opens no transaction) is pinned.
  4. Family predicate coverage — complete for the delete walk. On origin/main, engine.ts carried the master_detail/lookup family literal at three sites: buildSummaryIndex (11709), planCascadeAtomicity (17244) and cascadeRelationBehavior (17838). The diff replaces the latter two with statesDeleteSemantics; collectCascadeDeleteSet and walkReferencingRelations read through cascadeRelationBehavior and need no edit. The two single-type reads left inside the walk are right as literals: the resolution ternary (master_detail to cascade unless restrict; everything else to the authored value or set_null), under which a tree takes the second arm by design, and the master_detail plus authored set_null error log, which is about that type alone. buildSummaryIndex's literal resolves a roll-up's foreign key and is not the delete walk — right to leave. Outside engine.ts: no master_detail/lookup pair in packages/objectql/src; the dangling-reference audit reads referenceTargetOf, which admits tree; packages/verify/src/derive.ts's RELATIONAL set already carries tree (which is why the instrument reported f_tree); packages/rest has no delete-walk reader. Every reader of the walk asks the one predicate.
  5. A referenceless tree — does nothing, and this PR is right not to decide it. cascadeRelationBehavior reads the carrier through referenceCarrierOf, which answers undefined for an absent reference; IMPLICIT_REFERENCE_TARGETS names user only. The spec contradicts itself on the shape: refuseForeignTreeReference prescribes "Drop reference (it is optional on a tree)", while referenceTargetOf's docblock calls a tree with no reference "an authoring bug". An engine that filled in the owning object would be settling that reading in a consumer (Prime Directive Add comprehensive test suite for Zod schema validation #12). Escalated: the seat files it at ACCEPT as class (b), measured by the dev at the REST door on this head. Reach today: examples/app-showcase's f_tree carries reference, and no tree field exists in packages/platform-objects (the org_chart hits are views), so no shipped corpus row sits in the gap.
  6. multiple: true — refused at parse on a tree; a raw registration is read single-valued through isMultiValueField, the walk's one arbiter. Right: nothing was added for a shape the contract refuses.
  7. user stays out — right; the spec's .overwrite puts it on the non-reference side on purpose, and moving it is a declaration change.

② Semver level

The card's thread asks two things; both are answered from the repo's rules, not from lane memory.

③ Boundary flags

  • Deviation: the pins use the in-file snapshot-rollback driver of the five sibling engine-cascade-delete-* suites, not the real driver the claim named — accepted, as the seat accepted it: packages/objectql declares no driver dependency, and adding one is a lockfile and ledger change outside the surface. The re-runnable real-driver evidence is os verify --delete-behaviors on the showcase (hardFailures 1 to 0, f_tree proven at both doors, the other 23 rows identical); the SqlDriver scratch probe is uncommitted and therefore not evidence anyone can re-run — if the seat wants a committed real-driver pin, the dev's suggestion (examples/app-showcase/test) is a separate small change, not a condition here.
  • Deviation: one docs-only merge of main (this head); the model-free trailer pair — fine under AGENTS.md.
  • open_questions: none in 6125920806.
  • Finding 1 (referenceless tree) — escalated, correctly (judgment ①5); filed by the seat at ACCEPT.
  • Finding 2 (the two field.zod.ts comments that say the engine reads the key only behind a master_detail/lookup guard — the deleteBehavior TSDoc's FieldSchema.deleteBehavior defaults set_null onto EVERY field type, so built artifacts ship the key on text/datetime/number fields #9784 paragraph and the .overwrite block's "cascadeDeleteRelations reaches the key exclusively on master_detail/lookup fields") — may stay an Acceptance note on this PR, and must gain a carrier. Reasons it stays out: the claim's surface is engine.ts plus tests plus one changeset ("stop on a breach"); packages/spec is cross-lane; a TSDoc edit on a schema is a check:docs trigger under AGENTS.md's "Touched packages/spec?" protocol, so a comment rider is the scope expansion Prime Directive chore: version packages #10 refuses; and the comments have no runtime consequence. Reason it needs a carrier: once this lands the sentences are false, and the .overwrite block's user sentence ("sits outside today's cascade guard exactly like text does") is the rationale a future reader will trust. Decision: the seat lists both sites in the spec card it files at ACCEPT for finding 1 — same file, same tree delete-semantics reading, one domain:spec card — so the note's 承接者:无 becomes a card.
  • Finding 3 (an authored deleteBehavior on a user field parses and is not walked) — Acceptance note, right (the spec's own line).
  • Triage's measure-at-claim list (authored cascade, restrict, multiple: true) — all three reported before and after; the pin triage named (os verify --delete-behaviors from 1 to 0 with lookup/master_detail CONTROLS unchanged) is reported row by row.
  • 495 changed lines; no governed surface; nothing under content/docs/releases/. The docs-drift advisory lists seven hand-written pages by a set_null literal in a changed comment — advisory, and outside this brief's inputs; not re-verified here.

Adopted by domain:engine seat 2 at 2026-10-12T15:28Z, after a tool-use audit of the isolated reviewer's transcript. It made 64 tool calls: reads, git fetch into review refs, and the two record writes. It made no GitHub write and ran no build, test or gate. One self-check script went to the scratchpad root rather than this PR's scratch directory, and was deleted. One reviewer rendered this record and PR #23045's, each from its own brief.

Implemented-by: claude/issue-22957-tree-delete-behavior
Reviewed-by: session_01ADzJtzYTLUfgrRZHxkagkX

VERDICT: PASS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants