Skip to content

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

Merged
os-warren merged 6 commits into
mainfrom
claude/issue-9689-master-detail-set-null-r2
Aug 24, 2026
Merged

os-warren merged 6 commits into
mainfrom
claude/issue-9689-master-detail-set-null-r2

Conversation

@os-steve

@os-steve os-steve commented Aug 23, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #9689

Implements two maintainer rulings, both recorded on the card:

  • 2026-08-19 (「接受你的所有建议。」): Q1 = A (the Contract question: should publish-time validation reject a declared currency precision that contradicts the currency's ISO 4217 digits? #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).
  • 2026-08-24 (「四维分析一致的,接手你的建议。」, comment 5393503175): A — idempotent materialization. The .overwrite() no longer materializes set_null on master_detail; every other type keeps byte-identity; the superRefine rejection stays verbatim; artifacts stop carrying a value the schema itself refuses; parse becomes idempotent on the mainline create() → defineStack path. (The R2 increment's byte-identical materialization was measured self-rejecting on that path — every failing CI job died on @objectstack/example-showcase#build, whose 4 bare master_detail fields were baked set_null by parse Add metamodel interfaces for ObjectQL/ObjectUI contract #1 and rejected by parse ✨ Set up Copilot instructions #2. Diagnosis: comment 5387252153.)

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. Per the 2026-08-24 ruling, the .overwrite() carries one conditional stating the general principle in code doc: never materialize a default the schema itself would refuse as authored — a bare master_detail parses to output that OMITS deleteBehavior (the baked value was indistinguishable from an authored one by design, so parse output rejected itself on re-parse); every other type keeps byte-identity with the .default() era. The schema stays a ZodObject (shape enumerable — no pipe degradation). Accepted cost, same as the currency precision precedent: the inferred Field output type declares deleteBehavior? (ADR-0122 forbids hand-narrowing); at runtime a parsed field carries it on every type except master_detail, where absence is the honest spelling.

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; unchanged in R3) — the coercion stays (Q3 = B; engine behavior unchanged). It 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/artifacts stored before the tightening). Sanctioned #9750 shape: reach for error, fall back to warn, never an optional call. After the idempotent-materialization change, artifacts built from this version onward no longer carry the baked key on bare master_detail, so the #4447-class residual log narrows to pre-tightening artifacts and genuinely raw registrations.

Tests — spec: rejection pinned (path + message head + both remedies named); bare master_detail parses and its output omits deleteBehavior (flipped from the R2 byte-identity pin, per the ruling); parse idempotency pinned on the mainline chain (FieldSchema.parse output re-parses green; ObjectSchema.create() output re-parses green — the exact chain that carried the defect); the shape-position byte-identity pin re-targeted to lookup (the types that still materialize); 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 cascades stays true and untouched (registrations are raw; the engine never re-parses); the R2 describe pinning the coercion-site log (fires once on authored set_null, error path and warn-fallback path, NOT on bare/cascade) unchanged.

Docs (declared surface addition, unchanged in R3 — two sentences the diff falsifies): content/docs/protocol/objectql/types.mdx and the master_detail row of content/docs/data-modeling/field-types.mdx now state the parse-time rejection and the raw/stored residual.

Changeset: minor for @objectstack/spec and @objectstack/objectql; prose updated in R3 with the artifact honesty note (bare master_detail built artifacts stop carrying deleteBehavior; engine behavior measured unchanged).

Measurements (R3, on the branch with origin/main merged in)

  • Failure reproduced before the fix (PM assumption verified): at the merged head pre-fix, turbo run build --filter=@objectstack/example-showcase exits 2 — defineStack validation failed (4 issues) at exactly objects.9.fields.f_master_detail.deleteBehavior, objects.17.fields.team.deleteBehavior, objects.17.fields.project.deleteBehavior, objects.20.fields.project.deleteBehavior. Mechanism probes at the same head: FieldSchema.parse(bare md) output carries deleteBehavior: 'set_null' and re-parse REJECTS at deleteBehavior; ObjectSchema.create(bare md) output re-parse REJECTS at fields.parent.deleteBehavior.
  • Green after the one-conditional fix: the same turbo graph goes 61/61 tasks successful, exit 0; both probes flip to IDEMPOTENT; md_bare output omits the key.
  • Byte-identity re-measured: 14-fixture battery (bare/authored master_detail × all three values, four lookup variants, text/datetime/number/currency/tree, the data: created_at is client-writable on an ordinary PATCH — the audit anchor can be forged silently #4447 showcase-verbatim datetime) captured against the built dist BEFORE and AFTER the conditional: 13/14 entries byte-identical (JSON.stringify, key order included; md_set_null stays REJECTED at the same path). The single flip is md_bare, whose only movement is the omission of deleteBehavior — no other key or value moved.
  • Consumer sweep for the absent key (premise check, measured): every non-test deleteBehavior read site in the workspace enumerated; the engine reads raw definitions (absent ≡ set_null → cascade on master_detail — the FieldSchema accepts deleteBehavior: 'set_null' on a master_detail, and the engine silently resolves it to cascade #9689 exhaustion matrix); the one consumer that distinguishes absent — lint rule relationship/delete-behavior — consumes RAW authored objects (verified from its tests), where absent has always meant "not authored", so it is unaffected by parse-output shape. Its message text staleness (still offering set_null on master_detail) filed as lint relationship/delete-behavior suggestion still names set_null as declarable on a master_detail — a spelling #9689 makes a parse-time rejection #11668.
  • Mutation-on-disk proof: the conditional confirmed present in the rebuilt packages/spec/dist/data/index.mjs (esbuild spelling if (field.type === "master_detail") return field;) before any post-fix measurement was read.
  • TEST_DEBT ratchet drift found and repaired: check:type-check-debt --re-measure red at 355 vs the recorded 354 for @objectstack/objectql. Attributed by per-file tsc enumeration: the +1 is the R2 log-test helper's 1-arg registry.registerObject(o as any) (engine-cascade-delete.test.ts:577, TS2554 — the base file's identical pile member balances the rest out). Repaired with the gate's own author remedy — the house 2-arg spelling (batch-row-authoring-feedback.test.ts precedent); file re-run 16/16 green, re-measure back to "none above its recorded number".

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.

Second principle (ruled 2026-08-24, this card's R3): never materialize a default the schema itself would refuse as authored. The materialized default in serialized parse output is indistinguishable from an authored value to every later consumer — including the schema's own next parse. Where a per-type rule refuses the authored spelling of the default, baking that default makes parse output self-rejecting, and parse(parse(x)) breaks on any path that re-parses parse output (ObjectSchema.create() → defineStack is the mainline app-build path; measured: 4 bare master_detail fields failed the showcase build). On such types the default must stay absent in output — absence is the honest spelling, and idempotent parse is the more fundamental invariant than byte-stability. The currency precision twin of this landmine is #11423.

Effect on #9784

This PR now lands the master_detail slice of #9784's decision (bare master_detail artifacts stop carrying the inert-and-refused key), which was its most urgent half. What remains for #9784 is the inert-but-accepted key on non-reference types (text/datetime/number …), where byte-identity is preserved by this PR and removal would be a real artifact migration. The relocation still makes that edit a per-type conditional in one place (the .overwrite()), and #11423 (currency precision) follows the second principle above directly.

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 original claim's declared file list; accepted in the R2 contract review.
  3. R3 gate-derivation detail: dispatch-gates.mjs run without paths (the script derives the changed set from the merge base itself — the takeover prompt's explicit changed-paths argument conflicts with the standing no-paths rule; resolved toward the standing contract, recorded in the os-dev report).
  4. R3 file-surface extension, one line: the R3 claim's surface was field.zod.ts + its tests + PR body/changeset prose, but the debt-ratchet red above lands in packages/objectql/src/engine-cascade-delete.test.ts — a file of THIS PR's own R2 diff, red on a gate this patch round must drive green, with the fix being the gate's own prescribed author remedy (never a ledger raise). Declared here and in the report rather than left to a fork-back: the contract increment itself did not move.

Gates

R3 gates derived from the actual diff via node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (no paths passed) at both b8c3f8507 and the final head 2bed67490 — list identical. At the final head: full @objectstack/objectql suite green (231 files / 4106 tests), check:type-check-debt --re-measure green ("none above its recorded number"), the 41-entry light battery all exit 0 (including check:dev-prereqs after topping the closure to 67/67 built), check:generated green. Spec suite (420 files / 11226 tests), showcase build 61/61, showcase tsc --noEmit, and workspace typecheck 124/124 ran at b8c3f8507; carried to 2bed67490 by invariance — the only file the later commit touches is engine-cascade-delete.test.ts, excluded from every build/typecheck program (packages/objectql/tsconfig.json excludes **/*.test.ts) and reachable only by vitest and the debt program, both re-run green at the final head. Per-gate verdict lines in the os-dev report on #9689.

needs:contract-review re-fires on this new diff (clause-②): this PR stays draft and is held for contract review. Out of scope, filed separately: #11409 (skill), #11410 (field designer form), #11423 (currency twin), #11668 (lint message).


Generated by Claude Code

claude added 2 commits August 23, 2026 15:30
…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

github-actions Bot commented Aug 23, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

26 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 7181101a536b56dfa3004e91eb9811d1a1c5e34c.

⛔ 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 7181101a536b56dfa3004e91eb9811d1a1c5e34c → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 4b2a2072c1d748aeef4614e0bc3c6081999dad7f — the merge of head 2bed67490cdf8d6ad363daa404d08beb698f7ce3 into base 7181101a536b56dfa3004e91eb9811d1a1c5e34c, 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 4b2a2072c1d748aeef4614e0bc3c6081999dad7f && git checkout 4b2a2072c1d748aeef4614e0bc3c6081999dad7f
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7181101a536b56dfa3004e91eb9811d1a1c5e34c 2bed67490cdf8d6ad363daa404d08beb698f7ce3 && git checkout -B drift-repro 7181101a536b56dfa3004e91eb9811d1a1c5e34c && git merge --no-ff 2bed67490cdf8d6ad363daa404d08beb698f7ce3

node scripts/docs-audit/affected-docs.mjs --json 7181101a536b56dfa3004e91eb9811d1a1c5e34c

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

@github-actions github-actions Bot added documentation Improvements or additions to documentation protocol:data tests tooling labels Aug 23, 2026
claude added 2 commits August 24, 2026 11:36
… as authored — bare master_detail parse output omits deleteBehavior

Maintainer ruling 2026-08-24 (A — idempotent materialization) on #9689's R2
increment: the .overwrite() baked set_null onto bare master_detail, the
superRefine refuses the authored spelling, and the two are indistinguishable
by design — so ObjectSchema.create() output rejected itself on defineStack's
re-parse and every app build with a bare master_detail failed (showcase: 4
fields, reproduced red pre-fix, green post-fix). One conditional in the
.overwrite(); every other type keeps byte-identity (13/14 battery entries
byte-identical, the flip is the omitted key only); parse(parse(x)) pinned
idempotent at both FieldSchema and the create()->defineStack chain; the
shape-position pin re-targeted to lookup. Engine behavior unchanged (absent
was already cascade on this type).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rxnd8cyFnoU8V5y21PaTsy
…eep the TEST_DEBT ratchet at 354

The R2 helper's 1-arg call added a TS2554 to the frozen debt pile
(354 -> 355); check:type-check-debt --re-measure is shrink-only and the
author remedy is fixing the new error. House 2-arg spelling
(batch-row-authoring-feedback.test.ts precedent); file re-run green 16/16,
re-measure back to 'none above its recorded number'.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rxnd8cyFnoU8V5y21PaTsy
@os-warren
os-warren marked this pull request as ready for review August 24, 2026 14:23
@os-warren
os-warren added this pull request to the merge queue Aug 24, 2026
Merged via the queue into main with commit 9086761 Aug 24, 2026
42 checks passed
@os-warren
os-warren deleted the claude/issue-9689-master-detail-set-null-r2 branch August 24, 2026 14:39
os-zhuang pushed a commit that referenced this pull request Aug 24, 2026
The published objectstack-data skill taught `deleteBehavior: 'set_null'` on a
`master_detail` field in four places. That was already wrong before #9689 — the
engine resolved every non-`restrict` value on this type to `cascade`, so the
declaration promised keep-the-children and delivered delete-the-children — and
since #9689 landed (PR #11406) it is a named parse-time rejection: an AI author
following the skill produced metadata that fails publish with a 422.

Teaches the per-type vocabulary instead: `master_detail` admits `cascade` /
`restrict`; `set_null` belongs on `lookup`, where children survive the parent.

Correction, not expansion — net line delta is exactly 0 in both files.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RMTpSRF5CjMmQBFfPtPCwJ
os-project-manager pushed a commit that referenced this pull request Aug 31, 2026
…master_detail

The `relationship/delete-behavior` suggestion in `lintDataModel` told an
author an undeclared `master_detail.deleteBehavior` could be `cascade`,
`restrict`, or `set_null`. Since #9689 (PR #11406, maintainer ruling
2026-08-19), an authored `deleteBehavior: 'set_null'` on a `master_detail`
field is a named parse-time rejection, so following the suggestion literally
walked an author into a publish-time error.

The message now enumerates only the two values FieldSchema actually accepts
on a master_detail (cascade/restrict — matching object.form.ts's
master_detail deleteBehavior options), and keeps the outcome-naming courtesy
of the parse-time rejection message: it still names set_null to say plainly
it is not honored on this type, and points to `lookup` for the case where
children must survive the parent. The `fix` payload was already correct and
is unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC
akarma-synetal pushed a commit to akarma-synetal/framework that referenced this pull request Sep 1, 2026
…ly (objectstack-ai#11869)

The published objectstack-data skill taught `deleteBehavior: 'set_null'` on a
`master_detail` field in four places. That was already wrong before objectstack-ai#9689 — the
engine resolved every non-`restrict` value on this type to `cascade`, so the
declaration promised keep-the-children and delivered delete-the-children — and
since objectstack-ai#9689 landed (PR objectstack-ai#11406) it is a named parse-time rejection: an AI author
following the skill produced metadata that fails publish with a 422.

Teaches the per-type vocabulary instead: `master_detail` admits `cascade` /
`restrict`; `set_null` belongs on `lookup`, where children survive the parent.

Correction, not expansion — net line delta is exactly 0 in both files.


Claude-Session: https://claude.ai/code/session_01RMTpSRF5CjMmQBFfPtPCwJ

Co-authored-by: Claude <noreply@anthropic.com>
akarma-synetal pushed a commit to akarma-synetal/framework that referenced this pull request Sep 1, 2026
…master_detail (objectstack-ai#13668)

The `relationship/delete-behavior` suggestion in `lintDataModel` told an
author an undeclared `master_detail.deleteBehavior` could be `cascade`,
`restrict`, or `set_null`. Since objectstack-ai#9689 (PR objectstack-ai#11406, maintainer ruling
2026-08-19), an authored `deleteBehavior: 'set_null'` on a `master_detail`
field is a named parse-time rejection, so following the suggestion literally
walked an author into a publish-time error.

The message now enumerates only the two values FieldSchema actually accepts
on a master_detail (cascade/restrict — matching object.form.ts's
master_detail deleteBehavior options), and keeps the outcome-naming courtesy
of the parse-time rejection message: it still names set_null to say plainly
it is not honored on this type, and points to `lookup` for the case where
children must survive the parent. The `fix` payload was already correct and
is unchanged.


Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC

Co-authored-by: Claude <noreply@anthropic.com>
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 protocol:data size/m tests tooling

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

4 participants