Skip to content

fix(spec): reject an authored deleteBehavior 'set_null' on a master_detail at parse time; log the engine coercion loudly - #11406

Draft
os-steve wants to merge 3 commits into
mainfrom
claude/issue-9689-master-detail-set-null-r2
Draft

fix(spec): reject an authored deleteBehavior 'set_null' on a master_detail at parse time; log the engine coercion loudly#11406
os-steve wants to merge 3 commits into
mainfrom
claude/issue-9689-master-detail-set-null-r2

Conversation

@os-steve

Copy link
Copy Markdown
Collaborator

Fixes#9689

Implements the maintainer ruling of 2026-08-19 (「接受你的所有建议。」, recorded on the card): Q1 = A (the #7918 Option A relocation, exactly as spiked), Q2 = C (no automatic conversion — SemanticMigration entry only), Q3 = B (spec rejection plus a loud engine log at the coercion site), Q4 = B (maintainer authors the ADR — drafted below, docs/adr/** untouched).

What changed

packages/spec/src/data/field.zod.ts.default('set_null') moved off deleteBehavior to .optional() + .meta({ description, default: 'set_null' }); a per-type check in the field-level superRefine makes an AUTHORED set_null on a master_detail a named parse-time rejection (path deleteBehavior, message states the actual outcome — children DELETED, not kept — and names both legal re-declarations plus the lookup alternative); a post-check .overwrite() re-materializes 'set_null' at its shape position, so parse output is byte-identical to the .default() era. The schema stays a ZodObject (shape enumerable, 71 keys — no pipe degradation). Accepted cost, same as the currency precision precedent: the inferred Field output type now declares deleteBehavior? (ADR-0122 forbids hand-narrowing); a parsed field always carries it at runtime.

packages/spec/src/migrations/entries/semantic/18.field-master-detail-set-null-refused.ts (+ regenerated registry.ts) — the Q2 = C SemanticMigration entry, field-scale-precision-integer-refused precedent, with option B's reasoning stated in the replacement text ("restrict is the only rewrite that cannot lose data").

packages/objectql/src/engine.ts (cascadeDeleteRelations only) — the coercion stays (Q3 = B; engine behavior unchanged). It now logs loudly when an authored set_null on a master_detail reaches the coercion site — the populations parse-time rejection measurably cannot catch (raw registrations; rows stored before the tightening — the engine registers raw objects and never re-parses, measured: the pin suite is green with the spec change alone). Sanctioned #9750 shape: reach for error, fall back to warn, never an optional call.

Tests — spec: rejection pinned (path + message head + both remedies named); bare master_detail still parses and still materializes set_nullat shape position (byte-identity pin); authored cascade/restrict untouched; set_null on lookup (required and not) stays legal; the #4447 showcase-verbatim datetime shape stays legal; shape enumerability pinned. objectql: the #9625 pin a master_detail declaring an explicit deleteBehavior:set_null still cascadesstays true and untouched; a new describe pins the log firing exactly once on the authored case (error path and warn-fallback path), and NOT firing on bare/cascade (same resolved behavior, no divergence).

Docs (declared surface addition — two sentences the diff falsifies): content/docs/protocol/objectql/types.mdx and the master_detail row of content/docs/data-modeling/field-types.mdx said an explicit set_null is accepted-but-not-honored; both now state the parse-time rejection and the raw/stored residual. These are the exact sentences #9625 wrote to document the behavior this card changes; leaving them would make the docs affirmatively wrong about the authorable surface. Not in the claim's file list — flagged as a deviation, see below.

Changeset: minor for @objectstack/spec and @objectstack/objectql (launch window forbids major; check-changeset-no-major respected).

Measurements

Q4 = B — draft ADR entry (for the maintainer; NOT written to docs/adr/**)

A property-level Zod default destroys the authored-vs-absent distinction that per-type enforcement depends on. A .default(v) on a shared authorable key materializes at parse, so no later refinement can tell "the author wrote v" from "we defaulted v" — a per-type rule gated on the value fires on every untouched declaration (permanently noisy) or cannot exist. Three instances have each cost a card: currency precision (#7918), deleteBehavior: 'set_null' on a required lookup (#9625), deleteBehavior: 'set_null' on a master_detail (#9689). Prescription (the #7918 Option A shape): declare the key .optional() + .meta({ description, default }); judge it in superRefine, where undefined means "not authored"; re-materialize the same default AFTER the check via .overwrite(), rebuilt in shape order so parse output stays byte-identical. Corollary: the materialized default in serialized parse output is indistinguishable from an authored value to every later consumer — which is why built artifacts (#4447, #9784) should stop carrying defaults whose legality is type-dependent.

Required answer — effect on #9784

This change makes #9784more urgent and its edit easier, its decision slightly harder:

Deviations

  1. Branch name is claude/issue-9689-master-detail-set-null-r2 (claim says …-set-null). The stale probe branch at 2065e31d5 is not an ancestor of current main (old-main history; upstream history divergence), so a plain push is non-fast-forward; branch deletion is refused by the proxy (403 on both the git and REST paths), and force-push is contract-banned for this seat even though the dispatch offered it (prompt-vs-contract conflict resolved toward the contract, as the contract instructs). The stale branch carries zero commits of its own and was left in place.
  2. Docs surface addition (two sentences, rationale above) — not in the claim's declared file list.
  3. The dispatch said the prior run's engine-cascade-delete.test.ts pin sat at ~:181 — confirmed exactly.

Gates

Derived from the actual diff via node scripts/pm/dispatch-gates.mjs (no paths passed; answer verified as objectstack-ai/objectstack @ cbe02bf) — per-gate exit codes in the os-dev report on #9689. Full @objectstack/spec suite (419 files / 11143 tests), full @objectstack/objectql suite (229 files / 4063 tests at cbe02bf), spec regen discipline (build + check:generated — all 14 generated artifacts up to date, zero movement), and the derived families run locally.

needs:contract-review is hung card-side (clause-②): this PR stays draft and is held for contract review. Out of scope, filed separately: the published objectstack-data skill and the field designer form both still offer set_null on master_detail (issue numbers in the os-dev report; the skill fix is a governed surface and must be its own PR).


Generated by Claude Code

…l at parse time
Relocate the property-level .default('set_null') off deleteBehavior into a
post-check .overwrite() (the currency-precision Option A shape) so the
field-level superRefine can tell authored from defaulted; an authored
set_null on a master_detail is now a named parse-time rejection, while parse
output stays byte-identical for every accepted input (shape-position
re-materialization, measured 13/13 on a before/after battery). Ship the
SemanticMigration entry (no automatic conversion - only the author knows
whether they meant restrict or cascade), and log loudly at the engine's
coercion site when the combination still arrives via a raw registration or a
pre-tightening stored row (error, falling back to warn). Engine behavior is
unchanged; the existing cascade pin stays true. Docs rows that described the
silent acceptance now state the rejection.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T9cDbY2NBiVJWYx3BpWfH2
The coercion-site log fires whenever a parent delete computes a misdeclared
child field's behavior - with or without child rows - so the negative
control must not register the misdeclared object at all. Message wording
adjusted to not assert rows exist (CASCADES its referencing child rows).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T9cDbY2NBiVJWYx3BpWfH2
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

25 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json bb006446adb558e5a86c009df9c0b08e98833011.

5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 4 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 127 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 bb006446adb558e5a86c009df9c0b08e98833011packageMentionDocs.

Which tree this was computed on

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

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

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation protocol:data tests tooling labels Aug 23, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationprotocol:datasize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FieldSchema accepts deleteBehavior: 'set_null' on a master_detail, and the engine silently resolves it to cascade

2 participants

@os-steve@claude