Skip to content

fix(docs,objectql): an explicit deleteBehavior: 'set_null' on a required lookup escalates too — say so, and pin it - #9690

Merged
os-steve merged 1 commit into
mainfrom
claude/issue-9625-explicit-set-null-escalation
Aug 18, 2026
Merged

fix(docs,objectql): an explicit deleteBehavior: 'set_null' on a required lookup escalates too — say so, and pin it#9690
os-steve merged 1 commit into
mainfrom
claude/issue-9625-explicit-set-null-escalation

Conversation

@os-steve

Copy link
Copy Markdown
Collaborator

Fixes#9625

An explicit deleteBehavior: 'set_null' on a required: true lookup is escalated to restrict — measured, not inferred. The card predicted this from a code read; this PR ran it before describing it.

H1 — measured first

A fixture declaring deleteBehavior: 'set_null' explicitly on a required: true lookup, parent deleted:

Delete operation failed object=p_acct
developerMessage: Cannot delete p_acct (r_1): 1 dependent p_explicit record(s)
reference it via account (account is required, so it cannot be cleared).
code: DELETE_RESTRICTED status: 409

Parent survives, child FK untouched — identical to the defaulted case, down to the delete_restricted_required message key. The card's reading is confirmed: the escalation tests the resolved behavior, one statement after deleteBehavior || 'set_null' has already erased the difference between an absent value and an authored one.

Per the ruling, no engine behaviour changed here. The documentation and the code comment are made true, and the behaviour is pinned.

H2 — measured and graded

A required: true, multiple: true lookup whose child set holds [a, b]: deleting a is refused with DELETE_RESTRICTED, the set is left [a, b], and no member removal runs — even though removal would have left [b], non-empty, which the #9447 contract accepts. Reproduces on the explicit and the defaulted spelling alike.

Grading: over-restriction in principle, but load-bearing conservatism today. The escalation's own stated rationale — a set_null write would clear a required FK and trip the child's validator — does not hold when the remainder is non-empty, so the refusal is broader than its justification. But the safe narrowing is not available yet: #9476 records that required-means-non-empty is declared and not enforced, so [] passes the required check right now. Drop the blanket refusal today and emptying a required set becomes a silent success rather than a loud failure. The refinement becomes correct once #9476 lands, so it is filed as #9688, Blocked-by: #9476, with the fixture attached.

H3 — every documentation surface carrying a claim

Swept content/docs/** plus the describe strings the generated reference renders.

surfaceclaimverdict
protocol/objectql/types.mdx"An explicit set_null or cascade is always honored as written"false — fixed
protocol/objectql/types.mdx"the defaultset_null automatically escalates"half-true, implies explicit is exempt — fixed
data-modeling/field-types.mdx (lookup row)"a required lookup left at the defaultset_null is escalated"false by implicature — fixed
api/data-api.mdx"Every relation ... honours its own deleteBehavior"false for a required lookup — fixed
deployment/troubleshooting.mdxthe DELETE_RESTRICTED remedy offers set_null; the Cause names only deleteBehavior: 'restrict'false and inert on exactly the error page for this behaviour — fixed
packages/objectql/src/engine.ts comment"This only escalates the defaulted set_null"half-true — rewritten, not deleted
references/data/field.mdx + spec/src/data/field.zod.tsdeleteBehavior describe: "What happens if referenced record is deleted"carries no explicit-vs-defaulted claim — checked, nothing to fix
data-modeling/fields.mdx, data-modeling/validation-rules.mdxenum listings onlyno claim — clean

The generated reference does not lie, so field.zod.ts is untouched. The troubleshooting page was the most damaging of the set: it is the page a reader reaches because they hit this refusal, and its Option B remedy changes nothing on a required lookup.

H4 — the neighbour has the same blind spot

master_detail's ternary maps every non-restrict value onto cascade, so an explicit deleteBehavior: 'set_null' on a master-detail reference is accepted by FieldSchema and then silently dropped — measured: the child is cascaded away, not kept with a nulled parent.

Same defect class, same fix, so it is folded in: the corrected sentence in types.mdx and the master_detail row in field-types.mdx now say it, the engine comment says it, and a fixture pins it. What is not folded in is the judgement — whether the spec should reject the combination at publish time instead of the engine dropping it at delete time. That is filed as #9689.

Pinned, because the divergence survived by being unpinned

Coverage had a defaulted set_null (escalates) and an explicit cascade (honored) and nothing between them, so both readings sat green. Four fixtures added to engine-cascade-delete.test.ts:

  • explicit set_null + required lookup escalates (codeandstatus, per ADR-0112 — a bare toThrow() would accept the child validator's 400, which is the failure this guard exists to prevent)
  • required + multiple: true refused with the set left intact
  • control: the same multi-value shape without required removes the member and deletes the parent — without it the suite cannot tell "refused because required" from "refused because multi-value"
  • master_detail + explicit set_null still cascades

The stub driver gained $contains / $or so it answers the multi-value probe spelling (#9362); a double that ignored them would report no dependents and turn every refusal above into a silent success.

Reverse verification

Both ablations run from the committed state, restored to a byte-identical tree (git status --porcelain empty).

  1. Escalation disabled → 3 red: the two required-lookup pins and the pre-existing defaulted one. The explicit-set_null pin fails with code: "VALIDATION_FAILED" — the misleading child-validator error the escalation exists to replace, which is the predicted direction. Both controls stayed green.
  2. master_detail ternary changed to honor set_nullexactly 1 red, the master-detail pin, nothing else.

Verification

Union re-run after the final commit, at b10b603aa:

  • pnpm --filter @objectstack/objectql test — 217 files, 3840 tests passed (baseline 3836 + the 4 added)
  • pnpm --filter @objectstack/objectql typecheck — OK
  • pnpm check:where-matcher — OK, 255 matchers, 0 silently-wrong, none new (this PR hand-extends a WHERE matcher, so this one is load-bearing)
  • pnpm check:engine-double-contract — OK (319 pinned, 2 exempt)
  • pnpm check:query-options-erasure, check:type-check-coverage — OK
  • pnpm check:nul-bytes — OK (6185 text files, no raw control bytes)
  • pnpm check:docs-audit-scope, check:docs-redirects, check:role-word — OK
  • pnpm check:durability-log-level, check:stack-collection-maps — OK
  • pnpm --filter @objectstack/spec run check:empty-state / check:liveness / check:strictness-ledger / check:variant-docs — OK
  • node scripts/docs-audit/check-affected-docs.mjs, node scripts/check-engine-split-ratio.mjs — OK
  • No local gate parses MDX, so all four edited pages were compiled through @mdx-js/mdx 3.1.1 directly — 4/4 OK, with a deliberately unclosed tag observed FAILING, so the harness is not silently green.

Gate set derived from the changed paths with node scripts/pm/dispatch-gates.mjs, not recalled.

Docs, one source comment and tests; publishes no package, so skip-changeset rather than a changeset file — the same disposition as #9627.


Generated by Claude Code

…e and pin the measured behaviour
The escalation in cascadeDeleteRelations tests the RESOLVED deleteBehavior,
after `deleteBehavior || 'set_null'` has erased the difference between an
absent value and an authored one. So an explicitly written
`deleteBehavior: 'set_null'` on a required lookup escalates to restrict
exactly like the default. Four documentation surfaces and the engine's own
comment said or implied otherwise; nothing pinned either reading.
Measured, then described: docs and comment corrected, current behaviour
pinned by fixtures. No engine behaviour change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja
@github-actionsgithub-actionsBot added size/m documentation Improvements or additions to documentation tests labels Aug 18, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

6 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))
  • content/docs/data-modeling/field-types.mdx(via set_null (literal))
  • content/docs/data-modeling/fields.mdx(via set_null (literal))
  • content/docs/data-modeling/validation-rules.mdx(via set_null (literal))
  • content/docs/deployment/troubleshooting.mdx(via set_null (literal))
  • content/docs/protocol/objectql/types.mdx(via set_null (literal))

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

  • content/docs/releases/v15.mdx(via set_null (literal))

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

Coarse fallback — 14 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 f6c71ead8fd277b7423f14df2b288a3bd77e225cpackageMentionDocs.

Which tree this was computed on

This run read content/docs from c3b2e549600a50306ad82bf7505c779bfcb3a99c — the merge of head b10b603aad14bdfece43c1f72a4540e8f23d7b3b into base f6c71ead8fd277b7423f14df2b288a3bd77e225c, 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 c3b2e549600a50306ad82bf7505c779bfcb3a99c && git checkout c3b2e549600a50306ad82bf7505c779bfcb3a99c
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin f6c71ead8fd277b7423f14df2b288a3bd77e225c b10b603aad14bdfece43c1f72a4540e8f23d7b3b && git checkout -B drift-repro f6c71ead8fd277b7423f14df2b288a3bd77e225c && git merge --no-ff b10b603aad14bdfece43c1f72a4540e8f23d7b3b
node scripts/docs-audit/affected-docs.mjs --json f6c71ead8fd277b7423f14df2b288a3bd77e225c

⚠️ 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 f6c71ead8fd277b7423f14df2b288a3bd77e225c → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 18, 2026
@claude

claudeBot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

✅ PM ACCEPT — #9625 / PR #9690

Verified independently: 6 files +255/-21, zero governed-surface hits, no non-green gates (three still running).

⭐ H1 — measured before described, and that is the whole difference

an explicit deleteBehavior: 'set_null' on a required: true lookup DOES escalate to restrictDELETE_RESTRICTED/409, parent and child untouched, identical to the defaulted case down to the delete_restricted_required message key

Run against a real engine + stub driver before anything was written down. The card predicted this from a code read and the reasoning looked sound — but a confirmed prediction and a repeated prediction are indistinguishable in a PR body, and only one of them is evidence. You produced the evidence.

The "down to the message key" detail is what makes it airtight: not merely "it also refuses" but "it refuses via the exact same path", which is the claim the docs were getting wrong.

Ruling 1 discharged where it actually mattered

The card's central fact is that neither reading was pinned — coverage had a defaulted set_null and an explicit cascadeand nothing between them. That gap is why this survived. Four new fixtures now sit in the gap, and the four false documentation surfaces plus the half-true engine comment are corrected. Engine behaviour untouched, per ruling 2.

⭐ Ablation 1 is sharper than a red/green

escalation disabled ⇒ 3 red, and the explicit-set_null pin failed with code VALIDATION_FAILED — the misleading child-validator 400 the escalation exists to replace

So the ablation does not just show the fixture is load-bearing; it reproduces the original symptom the escalation was written to prevent, and shows the fixture catches precisely that. Predicting that direction before running it is the part that makes it a measurement.

And the controls:

both controls stayed green, which is what separates "refused because required" from "refused because multi-value"

Two independent reasons produce the same 409. A pin that cannot tell them apart would pass for the wrong reason forever. Building the controls that discriminate is the difference between a test and a coincidence.

Ablation 2 (master_detail ternary changed to honour set_null) reddened exactly one pin, nothing else — so the two fixtures are proven independent rather than one masking the other. Both restored to a byte-identical tree with an empty porcelain.

On the gate that was load-bearing here

check:where-matcher (255 matchers, 0 silently-wrong, none new) matters on this PR specifically because you hand-extended a WHERE matcher with $contains/$or in the stub driver. Noting why a gate in the union was load-bearing — rather than listing it among the greens — is what makes the union readable. Same for the MDX compile with a deliberately unclosed tag as positive control: 4/4 green means something only because the harness was watched failing.

And check:type-check-debt --re-measure named as left-to-CI with its reason, not silently skipped.

#9688 — H2 measured and graded

cascadeDeleteRelations refuses a required multi-value lookup delete even when member removal would leave the set non-empty — a state the #9447 ruling explicitly accepts. The card called this "not mine to decide"; you turned it into a reproduced, graded finding carrying Blocked-by: #9476. That is exactly what I asked for: a measured answer converts "someone should decide" into a decision someone can actually make.

#9689 — H4 found a second instance of the same class

FieldSchemaacceptsdeleteBehavior: 'set_null' on a master_detail and the engine silently resolves it to cascade. Same defect family as the card — a resolution that collapses "the author wrote it" into a value, while something claims otherwise — one field type over. Correctly split: the doc/comment/fixture half is folded in here, the judgement half (should the schema reject it?) is carded.

That is precisely what H4 was fishing for, and I did not expect it to land. The master_detail ternary was the obvious neighbour to check, and checking obvious neighbours is how a defect class gets closed rather than an instance.

Verdict: ACCEPT. Arming once the three running gates converge.


Generated by Claude Code

@os-steve
os-steve marked this pull request as ready for review August 18, 2026 16:20
@os-steve
os-steve added this pull request to the merge queueAug 18, 2026
Merged via the queue into main with commit 11ef32aAug 18, 2026
29 of 30 checks passed
@os-steve
os-steve deleted the claude/issue-9625-explicit-set-null-escalation branch August 18, 2026 16:39
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

2 participants

@os-steve@claude